# Phase 1 — Sale Module: Differences Report (API vs Blade)

Comparison of the REST Admin Sale APIs against their Blade source-of-truth controllers, produced before any code changes. Blade is the reference for business behavior unless flagged as a likely Blade bug.

**Sources compared**
- API: `packages\Webkul\RestApi\src\Http\Controllers\V1\Admin\Sale\{Order,Invoice,Shipment,Refund,Transaction}Controller.php`
- Blade: `packages\Webkul\Admin\src\Http\Controllers\Sales\{Order,Invoice,Shipment,Refund,Transaction}Controller.php`
- Routes: `packages\Webkul\RestApi\src\Routes\V1\Admin\sale-routes.php`

---

## Cross-cutting differences (apply to all Sale controllers)

- **No service layer:** All business logic lives in the API controllers. Per agreed standard (Q1), extract into `OrderService`, `InvoiceService`, `ShipmentService`, `RefundService`, `TransactionService` (singletons), keeping controllers thin. Behavior preserved/aligned to Blade.
- **Search endpoints build arrays manually:** `searchOrders/searchInvoices/searchShipment/refundSearch/searchTransaction` use `foreach` + `array_push(new XResource())`. These are **API-only** (Blade uses server-side DataGrids, no equivalent endpoint). Decision (Q3): keep + document; standardize wrapper to `{ data, message }` and reuse a service `search()` method. No pagination added (no Blade behavior to mirror).
- **`getResource` tenant scoping:** Base `getResource()` does `findOrFail($id)` with **no `company_id` check**, so an admin could fetch another company's order/invoice/etc. by ID. Blade `view()` loads by `order_no` + `firstOrFail` (also not explicitly company-scoped, but behind admin guard + channel). Impact: potential cross-tenant read. Recommendation: add `company_id` scoping in the service `getById()` used by API. **Decision Needed? Minor — see Q-S2.**
- **Auth model:** Routes use `auth:sanctum` + `sanctum.admin`. No Blade ACL gate parity (Q7 = keep Sanctum + `company_id`). Documented as intentional difference.

---

## Order endpoints

### `POST /sales/orders/{id}/cancel`
- **Current API:** `OrderRepository::cancel($id)`; returns `{message}`; wraps in try/catch. No activity logging.
- **Blade:** Same cancel call, **plus** `OrderActivityLogger::logCanceled($order->fresh(), ['old_status' => $oldStatus])` on success.
- **Impact:** API-cancelled orders are missing from the order activity timeline.
- **Recommendation:** Add `OrderActivityLogger::logCanceled` in service to match Blade.
- **Decision Needed?** No.

### `POST /sales/orders/{id}/complete`
- **Current API:** Adds preconditions NOT in Blade — rejects if status is `completed`/`canceled`/`closed`, and requires status `shipped`/`processing`. No `OrderActivityLogger::logCompleted`.
- **Blade:** Calls `complete($id)` directly (relies on repository result), logs `logCompleted`, no explicit status precondition checks.
- **Impact:** API may reject completes that Blade allows; missing activity log.
- **Recommendation:** Align to Blade: remove extra precondition gating, rely on repository result, add `logCompleted`. (Keeps `{data,message}` response.)
- **Decision Needed?** Q-S1 — confirm removing the API's extra status guards (they look like added safety, but violate parity).

### `POST /sales/orders/{id}/fastComplete`
- **Current API:** Param is order **id**; runs in a single `DB::transaction`; **returns a raw array** (not `response()`); pickup/active-carrier auto-selection; creates invoice + shipment + completes; no activity logging.
- **Blade:** Param is **order_no**; status-workflow stepwise (pending→processing→shipped→completed); logs `logFastCompleted`; carrier auto-select; richer logging; flashes session messages.
- **Impact:** Response inconsistency (raw array vs standardized response); missing activity log; subtle flow differences (Blade refreshes between steps, has retry-without-events fallback).
- **Recommendation:** Move logic to `OrderService::fastComplete()`, return standardized `{data,message}`, add `logFastCompleted`, mirror Blade's stepwise flow. Keep `{id}` route param (API convention) — documented difference vs Blade's `order_no`.
- **Decision Needed?** No (behavior aligned; param style documented).

### `POST /sales/orders/{id}/comments`
- **Current API:** Validates `comment` required; merges `order_id`; **`customer_notified` handling commented out**; dispatches before/after events; returns `{data: comment}`. No activity logging.
- **Blade:** Sets `customer_notified = isset(...) ? 1 : 0`; dispatches events; logs `logCommentAdded`.
- **Impact:** API never persists `customer_notified`; missing activity log.
- **Recommendation:** Restore `customer_notified` handling + `logCommentAdded` to match Blade.
- **Decision Needed?** No.

### `POST /sales/orders/store` (manual order create)
- **Current API:** Cart-based manual order; expects `related_products` (`"productId-variationId"`), `qty[]`, `branchId`, `customer_id`, `address_id`, `shipping_method`, `paymentMethod`, `token`. Creates order, checks qty, inserts a shipping address row, **always creates a paid/processing invoice**.
- **Blade:** `store()` + `storePOS()` cart-based manual order with broader handling (POS sets `pickup_pickup`, shipping_cost 0). Larger flow with address/customer helpers.
- **Impact:** Both are complex cart flows; risk of subtle divergence (invoice auto-creation, address duplication via raw `DB::table('addresses')->insert`).
- **Recommendation:** Extract into `OrderService::createManualOrder()`. Preserve current API behavior (API-specific manual-order contract) and document; align invoice-creation + address handling with Blade where they clearly differ. Detailed sub-diff to be completed during implementation.
- **Decision Needed?** Q-S3 — confirm manual-order API contract stays as-is (it differs from Blade's POS/edit flows which are UI-driven).

### API-only Order endpoints (no Blade counterpart) — keep + document
- `GET /sales/orders` (`allResources`), `GET /sales/orders/{id}` (`getResource`)
- `POST /sales/orders/search` (`searchOrders`)
- `GET /sales/orders/order-timeline/{id}` (`orderTimeLine`)
- `GET /sales/shippingRates` (`collectRates`)
- `GET /sales/shipment/source/{order_id}` (`sourceShipment`)
- `GET /sales/refund/create/{order_id}` (`refundCreate`)
- `POST /sales/orders/{id}/ship` (`ship`) — multi-inventory allocation; Blade equivalent is `fastShip` (different contract). Keep + document.

---

## Invoice endpoints

### `POST /sales/invoices/{order_id}` (store)
- **Current API:** `canInvoice()` guard → 400 `invoices.creation-error`; validates `invoice.items.* required|numeric|min:0`; `haveProductToInvoice` via `(int)$qty`; creates invoice; returns `{data,message}`.
- **Blade:** Same guards/validation; `haveProductToInvoice` via truthy `$qty`; uses message key `invoices.product-error` for the no-product case (API reuses `creation-error`); redirects on success.
- **Impact:** Minor message-key mismatch for the "no products to invoice" case.
- **Recommendation:** Align the no-product error to the Blade-equivalent message; keep `{data,message}` JSON. Behavior otherwise matches.
- **Decision Needed?** No.

### `GET /sales/invoice/print/{id}` (print)
- **Current API:** Renders `admin::sales.invoices.pdf` with `invoice` + `paperSize='a4'`; returns HTML string in `{data,message}`; 404 if not found.
- **Blade:** `print($id, $paperSize)` renders `admin::sales.invoices.print` with `invoice`, `order`, `branch`, `paperSize`.
- **Impact:** Different view + missing `order`/`branch` context; different paper size source.
- **Recommendation:** Document as API-simplified print. Optionally align to Blade's `print` view + context. **Decision Needed? Q-S4 — confirm whether to match Blade's print view.**

### API-only / gaps
- `POST /sales/invoice/search` (`searchInvoices`) — API-only, keep + document.
- `GET /sales/invoice/transaction/{id}` (`invecesTransaction`) — API-only, keep + document.
- **Gap:** Blade has `update()` (mark invoice paid). No API equivalent. Recommendation: note as potential addition (not adding unless requested).

---

## Shipment endpoints

### `POST /sales/shipments/{order_id}` (store)
- **Current API:** `canShip()` guard; validates `shipment.source`, `shipment.items.*.*`; `isInventoryValidate()`; carrier integration via `switch` on **`'smsa'|'mylerz'|...`** (bare keys); sets `track_number=null` default.
- **Blade:** Same guards/validation; `isInventoryValidate()` **resets `totalWeight=0`**, handles **variation inventories** and **backorder/allow-zero-quantity** config; carrier `switch` on **`'carriers.smsa'|...`** (prefixed keys); checks `session('shipping_Carrier_invalid')` and aborts with error.
- **Impact (multiple):**
  1. API `isInventoryValidate` never initializes `$this->totalWeight = 0` → weight accumulates incorrectly / null-add. **Bug.**
  2. API ignores variation inventories and backorder allow-zero logic → may wrongly reject/accept qty. **Parity gap.**
  3. Carrier key format differs (`smsa` vs `carriers.smsa`) → API carrier integration likely never matches → no tracking number generated. **Bug/parity gap.**
  4. API doesn't honor `shipping_Carrier_invalid` session flag.
- **Recommendation:** Extract to `ShipmentService`; match Blade exactly (totalWeight reset, variation + backorder logic, `carriers.*` keys, invalid-carrier handling).
- **Decision Needed?** No (clear Blade parity fixes).

### API-only
- `POST /sales/shipment/search` (`searchShipment`) — keep + document.

---

## Refund endpoints

### `POST /sales/refunds/orderItems/{order_id}` (store)
- **Current API:** `canRefund()` guard; validates `refund.items.*`; defaults `shipping=0`; computes totals; `maxRefundAmount`/`refundAmount` checks; creates refund. Message keys: `refunds.invalid-qty-error`, `refunds.invalid-amount-error`, `refunds.limit-error`.
- **Blade:** Same flow; **wraps `maxRefundAmount` calc in try/catch** → on failure flashes `refunds.creation-error`; message keys `refunds.invalid-refund-amount-error`, `refunds.refund-limit-error`.
- **Impact:** API lacks the try/catch guard around totals (possible 500 instead of clean 400); minor message-key differences.
- **Recommendation:** Add try/catch parity; align message keys where equivalents exist. Keep `{data,message}`.
- **Decision Needed?** No.

### API-only / gaps
- `POST /sales/refunds/{order_id}` (`fastRefund`) — API-only convenience, keep + document.
- `POST /sales/refund/search` (`refundSearch`) — keep + document.
- **Gap:** Blade `updateQty()` (refund totals preview) has no direct API route; API exposes `OrderController::refundCreate` instead. Document.

---

## Transaction endpoints

### `POST /sales/transactions` (store) — ⚠️ MAJOR DIVERGENCE
- **Current API:**
  - `$order = OrderRepository::find($invoice->order_id)` (correct order).
  - Saves `amount` into `transactionData`.
  - Sums **all** transactions for the invoice; if `total >= base_grand_total`: if a shipment exists → order `completed`, else `processing`; then invoice → `paid`.
  - Already-paid → 400; invoice missing → 400.
- **Blade:**
  - `$order = OrderRepository::find($invoice->id)` — **uses invoice id as order id (likely bug)**.
  - Does **not** save `amount` in `transactionData`.
  - If `base_grand_total == amount` (exact match only): order → `processing`, invoice → `paid`; **else** order → `pending`, invoice → `pending`.
  - No partial-sum accumulation; no shipment-based `completed`.
- **Impact:** Fundamentally different status outcomes and a probable Blade bug (`find($invoice->id)`). Strict Blade parity would be a **functional regression** (wrong order lookup, no partial payments, no completed state).
- **Recommendation:** Do **not** blindly mirror Blade here. Keep the API's correct `order_id` lookup and partial-sum logic; document as an intentional deviation (Blade bug). 
- **Decision Needed?** **YES — Q-S5 (blocking for Transaction only).**

### API-only
- `POST /sales/transactions/search` (`searchTransaction`) — keep + document.

---

## Questions raised in Phase 1

- **Q-S1:** `complete` — OK to remove the API's extra status guards (`completed/canceled/closed/must-be-shipped`) to match Blade, relying on repository result + adding activity logging? (Recommend: yes.)
- **Q-S2:** Add `company_id` scoping to single-resource `getResource` (orders/invoices/shipments/refunds/transactions) to prevent cross-tenant reads? Blade isn't explicitly scoped but is channel/guard bound. (Recommend: yes, scope it.)
- **Q-S3:** Keep the manual-order `POST /sales/orders/store` API contract as-is (it differs from Blade POS/edit UI flows)? (Recommend: keep + document.)
- **Q-S4:** Invoice `print` — match Blade's `print` view (`order`+`branch` context) or keep the API's simplified `pdf` HTML? (Recommend: keep API simplified, document.)
- **Q-S5 (blocking, Transaction only):** Transaction `store` — keep the API's corrected logic (order_id lookup, partial-payment sums, shipment-aware completed) instead of Blade's likely-buggy behavior? (Recommend: keep API logic, document Blade as buggy.)

Non-blocking items (Q-S1–Q-S4) have safe recommended defaults; I'll proceed on those unless you object. Q-S5 is the only true blocker before refactoring `TransactionController`.