diff --git a/katana_mcp_server/src/katana_mcp/resources/help.py b/katana_mcp_server/src/katana_mcp/resources/help.py index c54d64cf..cdfc1008 100644 --- a/katana_mcp_server/src/katana_mcp/resources/help.py +++ b/katana_mcp_server/src/katana_mcp/resources/help.py @@ -1483,6 +1483,18 @@ - `tracking_number`, `tracking_number_url`: Set if a carrier label is already known at creation time; otherwise patch in via `modify_sales_order.update_header.tracking_number`. +- `shipping_fees` (optional, list): Inline shipping fees to attach to the + new SO. Each entry: `amount` (decimal string, required, e.g. "8.95"), + `description` (label like "Standard shipping"), `tax_rate_id` (see + `list_tax_rates`). Eliminates the create-then-modify two-step that + every ecommerce-order workflow hits (#818). + - **Wire-level**: Katana's `POST /sales_orders` doesn't accept fees + inline, so the apply path creates the SO first and then fires + `POST /sales_order_shipping_fee` per fee. + - **Best-effort semantics**: SO success is durable. If any fee fails, + the response carries a warning and per-fee outcomes; the SO remains + intact. Retry failed fees via + `modify_sales_order(id=, add_shipping_fees=[...])`. **Ecommerce cross-references** (set when the SO mirrors an order from a storefront — Shopify, WooCommerce, etc.): @@ -1497,6 +1509,11 @@ **`preview`** (optional, default true): true=preview, false=create. +**Returns:** `SalesOrderResponse` with the standard SO fields plus +`shipping_fee_outcomes` (per-fee status rows when `shipping_fees` was +supplied — `succeeded=None` on preview; `succeeded=True/False` on apply +with `created_id` on success or `error` on failure). + --- ### list_sales_orders diff --git a/katana_mcp_server/src/katana_mcp/tools/foundation/sales_orders.py b/katana_mcp_server/src/katana_mcp/tools/foundation/sales_orders.py index 0aa1bddc..f79274be 100644 --- a/katana_mcp_server/src/katana_mcp/tools/foundation/sales_orders.py +++ b/katana_mcp_server/src/katana_mcp/tools/foundation/sales_orders.py @@ -50,6 +50,7 @@ ) from katana_mcp.tools.list_coercion import CoercedIntListOpt from katana_mcp.tools.tool_result_utils import ( + BLOCK_WARNING_PREFIX, UI_META, PaginationMeta, SoftDeletableResponse, @@ -216,6 +217,50 @@ class SalesOrderAddress(BaseModel): ) +class SOShippingFeeAdd(BaseModel): + """A new shipping fee to attach to a sales order. + + Shared between the ``create_sales_order`` inline-fees slot (#818) and + ``modify_sales_order.add_shipping_fees``. Defined at module top so it's + available to ``CreateSalesOrderRequest``; the modify path imports it + from the same location. + """ + + model_config = ConfigDict(extra="forbid") + + amount: str = Field(..., description="Fee amount (decimal string)") + description: str | None = Field( + default=None, + description="Customer-facing fee label (e.g., 'Standard shipping')", + ) + tax_rate_id: int | None = Field( + default=None, + description=("Tax rate ID. Look up via `list_tax_rates`."), + ) + + +class SOShippingFeeUpdate(BaseModel): + """Patch to an existing SO shipping fee. + + Note: Katana's API requires ``amount`` even on PATCH — it's a replace + semantic on the fee's amount, not a partial update. The other fields + are genuinely optional. + """ + + model_config = ConfigDict(extra="forbid") + + id: int = Field(..., description="Shipping fee ID to update") + amount: str = Field(..., description="Fee amount (required by API)") + description: str | None = Field( + default=None, + description="New customer-facing fee label", + ) + tax_rate_id: int | None = Field( + default=None, + description=("Tax rate ID. Look up via `list_tax_rates`."), + ) + + class CreateSalesOrderRequest(BaseModel): """Request to create a sales order.""" @@ -301,12 +346,69 @@ class CreateSalesOrderRequest(BaseModel): "field's configured type." ), ) + shipping_fees: list[SOShippingFeeAdd] | None = Field( + default=None, + description=( + "Optional shipping fees to attach to the new SO. Each entry: " + "`amount` (decimal string, required), `description` (label like " + "'Standard shipping'), `tax_rate_id` (see `list_tax_rates`). " + "Wire-level: Katana's `POST /sales_orders` does NOT accept " + "inline fees, so the apply path creates the SO first and then " + "fires `POST /sales_order_shipping_fee` per fee. Best-effort " + "semantics — if the SO succeeds but a fee fails, the response " + "carries a warning and the SO is preserved; retry the failed " + "fees via `modify_sales_order(id=, add_shipping_fees=[...])`." + ), + ) preview: bool = Field( default=True, description="If true (default), returns preview. If false, creates order.", ) +class ShippingFeeOutcome(BaseModel): + """Per-fee outcome row for ``create_sales_order``'s inline shipping fees. + + Used for both preview (planned fees with ``succeeded=None``) and apply + (per-fee results with ``succeeded=True``/``False``). The UI renders one + row per outcome — preview shows the planned amount/description, applied + swaps in an APPLIED/FAILED status pill. + + Best-effort semantics on apply: the parent SO can succeed while one or + more fees fail. Failed fees report ``error`` text; surviving rows can + be retried via ``modify_sales_order(id=, add_shipping_fees=[...])``. + """ + + description: str | None = Field( + default=None, + description="Customer-facing fee label, mirrors the request entry.", + ) + amount: str | None = Field( + default=None, + description="Fee amount (decimal string), mirrors the request entry.", + ) + tax_rate_id: int | None = Field( + default=None, + description="Tax rate ID associated with the fee, if any.", + ) + succeeded: bool | None = Field( + default=None, + description=( + "None on preview; True/False on apply. False indicates the SO " + "succeeded but this fee POST failed — retry via " + "`modify_sales_order(id=, add_shipping_fees=[...])`." + ), + ) + created_id: int | None = Field( + default=None, + description="Server-assigned shipping-fee ID on success.", + ) + error: str | None = Field( + default=None, + description="Error message when this fee failed to create.", + ) + + class SalesOrderResponse(BaseModel): """Response from creating a sales order.""" @@ -321,6 +423,15 @@ class SalesOrderResponse(BaseModel): delivery_date: str | None = None item_count: int | None = None is_preview: bool + shipping_fee_outcomes: list[ShippingFeeOutcome] = Field( + default_factory=list, + description=( + "Per-fee outcome rows when ``shipping_fees`` was supplied. " + "Preview: planned fees with ``succeeded=None``. Apply: per-fee " + "results with ``succeeded`` + ``created_id`` (on success) or " + "``error`` (on failure). Empty when no fees were requested." + ), + ) warnings: list[str] = Field( default_factory=list, description="Operator-facing warnings raised during the operation.", @@ -333,6 +444,93 @@ class SalesOrderResponse(BaseModel): katana_url: str | None = None +def _validate_shipping_fee_amounts( + fees: list[SOShippingFeeAdd], +) -> list[str]: + """Sanity-check shipping-fee ``amount`` strings on the request. + + Returns a list of ``BLOCK:``-prefixed warning strings — one per fee that + fails to parse as a non-negative decimal. The Katana wire shape carries + ``amount`` as a decimal string, so the simplest meaningful validation + we can do at request time is to confirm each amount actually parses. + + Note: ``SOShippingFeeAdd`` does not carry a per-fee currency field + (shipping fees inherit the parent SO's currency at the wire level), + so a "currency mismatch" check is structurally impossible on the + current schema. This guard catches the closely-related class of + request-shape bugs the wire-level POST would otherwise 422 on. + + ``Decimal`` constructor accepts "NaN" and "Infinity" as valid strings + (they raise ``InvalidOperation`` on subsequent arithmetic, not parse). + Explicitly reject non-finite values so the user gets a clear BLOCK + warning instead of letting the wire POST fail with a generic 422 + (or worse — a later comparison raising ``InvalidOperation``). + """ + from decimal import Decimal, InvalidOperation + + warnings: list[str] = [] + for idx, fee in enumerate(fees, start=1): + try: + parsed = Decimal(fee.amount) + except (InvalidOperation, ValueError): + warnings.append( + f"{BLOCK_WARNING_PREFIX} shipping_fees[{idx}].amount " + f"({fee.amount!r}) is not a valid decimal — Katana's " + f"POST /sales_order_shipping_fee requires a parseable " + f"decimal string." + ) + continue + if not parsed.is_finite(): + warnings.append( + f"{BLOCK_WARNING_PREFIX} shipping_fees[{idx}].amount " + f"({fee.amount!r}) is not a finite decimal (NaN/Infinity) " + f"— Katana's POST /sales_order_shipping_fee requires a " + f"finite decimal string." + ) + continue + try: + is_negative = parsed < 0 + except InvalidOperation: + # Defensive — by here ``parsed`` is finite, but ``< 0`` on a + # signaling NaN could still raise. Treat as invalid. + warnings.append( + f"{BLOCK_WARNING_PREFIX} shipping_fees[{idx}].amount " + f"({fee.amount!r}) could not be compared as a decimal — " + f"Katana's POST /sales_order_shipping_fee requires a " + f"comparable decimal string." + ) + continue + if is_negative: + warnings.append( + f"{BLOCK_WARNING_PREFIX} shipping_fees[{idx}].amount " + f"({fee.amount!r}) is negative — shipping fees must be " + f"non-negative." + ) + return warnings + + +def _outcomes_from_planned_fees( + fees: list[SOShippingFeeAdd], +) -> list[ShippingFeeOutcome]: + """Convert request-side ``SOShippingFeeAdd`` list to outcome rows. + + Used for both the preview (succeeded=None, no created_id/error) and + as the seed list for the apply path — each outcome's ``succeeded`` / + ``created_id`` / ``error`` is mutated in place as the per-fee POSTs + land. ``ShippingFeeOutcome`` intentionally omits ``ConfigDict(frozen= + True)`` to allow this in-place mutation; tests pin the mutation + behavior. + """ + return [ + ShippingFeeOutcome( + description=fee.description, + amount=fee.amount, + tax_rate_id=fee.tax_rate_id, + ) + for fee in fees + ] + + async def _create_sales_order_impl( request: CreateSalesOrderRequest, context: Context ) -> SalesOrderResponse: @@ -359,6 +557,8 @@ async def _create_sales_order_impl( for item in request.items ) + requested_fees = request.shipping_fees or [] + if request.preview: logger.info( f"Preview mode: SO {request.order_number} would have {len(request.items)} items" @@ -380,6 +580,11 @@ async def _create_sales_order_impl( warnings.append( "No delivery_date specified - order will have no delivery deadline" ) + # Validate shipping-fee amounts at preview time — surfacing a BLOCK + # warning gates the Confirm button so the user fixes the request + # before the wire-level fee POST would 422. + if requested_fees: + warnings.extend(_validate_shipping_fee_amounts(requested_fees)) return SalesOrderResponse( order_number=request.order_number, @@ -394,6 +599,7 @@ async def _create_sales_order_impl( else None, item_count=len(request.items), is_preview=True, + shipping_fee_outcomes=_outcomes_from_planned_fees(requested_fees), warnings=warnings, next_actions=[ "Review the order details", @@ -406,6 +612,40 @@ async def _create_sales_order_impl( try: services = get_services(context) + # Apply-path BLOCK validation: amounts must parse to non-negative + # decimals BEFORE we POST the SO. We don't roll back the SO on a + # later fee failure, so a malformed amount caught after the SO + # POST would leave a created SO + zero applied fees on the books. + # Preventing the SO POST is the only clean exit. (Same validation + # the preview path emits as a BLOCK warning to gate the Confirm + # button — an agent that called ``preview=False`` directly bypasses + # that gate, so we re-check here.) + # + # On failure we return ``is_preview=True`` (not False) so the card + # UI seeds ``state.applied=False`` and renders as a preview with + # BLOCK warnings disabling the Confirm button — visually matches + # the actual outcome (nothing was created). Returning + # ``is_preview=False`` would flip the card into the "CREATED" + # rendering despite no SO existing, which misleads the operator. + if requested_fees: + block_warnings = _validate_shipping_fee_amounts(requested_fees) + if block_warnings: + return SalesOrderResponse( + order_number=request.order_number, + customer_id=request.customer_id, + is_preview=True, + shipping_fee_outcomes=_outcomes_from_planned_fees(requested_fees), + warnings=block_warnings, + next_actions=[ + "Fix the failing shipping_fees amounts and retry", + ], + message=( + f"Sales order {request.order_number} was NOT created — " + f"one or more shipping_fees amounts failed validation. " + f"Fix the amounts and re-issue with preview=false." + ), + ) + # Build sales order rows so_rows = [] for item in request.items: @@ -494,6 +734,63 @@ async def _create_sales_order_impl( currency = unwrap_unset(so.currency, None) total = unwrap_unset(so.total, None) + # Inline shipping fees (#818). The wire endpoint POST /sales_orders + # does NOT accept fees inline, so we fire POST /sales_order_shipping_fee + # per fee against the just-created SO. Best-effort semantics: SO + # success is durable; per-fee failures surface as outcomes + a + # warning telling the user how to retry the failed fees via + # modify_sales_order(id=, add_shipping_fees=[...]). + fee_outcomes = _outcomes_from_planned_fees(requested_fees) + fee_warnings: list[str] = [] + # ``SalesOrder.id`` is non-optional in the generated model — ``unwrap_as`` + # raises if the wire shape is missing it, so by this point ``so.id`` is + # guaranteed populated; no need to guard. + if requested_fees: + for outcome, fee in zip(fee_outcomes, requested_fees, strict=True): + fee_body = _build_create_shipping_fee_request(so.id, fee) + try: + fee_resp = await api_create_so_shipping_fee.asyncio_detailed( + client=services.client, body=fee_body + ) + created_fee = unwrap_as(fee_resp, SalesOrderShippingFee) + except Exception as fee_exc: + logger.warning( + "create_sales_order shipping fee POST failed", + sales_order_id=so.id, + amount=fee.amount, + description=fee.description, + error=str(fee_exc), + ) + outcome.succeeded = False + outcome.error = str(fee_exc) + continue + outcome.succeeded = True + outcome.created_id = created_fee.id + + failed_count = sum(1 for o in fee_outcomes if o.succeeded is False) + if failed_count: + fee_warnings.append( + f"{failed_count} of {len(requested_fees)} shipping fee(s) " + f"failed to create on SO {so.id} — the sales order itself " + f"is preserved. Retry the failed fees via " + f"`modify_sales_order(id={so.id}, add_shipping_fees=[...])`." + ) + + next_actions = [ + f"Sales order created with ID {so.id}", + "Use fulfill_order to ship items when ready", + ] + if any(o.succeeded is False for o in fee_outcomes): + next_actions.append( + f"Retry failed shipping fees via " + f"modify_sales_order(id={so.id}, add_shipping_fees=[...])" + ) + + message = f"Successfully created sales order {so.order_no} (ID: {so.id})" + if requested_fees: + ok_count = sum(1 for o in fee_outcomes if o.succeeded is True) + message += f" with {ok_count}/{len(requested_fees)} shipping fee(s) applied" + return SalesOrderResponse( id=so.id, order_number=so.order_no, @@ -503,12 +800,11 @@ async def _create_sales_order_impl( total=total, currency=currency, is_preview=False, + shipping_fee_outcomes=fee_outcomes, + warnings=fee_warnings, katana_url=katana_web_url("sales_order", so.id), - next_actions=[ - f"Sales order created with ID {so.id}", - "Use fulfill_order to ship items when ready", - ], - message=f"Successfully created sales order {so.order_no} (ID: {so.id})", + next_actions=next_actions, + message=message, ) except Exception as e: @@ -1738,44 +2034,6 @@ class SOFulfillmentUpdate(BaseModel): ) -class SOShippingFeeAdd(BaseModel): - """A new shipping fee to attach to the SO.""" - - model_config = ConfigDict(extra="forbid") - - amount: str = Field(..., description="Fee amount (decimal string)") - description: str | None = Field( - default=None, - description="Customer-facing fee label (e.g., 'Standard shipping')", - ) - tax_rate_id: int | None = Field( - default=None, - description=("Tax rate ID. Look up via `list_tax_rates`."), - ) - - -class SOShippingFeeUpdate(BaseModel): - """Patch to an existing SO shipping fee. - - Note: Katana's API requires ``amount`` even on PATCH — it's a replace - semantic on the fee's amount, not a partial update. The other fields - are genuinely optional. - """ - - model_config = ConfigDict(extra="forbid") - - id: int = Field(..., description="Shipping fee ID to update") - amount: str = Field(..., description="Fee amount (required by API)") - description: str | None = Field( - default=None, - description="New customer-facing fee label", - ) - tax_rate_id: int | None = Field( - default=None, - description=("Tax rate ID. Look up via `list_tax_rates`."), - ) - - class ModifySalesOrderRequest(ConfirmableRequest): """Unified modification request for a sales order. diff --git a/katana_mcp_server/src/katana_mcp/tools/prefab_ui.py b/katana_mcp_server/src/katana_mcp/tools/prefab_ui.py index ecb7ae3b..b9da1e98 100644 --- a/katana_mcp_server/src/katana_mcp/tools/prefab_ui.py +++ b/katana_mcp_server/src/katana_mcp/tools/prefab_ui.py @@ -68,6 +68,7 @@ from __future__ import annotations +from decimal import Decimal, InvalidOperation from typing import TYPE_CHECKING, Any, Literal if TYPE_CHECKING: @@ -2457,14 +2458,18 @@ def _format_cost(cost: float | int | None) -> str: return f"{cost:.2f}" -def _format_money(amount: float | int | None, currency: str | None) -> str: +def _format_money(amount: float | int | Decimal | None, currency: str | None) -> str: """Format a Metric ``Total`` value using ISO 4217 currency-aware rules. Delegates to :func:`babel.numbers.format_currency` so the rendered string picks up the right symbol, decimal-digit count, and grouping for the currency (``$1,500.00`` for USD, ``€1,500.00`` for EUR, ``¥1,500`` for JPY with no decimals). Integer ``amount`` is passed through unchanged - — Babel handles ``int`` and ``float`` identically. + — Babel handles ``int``, ``float``, and ``Decimal`` identically (and + actually rounds exact-decimal sums correctly when passed as + :class:`~decimal.Decimal`, so call sites accumulating money should + pass Decimal here rather than coercing back to float and reintroducing + binary-representation drift). Katana has two currency concepts: @@ -4881,6 +4886,285 @@ def build_bom_modify_ui( return app +def _render_so_shipping_fees_section( + outcomes: list[dict[str, Any]], + *, + is_preview: bool, + currency: str | None, +) -> None: + """Render the inline shipping-fees sub-section for ``build_so_create_ui``. + + Layout: + - Section header with a Separator + ``Shipping fees`` muted label + (collapses fee block into a clearly demarcated Tier-3 sub-section). + - Preview rows (and apply rows still pending) — `` description: + amount`` (formatted via :func:`_format_money` with the SO currency), + tax_rate_id when present. Leading 2-char gutter reserves space so an + applied-state morph to ``✗`` doesn't reflow the row horizontally. + - Applied / success rows — same row text plus an ``APPLIED`` status + Badge (variant=default). + - Applied / failure rows — ``✗ `` glyph prefix on the row text plus a + ``FAILED`` Badge (variant=destructive); the per-row error message + tails the row text. When at least one fee failed, a destructive + Alert below the section coaches the operator toward retrying via + ``modify_sales_order(id=, add_shipping_fees=[...])``. + - No fees: caller skips this helper entirely. + + The ``✗`` failure glyph matches the convention from + :func:`_render_field_diff_line` (failure-row prefix; non-failed rows + reserve the 2-char gutter). The per-row ``APPLIED`` / ``FAILED`` Badge + is *additional* to that convention — diff lines on scalar-field modify + cards omit it because the card-header Badge already carries the signal, + but the shipping-fees section is a list of multiple parallel outcomes + so each one needs its own status pill. + + Monetary totals accumulate via :class:`~decimal.Decimal` for the same + reason the rest of this codebase uses Decimal for sums — float + addition can drift on common shipping amounts (``0.1 + 0.2`` etc.). + """ + if not outcomes: + return + + Separator() + Muted(content="Shipping fees:") + + failed_outcomes: list[dict[str, Any]] = [] + # Two running totals because preview vs. apply have different "what's + # in the total" semantics: + # - Preview: ``planned_total`` = sum of every parseable fee. There's + # no succeeded info yet; the total represents what the user will + # be charged if everything attaches. + # - Apply: ``succeeded_total`` = sum of fees that actually landed. + # Summing failed fees too would misrepresent the SO's final state + # — "$30 total" when only $20 attached is a wrong number, not a + # "planned" number. Render rule below switches between them. + planned_total = Decimal("0") + planned_parseable_count = 0 + succeeded_total = Decimal("0") + succeeded_parseable_count = 0 + succeeded_count = 0 + + for outcome in outcomes: + description = outcome.get("description") or "Shipping fee" + amount_str = outcome.get("amount") + tax_rate_id = outcome.get("tax_rate_id") + succeeded = outcome.get("succeeded") + error = outcome.get("error") + created_id = outcome.get("created_id") + + if succeeded is True: + succeeded_count += 1 + + # Parse the wire-shape decimal string into Decimal once so both + # the per-row label and the running totals format from the same + # exact value — passing ``float(amount_str)`` to ``_format_money`` + # would defeat the Decimal accumulation by reintroducing binary- + # representation drift in the per-row text (and the totals then + # disagree with the sum-of-displayed-rows the user can mentally + # verify). + # + # ``Decimal("NaN")`` / ``Decimal("Infinity")`` parse without + # raising but pollute the totals (NaN + Decimal → NaN) and would + # render as a garbage formatted-money string. Guard with + # ``is_finite()`` so non-finite values fall through to the + # raw-string label and are excluded from both totals. The + # validator emits BLOCK warnings for these on the request side, + # so they only reach the renderer when an agent bypassed preview + # — defensive belt + suspenders. + amount_dec: Decimal | None = None + if amount_str is not None: + try: + parsed = Decimal(str(amount_str)) + except (TypeError, ValueError, InvalidOperation): + parsed = None + if parsed is not None and parsed.is_finite(): + amount_dec = parsed + planned_total += amount_dec + planned_parseable_count += 1 + if succeeded is True: + succeeded_total += amount_dec + succeeded_parseable_count += 1 + amount_label = ( + _format_money(amount_dec, currency) + if amount_dec is not None + else (str(amount_str) if amount_str is not None else "—") + ) + + tax_suffix = f" · tax rate #{tax_rate_id}" if tax_rate_id is not None else "" + + if is_preview: + # Preview row: leading 2-char gutter matches the diff-line + # convention so an applied-state morph would shift to ``✗`` + # without reflowing the row text horizontally. + Text(content=f" {description}: {amount_label}{tax_suffix}") + continue + + if succeeded is None: + # Apply path with an outcome not yet resolved (defensive — the + # apply pipeline always sets succeeded True/False before + # returning, but the renderer shouldn't crash on a partially + # populated outcome dict). + Text(content=f" {description}: {amount_label}{tax_suffix}") + continue + + if succeeded is True: + id_suffix = f" (id={created_id})" if created_id is not None else "" + with Row(gap=2): + Text(content=f" {description}: {amount_label}{tax_suffix}{id_suffix}") + Badge(label="APPLIED", variant="default") + else: + failed_outcomes.append(outcome) + error_suffix = f" — {error}" if error else "" + with Row(gap=2): + Text( + content=f"✗ {description}: {amount_label}{tax_suffix}{error_suffix}" + ) + Badge(label="FAILED", variant="destructive") + + # Render the running total. Three rules cover the cases: + # + # 1. *Preview* / no apply outcomes yet — show the planned total when + # every fee parsed. A partial parse-sum would be misleading + # (BLOCK warnings surface the unparseable rows separately; + # showing "Total shipping: $X · 3 fee(s)" when only 2 of the 3 + # amounts contributed implies the plan totals to $X — it doesn't). + # + # 2. *Apply, all succeeded* — same shape as preview but the + # counter reads "applied" so the user knows the SO actually + # carries this total. + # + # 3. *Apply, partial failure* — show ONLY the succeeded subset so + # the number reflects what landed on the SO. Showing the planned + # total would misrepresent the SO's actual state ("$30 total" + # when only $20 attached is a wrong number, not a "planned" + # number). Label calls out the M-of-N shape explicitly. + # + # Pass Decimal directly to ``_format_money`` so the formatted total + # stays exact (no float-conversion rounding drift between the per-row + # labels and the total). + n_outcomes = len(outcomes) + failed_count = len(failed_outcomes) + if is_preview or failed_count == 0: + # Preview path or all-succeeded apply path: show planned/applied + # total when every parseable. + if planned_parseable_count and planned_parseable_count == n_outcomes: + total_label = _format_money(planned_total, currency) + counter = ( + f"{n_outcomes} fee(s)" if is_preview else f"{n_outcomes} fee(s) applied" + ) + Muted(content=f" Total shipping: {total_label} · {counter}") + elif succeeded_count and succeeded_parseable_count == succeeded_count: + # Partial failure: show only the succeeded subset, labeled + # explicitly so the user doesn't confuse it with the requested + # total. Skip entirely if no succeeded amounts parsed (rare). + total_label = _format_money(succeeded_total, currency) + Muted( + content=( + f" Total shipping applied: {total_label} · " + f"{succeeded_count} of {n_outcomes} fee(s) applied" + ) + ) + + if failed_outcomes and not is_preview: + with Alert(variant="destructive", icon="circle-alert"): + n = len(failed_outcomes) + AlertTitle( + content=( + f"{n} of {len(outcomes)} shipping fee(s) failed — " + f"sales order itself was created" + ) + ) + AlertDescription( + content=( + "Retry the failed fees via " + "modify_sales_order(id=, add_shipping_fees=[...]) — " + "the SO ID is shown above. Each failed fee preserves its " + "description / amount / tax_rate_id in the row above so " + "you can copy them straight into the modify call." + ) + ) + + +def _so_shipping_fees_apply_state( + outcomes: list[dict[str, Any]], + *, + so_id: int | None = None, +) -> dict[str, Any]: + """Pre-compute the apply-outcome state slots for the SO shipping + fees section so the preview→Confirm in-place morph can surface the + actual result. + + The build-time ``_render_so_shipping_fees_section`` paints rows + once at preview time and won't repaint on morph (Python-time + ``is_preview`` stays True under the iframe's morphed state). We + instead bind a state-driven ``If("applied")`` summary block to + these slots; the preview-side ``on_success`` chain SetStates them + from ``$result.state.*`` after the apply lands. + + Why state slots + ``$result.state.*`` references and not + ``$result.`` directly: ``$result`` in the on_success Rx + context resolves to the apply tool's wire-shape ``structured_content`` + — a PrefabApp envelope keyed by ``$prefab`` / ``view`` / ``state``, + NOT the raw ``SalesOrderResponse``. So the apply-time builder seeds + these slots into its own ``state.*``, and the preview iframe reads + off ``$result.state.*`` to morph (documented in + ``test_apply_button_morphs_card_to_applied_state``). + + Returns a dict with: + - ``applied_fees_summary`` — operator-facing one-liner. + - ``applied_fees_failed_count`` — int gating the failed-fees Alert. + - ``applied_fees_failed_summary`` — pre-formatted multi-line string + with ``Failed — : `` per failed fee, then a + retry-coaching line referencing the SO id so the operator knows + how to recover. The build-time + :func:`_render_so_shipping_fees_section` Alert carries the same + coaching but doesn't morph; embedding it here means the + preview→Confirm morph keeps the recovery instructions visible. + """ + total = len(outcomes) + succeeded = sum(1 for o in outcomes if o.get("succeeded") is True) + failed_outcomes = [o for o in outcomes if o.get("succeeded") is False] + failed = len(failed_outcomes) + if total == 0 or (succeeded == 0 and failed == 0): + # Preview path or no fees — empty summary; the If("applied") + # gating + If(Rx("applied_fees_failed_count") > 0) gating keep + # the block hidden until the apply lands. + return { + "applied_fees_summary": "", + "applied_fees_failed_count": 0, + "applied_fees_failed_summary": "", + } + if failed == 0: + summary = f"All {total} shipping fee(s) applied successfully." + elif succeeded == 0: + summary = f"0 of {total} shipping fee(s) applied — every fee failed." + else: + summary = f"{succeeded} of {total} shipping fee(s) applied — {failed} failed." + failed_lines: list[str] = [] + for o in failed_outcomes: + desc = o.get("description") or "Shipping fee" + err = o.get("error") or "unknown error" + failed_lines.append(f"Failed — {desc}: {err}") + if failed_lines: + # Retry coaching — referencing the literal SO id so the operator + # can copy/paste the modify call without bouncing through the + # response object. ``so_id is None`` falls back to a placeholder + # since the preview path doesn't have an SO yet (and won't have + # failed fees either, so the path is unreachable in practice). + so_ref = str(so_id) if so_id is not None else "" + failed_lines.append("") + failed_lines.append( + f"Retry the failed fee(s) via modify_sales_order(id={so_ref}, " + f"add_shipping_fees=[...]). Each failed row above preserves its " + f"description / amount / tax_rate_id — copy them straight in." + ) + return { + "applied_fees_summary": summary, + "applied_fees_failed_count": failed, + "applied_fees_failed_summary": "\n".join(failed_lines), + } + + def build_so_create_ui( response: dict[str, Any], *, @@ -4896,9 +5180,19 @@ def build_so_create_ui( ``status_badge_variant("sales_order", status)``). - Tier 2: Total, Line Items, Delivery date. - Tier 3: customer, location (ID-only — SO response has no - ``location_name``), warnings. + ``location_name``), inline shipping-fees sub-section (#818) when + ``shipping_fee_outcomes`` is non-empty, warnings, state-driven + apply-outcome summary block (visible after morph). - Tier 4: Confirm/Cancel (preview) or View in Katana + Fulfill Order (applied). + + Shipping fees (#818): the apply path creates the SO first, then + fires ``POST /sales_order_shipping_fee`` per fee. The card surfaces + each planned fee at preview time. After Confirm the iframe's + in-place morph reveals an ``If("applied")``-gated state-driven + summary block (per-fee badges still don't morph — the Python tree + is paint-once — but the operator-facing summary + failed-fee retry + coaching DO morph correctly via SetState from ``$result.state.*``). """ order_number = response.get("order_number") or "N/A" status = response.get("status") @@ -4906,12 +5200,49 @@ def build_so_create_ui( currency = response.get("currency") item_count = response.get("item_count") delivery_date = response.get("delivery_date") + is_preview = bool(response.get("is_preview", True)) + shipping_fee_outcomes: list[dict[str, Any]] = ( + response.get("shipping_fee_outcomes") or [] + ) + # Pass ``so_id`` so the apply-time failed-fees summary can embed the + # exact ``modify_sales_order(id=…, add_shipping_fees=[…])`` call for + # retry. On preview the response has no ``id`` yet (the SO isn't + # created) — falls back to ```` placeholder text, which is + # only reachable if there are also failed fees (impossible on the + # preview path). + applied_fees_state = _so_shipping_fees_apply_state( + shipping_fee_outcomes, + so_id=response.get("id"), + ) - apply_action = _build_apply_action(confirm_tool, confirm_request) + # The apply response's PrefabApp envelope carries these slots in + # ``state`` (initialized below); the preview iframe's on_success + # chain reads ``$result.state.*`` to morph the summary block. + apply_action = _build_apply_action( + confirm_tool, + confirm_request, + extra_on_success=[ + SetState( + "applied_fees_summary", + "{{ $result.state.applied_fees_summary }}", + ), + SetState( + "applied_fees_failed_count", + "{{ $result.state.applied_fees_failed_count }}", + ), + SetState( + "applied_fees_failed_summary", + "{{ $result.state.applied_fees_failed_summary }}", + ), + ], + ) cancel_action = _build_cancel_action("that sales order") + state = _init_create_card_state(response) + state.update(applied_fees_state) + with ( - PrefabApp(state=_init_create_card_state(response), css_class="p-4") as app, + PrefabApp(state=state, css_class="p-4") as app, Card(), ): _render_preview_header( @@ -4940,6 +5271,31 @@ def build_so_create_ui( name=None, # SalesOrderResponse has no location_name entity_id=response.get("location_id"), ) + _render_so_shipping_fees_section( + shipping_fee_outcomes, + is_preview=is_preview, + currency=currency, + ) + + # State-driven apply-outcome surface — visible after the + # in-place morph (the per-row badges painted by + # ``_render_so_shipping_fees_section`` above don't morph + # because they're built at Python time; the summary line + # + failed-fee Alert below DO morph via SetState). + with If("applied"): + Muted(content="{{ applied_fees_summary }}") + with ( + If(Rx("applied_fees_failed_count") > 0), + Alert(variant="destructive", icon="circle-alert"), + ): + AlertTitle( + content=( + "{{ applied_fees_failed_count }} shipping fee(s) " + "failed — sales order itself was created" + ) + ) + AlertDescription(content="{{ applied_fees_failed_summary }}") + block_warnings = _render_warnings_block(response.get("warnings")) _render_preview_footer( title_prefix="Sales Order", diff --git a/katana_mcp_server/tests/browser/render_test_server.py b/katana_mcp_server/tests/browser/render_test_server.py index 6f1e6e34..012dde73 100644 --- a/katana_mcp_server/tests/browser/render_test_server.py +++ b/katana_mcp_server/tests/browser/render_test_server.py @@ -36,6 +36,7 @@ build_modification_result_ui, build_product_bom_ui, build_search_results_ui, + build_so_create_ui, build_stock_adjustment_create_ui, build_stock_adjustment_delete_ui, build_stock_adjustment_update_ui, @@ -701,6 +702,47 @@ def _customer_create_response(*, is_preview: bool, with_addresses: bool) -> dict return base +def _so_create_response_with_shipping_fees( + *, is_preview: bool, fee_outcomes: list[dict] | None = None +) -> dict: + """Build a canned SalesOrderResponse dict for the SO-with-fees card. + + Used by the #818 browser scenarios to pin the inline-shipping-fees + rendering on the create_sales_order card across the preview / + all-success-apply / partial-failure states. + """ + base: dict = { + "id": None if is_preview else 9001, + "order_number": "SO-FEES-001", + "customer_id": 1501, + "customer_name": "Buyer Co", + "location_id": 1, + # ``status`` mirrors the real tool's two paths: preview emits + # ``PENDING`` (hardcoded — no SO exists yet), apply reads + # ``so.status.value`` from the just-created SO (which Katana + # stamps ``NOT_SHIPPED`` for a freshly-created order). The + # ternary reads in the same order as the impl for parity. + "status": "PENDING" if is_preview else "NOT_SHIPPED", + "total": 100.0, + "currency": "USD", + "delivery_date": "2026-06-01", + "item_count": 2, + "is_preview": is_preview, + "shipping_fee_outcomes": fee_outcomes or [], + "warnings": [], + "next_actions": [], + "message": ( + "Preview: Sales order SO-FEES-001 with 2 items totaling 100.00" + if is_preview + else "Successfully created sales order SO-FEES-001 (ID: 9001)" + ), + "katana_url": ( + None if is_preview else "https://factory.katanamrp.com/salesorder/9001" + ), + } + return base + + def _stock_adjustment_response(*, is_preview: bool, n_rows: int = 3) -> dict: """Build a canned StockAdjustmentResponse dict with N rows.""" rows = [ @@ -1035,6 +1077,93 @@ def _bom_modify_response(*, is_preview: bool, succeeded: bool | None) -> dict: confirm_request=_StubRequest(), confirm_tool="create_customer", ), + # SO create card with inline shipping fees (#818) — preview / all-success + # apply / partial-failure apply. Pins the per-fee row rendering, the + # APPLIED / FAILED status pills, and the destructive Alert on partial + # failure so a future regression (e.g. the rows-binding shape from #629) + # can't silently break the surface. + "so_create_with_fees_preview": lambda: build_so_create_ui( + _so_create_response_with_shipping_fees( + is_preview=True, + fee_outcomes=[ + { + "description": "Standard shipping", + "amount": "8.95", + "tax_rate_id": 301, + "succeeded": None, + "created_id": None, + "error": None, + }, + { + "description": "Handling", + "amount": "2.50", + "tax_rate_id": None, + "succeeded": None, + "created_id": None, + "error": None, + }, + ], + ), + confirm_request=_StubRequest(), + confirm_tool="create_sales_order", + ), + "so_create_with_fees_applied_all_success": lambda: build_so_create_ui( + _so_create_response_with_shipping_fees( + is_preview=False, + fee_outcomes=[ + { + "description": "Standard shipping", + "amount": "8.95", + "tax_rate_id": 301, + "succeeded": True, + "created_id": 5001, + "error": None, + }, + { + "description": "Handling", + "amount": "2.50", + "tax_rate_id": None, + "succeeded": True, + "created_id": 5002, + "error": None, + }, + ], + ), + confirm_request=_StubRequest(), + confirm_tool="create_sales_order", + ), + "so_create_with_fees_applied_partial_failure": lambda: build_so_create_ui( + { + **_so_create_response_with_shipping_fees( + is_preview=False, + fee_outcomes=[ + { + "description": "Standard shipping", + "amount": "8.95", + "tax_rate_id": None, + "succeeded": True, + "created_id": 5001, + "error": None, + }, + { + "description": "Handling", + "amount": "2.50", + "tax_rate_id": 9999, + "succeeded": False, + "created_id": None, + "error": "422: invalid tax rate id", + }, + ], + ), + "warnings": [ + "1 of 2 shipping fee(s) failed to create on SO 9001 — " + "the sales order itself is preserved. Retry the failed fees " + "via `modify_sales_order(id=9001, add_shipping_fees=[...])`." + ], + }, + confirm_request=_StubRequest(), + confirm_tool="create_sales_order", + ), "modify_item_single_preview": lambda: build_modification_preview_ui( { "entity_type": "product", diff --git a/katana_mcp_server/tests/browser/test_other_cards_render.py b/katana_mcp_server/tests/browser/test_other_cards_render.py index 820878b4..b1bbca6b 100644 --- a/katana_mcp_server/tests/browser/test_other_cards_render.py +++ b/katana_mcp_server/tests/browser/test_other_cards_render.py @@ -280,6 +280,74 @@ def test_variant_batch_renders(self, render_scenario): assert frame.locator("text=DOES-NOT-EXIST-1").count() >= 1 assert frame.locator("text=999999999").count() >= 1 + def test_so_create_with_fees_preview_renders(self, render_scenario): + """``build_so_create_ui`` with inline shipping fees on preview (#818). + + The card surfaces each planned fee with description / amount / + tax-rate suffix and a total-summary line. No APPLIED / FAILED + status pills on preview. The PREVIEW state header keeps the + Confirm button available (no BLOCK warnings). + """ + frame = render_scenario("so_create_with_fees_preview") + # Tier 1 — title + state. + assert frame.locator("text=Sales Order Preview").count() >= 1 + assert frame.locator("text=PREVIEW").count() >= 1 + # Shipping fees section header + per-fee rows. + assert frame.locator("text=Shipping fees:").count() >= 1 + assert frame.locator("text=Standard shipping").count() >= 1 + assert frame.locator("text=Handling").count() >= 1 + # Tax-rate suffix renders. + assert frame.locator("text=tax rate #301").count() >= 1 + # Summary line. + assert frame.locator("text=Total shipping").count() >= 1 + assert frame.locator("text=2 fee(s)").count() >= 1 + # Confirm available; no failure chrome. + assert frame.locator("text=Confirm & Create Sales Order").count() >= 1 + assert frame.locator("text=APPLIED").count() == 0 + assert frame.locator("text=FAILED").count() == 0 + + def test_so_create_with_fees_applied_all_success_renders(self, render_scenario): + """``build_so_create_ui`` applied state with every fee succeeded. + + Each row gains the APPLIED pill + the server-assigned id surfaces + (id=5001 / id=5002). No retry coaching since nothing failed. + """ + frame = render_scenario("so_create_with_fees_applied_all_success") + # Tier 1 — applied chrome. + assert frame.locator("text=Sales Order Created").count() >= 1 + # APPLIED pills, one per fee. + assert frame.locator("text=APPLIED").count() >= 2 + # Server-assigned ids surface. + assert frame.locator("text=id=5001").count() >= 1 + assert frame.locator("text=id=5002").count() >= 1 + # View-in-Katana + Fulfill button rail. + assert frame.locator("text=View in Katana").count() >= 1 + assert frame.locator("text=Fulfill Order").count() >= 1 + # No retry-coaching Alert. + assert frame.locator("text=Retry the failed fees").count() == 0 + + def test_so_create_with_fees_applied_partial_failure_renders(self, render_scenario): + """Partial failure: SO created + one fee succeeded + one fee failed. + + Pins the per-fee status pill mix (APPLIED + FAILED) and the + destructive Alert at the bottom coaching the operator toward + ``modify_sales_order(id=, add_shipping_fees=[...])``. + """ + frame = render_scenario("so_create_with_fees_applied_partial_failure") + # Tier 1 — applied chrome (the SO itself succeeded). + assert frame.locator("text=Sales Order Created").count() >= 1 + # Mixed status pills + inline error text. + assert frame.locator("text=APPLIED").count() >= 1 + assert frame.locator("text=FAILED").count() >= 1 + assert frame.locator("text=422: invalid tax rate id").count() >= 1 + # Destructive Alert with retry coaching at the bottom of the + # shipping-fees section. + assert frame.locator("text=shipping fee(s) failed").count() >= 1 + assert frame.locator("text=modify_sales_order").count() >= 1 + assert frame.locator("text=add_shipping_fees").count() >= 1 + # Partial-failure warning surfaces as a regular (non-BLOCK) Badge. + assert frame.locator("text=1 of 2 shipping fee(s) failed").count() >= 1 + class TestDataTableRowClickBinding: """Verify DataTable ``onRowClick`` ``{{ field }}`` per-row bindings diff --git a/katana_mcp_server/tests/test_prefab_ui.py b/katana_mcp_server/tests/test_prefab_ui.py index 019b8c62..96dd466b 100644 --- a/katana_mcp_server/tests/test_prefab_ui.py +++ b/katana_mcp_server/tests/test_prefab_ui.py @@ -1874,6 +1874,179 @@ def test_customer_renders_as_link_to_katana(self): assert "/contacts/customers/1" in rendered assert "Buyer Co" in rendered + def test_inline_shipping_fees_render_preview(self): + """Preview state: planned shipping fees surface as rows with + description / amount / tax. Section header + total summary line + appear. No APPLIED / FAILED status pills (those are apply-only). + """ + response = dict( + self._SO_RESPONSE, + shipping_fee_outcomes=[ + { + "description": "Standard shipping", + "amount": "8.95", + "tax_rate_id": 301, + "succeeded": None, + "created_id": None, + "error": None, + }, + { + "description": "Handling", + "amount": "2.50", + "tax_rate_id": None, + "succeeded": None, + "created_id": None, + "error": None, + }, + ], + ) + app = build_so_create_ui( + response, + confirm_request=_StubRequest(), + confirm_tool="create_sales_order", + ) + rendered = str(app.to_json()) + # Section header + per-fee rows show up. + assert "Shipping fees" in rendered + assert "Standard shipping" in rendered + assert "Handling" in rendered + # Tax-rate suffix surfaces when present. + assert "tax rate #301" in rendered + # Summary line surfaces the total + count. + assert "Total shipping" in rendered + assert "2 fee(s)" in rendered + # No status pills on preview. + assert "APPLIED" not in rendered + assert "FAILED" not in rendered + + def test_inline_shipping_fees_render_applied_success(self): + """Applied + all-success: each fee row gains an APPLIED pill + + the server-assigned created_id surfaces.""" + response = dict( + self._SO_RESPONSE, + is_preview=False, + id=99, + katana_url="https://factory.katanamrp.com/salesorder/99", + shipping_fee_outcomes=[ + { + "description": "Standard shipping", + "amount": "8.95", + "tax_rate_id": None, + "succeeded": True, + "created_id": 5001, + "error": None, + }, + ], + ) + app = build_so_create_ui( + response, + confirm_request=_StubRequest(), + confirm_tool="create_sales_order", + ) + rendered = str(app.to_json()) + assert "APPLIED" in rendered + assert "id=5001" in rendered + # No failure retry coaching on a clean run. + assert "modify_sales_order" not in rendered + + def test_inline_shipping_fees_render_applied_partial_failure(self): + """Partial failure: failed rows get the FAILED pill + ✗ glyph + + inline error text, and a destructive Alert at the bottom coaches + the operator toward retrying via modify_sales_order.""" + response = dict( + self._SO_RESPONSE, + is_preview=False, + id=99, + katana_url="https://factory.katanamrp.com/salesorder/99", + shipping_fee_outcomes=[ + { + "description": "Standard shipping", + "amount": "8.95", + "tax_rate_id": None, + "succeeded": True, + "created_id": 5001, + "error": None, + }, + { + "description": "Handling", + "amount": "2.50", + "tax_rate_id": 9999, + "succeeded": False, + "created_id": None, + "error": "422: invalid tax rate", + }, + ], + ) + app = build_so_create_ui( + response, + confirm_request=_StubRequest(), + confirm_tool="create_sales_order", + ) + rendered = str(app.to_json()) + assert "APPLIED" in rendered + assert "FAILED" in rendered + assert "✗" in rendered + assert "422: invalid tax rate" in rendered + # Retry coaching surfaces in the Alert. + assert "modify_sales_order" in rendered + assert "add_shipping_fees" in rendered + # The bottom-line total reflects ONLY succeeded fees, not the + # full requested set — currency is EUR per ``_SO_RESPONSE``, + # so the formatted strings appear with the € symbol. Showing + # the full €11.45 when only €8.95 attached would misrepresent + # the SO's actual state. + assert "Total shipping applied" in rendered + assert "1 of 2 fee(s) applied" in rendered + assert "€11.45" not in rendered + assert "8.95" in rendered # the succeeded fee's amount + + def test_inline_shipping_fees_render_applied_all_failed(self): + """Every fee failed: the SO exists but no fees attached. The + total line is skipped entirely (no succeeded fees → nothing to + sum); the failed-row Alert below still surfaces.""" + response = dict( + self._SO_RESPONSE, + is_preview=False, + id=99, + katana_url="https://factory.katanamrp.com/salesorder/99", + shipping_fee_outcomes=[ + { + "description": "Standard shipping", + "amount": "8.95", + "succeeded": False, + "error": "500 upstream", + }, + { + "description": "Handling", + "amount": "2.50", + "succeeded": False, + "error": "422 invalid tax", + }, + ], + ) + app = build_so_create_ui( + response, + confirm_request=_StubRequest(), + confirm_tool="create_sales_order", + ) + rendered = str(app.to_json()) + assert "FAILED" in rendered + # No total line — every fee failed, nothing landed on the SO. + assert "Total shipping" not in rendered + # Retry coaching still surfaces. + assert "modify_sales_order" in rendered + + def test_inline_shipping_fees_section_skipped_when_no_fees(self): + """No shipping_fee_outcomes → the section is omitted entirely.""" + app = build_so_create_ui( + dict(self._SO_RESPONSE, shipping_fee_outcomes=[]), + confirm_request=_StubRequest(), + confirm_tool="create_sales_order", + ) + rendered = str(app.to_json()) + assert "Shipping fees" not in rendered + assert "Total shipping" not in rendered + class TestBuildMOCreateUI: """``build_mo_create_ui`` — MO response uses ``order_no`` (not diff --git a/katana_mcp_server/tests/tools/test_sales_orders.py b/katana_mcp_server/tests/tools/test_sales_orders.py index d86feed7..36a67a7c 100644 --- a/katana_mcp_server/tests/tools/test_sales_orders.py +++ b/katana_mcp_server/tests/tools/test_sales_orders.py @@ -16,6 +16,7 @@ SalesOrderItem, SOHeaderPatch, SORowAdd, + SOShippingFeeAdd, _create_sales_order_impl, _delete_sales_order_impl, _get_sales_order_impl, @@ -564,6 +565,467 @@ async def test_create_sales_order_confirm_with_minimal_fields(): create_so_module.asyncio_detailed = original_asyncio_detailed +# ============================================================================ +# Inline shipping-fees on create (#818) +# ============================================================================ + + +@pytest.mark.asyncio +async def test_create_sales_order_preview_with_shipping_fees(): + """Preview surfaces planned shipping fees on the response. + + The card binds to ``shipping_fee_outcomes`` — each planned fee must + land with ``succeeded=None`` (preview), no ``created_id`` / ``error``, + and the description/amount/tax_rate_id mirroring the request. + """ + context, lifespan_ctx = create_mock_context() + lifespan_ctx.typed_cache.catalog.get_by_id = AsyncMock( + return_value={"id": 1501, "name": "Acme"} + ) + + request = CreateSalesOrderRequest( + customer_id=1501, + order_number="SO-FEE-PREV-001", + items=[SalesOrderItem(variant_id=2101, quantity=1, price_per_unit=100.0)], + location_id=1, + delivery_date=datetime(2026, 6, 1, tzinfo=UTC), + currency="USD", + shipping_fees=[ + SOShippingFeeAdd( + amount="8.95", description="Standard shipping", tax_rate_id=301 + ), + SOShippingFeeAdd(amount="2.50", description="Handling"), + ], + preview=True, + ) + result = await _create_sales_order_impl(request, context) + + assert result.is_preview is True + assert len(result.shipping_fee_outcomes) == 2 + first, second = result.shipping_fee_outcomes + assert first.description == "Standard shipping" + assert first.amount == "8.95" + assert first.tax_rate_id == 301 + assert first.succeeded is None + assert first.created_id is None + assert first.error is None + assert second.description == "Handling" + assert second.amount == "2.50" + assert second.tax_rate_id is None + assert second.succeeded is None + # No BLOCK warnings — both amounts parse cleanly. + assert not any(w.startswith("BLOCK:") for w in result.warnings) + + +@pytest.mark.asyncio +async def test_create_sales_order_apply_with_shipping_fees(): + """Apply path creates the SO then each shipping fee in sequence. + + On full success: each outcome carries ``succeeded=True`` plus the + server-assigned ``created_id``, and the response message reflects + the all-ok ratio. + """ + from katana_public_api_client.models import SalesOrderShippingFee + + context, _lifespan_ctx = create_mock_context() + + mock_so = SalesOrder( + id=3001, + customer_id=1501, + order_no="SO-FEE-OK-001", + location_id=1, + status=SalesOrderStatus.NOT_SHIPPED, + currency="USD", + total=100.0, + ) + mock_so_response = MagicMock() + mock_so_response.status_code = 200 + mock_so_response.parsed = mock_so + + fee_a = SalesOrderShippingFee( + id=5001, sales_order_id=3001, amount="8.95", description="Standard shipping" + ) + fee_b = SalesOrderShippingFee( + id=5002, sales_order_id=3001, amount="2.50", description="Handling" + ) + fee_responses = [] + for fee in (fee_a, fee_b): + resp = MagicMock() + resp.status_code = 200 + resp.parsed = fee + fee_responses.append(resp) + + mock_create_so = AsyncMock(return_value=mock_so_response) + mock_create_fee = AsyncMock(side_effect=fee_responses) + + import katana_public_api_client.api.sales_order.create_sales_order as so_module + import katana_public_api_client.api.sales_orders.create_sales_order_shipping_fee as fee_module + + so_orig = so_module.asyncio_detailed + fee_orig = fee_module.asyncio_detailed + cast(Any, so_module).asyncio_detailed = mock_create_so + cast(Any, fee_module).asyncio_detailed = mock_create_fee + + try: + request = CreateSalesOrderRequest( + customer_id=1501, + order_number="SO-FEE-OK-001", + items=[SalesOrderItem(variant_id=2101, quantity=1, price_per_unit=100.0)], + currency="USD", + shipping_fees=[ + SOShippingFeeAdd(amount="8.95", description="Standard shipping"), + SOShippingFeeAdd(amount="2.50", description="Handling"), + ], + preview=False, + ) + result = await _create_sales_order_impl(request, context) + finally: + cast(Any, so_module).asyncio_detailed = so_orig + cast(Any, fee_module).asyncio_detailed = fee_orig + + assert result.is_preview is False + assert result.id == 3001 + assert len(result.shipping_fee_outcomes) == 2 + first, second = result.shipping_fee_outcomes + assert first.succeeded is True + assert first.created_id == 5001 + assert first.error is None + assert second.succeeded is True + assert second.created_id == 5002 + # Each fee POST received the parent SO id. + assert mock_create_fee.call_count == 2 + for call in mock_create_fee.call_args_list: + assert call.kwargs["body"].sales_order_id == 3001 + # No best-effort warning emitted on a clean run. + assert not any("failed to create" in w for w in result.warnings) + assert "2/2 shipping fee(s) applied" in result.message + + +@pytest.mark.asyncio +async def test_create_sales_order_apply_partial_shipping_fee_failure(): + """SO success + 1 fee success + 1 fee failure → best-effort partial. + + The SO stays created. The failing fee carries ``succeeded=False`` + + ``error``; the surviving fee carries ``succeeded=True`` + a + ``created_id``. A non-BLOCK warning surfaces the partial-failure + summary with the retry hint pointing at ``modify_sales_order``. + """ + from katana_public_api_client.models import SalesOrderShippingFee + + context, _lifespan_ctx = create_mock_context() + + mock_so = SalesOrder( + id=3002, + customer_id=1501, + order_no="SO-FEE-PART-001", + location_id=1, + status=SalesOrderStatus.NOT_SHIPPED, + currency="USD", + ) + mock_so_response = MagicMock() + mock_so_response.status_code = 200 + mock_so_response.parsed = mock_so + + fee_a = SalesOrderShippingFee( + id=5003, sales_order_id=3002, amount="8.95", description="Standard shipping" + ) + success_resp = MagicMock() + success_resp.status_code = 200 + success_resp.parsed = fee_a + + mock_create_so = AsyncMock(return_value=mock_so_response) + # First fee POST succeeds; second raises a typed exception. + mock_create_fee = AsyncMock( + side_effect=[success_resp, APIError("422: invalid tax rate", 422)] + ) + + import katana_public_api_client.api.sales_order.create_sales_order as so_module + import katana_public_api_client.api.sales_orders.create_sales_order_shipping_fee as fee_module + + so_orig = so_module.asyncio_detailed + fee_orig = fee_module.asyncio_detailed + cast(Any, so_module).asyncio_detailed = mock_create_so + cast(Any, fee_module).asyncio_detailed = mock_create_fee + + try: + request = CreateSalesOrderRequest( + customer_id=1501, + order_number="SO-FEE-PART-001", + items=[SalesOrderItem(variant_id=2101, quantity=1, price_per_unit=100.0)], + currency="USD", + shipping_fees=[ + SOShippingFeeAdd(amount="8.95", description="Standard shipping"), + SOShippingFeeAdd( + amount="2.50", description="Handling", tax_rate_id=9999 + ), + ], + preview=False, + ) + result = await _create_sales_order_impl(request, context) + finally: + cast(Any, so_module).asyncio_detailed = so_orig + cast(Any, fee_module).asyncio_detailed = fee_orig + + # SO succeeded and stays in the response — best-effort, not rollback. + assert result.is_preview is False + assert result.id == 3002 + assert len(result.shipping_fee_outcomes) == 2 + first, second = result.shipping_fee_outcomes + assert first.succeeded is True + assert first.created_id == 5003 + assert second.succeeded is False + assert second.created_id is None + assert second.error is not None + assert "invalid tax rate" in second.error + # Partial-failure summary warning is informational (no BLOCK prefix — + # the SO already exists, there's nothing to gate). + assert any( + "1 of 2 shipping fee(s) failed" in w and not w.startswith("BLOCK:") + for w in result.warnings + ) + # Retry coaching surfaces the SO id and the modify path. + assert any( + f"modify_sales_order(id={result.id}, add_shipping_fees=[...])" in action + for action in result.next_actions + ) + assert "1/2 shipping fee(s) applied" in result.message + + +@pytest.mark.asyncio +async def test_create_sales_order_apply_all_shipping_fees_fail(): + """SO success + every fee failure → 0/N applied, SO still preserved. + + Symmetric to the partial-failure case. Pins the all-fail message + shape (``0/2 shipping fee(s) applied``), the ``next_actions`` retry + coaching, and the consolidated warning's framing — the SO id stays + available so the operator can re-issue all fees via + ``modify_sales_order(id=, add_shipping_fees=[...])`` without + re-creating the SO. + """ + context, _lifespan_ctx = create_mock_context() + + mock_so = SalesOrder( + id=3003, + customer_id=1501, + order_no="SO-FEE-ALLFAIL-001", + location_id=1, + status=SalesOrderStatus.NOT_SHIPPED, + currency="USD", + ) + mock_so_response = MagicMock() + mock_so_response.status_code = 200 + mock_so_response.parsed = mock_so + + mock_create_so = AsyncMock(return_value=mock_so_response) + # Both fee POSTs raise — independent failures so the impl runs through + # every fee rather than short-circuiting. + mock_create_fee = AsyncMock( + side_effect=[ + APIError("500: upstream timeout", 500), + APIError("422: bad tax rate id", 422), + ] + ) + + import katana_public_api_client.api.sales_order.create_sales_order as so_module + import katana_public_api_client.api.sales_orders.create_sales_order_shipping_fee as fee_module + + so_orig = so_module.asyncio_detailed + fee_orig = fee_module.asyncio_detailed + cast(Any, so_module).asyncio_detailed = mock_create_so + cast(Any, fee_module).asyncio_detailed = mock_create_fee + + try: + request = CreateSalesOrderRequest( + customer_id=1501, + order_number="SO-FEE-ALLFAIL-001", + items=[SalesOrderItem(variant_id=2101, quantity=1, price_per_unit=100.0)], + currency="USD", + shipping_fees=[ + SOShippingFeeAdd(amount="8.95", description="Standard shipping"), + SOShippingFeeAdd( + amount="2.50", description="Handling", tax_rate_id=9999 + ), + ], + preview=False, + ) + result = await _create_sales_order_impl(request, context) + finally: + cast(Any, so_module).asyncio_detailed = so_orig + cast(Any, fee_module).asyncio_detailed = fee_orig + + # SO durability guarantee: SO succeeded despite every fee failing. + assert result.is_preview is False + assert result.id == 3003 + assert len(result.shipping_fee_outcomes) == 2 + # Each outcome carries succeeded=False + error. + assert all(o.succeeded is False for o in result.shipping_fee_outcomes) + assert all(o.error is not None for o in result.shipping_fee_outcomes) + assert all(o.created_id is None for o in result.shipping_fee_outcomes) + # The consolidated warning counts all fees as failed. + assert any( + "2 of 2 shipping fee(s) failed" in w and not w.startswith("BLOCK:") + for w in result.warnings + ) + # Retry coaching still surfaces the SO id. + assert any( + f"modify_sales_order(id={result.id}, add_shipping_fees=[...])" in action + for action in result.next_actions + ) + # Message frames it as 0/N applied — the SO id stays in the prefix so + # the operator can copy-paste the retry call. + assert "0/2 shipping fee(s) applied" in result.message + assert str(result.id) in result.message + + +@pytest.mark.asyncio +async def test_create_sales_order_apply_blocks_on_invalid_amount(): + """Apply path validates fee amounts BEFORE the SO POST. + + An agent that bypasses preview (calls with ``preview=False`` directly) + and supplies a malformed ``amount`` would otherwise leave a created + SO + zero applied fees on the books (no rollback on best-effort). + The apply path runs the same ``_validate_shipping_fee_amounts`` gate + the preview path emits as a BLOCK warning. On failure the SO POST is + never issued — the response carries BLOCK warnings, ``id=None``, and + ``is_preview=True`` (so the card UI seeds ``state.applied=False`` and + renders as a preview with BLOCK warnings disabling the Confirm button + — matches the actual outcome: nothing was created). + """ + context, _lifespan_ctx = create_mock_context() + + # SO POST mock — if validation runs first, this should never be called. + mock_create_so = AsyncMock() + mock_create_fee = AsyncMock() + + import katana_public_api_client.api.sales_order.create_sales_order as so_module + import katana_public_api_client.api.sales_orders.create_sales_order_shipping_fee as fee_module + + so_orig = so_module.asyncio_detailed + fee_orig = fee_module.asyncio_detailed + cast(Any, so_module).asyncio_detailed = mock_create_so + cast(Any, fee_module).asyncio_detailed = mock_create_fee + + try: + request = CreateSalesOrderRequest( + customer_id=1501, + order_number="SO-FEE-BADAMT-001", + items=[SalesOrderItem(variant_id=2101, quantity=1, price_per_unit=100.0)], + currency="USD", + shipping_fees=[ + SOShippingFeeAdd(amount="garbage", description="Bad amount"), + ], + preview=False, # bypassing the preview gate + ) + result = await _create_sales_order_impl(request, context) + finally: + cast(Any, so_module).asyncio_detailed = so_orig + cast(Any, fee_module).asyncio_detailed = fee_orig + + # Critically: the SO POST never fired, so nothing was created. + mock_create_so.assert_not_awaited() + mock_create_fee.assert_not_awaited() + # Response shape signals the block clearly. is_preview=True (not + # False) so the card UI seeds state.applied=False and renders as a + # preview with BLOCK warnings disabling the Confirm button — that + # matches the actual outcome (nothing was created). + assert result.is_preview is True + assert result.id is None + block_warnings = [w for w in result.warnings if w.startswith("BLOCK:")] + assert len(block_warnings) == 1 + assert "not a valid decimal" in block_warnings[0] + # Planned outcomes still surface so the UI can show the row the user + # has to fix. + assert len(result.shipping_fee_outcomes) == 1 + assert "NOT created" in result.message + + +@pytest.mark.asyncio +async def test_create_sales_order_preview_nan_infinity_amounts_blocked(): + """Python's ``Decimal`` constructor accepts ``"NaN"`` and ``"Infinity"`` + as valid strings — they don't raise on parse, only on subsequent + arithmetic. The validator must explicitly reject non-finite values + so the user sees a clear BLOCK warning instead of letting them flow + through to a wire 422 (or worse, ``InvalidOperation`` on the + negativity check). + """ + context, lifespan_ctx = create_mock_context() + lifespan_ctx.typed_cache.catalog.get_by_id = AsyncMock( + return_value={"id": 1501, "name": "Acme"} + ) + + request = CreateSalesOrderRequest( + customer_id=1501, + order_number="SO-FEE-NONFINITE-001", + items=[SalesOrderItem(variant_id=2101, quantity=1, price_per_unit=100.0)], + location_id=1, + delivery_date=datetime(2026, 6, 1, tzinfo=UTC), + currency="USD", + shipping_fees=[ + SOShippingFeeAdd(amount="NaN", description="NaN amount"), + SOShippingFeeAdd(amount="Infinity", description="Infinite amount"), + SOShippingFeeAdd(amount="-Infinity", description="Negative infinity"), + ], + preview=True, + ) + result = await _create_sales_order_impl(request, context) + + assert result.is_preview is True + block_warnings = [w for w in result.warnings if w.startswith("BLOCK:")] + # All three non-finite values get BLOCKed with the "not a finite + # decimal" message — distinct from the unparseable / negative buckets. + assert len(block_warnings) == 3 + assert all("not a finite decimal" in w for w in block_warnings) + + +@pytest.mark.asyncio +async def test_create_sales_order_preview_invalid_fee_amount_blocks_confirm(): + """Unparseable / negative fee amounts emit BLOCK warnings. + + ``SOShippingFeeAdd.amount`` is a decimal string on the wire — the + request schema can't validate the format at parse time, so the impl + layer sanity-checks before exposing the Confirm button. Each + invalid fee gets a ``BLOCK:``-prefixed warning that the UI builder + strips + renders as a destructive Badge, gating the Confirm button. + + Note: ``SOShippingFeeAdd`` carries no currency field (shipping fees + inherit the parent SO's currency at the wire level), so a literal + "currency mismatch" check is structurally impossible against the + current schema. This test pins the closely-related amount-shape + validation that the wire-level fee POST would otherwise 422 on. + """ + context, lifespan_ctx = create_mock_context() + lifespan_ctx.typed_cache.catalog.get_by_id = AsyncMock( + return_value={"id": 1501, "name": "Acme"} + ) + + request = CreateSalesOrderRequest( + customer_id=1501, + order_number="SO-FEE-BAD-001", + items=[SalesOrderItem(variant_id=2101, quantity=1, price_per_unit=100.0)], + location_id=1, + delivery_date=datetime(2026, 6, 1, tzinfo=UTC), + currency="USD", + shipping_fees=[ + SOShippingFeeAdd(amount="not a number", description="Garbage amount"), + SOShippingFeeAdd(amount="-1.00", description="Negative shipping"), + SOShippingFeeAdd(amount="9.95", description="Legitimate fee"), + ], + preview=True, + ) + result = await _create_sales_order_impl(request, context) + + assert result.is_preview is True + block_warnings = [w for w in result.warnings if w.startswith("BLOCK:")] + # Two BLOCKs: one for unparseable, one for negative. The legitimate + # fee passes through. + assert len(block_warnings) == 2 + assert any("not a valid decimal" in w for w in block_warnings) + assert any("negative" in w for w in block_warnings) + # All planned fees still surface in outcomes so the UI can render the + # row that the operator needs to fix. + assert len(result.shipping_fee_outcomes) == 3 + + # ============================================================================ # Validation Tests # ============================================================================ diff --git a/uv.lock b/uv.lock index 49567481..630e5eb9 100644 --- a/uv.lock +++ b/uv.lock @@ -1329,7 +1329,7 @@ wheels = [ [[package]] name = "katana-mcp-server" -version = "0.94.0" +version = "0.96.0" source = { editable = "katana_mcp_server" } dependencies = [ { name = "aiosqlite" },