Skip to content

docs: order-management feature README + strip internal GUS refs @W-22821845 - #3907

Merged
sf-jie-dai merged 6 commits into
feature/264-order-managementfrom
jie.dai-W-22821845-stripInternalRefs
Jun 30, 2026
Merged

docs: order-management feature README + strip internal GUS refs @W-22821845#3907
sf-jie-dai merged 6 commits into
feature/264-order-managementfrom
jie.dai-W-22821845-stripInternalRefs

Conversation

@sf-jie-dai

Copy link
Copy Markdown
Contributor

What

Two documentation/hygiene changes for the order-management feature (returns, cancellation, tracking) on the 264 epic branch.

Changes

1. Strip internal GUS work-item references

The repo is customer-facing, so internal GUS W-/WI- identifiers don't belong in shipped comments or test titles. This removes them from the return-feature comments (return-utils.js, return-error-utils.js, return-items-modal/{index.jsx,index.test.js,constants.js}, order-detail.jsx, order-status-utils.js) and from orders.test.js describe/test titles, rewording each to describe the behavior instead of the work item. No behavior or test-coverage change — titles and comments only.

2. Add a feature-branch README

New packages/template-retail-react-app/docs/README-ORDER-MANAGEMENT.md, an overview of the three OMS shopper actions on the order detail page, in the style of storefront-next's README-SHOPPER-CONTEXT.md:

  • Returns — data-driven eligibility (isRegistered && ownsOrder + !!order.omsData, no feature flag), the request lifecycle (getReturnableItemsuseOmsMetaData reasons → buildReturnProductItemsReturnOmsOrder), OMS-driven reason codes, and the ReturnErrorKind error-code contract.
  • Cancellation — all-or-nothing eligibility (quantityAvailableToCancel === quantityOrdered across all items), the CancelOmsOrder mutation, and terminal 404/409 handling.
  • Tracking — carrier-link hardening via ensureExternalUrl, the epoch date-guard, and the flat-list / no-address multi-shipment scope (grouped-by-address deferred to a TD pending correlated SCAPI data; see feat: order-detail tracking cards + Track Shipment action @W-23091033 #3906).
  • Status badge — the shared per-unit aggregation in getOrderDisplayStatus / getItemUnitBuckets.

Testing

Comments-and-docs only; no runtime code touched. The existing account-area Jest suites remain green (only describe/test string literals changed in orders.test.js).

🤖 Generated with Claude Code

…t titles

The template repo is customer-facing; internal W-/WI- numbers carry no
meaning for external consumers. Strip them from return-feature code comments
and Jest describe/test titles, preserving the surrounding explanations.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@sf-jie-dai
sf-jie-dai requested a review from a team as a code owner June 29, 2026 19:01
@git2gus

git2gus Bot commented Jun 29, 2026

Copy link
Copy Markdown

Git2Gus App is installed but the .git2gus/config.json doesn't have right values. You should add the required configuration.

@cc-prodsec

cc-prodsec commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@sf-jie-dai sf-jie-dai self-assigned this Jun 29, 2026
@sf-jie-dai sf-jie-dai added the skip changelog Skip the "Changelog Check" GitHub Actions step even if the Changelog.md files are not updated label Jun 29, 2026
@sf-jie-dai sf-jie-dai closed this Jun 29, 2026
@sf-jie-dai sf-jie-dai reopened this Jun 29, 2026
@sf-jie-dai
sf-jie-dai force-pushed the jie.dai-W-22821845-stripInternalRefs branch 2 times, most recently from ef67721 to b8d6762 Compare June 29, 2026 20:17
Feature-branch overview covering the three OMS shopper actions on the
order detail page: returns (eligibility, request lifecycle, reason
codes, the error-kind contract), cancellation (all-or-nothing
eligibility, terminal 404/409 handling), and shipment tracking
(carrier-link hardening, date guarding, and the flat-list / no-address
multi-shipment scope). Also documents the per-unit status-badge
aggregation shared across all three. Mirrors the
README-SHOPPER-CONTEXT.md style.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@sf-jie-dai
sf-jie-dai force-pushed the jie.dai-W-22821845-stripInternalRefs branch from b8d6762 to 8c731b1 Compare June 29, 2026 20:20

@ddiazccrz ddiazccrz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Verdict: Approve with changes. The intent is sound and most of the README is exceptionally accurate (I verified the claims against the implementation on the PR head, not just the diff). But there is one fabricated feature claim that should be fixed before merge, plus a few small accuracy/polish items. The strip of internal refs is surgically clean — no logic, message ids, or assertions changed.

🔴 Must fix

1. README documents a "Track Shipment" action that doesn't existdocs/README-ORDER-MANAGEMENT.md:190-191

"A single order-level Track Shipment action links to the first shipment with a carrier URL (disabled when none)."

This is fabricated. git grep -niE 'track[ _-]?shipment' over the whole package returns only this README line. There is no order-level CTA, no "first shipment with a carrier URL" selection, and no "disabled when none" state. What actually happens: each shipment's tracking number itself is rendered as an inline carrier hyperlink via ensureExternalUrl(trackingUrl) (order-tracking/index.jsx:62, 107-110), and as plain text when no URL is present. The four <Button>s in order-detail.jsx are Cancel/Return/back — none is a tracking action.

→ Delete the bullet, or rewrite it as: "each shipment's tracking number is rendered as an inline carrier hyperlink (via ensureExternalUrl) when a tracking URL is present, and as plain text otherwise — there is no separate order-level action."

🟡 Should fix (small accuracy)

2. Wrong UI label — "Start a return":34

"A shopper opens the Start a return modal…"

No such label exists. The CTA accessible name is "Return Items" (order-detail.jsx:733, asserted in orders.test.js) and the modal title is "Return items from order #…" (return-items-modal/constants.js:12). The PR's own test comment even records that the label was renamed away from "Start return". Use "Return Items".

3. useOmsMetaData().returnReasonCodes is missing the .data layer:56-57 and :79

It's a React Query hook; reasons live under .data. Actual code: reviewQuery.data?.returnReasonCodes (return-items-modal/index.jsx:246), const {data: omsMetaData} = useOmsMetaData(...) (order-detail.jsx:259). The sibling README.md:73 already writes useOmsMetaData().data.returnReasonCodes. Line 79 gives the literal expression, so a reader copying it gets undefined. → Write .data.returnReasonCodes in both spots.

4. Incomplete order.status fallback expression:28 and :232

README says the badge "falls back to the raw order.status", but the code (and order-status-utils.js:185's own docstring) is order.status || order.omsData?.status. The dropped half matters for OMS orders that expose only an order-level status. → Write order.status, then order.omsData?.status.

🟢 Nits (optional)

  • Status-badge blockquote absolutism (:207-209): "never trusts the order-level status field" is contradicted by the green-badge fallback at order-status-badge/index.jsx:86. getOrderDisplayStatus itself genuinely never reads order.omsData.status (confirmed), so scope the claim to the aggregation path.
  • Returns eligibility table (OMS-managed order row): lists Source !!order.omsData, but that exact expression is the cancel gate (order-detail.jsx:370); for returns the OMS dependency is enforced indirectly via getReturnableItems. The described outcome is correct — only the cited source is misattributed.
  • Leftover GUS ref the strip missed (order-tracking/index.test.js:151): (Found in QA on W-22918455.). It's in the order-management test surface and is exactly the kind of customer-facing ref this PR targets — but the PR never enumerated this file, so it's a bounded completeness gap, not a regression. Worth catching while you're here: (Found in QA.).
  • Two bare "WI" jargon comments remain (order-detail.jsx:229, order-tracking/index.jsx:30) — no GUS number, so outside the literal pattern. Optional de-jargoning.

✅ Verified accurate (the bulk of the PR)

  • Strip is clean: the entire 8-file code/test diff changes only comments, JSDoc, and Jest titles — zero logic, zero message-id, zero assertion changes. All 9 enumerated files are free of numbered W-/WI- refs.
  • ReturnErrorKind error table is exact — all 7 kinds, HTTP/errorCode triggers, exact spellings (InvalidReasonCode/UnknownProductItemIds/ReturnQuantityExceeded), and the OrderReturnFailed/5xxUNKNOWN fall-through all match ERROR_CODE_TO_KIND and classifyReturnError verbatim.
  • Status badge unit-level aggregation, the four return enums, bucket fields, color branches, and getOrderDisplayStatus being presentation-free — all match.
  • ensureExternalUrl safety claims (rejects javascript:, https://…@evil.com host-spoof, relative paths → undefined) match url.js:357.
  • Cancel eligibility/flow, the epoch date-guard, the multi-shipment "no address association" scope, "no feature flag", and every relative link + the #error-handling anchor — all resolve and are accurate.

Net: fix #1 (and ideally #2–4, which are one-line doc edits) and this is a solid, unusually well-researched feature README.

The single order-level Track Shipment button is not on this branch yet;
it arrives via the order-tracking-cards PR. Note that in the
multi-shipment section so the doc matches the current branch state.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@sf-jie-dai

Copy link
Copy Markdown
Contributor Author

@ddiazccrz thanks for the close read — you're right that the Track Shipment action does not exist on this branch (must-fix item 1). It isn't invented, though: it's the order-level CTA being added in #3906 (sprasoon/W-23091033), whose description specs exactly "a single Track Shipment button → first shipment with a carrier URL, disabled when none, mirroring storefront-next's getTrackShipmentHref".

The multi-shipment section is deliberately written toward the converged epic-branch state (the flat-list / no-address scope it documents also lands with that PR), so rather than delete the bullet I've annotated it as incoming — "Arriving via PR #3906; not yet on this branch." — in 4704f43.

If you'd prefer the README describe only what's on the branch today, I'm happy to drop the bullet entirely and let #3906 add it back with the rest of its tracking changes. Your call. The other inline-hyperlink behavior you describe (ensureExternalUrl(trackingUrl) on the tracking number) is already covered under the Order Tracking "carrier link safety" bullet.

sf-jie-dai and others added 3 commits June 30, 2026 12:54
PR #3906 (order-detail tracking cards + Track Shipment action) merged
into the feature branch, so document what actually shipped: bordered
per-shipment tracking cards in a flat Tracking section, the three-state
Track Shipment action (disabled / single link / multi-shipment
dropdown), and the items-grouped-by-delivery-shipment-with-address
layout. Replaces the earlier "arriving via #3906" placeholder and the
single-order-level-address description.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…to jie.dai-W-22821845-stripInternalRefs

# Conflicts:
#	packages/template-retail-react-app/app/pages/account/orders.test.js
…US ref

- README: 'Return Items' label (not 'Start a return'), useOmsMetaData().data.returnReasonCodes, order.status || order.omsData?.status fallback in both spots
- README: scope status-badge blockquote to getOrderDisplayStatus aggregation path (green-badge fallback does read order-level status)
- order-tracking/index.test.js: drop leftover W- ref the strip missed
- order-detail.jsx: de-jargon bare 'WI' comment

@ddiazccrz ddiazccrz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review — Approve ✅

The must-fix from my prior review is resolved, and I've pushed the remaining nits directly to the branch (9e9d242).

What changed since my last review (8c731b19e9d242)

  • Must-fix #1 (Track Shipment) — resolved. The tracking-cards / Track Shipment implementation merged onto feature/264-order-management, so the README now describes real, merged code rather than a phantom feature. I re-verified the rewritten ### Track Shipment action and ### Shipments and addresses sections against order-detail.jsx:
    • Source is trackingUrlOptions = order.omsData.shipmentsensureExternalUrl → filtered to externalizable URLs (order-detail.jsx:244-255) ✓
    • Three states match exactly: 0 URLsaria-disabled button + VisuallyHidden hint "Tracking is not available for this order yet." (:845-857); 1 URL → external ChakraLink button (:816-834); >1 → Popover dropdown, one carrier link per shipment (:760-815) ✓
    • Items↔address (reliable, via shipmentId) vs Tracking↔shipment (not associated), BOPIS flat-list fallback — all match (:1133-1143) ✓
    • The subtle "single-shipment carrier-name fallback only" claim matches trackingEntries.singleMethodFallback (:543-546) ✓

Nits I just pushed (9e9d242)

  • README: "Return Items" label (was "Start a return"); useOmsMetaData().data.returnReasonCodes (added the missing .data); order.status || order.omsData?.status in both fallback spots.
  • README: scoped the status-badge blockquote absolutism to the getOrderDisplayStatus aggregation path (the green-badge fallback does read order-level status).
  • order-tracking/index.test.js:146: dropped the leftover W-22918455 ref the strip missed.
  • order-detail.jsx:267: de-jargoned the bare "WI" comment.

Re-confirmed clean

  • Strip remains comments/JSDoc/Jest-titles only — zero logic, message-id, or assertion changes (orders.test.js has no non-title line changes).
  • No numbered W-/WI- (or bare WI) refs remain anywhere in the order-management surface.

Approving.

@sf-jie-dai
sf-jie-dai merged commit 7974ee3 into feature/264-order-management Jun 30, 2026
40 of 41 checks passed
@sf-jie-dai
sf-jie-dai deleted the jie.dai-W-22821845-stripInternalRefs branch June 30, 2026 19:17
# Order Management

The order detail page surfaces three [Salesforce Order Management
(OMS/SOM)](https://help.salesforce.com/s/articleView?id=commerce.order_management.htm)

@knhage knhage Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This link doesn't work - returns 404. I found this other doc:
Salesforce Order Management
And
Integrate Order Management with B2C Commerce

server-side via the corresponding order action.

There is **no feature flag**. Each action is gated entirely on data and shopper
identity in [`order-detail.jsx`](../app/pages/account/order-detail.jsx). ECOM-only

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
identity in [`order-detail.jsx`](../app/pages/account/order-detail.jsx). ECOM-only
identity in [`order-detail.jsx`](../app/pages/account/order-detail.jsx). B2C Commerce-only


## Prerequisites

These features require a **Salesforce Order Management (SOM) core org connected to

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
These features require a **Salesforce Order Management (SOM) core org connected to
These features require a Salesforce Order Management (SOM) core org connected to

the storefront's B2C Commerce instance**. SOM is what enriches orders with the
`omsData` this UI depends on — order- and item-level OMS data (returnable/cancellable
quantities, item statuses, shipment tracking) and the `oms-return-order` /
`oms-cancel-order` SCAPI order actions. See the [Order Management

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
`oms-cancel-order` SCAPI order actions. See the [Order Management
`oms-cancel-order` B2C Commerce API (SCAPI) order actions. See the [Order Management

`omsData` this UI depends on — order- and item-level OMS data (returnable/cancellable
quantities, item statuses, shipment tracking) and the `oms-return-order` /
`oms-cancel-order` SCAPI order actions. See the [Order Management
setup](https://help.salesforce.com/s/articleView?id=commerce.order_management.htm)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This link doesn't work. Replace with:
Integrate Order Management with B2C Commerce

setup](https://help.salesforce.com/s/articleView?id=commerce.order_management.htm)
docs for connecting and provisioning the org.

Without a connected SOM org, orders carry no `omsData`: they are treated as ECOM-only,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Without a connected SOM org, orders carry no `omsData`: they are treated as ECOM-only,
Without a connected SOM org, orders carry no `omsData`: they are treated as B2C Commerce-only,

These features require a **Salesforce Order Management (SOM) core org connected to
the storefront's B2C Commerce instance**. SOM is what enriches orders with the
`omsData` this UI depends on — order- and item-level OMS data (returnable/cancellable
quantities, item statuses, shipment tracking) and the `oms-return-order` /

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to provide links for those SCAPI order actions. I searched on the web and the doc portal but couldn't find the SCAPI endpoints for these: oms-return-order and oms-cancel-order.


Without a connected SOM org, orders carry no `omsData`: they are treated as ECOM-only,
the return and cancel actions never render, return reasons can't load, and the status
badge falls back to the raw `order.status || order.omsData?.status`. Nothing errors — the features simply stay

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
badge falls back to the raw `order.status || order.omsData?.status`. Nothing errorsthe features simply stay
badge falls back to the raw `order.status || order.omsData?.status`. Nothing errorsthe features simply stay

| --- | --- | --- |
| Registered shopper | `useCustomerType().isRegistered` | Guests never see the return UI. |
| Owns the order | `order.customerInfo.customerId === customerId` | A shopper can only return their own orders. |
| OMS-managed order | `!!order.omsData` | ECOM-only orders carry no OMS data. |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| OMS-managed order | `!!order.omsData` | ECOM-only orders carry no OMS data. |
| OMS-managed order | `!!order.omsData` | B2C Commerce-only orders carry no OMS data. |


Registered shoppers can cancel an order that hasn't started fulfillment. The
**Cancel order** button renders unconditionally (so its position is stable) but is
disabled — with a screen-reader hint explaining why — when the order isn't eligible,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
disabledwith a screen-reader hint explaining whywhen the order isn't eligible,
disabledwith a screen-reader hint explaining whywhen the order isn't eligible,

## Order Cancellation

Registered shoppers can cancel an order that hasn't started fulfillment. The
**Cancel order** button renders unconditionally (so its position is stable) but is

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
**Cancel order** button renders unconditionally (so its position is stable) but is
**Cancel Order** button renders unconditionally (so its position is stable) but is

| Every item fully cancellable | `item.omsData.quantityAvailableToCancel === item.omsData.quantityOrdered` for **all** `productItems` |

Cancellation is all-or-nothing: it's offered only when every line can still be
cancelled in full. Once any unit has shipped, the order is no longer cancellable.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
cancelled in full. Once any unit has shipped, the order is no longer cancellable.
cancelled in full. Once any unit has shipped, the order is no longer cancelable.

(`POST .../actions/oms-cancel-order`); the `reason` is sent only when provided.
3. On success an "Order cancelled" alert is shown (after a short delay so screen
readers finish announcing the modal close) and the order status badge flips to
**Cancelled**.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
**Cancelled**.
**Canceled**.

**Cancelled**.
4. On failure, cancellation follows the same convention as returns (see
[Error Handling](#error-handling) above): a `404` or `409` is **terminal** — the
order can no longer be cancelled, so the button is permanently disabled with an

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
order can no longer be cancelled, so the button is permanently disabled with an
order can no longer be canceled, so the button is permanently disabled with an

## Order Tracking

[`OrderTracking`](../app/components/order-tracking) is a presentational, bordered
**tracking card** — one per shipment — rendered in a single flat **Tracking** section

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per our CX style, em dashes (—) should not be surrounded with spaces. I edited some of these but won't do that for all. Feel free to do a search and replace.
Search for: ' — '
Replace with: '—'

items; see [Shipments and addresses](#shipments-and-addresses) below).

The cards are built from `trackingEntries` (`order-detail.jsx`): one entry per
`order.omsData.shipments[]`, falling back to `order.shipments[]` (ECOM) only when there

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
`order.omsData.shipments[]`, falling back to `order.shipments[]` (ECOM) only when there
`order.omsData.shipments[]`, falling back to `order.shipments[]` (B2C Commerce) only when there


The cards are built from `trackingEntries` (`order-detail.jsx`): one entry per
`order.omsData.shipments[]`, falling back to `order.shipments[]` (ECOM) only when there
are no OMS shipments. ECOM-fallback cards have no provider, tracking URL, or dates

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
are no OMS shipments. ECOM-fallback cards have no provider, tracking URL, or dates
are no OMS shipments. B2C Commerce-fallback cards have no provider, tracking URL, or dates

rather than a misleading value. The `!value` guard is load-bearing: `new Date(null)`
returns the epoch (1970-01-01), not an Invalid Date, so without it a null delivery
date would display "31 Dec 1969".
- **OMS-over-ECOM fallback.** The component receives already-resolved scalar props;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ECOM is only used internally. We should use "B2C Commerce" instead. I edited several instances of this. I suggest doing a search and replace to get them all.
Search for: 'ECOM'
Replace with: 'B2C Commerce'

### Shipments and addresses

Shipment data arrives on the order in two lists with no correlation key between them:
tracking info (status, tracking number/URL, dates) in `order.omsData.shipments`, and

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
tracking info (status, tracking number/URL, dates) in `order.omsData.shipments`, and
tracking info (status, tracking number/URL, and dates) in `order.omsData.shipments`, and

sf-jie-dai added a commit that referenced this pull request Jun 30, 2026
Address knhage's unresolved review feedback from #3907 (merged before the
suggestions were applied):
- Fix two broken help.salesforce.com links (om_order_management,
  om_impl_storefront_integration)
- Rename "ECOM-only" -> "B2C Commerce-only" throughout
- Clarify the oms-return-order/oms-cancel-order actions are B2C Commerce
  API (SCAPI) order actions
- "Cancel order" -> "Cancel Order" button label + disabled-hint wording

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sf-jie-dai added a commit that referenced this pull request Jun 30, 2026
…order

Addresses knhage's L21 question on #3907 (couldn't find SCAPI endpoint docs).
Add the operationIds + POST paths inline and link the Shopper Orders SCAPI
reference, which documents oms-return-order (returnOmsOrder) and
oms-cancel-order (cancelOmsOrder).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sf-jie-dai added a commit that referenced this pull request Jun 30, 2026
Address the rest of knhage's #3907 review comments on the OMS README:
ECOM -> B2C Commerce (internal-only term), de-space em dashes per CX
style, and one-l US 'canceled' spellings in the cancellation section.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sf-jie-dai added a commit that referenced this pull request Jul 1, 2026
* @W-22931003 Add cancelOmsOrder and getOmsMetaData hooks to commerce-sdk-react (#3864)

* @W-22931003@ Add cancelOmsOrder and getOmsMetaData hooks to commerce-sdk-react

- Add CancelOmsOrder to ShopperOrdersMutations enum (mutation.ts)
- Add useOmsMetaData query hook for fetching cancel/return reason codes (query.ts)
- Add getOmsMetaData query key helper (queryKeyHelpers.ts)
- Add cancelOmsOrder cache invalidation: invalidates order query on success (cache.ts)
- Update query test map with useOmsMetaData
- Add returnOmsOrder to unimplemented endpoints list (available in SDK but no hook yet)

Requires commerce-sdk-isomorphic with shopper-orders-oas v1.14.1
(branch: update-shopper-orders-1.14.1, not yet published to npm)

* Add changelog entry for cancelOmsOrder and getOmsMetaData hooks

* Address PR review feedback

- Invalidate getCustomerOrders cache on cancel (order history shows updated status)
- Add comments for unimplemented endpoints in test file
- Bump commerce-sdk-isomorphic to 5.1.0-unstable-20260611093359 (preview with OAS v1.14.1)

* Update package-lock.json for commerce-sdk-isomorphic unstable version

* Fix prettier: inline single-element array in cancelOmsOrder cache

* Fix tests for unstable SDK: remove endpoints no longer in preview version

- Remove setupCustomerPaymentMethodReference from ShopperCustomers unimplemented list
- Remove resolveQualifiers from ShopperExperience unimplemented list

* Remove changelog entry (will add when merging to develop)

* Add unit tests for cancelOmsOrder mutation and cache invalidation

* @W-22821835@ Add returnOmsOrder mutation to commerce-sdk-react (#3869)

Add returnOmsOrder mutation

* @W-22918057 Extract standalone OrderTracking component (#3865)

@W-22918057 Extract standalone OrderTracking component

Pure refactor: move the inline tracking render from renderShippingMethod in
order-detail.jsx into a standalone, testable app/components/order-tracking
component, wired at both call sites (delivery/OMS-fallback and OMS-multi). The
OMS-multi call site no longer needs a wrapping React.Fragment (key moves onto
OrderTracking), and the now-unused ChakraLink import is removed from
order-detail.jsx.

DOM output is identical and i18n message IDs are unchanged (shipping-status
descriptors are kept as inline formatMessage literals so babel-plugin-formatjs
still statically extracts them). The existing orders.test.js suite plus new
isolated component tests pass (96/96); lint clean.

First of the Order Tracking epic work items.

* @W-22918455 Render estimated delivery date on order tracking (#3867)

@W-22918455 Render estimated delivery date on order tracking

Render expectedDeliveryDate and actualDeliveryDate on the OrderTracking
component as locale-formatted "Expected delivery: <date>" / "Delivered: <date>"
lines, shown when present and omitted when absent (or when the date is missing
or malformed — formatTrackingDate returns null on an unparseable value so it
never renders "Invalid Date" or throws). Wired at both order-detail call sites;
these fields are OMS-only (no ECOM fallback — they exist only on
omsData.shipments[]).

Adds two i18n keys (extracted + compiled). Unit and page tests cover the
present, absent, and malformed cases across single, multi-shipment, and
ECOM-fallback paths.

* @W-22918457 Guard null delivery dates in OrderTracking (#3872)

* @W-22918457 Guard null delivery dates in OrderTracking

QA on W-22918455 found that new Date(null) returns the epoch (1970-01-01), not Invalid Date, so a null expectedDeliveryDate/actualDeliveryDate slipped past the isNaN guard and rendered '31 Dec 1969' to the shopper. Add a falsy guard (catches null, undefined, '') to formatTrackingDate, with a regression test for the explicit-null case. Defensive regardless of whether OMS serializes null on the wire.

* @W-22918457 Re-trigger CI (flaky e2e SLAS login)

* @W-22918462 Add error/fallback state to order-detail fetch (#3871)

Consume useOrder's isError and render a full-card error (OrderNotFoundCard-style: square corners, title + description + Back to Order History) instead of hanging on the loading skeleton forever (the AC6 bug). A successful order with no omsData is NOT an error — it renders normally via the ECOM fallback. Adds the OrderLoadError component, i18n keys, and unit tests for the error, back-link, and no-omsData paths. Partial (products-only) failure keeps graceful degradation per the team's decision.

* @W-22952388@ Sync payment-instrument amount before createOrder so OMS can ingest(#3875)

@W-22952388@ Sync payment-instrument amount before createOrder so OMS can ingest

* fix: invalidate `useCustomerOrders` after `createOrder` @W-22821836 (#3878)

* @W-22821836@ Invalidate customer orders cache after createOrder

After a registered shopper places an order, useCustomerOrders kept
serving the stale (pre-order) result and only refreshed on a fresh
login, because createOrder's cache-update matrix only invalidated
getCustomerBaskets. Add getCustomerOrders to the same invalidate list
so the account order-history page reflects the new order immediately.
Guest checkouts (no customerId) remain a no-op.

* @W-22806925@ Integrate SCAPI Cancel Order API with UI (#3873)

* @W-22806925@ Integrate SCAPI Cancel Order API with UI

- Replace dummy cancel handler with real useShopperOrdersMutation(CancelOmsOrder)
- Fetch reason codes from useOmsMetaData API when modal opens
- Update cancel eligibility: omsData must exist + quantityAvailableToCancel === quantityOrdered for all items
- Remove oms.enabled config flag — cancel visibility driven by order.omsData presence
- Remove hardcoded CANCELLATION_REASONS — dropdown populated from API response
- Hide dropdown when no reason codes available (allows cancel without reason)
- Pass isSubmitting from mutation state to disable modal buttons during request
- Show skeleton while reason codes are loading
- Remove extracted translation messages for hardcoded reasons
- Add mock for useShopperOrdersMutation/useOmsMetaData in order detail tests

* Fix: keep modal open during API call, close only after response

* Fix CI: prettier formatting and add changelog entry

* Revert changelog entry — will add when merging to develop

* Fix prettier: correct indentation for map, revert formatMessage to multiline

* Pre-fetch OMS metadata on page load instead of on modal open

* Fix prettier: inline useOmsMetaData and formatMessage calls

* Fix prettier conflict: extract formatMessage to variables (under 100 char width)

* Fix prettier: match showCancelSuccess format with showCancelError

* Address Jie's review: initialize default reason and add no-reason test

- Pre-select the default reason code when modal opens (from API response)
- Add test: passes empty string when no reason codes provided and confirm clicked
- Update existing tests to expect default reason pre-selection

* Fix lint and tighten canCancel check

- Add item.omsData != null guard to prevent undefined === undefined passing
- Inline formatMessage in showCancelSuccess (satisfies generated-project prettier)
- Inline test props (satisfies generated-project prettier)

* Fix prettier: revert showCancelSuccess formatMessage to multiline (matches showCancelError)

* feat: surface OMS return eligibility on order detail @W-22821836 (#3880)

feat: surface OMS return eligibility on order detail

* @W-22930993 Handle cancel order API error scenarios with specific messages (#3884)

* @W-22930993@ Handle cancel order API error scenarios with specific messages

- 404: "Unable to cancel order" / "We could not find this order. Please refresh and try again."
- 409: "Unable to cancel order" / "This order is already being processed and cannot be canceled. Please reach out to the Merchant."
- Everything else (400, 401, 500, network): "Something went wrong" / "We couldn't process your cancellation right now. Please wait a moment and try again."

Error status is read from error.response.status (commerce-sdk-isomorphic ResponseError shape).
Cancel button remains enabled on error to allow retry.

* Fix prettier: inline defaultMessage values

* Address review: disable button on terminal errors, fix copy

- Disable cancel button after 404/409 (terminal — retrying won't help)
- Keep button enabled for transient errors (500/network) to allow retry
- Fix spelling: "canceled" → "cancelled", "Merchant" → "merchant"
- Reword 409 copy: "Please contact the merchant for assistance."

* Address review: revert copy change, fix spelling only, add error tests

- Revert message rewording — keep original copy, only fix "canceled" → "cancelled"
  and "Merchant" → "merchant"
- Add unit tests for cancel error scenarios (404, 409, 500, network failure)
- Tests verify correct title and description for each error code

* Fix lint: split long defaultMessage lines, multiline isDisabled, remove unused act

* @W-22930989 Test and polish cancel modal behavior when OMS metadata is unavailable (#3883)

* @W-22930989@ Hide cancellation reason dropdown when OMS metadata API fails

The dropdown is already hidden when reasonCodes is undefined (metadata
fetch failed). Add retry: 1 to the useOmsMetaData query so React Query
automatically retries once before giving up. If both attempts fail,
the modal renders without the dropdown and the shopper can still
confirm cancellation — the server applies the default reason.

* Add unit tests for metadata API failure scenarios

* Address review: remove retry override, conditional modal text

- Remove retry: 1 — let TanStack default (3) handle resilience
- Show "Confirm cancellation below." when dropdown is hidden (no reason codes)
- Show "Select a reason and confirm cancellation." when dropdown is visible
- Update test for conditional text

* feat: return-item-selection modal on order detail @W-22821837 (#3886)

feat: return-item-selection modal on order detail

* feat: add return review step + returnOmsOrder submission @W-22821838 (#3895)

feat: add return review step + returnOmsOrder submission

* @W-22806929 Add comprehensive cancel order integration tests (#3896)

* @W-22806929@ Add comprehensive cancel order integration tests

12 tests covering:
- Eligibility: OMS order shows cancel, non-OMS hides it, partially shipped
  disables, already-cancelled disables, wrong customer disables
- Happy path: modal opens, API called with correct payload, success feedback
- Reason selection: submits selected reason to API
- Error states: button disabled after 409 (terminal), enabled after 500 (retry)
- Modal UX: keep order closes without API call, no-reason-codes shows
  alternative text

* Address review: fix vacuous tests and lint errors

- Fix "cancel submits with selected reason": rewrite as "cancel submits
  empty body when no reason codes available" — tests the actual contract
  (cancelReasonCodes: [] means no dropdown, empty body sent to API)
- Fix "does not show cancel button for different customer": rename to
  "disables cancel button", assert unconditionally (button IS rendered
  but disabled for OMS orders with wrong customerId)
- Rename "quantityAvailableToCancel < quantityOrdered" to "not fully
  cancellable" — clearer wording matching the strict-equality contract
- Fix prettier: multiline mock resolved values
- Remove conditional expects (jest/no-conditional-expect lint rule)

* chore: merge develop into feature/264-order-management (#3901)

merge develop into feature/264-order-management

* feat: order-level status matrix util from item omsData @W-23163246 (#3900)

 order-level status matrix util from item omsData

* feat: item-level return error states @W-22821839 (#3897)

item-level return error states

* @W-23093717 wire getOrderDisplayStatus for cancelled status into order badges  (#3902)

* feat: wire getOrderDisplayStatus into order badges @W-23093717

Use item-level status derivation to show Cancelled badge (red with ×
icon) on both order-detail and order-history pages when all items are
cancelled. Non-cancelled orders retain the existing raw status display.

* Address review: hoist isCancelled out of IIFEs, add negative tests

- Hoist `isCancelled` derivation above the JSX return in order-detail
  and to the top of the .map() callback in order-history, removing the
  inline IIFEs.
- Add negative tests asserting a partially-cancelled order does NOT
  render the "Cancelled" badge (regression guard).

* Address review: memoize isCancelled, add OMS-only scope comment

- Wrap isCancelled in useMemo on order-detail to avoid recomputing on
  every render (matches canCancel/returnableItems pattern).
- Add comment clarifying the derivation is OMS-only; pure ECOM
  cancellations fall through to the raw status string in the badge.
- Fix broken import from merge (return-error-utils missing `import {`).

* Extract shared OrderStatusBadge component

De-duplicate the cancelled badge JSX from order-detail and order-history
into a reusable component with memoized status derivation. Unifies the
message ID (order_status_badge.label.cancelled) so translators only
localize the string once. Prepares a clean extension point for follow-up
status WIs.

---------

Signed-off-by: sf-madhuri-uppu <madhuri.uppu@salesforce.com>

* feat: show return status in order status badge @W-23163243 (#3904)

feat: show return status in order status badge

* @W-23025048 fix(order-tracking): make carrier tracking links external (#3898)

* fix(order-tracking): make carrier tracking links external

A carrier tracking URL can arrive without a scheme (e.g. `www.carrier.com/t`).
Rendered directly in an `href`, the browser treats it as a relative path and
resolves it against the current page, so the tracking-number link navigated
inside the app (`/account/orders/www.carrier.com/t`) instead of going to the
carrier.

Add `ensureExternalUrl` (app/utils/url.js): prepend `https://` to a scheme-less
URL, keep already-absolute http(s) URLs, and reject unsafe or non-web values
(`javascript:`, `data:`, `mailto:`, app-internal paths, …) by returning
`undefined` so the link renders inactive rather than executable or wrong. Apply
it to the OrderTracking tracking-number link.

* chore: trigger CI re-run

* harden ensureExternalUrl per review feedback

Address review findings on the carrier-tracking-link normalizer. The function
trusted the parsed scheme but never validated the resulting host, so several
inputs produced an href that read like the carrier yet navigated elsewhere.

- Reject userinfo: `https://www.ups.com@evil.com` parses to host `evil.com`
  with `www.ups.com` as the username — now rejected on both parse branches.
- Reject backslash forms (`\` is treated as `/` by the URL parser) and run the
  relative-path guard on the raw input so a control char between slashes can't
  collapse `/x/host` into a protocol-relative `//host`.
- Reject non-string input (was throwing on a non-string and crashing the render).
- Reject bare filenames / empty-label hosts (e.g. `data.html`, `a..b`) that
  aren't real external hosts.
- Drop the unreachable http branch and the dead IPv4 check.

Centralize the checks in `isSafeExternalUrl` (http(s) only, no userinfo,
plausible host), applied to both the absolute and prepended candidates. Add
regression tests asserting the resolved host for each case.

* harden ensureExternalUrl against dotted-protocol host confusion

A scheme-less value shaped `label.tld://host` parses to a dotted protocol, so it skipped the dot-less validation and fell through to be re-prepended into `https://label.tld//host` — a link that reads like one host but navigates to another (same class as the userinfo spoof). Reject a dotted-protocol parse that carries an authority; a genuine `host:port` has an empty host on that parse and still resolves. Review feedback.

* feat: order-detail tracking cards + Track Shipment action @W-23091033 (#3906)

* feat: add Track Shipment button to order detail @W-23091033

Render a "Track Shipment" order action per OMS shipment that has a
tracking URL — each button opens its own shipment's carrier URL in a new
tab (same destination as the tracking-number link), with no numeric
suffix. When no shipment has a tracking URL, a single disabled button is
shown. The Order Actions row is now full-width stacked on mobile and
inline from the sm breakpoint up, so additional buttons no longer
overflow on narrow screens.

* feat: per-shipment tracking cards + single track action @W-23091033

Align the order-detail tracking UI with storefront-next and the order
tracking epic's "flat list, no address association" scope:

- Render tracking as a flat list of per-shipment cards (one card per
  omsData.shipments[] entry, ECOM order.shipments[] fallback), each with
  carrier, status, tracking-number link, and delivery dates. No address
  on the cards and no positional OMS-to-ECOM index join.
- Consolidate the Track Shipment order action to a single button that
  links to the first shipment with a carrier URL (disabled when none),
  matching storefront-next getTrackShipmentHref.
- Move the shipping address to a single order-level block shown only for
  single-shipment delivery orders; multi-shipment orders no longer
  associate an address per shipment.

Updates the order detail tests and extracts the new Tracking heading
message.

* feat: per-shipment item boxes on order detail @W-23091033

Restructure the Items Ordered area to mirror storefront-next: render one
bordered box per ECOM delivery shipment, each with a "Shipment N" header,
that shipment's status, its items (grouped strictly by shipmentId; all items
when there's a single delivery shipment), and its own native shipping address
and method. Pickup-only orders (no delivery shipment) fall back to a flat item
list.

The top summary card no longer carries a standalone shipping-address block —
the address now lives inside each shipment box. The Tracking cards stay in a
separate flat section (OMS-preferred, ECOM fallback) and carry no address:
there is no reliable join key between an OMS shipment and a specific ECOM
shipment, so a per-shipment box never index-joins OMS data (the deferred TD).

Reconcile the multi-shipment / BOPIS tests to the new per-box behavior: they
previously asserted addresses were hidden and only passed because the products
endpoint was unmocked (which suppressed the box render); they now mock products
and assert each shipment's own address renders in its box, plus strict
item-to-box placement by shipmentId.

* feat: move tracking inside the first shipment box @W-23091033

Match the storefront-next design branch: the flat tracking list now renders
INSIDE the first shipment box, after its items and before its shipping address,
instead of as a standalone section above the items. It is still the whole flat
list (rendered in box 1 only, never split per shipment), so there is no
positional OMS↔ECOM index-join. Pickup-only orders (no delivery box) append the
flat tracking list after the flat item list so tracking is never lost.

Update the regression-lock test: it previously asserted tracking was a separate
section; it now asserts tracking sits inside the first shipment box with the
shipping address as a sibling block (no address inside a tracking card).

* feat: tracking as flat section below shipment boxes @W-23091033

OMS tracking (omsData.shipments[]) has no join key back to a specific ECOM
shipment (deferred TD-0326366), so the layout must not imply which tracking
belongs to which shipment. Move the flat tracking list out of the first
shipment box and render it as a single section BELOW all the boxes — a peer of
the boxes, not nested in one. Each shipment box still shows its own products
(grouped by ECOM shipmentId) and its own native address, both of which ARE
known; only the tracking↔shipment link is unknown, and the flat list is honest
about that. One code path covers single, multi, and pickup-only orders.

Update the regression-lock test: it now asserts tracking renders OUTSIDE (not
nested in) the shipment boxes, with no address/payment text in the section.

* feat: Track Shipment dropdown for multi-shipment orders @W-23091033

UX option A for multi-shipment tracking (one of two branches for review).

When an order has more than one shipment with a carrier tracking URL, the
Track Shipment order action becomes a dropdown (Popover — the shared UI barrel
exposes no Chakra Menu) listing each tracking number as an external link, so
the shopper can pick which carrier link to open. Single-shipment orders keep
the simple external-link button; orders with no tracking URL keep the single
disabled button. This is interim: we still cannot say which tracking maps to
which shipment (deferred TD-0326366), so options are labeled by tracking
number, not by shipment contents.

* chore(order-detail): self-contained rationale in tracking comments

Rewrite the order-detail and test comments to explain the multi-shipment
tracking behavior in project-internal terms (OMS and ECOM shipment arrays share
no join key, so a tracking entry can't be tied to a specific shipment) instead
of referencing tracking artifacts or another product. Customer-facing code
should not carry process/cross-product references; the rationale lives in the
spec and PR instead.

* fix(order-detail): address review findings on tracking layout @W-23091033

- Surface items that match no shipment box (untagged, or a shipmentId naming no
  delivery shipment) in an "Other items" box, so multi-shipment orders never
  silently drop purchased items while the item-count header still counts them.
- Localize the per-shipment box status pill via a shared formatter, so the box
  header and the tracking card render the same localized status instead of one
  showing a raw snake_case token in non-English locales.
- Make the disabled Track Shipment button use aria-disabled + a hidden
  disabled-reason hint (keyboard/SR focusable), matching the Cancel/Return
  buttons in the same row, instead of a native disabled button.

* fix(order-detail): meet AA contrast on shipment status pill @W-23091033

The "Shipment N" status pill rendered gray.700 text on gray.200 (4.04:1),
which fails WCAG AA (4.5:1) and tripped the registered happy-path a11y
snapshot (the color-contrast violation captured on the order detail page).
Darken the text to gray.800 on the same gray.200 background for 5.88:1.

* docs: order-management feature README + strip internal GUS refs @W-22821845 (#3907)

order-management feature README + strip internal GUS refs

* @W-22821845 fix(order-return): return focus to the trigger when the return modal closes (#3910)

return focus to the trigger when the return modal closes

* fix(oms-ui): apply CX UI text guidelines to order & account UI (#3912)

apply CX UI text guidelines to order & account UI

* docs(commerce-sdk-react): changelog for OMS order-action hooks

The cancelOmsOrder/returnOmsOrder mutations and getOmsMetaData hook
(#3864, #3869) landed without a commerce-sdk-react CHANGELOG entry, so
the integration PR failed the per-package changelog check. Add it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(commerce-sdk-react): pin commerce-sdk-isomorphic to stable 5.4.0

Replace the temporary 5.1.0-unstable-* dev build with stable 5.4.0, the
first release carrying the OMS Shopper Orders endpoints (oms-return-order,
oms-cancel-order, oms-meta-data). develop's 5.2.0 has no OMS endpoints, so
this is required for the OMS hooks to work rather than a plain bump.

Build + tsc clean; ShopperOrders hooks 33/33 pass against 5.4.0.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(commerce-sdk-react): list new 5.4.0 endpoints as unimplemented

The isomorphic 5.2.0 -> 5.4.0 bump adds endpoints with no React hooks
yet, so the 'all endpoints have hooks' guard tests failed. Add each new
endpoint (requestOtp/verifyOtp, setupCustomerPaymentMethodReference,
promoteTemporaryBasket, getComponent/resolveQualifiers, getProduct
Images/Prices/Promotions) to the expected unimplemented list with a TODO,
matching the test's documented convention. Implementing the hooks is out
of scope for the OMS integration.

Full commerce-sdk-react suite: 778/778 pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(oms-ui): bump vendor.js bundle-size budget to 398 kB

The commerce-sdk-isomorphic 5.2.0 -> 5.4.0 upgrade pushes vendor.js to
397.07KB, just over the 397 kB budget. Bump to 398 kB (main.js unchanged).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: address PR #3914 follow-up review comments @W-23254713

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(oms-ui): apply CX UI text guidelines to cancel order modal @W-23254713
CX UI text guideline caps for buttons

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Signed-off-by: sf-madhuri-uppu <madhuri.uppu@salesforce.com>
Co-authored-by: sf-madhuri-uppu <madhuri.uppu@salesforce.com>
Co-authored-by: sf-shikhar-prasoon <214730309+sf-shikhar-prasoon@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip changelog Skip the "Changelog Check" GitHub Actions step even if the Changelog.md files are not updated

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants