Skip to content

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

Merged
sf-shikhar-prasoon merged 5 commits into
feature/264-order-managementfrom
t/cc-sharks/W-23091033/tracking-url-external/main
Jun 29, 2026
Merged

@W-23025048 fix(order-tracking): make carrier tracking links external#3898
sf-shikhar-prasoon merged 5 commits into
feature/264-order-managementfrom
t/cc-sharks/W-23091033/tracking-url-external/main

Conversation

@sf-shikhar-prasoon

Copy link
Copy Markdown
Contributor

What

A carrier tracking URL can arrive without a scheme (e.g. www.carrier.com/track). 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 on Order Details navigated inside the app (/account/orders/www.carrier.com/track) instead of going to the carrier.

This adds ensureExternalUrl and applies it where the URL becomes an href.

Changes

  • app/utils/url.js — new ensureExternalUrl(input):
    • prepends https:// to a scheme-less URL (www.carrier.comhttps://www.carrier.com/)
    • keeps already-absolute http(s) URLs (normalized by the native URL parser)
    • handles protocol-relative //host, host:port, and IPv4 hosts
    • rejects unsafe / non-web values (javascript:, data:, vbscript:, mailto:, tel:, app-internal/relative paths) by returning undefined so the caller renders an inactive link rather than an executable or wrong href
  • app/components/order-tracking/index.jsx — the tracking-number ChakraLink uses the normalized href; falls back to plain text when there's no safe external URL.

Testing / Self-QA

  • ensureExternalUrl unit tests (url.test.js): scheme-less, already-absolute, protocol-relative, host:port, IPv4, dangerous/non-web schemes, internal/relative paths, control-char stripping, empty/nullish.
  • Render test (order-tracking/index.test.js): a scheme-less trackingUrl resolves to an absolute external href (regression lock for the relative-path bug).
  • Unit tests pass (url.test.js + order-tracking/index.test.js — 139 tests). Lint clean on the touched files.
  • Verified live on a fulfilled SOM order with a real tracking URL: the link now targets the external carrier URL (https://...) instead of an internal route.

@git2gus

git2gus Bot commented Jun 24, 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 24, 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.

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.
@sf-shikhar-prasoon
sf-shikhar-prasoon force-pushed the t/cc-sharks/W-23091033/tracking-url-external/main branch from 444a0dd to f351bdf Compare June 24, 2026 21:21
@sf-shikhar-prasoon sf-shikhar-prasoon added the skip changelog Skip the "Changelog Check" GitHub Actions step even if the Changelog.md files are not updated label Jun 24, 2026
@sf-shikhar-prasoon
sf-shikhar-prasoon marked this pull request as ready for review June 24, 2026 21:23
@sf-shikhar-prasoon
sf-shikhar-prasoon requested a review from a team as a code owner June 24, 2026 21:23

@sf-jie-dai sf-jie-dai 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.

Code-review findings (2 substantive). Both verified empirically against the WHATWG URL parser in Node.

}

// Require a host-like value so a bare word ("track") isn't externalized.
const host = sanitized.replace(/^\/+/, '').split(/[/?#]/)[0]

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.

Authority-confusion bypass — link reads like the carrier but navigates elsewhere.

The host-like guard approves a string because it contains a ., but the WHATWG parser resolves the actual host to something else. Verified in Node:

ensureExternalUrl('ups.com:@evil.com/track')  // -> 'https://ups.com@evil.com/track'  (host = evil.com)
ensureExternalUrl('@evil.com/track')           // -> 'https://evil.com/track'

Here ups.com: parses as a (dotted) "protocol" → falls through to the prepend path; host extraction yields ups.com:@evil.com, which passes .includes('.'). The produced URL's userinfo is ups.com and the host is evil.com. Since the whole point of this helper is to safely externalize merchant/OMS-sourced tracking URLs, a value that renders as a UPS-looking link but navigates to an attacker host defeats the guard. Consider validating the parsed URL's host/hostname (and rejecting any username/password) rather than the raw substring.

export const ensureExternalUrl = (input) => {
if (!input) return undefined
// eslint-disable-next-line no-control-regex -- strip control chars so they can't smuggle past the scheme checks
const sanitized = input.replace(/[\x00-\x1f\x7f]/g, '').trim()

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.

Non-string input throws, breaking the documented "never throws" contract.

trackingUrl is PropTypes.string (dev-only) and flows from API data (omsShipment?.trackingUrl / shipment.trackingUrl). A malformed response that yields a number crashes the render:

ensureExternalUrl(1234)  // TypeError: input.replace is not a function

!1234 is false, so the falsy guard is skipped and .replace throws — the OrderTracking render tree crashes. The old href={trackingUrl} coerced harmlessly. Suggest a typeof input !== 'string' guard (or String(input ?? '')) right after the falsy check.

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.
@sf-shikhar-prasoon sf-shikhar-prasoon changed the title @W-23091033 fix(order-tracking): make carrier tracking links external @W-23025048 fix(order-tracking): make carrier tracking links external Jun 25, 2026
@sf-shikhar-prasoon

Copy link
Copy Markdown
Contributor Author

Thanks for the review — both were good catches. Pushed fixes:

  • Authority-confusion @ bypass: the normalizer now validates the parsed URL on both the absolute and prepended branches (shared isSafeExternalUrl, rejecting any username/password), so https://www.ups.com@evil.com and the ups.com:@evil.com variant → undefined.
  • Non-string input throws: added a typeof input !== 'string' guard up front, so a non-string trackingUrl returns undefined instead of throwing in render.

Since this is the same helper as the storefront-next side, I also folded in the equivalent hardening flagged there — reject backslash-authority forms, run the relative-path guard on the raw input, reject bare-filename hosts, and drop a dead branch — so both stay in parity. Added regression tests asserting the resolved host for each case.

// protocol; a scheme-less `host:port` (`carrier.com:8080`) mis-parses to a
// dotted protocol — only the former should be validated/rejected as-is.
const parsed = new URL(sanitized)
if (!parsed.protocol.replace(/:$/, '').includes('.')) {

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.

Residual host-confusion bypass — same class as the userinfo spoof, via the dotted-protocol fallthrough.

The intent here is "a scheme-less host:port (carrier.com:8080) mis-parses to a dotted protocol → fall through and prepend https://." But an input shaped label.tld://… also parses to a dotted protocol, so it skips isSafeExternalUrl and falls through to be re-prepended — smuggling the fake scheme in as the real host. Verified against Node's WHATWG URL:

ensureExternalUrl('attacker.com://ups.com/track/12345')
  // -> 'https://attacker.com//ups.com/track/12345'   (host = attacker.com)
ensureExternalUrl('foo.bar://evil.com')
  // -> 'https://foo.bar//evil.com'                     (host = foo.bar)

The href navigates to attacker.com while the string reads like ups.com — exactly the "reads like the carrier, navigates elsewhere" defect this helper documents itself as rejecting (the suite already asserts https://www.ups.com@evil.com → undefined; this is the same outcome through a different door).

Severity is moderate, not critical: the rendered link text is the tracking number, not the URL, and label.tld://… isn't a shape any real carrier URL takes. But it's an open hole in a function whose sole job is host-confusion-safe externalization.

The discriminator between the legit and malicious cases is the // authority: a scheme-less host:port has no // after the colon, whereas attacker.com://… does. So reject when the first parse yields a dotted protocol that carries an authority, and only treat the no-// host:port form as scheme-less. (Note: simply "always run isSafeExternalUrl on the first parse" would wrongly reject carrier.com:8080, since its protocol isn't http/https.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch — fixed in d80fc161f1 with the //-authority discriminator you suggested. After the first parse, a dotted "protocol" that carries an authority is now rejected, while a genuine scheme-less host:port (empty host on that parse) still falls through and resolves:

const parsed = new URL(sanitized)
if (!parsed.protocol.replace(/:$/, '').includes('.')) {
    return isSafeExternalUrl(parsed) ? parsed.toString() : undefined
}
// Dotted protocol WITH an authority (`attacker.com://ups.com/t`) is a host-confusion spoof.
if (parsed.host) return undefined

Added regression tests: attacker.com://ups.com/track/12345 and foo.bar://evil.comundefined; carrier.com:8080 (and /track) still → https://carrier.com:8080/... so the legit host:port form isn't over-rejected. Ready for another look.

@sf-jie-dai sf-jie-dai 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: both findings from the previous pass are fixed — verified empirically against Node's WHATWG URL parser.

  • Userinfo spoof (https://www.ups.com@evil.com, ups.com:@evil.com) → now undefined (isSafeExternalUrl rejects username/password). ✅
  • Non-string crash (1234, {}, null) → now undefined, never throws (the typeof input !== 'string' guard). ✅

One residual issue in the same host-confusion class, so holding approval — see the inline comment.

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.

@sf-jie-dai sf-jie-dai 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.

Approving — reviewed the ensureExternalUrl normalizer and its order-tracking usage at high effort. The security spoof vectors are genuinely closed and tested (userinfo @, backslash authority, dotted-protocol label.tld://host, javascript:/data:), and since the result is a client-side href opened with rel=noopener noreferrer, the host-confusion/decimal-IP cases don't apply here. No blocking issues.

Non-blocking notes for a follow-up (optional):

  • FILENAME_HOST is an enumerated file-extension blocklist run against url.hostname — fragile and largely redundant; isSafeExternalUrl (http(s) + no userinfo + dotted multi-label host) already covers the intent. Consider dropping it.
  • Slight over-strictness: trailing-dot FQDN (https://tracking.ups.com.) and IPv6 literals are rejected. Low real-world risk for retail carriers.
  • The if (parsed.host) return undefined guard is load-bearing on a WHATWG quirk (scheme-less host:port → empty host). Correct today; worth keeping the regression tests that pin it.
  • The two try/catch parse branches duplicate the same validate-and-return tail.

…to t/cc-sharks/W-23091033/tracking-url-external/main
@sf-shikhar-prasoon
sf-shikhar-prasoon merged commit e546e0d into feature/264-order-management Jun 29, 2026
40 of 41 checks passed
@sf-shikhar-prasoon
sf-shikhar-prasoon deleted the t/cc-sharks/W-23091033/tracking-url-external/main branch June 29, 2026 15:56
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.

3 participants