Skip to content

(SP: 3) [SHOP] complete phase 3 admin operations surface and admin UI polish#418

Open
liudmylasovetovs wants to merge 12 commits intodevelopfrom
lso/feat/shop-legal
Open

(SP: 3) [SHOP] complete phase 3 admin operations surface and admin UI polish#418
liudmylasovetovs wants to merge 12 commits intodevelopfrom
lso/feat/shop-legal

Conversation

@liudmylasovetovs
Copy link
Collaborator

@liudmylasovetovs liudmylasovetovs commented Mar 26, 2026

Description

This PR completes Phase 3 of the Shop execution plan by strengthening the internal admin surface for day-to-day operations.

The work focuses on making the Shop admin usable as a real operational tool:

  • Admin Orders now supports real end-to-end filtering instead of decorative UI.
  • Admin Order Detail now serves as the main operational surface with customer summary, history timeline, controlled lifecycle actions, and wired existing shipping/refund/payment controls.
  • Shop admin UI was polished and aligned with the Blog admin visual language, including follow-up fixes for Orders and Products admin surfaces.

Related Issue

Issue: #<issue_number>


Changes

  • Implemented ADM-03 for Admin Orders:

    • added real filters for status, dateFrom, and dateTo
    • wired filters through page -> API -> query layer
    • preserved filter state in pagination
    • fixed admin-orders filter i18n labels and polished the Orders admin UI
  • Completed Admin Orders UI polish:

    • aligned the Orders admin page with Blog admin visual patterns
    • improved filter toolbar, table presentation, badges, and action surface
    • replaced raw technical error presentation with more admin-friendly localized messaging
  • Polished Admin Products admin UI:

    • aligned Products admin styling with Blog admin
    • improved table/layout fit for Ukrainian locale
    • compacted overcrowded headers/actions and improved admin readability
  • Implemented ADM-04 for Admin Order Detail:

    • enriched detail data with a proper customer summary
    • surfaced account/shipping/customer snapshot fields
    • added human-readable shipping labels
    • added order history / audit timeline surface
  • Implemented ADM-05 for Admin lifecycle actions:

    • added controlled confirm / cancel / complete admin actions
    • added backend-guarded lifecycle handling with audit writes
    • wired existing shipping/refund/payment action components into the detail page
  • Made the Admin Order Detail page the primary operational admin surface:

    • customer summary
    • timeline/history
    • lifecycle actions
    • shipping controls
    • refund / cancel unpaid payment controls

Database Changes (if applicable)

  • Schema migration required
  • Seed data updated
  • Breaking changes to existing queries
  • Transaction-safe migration
  • Migration tested locally on Neon

How Has This Been Tested?

  • Tested locally
  • Verified in development environment
  • Checked responsive layout (if UI-related)
  • Tested accessibility (keyboard / screen reader)

Additional notes:

  • Narrow Vitest coverage was run for Admin Orders filters, Admin Order Detail customer summary, history timeline, lifecycle actions, and operational action rendering.
  • Narrow eslint checks were run for touched files.
  • Repo-wide TypeScript noise outside Shop scope still exists, but touched Shop flows were validated through focused checks and tests.

Screenshots (if applicable)

  • Admin Orders filters and UI polish
  • Admin Products compact table/layout polish
  • Admin Order Detail customer summary, history timeline, lifecycle controls, and operational actions surface

Checklist

Before submitting

  • Code has been self-reviewed
  • No TypeScript or console errors
  • Code follows project conventions
  • Scope is limited to this feature/fix
  • No unrelated refactors included
  • English used in code, commits, and docs
  • New dependencies discussed with team
  • Database migration tested locally (if applicable)
  • GitHub Projects card moved to In Review

Reviewers

Summary by CodeRabbit

  • New Features

    • Admin order lifecycle controls (Confirm/Cancel/Complete), order timeline/history, cancel-payment and refund controls, plus preserved filters on the orders list.
  • Improvements

    • Refreshed action UIs with clearer, bordered amber error alerts, improved accessibility (aria-busy/alerts/aria-describedby), disabled/in‑flight behavior, multi-panel order detail layout, consistent date formatting, localized “back to list” links, and enhanced product list/form styling.
  • Tests

    • Extensive new tests for lifecycle actions, timeline/query behavior, filters, visibility rules, and localized UI error messaging.

@vercel
Copy link
Contributor

vercel bot commented Mar 26, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
devlovers-net Ignored Ignored Preview Mar 26, 2026 10:49pm

Request Review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 26, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds admin order lifecycle server handlers and service, new/updated admin UI controls for shipping/refunds/payments with normalized error mapping and accessibility, expanded DB queries/timeline and order filters with Zod validation, many i18n updates, and extensive tests across UI, routes, and services.

Changes

Cohort / File(s) Summary
Order controls & visibility
frontend/app/[locale]/admin/shop/orders/[id]/CancelPaymentButton.tsx, frontend/app/[locale]/admin/shop/orders/[id]/RefundButton.tsx, frontend/app/[locale]/admin/shop/orders/[id]/ShippingActions.tsx, frontend/app/[locale]/admin/shop/orders/[id]/shippingActionVisibility.ts
Added CancelPaymentButton; refactored RefundButton and ShippingActions to use error normalization/mapping, submission guards (isSubmitting/useTransition), improved accessibility and alert-style error UI; added shipping action visibility helper/type.
Lifecycle route & service
frontend/app/[locale]/admin/shop/orders/[id]/lifecycle/route.ts, frontend/lib/services/shop/admin-order-lifecycle.ts
New POST route performing same-origin/CSRF/admin checks and redirects; new lifecycle service implementing `confirm
Order detail page & data shaping
frontend/app/[locale]/admin/shop/orders/[id]/page.tsx, frontend/db/queries/shop/admin-orders.ts
Replaced previous single-query approach with getAdminOrderDetail and getAdminOrderTimeline; expanded AdminOrderDetail shape (customer/account, fulfillmentStage, shipping/shipment fields), added humanizers, issued CSRF tokens, and wired new action components.
Orders list, API & validation
frontend/app/[locale]/admin/shop/orders/page.tsx, frontend/app/api/shop/admin/orders/route.ts, frontend/lib/validation/shop-admin-orders.ts, frontend/components/admin/shop/AdminPagination.tsx
Added server-side filter parsing (status/dateFrom/dateTo) with Zod validation and cross-field checks, normalized date bounds, GET filters UI, and pagination preserving query parameters.
Products UI & shared styles
frontend/app/[locale]/admin/shop/products/..., frontend/components/admin/shop/AdminProductDeleteButton.tsx, frontend/components/admin/shop/AdminProductStatusToggle.tsx
Introduced shared Tailwind class constants, adjusted layouts/headings, added back-to-list links and server-side i18n, and updated product action/toggle/delete styling plus optional className props.
Localization
frontend/messages/en.json, frontend/messages/pl.json, frontend/messages/uk.json
Large i18n additions for admin products and orders: filters, lifecycle controls and errors, shipping/payment controls, customer summary, and order history strings.
DB & timeline
frontend/db/queries/shop/admin-orders.ts
Added filters to getAdminOrdersPage, new AdminOrderHistoryEntry type, getAdminOrderTimeline with canonical+legacy merge/dedupe logic, and expanded AdminOrderDetail result fields.
Tests
frontend/lib/tests/... (many new files)
Extensive Vitest/Jest suites for lifecycle service, timeline integration, page rendering, action visibility, error mapping, route redirects, filter parsing, and i18n presence; also UI interaction tests for action error handling.
sequenceDiagram
    participant Admin as Admin User
    participant UI as React Component
    participant Route as Next.js Route
    participant Service as Admin Lifecycle Service
    participant DB as Database
    participant Router as Next.js Router

    Admin->>UI: Click lifecycle action (confirm/cancel/complete)
    UI->>UI: Clear errors, set submitting / aria-busy
    UI->>Route: POST action + CSRF header
    Route->>Route: Validate params, same-origin, CSRF, admin auth
    Route->>Service: applyAdminOrderLifecycleAction(orderId, action, actor, requestId)
    Service->>DB: Load order + latest shipment/state
    Service->>DB: Validate eligibility & perform guarded update(s)
    alt Confirm
        Service->>DB: Update to PAID, maybe create queued shipment
    else Cancel
        Service->>DB: Cancel/restock/refund checks and updates
    else Complete
        Service->>DB: Verify shipment succeeded, set delivered
    end
    Service->>DB: Write admin audit log
    Service-->>Route: Return result
    Route-->>UI: Redirect / success response
    alt Success
        UI->>Router: router.refresh()
    else Error
        UI->>UI: Map code -> i18n message, render <span role="alert">
    end
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

Suggested labels

enhancement, UI, UX

Suggested reviewers

  • LesiaUKR
  • AM1007
  • ViktorSvertoka

Poem

🐇 I hopped through code and stitched a seam,
Buttons ready, timeline gleam,
Errors tamed to friendly prose,
Audits logged where history grows,
Tests and translations—down the burrow we go.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title accurately describes the main objective: completing phase 3 of the Shop admin operations surface with UI polish, matching the substantial changes across admin orders, order detail, and products.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch lso/feat/shop-legal

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ab45a85456

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
frontend/messages/uk.json (1)

932-964: ⚠️ Potential issue | 🟡 Minor

Indentation issue in fulfillmentStages object requires correction.

The fulfillmentStages object (lines 957–964) has inconsistent indentation. The keys "delivered", "canceled", and "returned" are under-indented (10 spaces) compared to "shipped" (14 spaces). This is a code formatting issue that should be corrected for consistency, even though JSON parsers ignore whitespace. Note: the trailing comma after "Відправлено" on line 961 is present.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/messages/uk.json` around lines 932 - 964, The fulfillmentStages
object has inconsistent indentation and a trailing comma after the "shipped"
entry; fix the formatting by aligning the keys "delivered", "canceled", and
"returned" to the same indentation level as "processing", "packed", and
"shipped" within the fulfillmentStages object and remove the trailing comma
after the "shipped" value so the JSON is consistently formatted and valid;
locate the fulfillmentStages block and adjust spacing for the keys and delete
the extra comma.
🧹 Nitpick comments (8)
frontend/lib/tests/shop/admin-orders-messages.test.ts (1)

46-57: Consider including English locale for completeness.

The negative test only checks Ukrainian and Polish locales. If the intent is to verify that filter labels are not accidentally placed under shop.admin.products, including English would provide more comprehensive coverage.

Proposed fix
   it('does not keep order filter labels under shop.admin.products', () => {
-    for (const messages of [uk, pl]) {
+    for (const messages of [en, uk, pl]) {
       expect(
         getAtPath(messages as Record<string, unknown>, [
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/lib/tests/shop/admin-orders-messages.test.ts` around lines 46 - 57,
The test "does not keep order filter labels under shop.admin.products" currently
iterates only over uk and pl; include the English locale variable (e.g., en)
alongside uk and pl in the for loop so getAtPath(messages,
['shop','admin','products','filters']) is also asserted undefined for English.
Update the array in the test (where uk and pl are referenced) to include en and
keep using the existing getAtPath and expect call.
frontend/app/[locale]/admin/shop/orders/[id]/ShippingActions.tsx (1)

102-106: Network error message mapping may not match expected codes.

In the catch block, err.message contains a browser error string (e.g., "Failed to fetch" or "NetworkError") rather than the expected code 'NETWORK_ERROR'. This means mapShippingError will almost always hit the default case, returning t('errors.generic') instead of t('errors.network').

Consider detecting network failures explicitly:

Proposed fix
     } catch (err) {
-      const msg =
-        err instanceof Error && err.message ? err.message : 'NETWORK_ERROR';
-      setError(mapShippingError(msg, t));
+      setError(mapShippingError('NETWORK_ERROR', t));
       return;
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/app/`[locale]/admin/shop/orders/[id]/ShippingActions.tsx around
lines 102 - 106, The catch block currently forwards raw browser error messages
to mapShippingError, causing network failures (e.g., "Failed to fetch",
"NetworkError") to miss the 'NETWORK_ERROR' mapping; update the ShippingActions
catch handler to detect network failures explicitly (for example check if err is
a TypeError or err.message contains "Failed to fetch" / "NetworkError" /
similar) and normalize those cases to the token 'NETWORK_ERROR' before calling
mapShippingError, otherwise pass through the existing err.message; reference the
catch handler around setError/mapShippingError and ensure mapShippingError
receives 'NETWORK_ERROR' for detected network failures.
frontend/lib/tests/shop/admin-orders-query-filters.test.ts (1)

91-96: Misleading variable names don't match actual dates.

Several variables use month names that don't match the actual dates:

  • paidLateMarchId at line 93 uses April 30
  • paidLateMarchId at line 137 uses May 31
  • aprilId at line 141 uses June 2
  • paidMidMarchId/paidLateMarchId at lines 191/195 use June dates

Consider renaming to reflect actual months or use neutral names like paidLateId, paidMidId for clarity.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/lib/tests/shop/admin-orders-query-filters.test.ts` around lines 91 -
96, Several test variables are misnamed relative to their seed dates (e.g.,
paidLateMarchId used with new Date('2099-04-30...'), other mismatches include
aprilId with June date and paidMid/paidLate with June dates); rename these
variables (paidLateMarchId, paidMidMarchId, aprilId, etc.) to accurate, neutral
names like paidLateId, paidMidId, aprilId->juneId or better generic names
(paidLateId, paidMidId, paidEarlyId) to match the seeded dates in seedOrder
calls and update all references in the test file
(admin-orders-query-filters.test.ts) accordingly so variable names reflect their
actual createdAt dates.
frontend/messages/en.json (1)

830-836: Inconsistent indentation in the detail section.

Starting at line 830, the nesting level appears inconsistent with the surrounding structure. Keys like customerSummary, lifecycle, shippingMethods, history, and fulfillmentStages have extra leading indentation compared to sibling keys like total (line 901) and paymentStatus (line 902).

While JSON parsers ignore whitespace, this makes the file harder to maintain. Consider normalizing the indentation.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/messages/en.json` around lines 830 - 836, The JSON block containing
keys like "customerSummary", "lifecycle", "shippingMethods", "history", and
"fulfillmentStages" has inconsistent leading indentation compared to sibling
keys such as "total" and "paymentStatus"; fix this by normalizing whitespace so
those keys align with their siblings (e.g., remove the extra leading spaces
before "customerSummary" and the nested "lifecycle" block) to match the
surrounding indentation style used elsewhere in the file.
frontend/app/[locale]/admin/shop/orders/[id]/CancelPaymentButton.tsx (2)

63-68: Same network error handling pattern as RefundButton.

The err.message passed to mapCancelPaymentError will be browser error text, not a code. Consider using 'NETWORK_ERROR' directly as suggested for RefundButton.tsx.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/app/`[locale]/admin/shop/orders/[id]/CancelPaymentButton.tsx around
lines 63 - 68, The catch block in the CancelPaymentButton component currently
extracts err.message and passes it to mapCancelPaymentError, which yields
browser text instead of a standardized code; change the error mapping to pass
the literal 'NETWORK_ERROR' (or the actual error code when available) to
mapCancelPaymentError and then call setError with its result — update the catch
in CancelPaymentButton to use 'NETWORK_ERROR' instead of err.message, keeping
the existing setError(mapCancelPaymentError(..., t)) call and preserving
behavior for known error codes if you detect them.

77-85: Inconsistent error field extraction vs RefundButton.

This component uses json?.code ?? json?.message while RefundButton uses json?.error ?? json?.code. Since the API returns { code, message }, both work, but the inconsistency could cause confusion or bugs if API responses change.

Consider aligning both components:

-          json?.code ?? json?.message ?? `HTTP_${res.status}`,
+          json?.code ?? json?.error ?? `HTTP_${res.status}`,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/app/`[locale]/admin/shop/orders/[id]/CancelPaymentButton.tsx around
lines 77 - 85, CancelPaymentButton currently extracts error as json?.code ??
json?.message while RefundButton uses json?.error ?? json?.code; make them
consistent by standardizing the extraction to a single fallback chain (e.g.,
json?.error ?? json?.code ?? json?.message) in CancelPaymentButton and
RefundButton so both components use the same logic, update the setError call in
CancelPaymentButton (and adjust RefundButton if needed) to use that standardized
chain and keep the mapping function mapCancelPaymentError/mapRefundError
compatible with the chosen fields.
frontend/lib/tests/shop/admin-order-timeline-query.test.ts (1)

40-46: Empty afterEach can be removed.

The comment explains cleanup happens in finally blocks, but the empty afterEach hook serves no purpose and can be deleted.

 describe.sequential('admin order timeline query', () => {
-  afterEach(async () => {
-    // tests own their rows; cleanup happens in each test finally block
-  });
-
   it('returns canonical audit history...
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/lib/tests/shop/admin-order-timeline-query.test.ts` around lines 40 -
46, Remove the no-op afterEach hook inside the describe.sequential('admin order
timeline query', ...) block: delete the empty afterEach(async () => { /* ... */
}); since cleanup is handled in each test's finally blocks; ensure only the
describe.sequential and its it('returns canonical audit history...') test remain
unchanged.
frontend/app/[locale]/admin/shop/orders/[id]/RefundButton.tsx (1)

58-62: Network error handling will always produce generic message.

When fetch throws (network failure), err.message contains browser error text like "Failed to fetch", not an error code. Passing this to mapRefundError will always hit the default case.

Consider simplifying to always use 'NETWORK_ERROR':

     } catch (err) {
-      const msg =
-        err instanceof Error && err.message ? err.message : 'NETWORK_ERROR';
-      setError(mapRefundError(msg, t));
+      setError(mapRefundError('NETWORK_ERROR', t));
       return;
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/app/`[locale]/admin/shop/orders/[id]/RefundButton.tsx around lines
58 - 62, In the catch block of RefundButton.tsx, don't pass the thrown Error's
message into mapRefundError because network failures produce browser text that
won't match your error mapping; instead set msg to the literal 'NETWORK_ERROR'
(or otherwise hardcode the network error key) and call
setError(mapRefundError('NETWORK_ERROR', t)); update the catch around the refund
action (the block that currently computes msg from err) to always use
'NETWORK_ERROR' so mapRefundError can return the intended network-specific
message.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@frontend/app/`[locale]/admin/shop/orders/[id]/lifecycle/route.ts:
- Around line 91-94: The handler currently returns raw JSON via
NextResponse.json (the branches around the NextResponse.json calls) which breaks
normal <form method="post"> submissions; replace these JSON responses with
redirects: for invalid/stale/order-not-found or access-revoked cases in the POST
handler, return a NextResponse redirect back to the order detail page
(preserving locale and id) with a short error code as a query parameter (e.g.
?error=INVALID_ORDER_ID) so the UI can show a localized message, and for
session-expired/unauthenticated cases redirect to the login page with a
post-login returnTo pointing at the order detail; update all similar branches
(including the other block noted) to follow the same pattern and ensure you
reference the same route/id/locale when building the redirect URL.

In `@frontend/app/`[locale]/admin/shop/orders/[id]/page.tsx:
- Around line 266-283: The lifecycleEnabled function returns button visibility
that diverges from the backend/service guards; update it to derive its booleans
from the same rule set used by the admin-order-lifecycle service (i.e., reuse or
call the service-side predicates used by applyComplete() rather than duplicating
simplified checks). Specifically, replace the current complete predicate in
lifecycleEnabled with the same conditions used by applyComplete() (including
handling of shipping provider checks like non‑"nova_poshta" and states treated
as already complete), and do the same for the other lifecycle flags to ensure
the page only shows actions that the service will actually perform (also apply
the same change to the other similar block mentioned around 405-414).
- Around line 653-676: Replace raw enum renderings with humanized/translated
labels: for paymentStatus, paymentProvider, shippingStatus and history
transition statuses use the existing translation pattern (e.g., call t(...) with
a label key helper) or reuse the detailValue helper instead of String(...) so
admins see localized text. Specifically, change usages like
String(order.paymentStatus), order.paymentProvider, order.shippingStatus and
history transition rendering to pass through a mapping function (e.g.,
paymentStatusLabelKey(order.paymentStatus),
paymentProviderLabelKey(order.paymentProvider),
shippingStatusLabelKey(order.shippingStatus) or
detailValue(order.shippingStatus)) and then call t(...) to render the localized
human-readable string; apply the same approach to the history status transition
rendering sections referenced (around the other ranges).

In `@frontend/db/queries/shop/admin-orders.ts`:
- Around line 383-395: The rows array currently contains one row per shipment
which causes duplicated items and an arbitrary shipmentStatus used by
deriveCanonicalFulfillmentStage; collapse shipment rows first (e.g., group by
order id and pick the latest shipment or join via a latest-shipment
subquery/helper) then set base = collapsedRows[0]!.order and shipmentStatus =
collapsedRows[0]?.shipping.shipmentStatus ?? null, and build items from the
shipment-free row set by mapping collapsedRows.map(r =>
toAdminOrderItem(r.item)). Apply the same collapse-and-build approach to the
other similar block that constructs items and computes fulfillmentStage (the
block using toAdminOrderItem and deriveCanonicalFulfillmentStage).
- Around line 290-321: The current early return when auditRows.length > 0
discards legacy entries; instead fetch legacyOrder (pspMetadata) even when
auditRows exist, convert legacy entries via
parseLegacyShippingAdminAudit(legacyOrder?.pspMetadata) into an array, map
auditRows into the same shape (using isRecord, normalizeHistoryAction,
readString, etc.), concat the two arrays, sort the combined list by occurredAt
(ascending or descending matching existing behavior), and return the sorted
merged array so both auditRows and orders.pspMetadata.shippingAdminAudit are
preserved.

In `@frontend/lib/services/shop/admin-order-lifecycle.ts`:
- Around line 194-266: The update moves the order to PAID before running
ensureQueuedInitialShipment and writeAdminAudit so exceptions leave state
changed but side-effects missing; fix by wrapping the status update,
ensureQueuedInitialShipment(...) and writeAdminAudit(...) in a single
transactional unit (use the same db transaction used by the
db.update(orders).set(...) call) so either all side-effects commit or none do,
or if a DB transaction across external systems isn’t possible, make a
compensating idempotent repair: after a failed
ensureQueuedInitialShipment/writeAdminAudit attempt, catch the error, run
idempotent ensureQueuedInitialShipment(...) and the audit write (or queue the
audit) again before rethrowing; also update the loadLifecycleState branch that
returns the already-PAID no-op to attempt the same idempotent repair (call
ensureQueuedInitialShipment and writeAdminAudit) when latest?.status === 'PAID'
so retries fix missing side-effects.

---

Outside diff comments:
In `@frontend/messages/uk.json`:
- Around line 932-964: The fulfillmentStages object has inconsistent indentation
and a trailing comma after the "shipped" entry; fix the formatting by aligning
the keys "delivered", "canceled", and "returned" to the same indentation level
as "processing", "packed", and "shipped" within the fulfillmentStages object and
remove the trailing comma after the "shipped" value so the JSON is consistently
formatted and valid; locate the fulfillmentStages block and adjust spacing for
the keys and delete the extra comma.

---

Nitpick comments:
In `@frontend/app/`[locale]/admin/shop/orders/[id]/CancelPaymentButton.tsx:
- Around line 63-68: The catch block in the CancelPaymentButton component
currently extracts err.message and passes it to mapCancelPaymentError, which
yields browser text instead of a standardized code; change the error mapping to
pass the literal 'NETWORK_ERROR' (or the actual error code when available) to
mapCancelPaymentError and then call setError with its result — update the catch
in CancelPaymentButton to use 'NETWORK_ERROR' instead of err.message, keeping
the existing setError(mapCancelPaymentError(..., t)) call and preserving
behavior for known error codes if you detect them.
- Around line 77-85: CancelPaymentButton currently extracts error as json?.code
?? json?.message while RefundButton uses json?.error ?? json?.code; make them
consistent by standardizing the extraction to a single fallback chain (e.g.,
json?.error ?? json?.code ?? json?.message) in CancelPaymentButton and
RefundButton so both components use the same logic, update the setError call in
CancelPaymentButton (and adjust RefundButton if needed) to use that standardized
chain and keep the mapping function mapCancelPaymentError/mapRefundError
compatible with the chosen fields.

In `@frontend/app/`[locale]/admin/shop/orders/[id]/RefundButton.tsx:
- Around line 58-62: In the catch block of RefundButton.tsx, don't pass the
thrown Error's message into mapRefundError because network failures produce
browser text that won't match your error mapping; instead set msg to the literal
'NETWORK_ERROR' (or otherwise hardcode the network error key) and call
setError(mapRefundError('NETWORK_ERROR', t)); update the catch around the refund
action (the block that currently computes msg from err) to always use
'NETWORK_ERROR' so mapRefundError can return the intended network-specific
message.

In `@frontend/app/`[locale]/admin/shop/orders/[id]/ShippingActions.tsx:
- Around line 102-106: The catch block currently forwards raw browser error
messages to mapShippingError, causing network failures (e.g., "Failed to fetch",
"NetworkError") to miss the 'NETWORK_ERROR' mapping; update the ShippingActions
catch handler to detect network failures explicitly (for example check if err is
a TypeError or err.message contains "Failed to fetch" / "NetworkError" /
similar) and normalize those cases to the token 'NETWORK_ERROR' before calling
mapShippingError, otherwise pass through the existing err.message; reference the
catch handler around setError/mapShippingError and ensure mapShippingError
receives 'NETWORK_ERROR' for detected network failures.

In `@frontend/lib/tests/shop/admin-order-timeline-query.test.ts`:
- Around line 40-46: Remove the no-op afterEach hook inside the
describe.sequential('admin order timeline query', ...) block: delete the empty
afterEach(async () => { /* ... */ }); since cleanup is handled in each test's
finally blocks; ensure only the describe.sequential and its it('returns
canonical audit history...') test remain unchanged.

In `@frontend/lib/tests/shop/admin-orders-messages.test.ts`:
- Around line 46-57: The test "does not keep order filter labels under
shop.admin.products" currently iterates only over uk and pl; include the English
locale variable (e.g., en) alongside uk and pl in the for loop so
getAtPath(messages, ['shop','admin','products','filters']) is also asserted
undefined for English. Update the array in the test (where uk and pl are
referenced) to include en and keep using the existing getAtPath and expect call.

In `@frontend/lib/tests/shop/admin-orders-query-filters.test.ts`:
- Around line 91-96: Several test variables are misnamed relative to their seed
dates (e.g., paidLateMarchId used with new Date('2099-04-30...'), other
mismatches include aprilId with June date and paidMid/paidLate with June dates);
rename these variables (paidLateMarchId, paidMidMarchId, aprilId, etc.) to
accurate, neutral names like paidLateId, paidMidId, aprilId->juneId or better
generic names (paidLateId, paidMidId, paidEarlyId) to match the seeded dates in
seedOrder calls and update all references in the test file
(admin-orders-query-filters.test.ts) accordingly so variable names reflect their
actual createdAt dates.

In `@frontend/messages/en.json`:
- Around line 830-836: The JSON block containing keys like "customerSummary",
"lifecycle", "shippingMethods", "history", and "fulfillmentStages" has
inconsistent leading indentation compared to sibling keys such as "total" and
"paymentStatus"; fix this by normalizing whitespace so those keys align with
their siblings (e.g., remove the extra leading spaces before "customerSummary"
and the nested "lifecycle" block) to match the surrounding indentation style
used elsewhere in the file.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0dff6fae-1620-490c-984a-4f30814e07af

📥 Commits

Reviewing files that changed from the base of the PR and between f30ff8d and ab45a85.

📒 Files selected for processing (31)
  • frontend/app/[locale]/admin/shop/orders/[id]/CancelPaymentButton.tsx
  • frontend/app/[locale]/admin/shop/orders/[id]/RefundButton.tsx
  • frontend/app/[locale]/admin/shop/orders/[id]/ShippingActions.tsx
  • frontend/app/[locale]/admin/shop/orders/[id]/lifecycle/route.ts
  • frontend/app/[locale]/admin/shop/orders/[id]/page.tsx
  • frontend/app/[locale]/admin/shop/orders/page.tsx
  • frontend/app/[locale]/admin/shop/products/[id]/edit/page.tsx
  • frontend/app/[locale]/admin/shop/products/_components/ProductForm.tsx
  • frontend/app/[locale]/admin/shop/products/new/page.tsx
  • frontend/app/[locale]/admin/shop/products/page.tsx
  • frontend/app/api/shop/admin/orders/route.ts
  • frontend/components/admin/shop/AdminPagination.tsx
  • frontend/components/admin/shop/AdminProductDeleteButton.tsx
  • frontend/components/admin/shop/AdminProductStatusToggle.tsx
  • frontend/components/tests/shop/admin-order-detail-action-errors.test.tsx
  • frontend/db/queries/shop/admin-orders.ts
  • frontend/lib/services/shop/admin-order-lifecycle.ts
  • frontend/lib/tests/shop/admin-edit-product-page-error-handling.test.ts
  • frontend/lib/tests/shop/admin-order-detail-customer-summary.test.ts
  • frontend/lib/tests/shop/admin-order-detail-history.test.ts
  • frontend/lib/tests/shop/admin-order-detail-operational-actions.test.ts
  • frontend/lib/tests/shop/admin-order-lifecycle-actions.test.ts
  • frontend/lib/tests/shop/admin-order-timeline-query.test.ts
  • frontend/lib/tests/shop/admin-orders-messages.test.ts
  • frontend/lib/tests/shop/admin-orders-page-filters.test.ts
  • frontend/lib/tests/shop/admin-orders-query-filters.test.ts
  • frontend/lib/tests/shop/admin-orders-route-filters.test.ts
  • frontend/lib/validation/shop-admin-orders.ts
  • frontend/messages/en.json
  • frontend/messages/pl.json
  • frontend/messages/uk.json

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
frontend/app/[locale]/admin/shop/orders/[id]/ShippingActions.tsx (1)

96-116: ⚠️ Potential issue | 🟠 Major

Disable shipping actions for the full request duration, not just for router.refresh().

The disabled={isPending} check at line 202 only covers the startTransition() call at lines 133–135, not the preceding fetch and response parsing (lines 107–131). A fast double-click during the fetch request will keep the button enabled, allowing the same shipping mutation to be sent multiple times before the first response returns. Add a local submitting flag and disable the button against both states: disabled={isSubmitting || isPending}.

Also applies to: 133–135, 197–203

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/app/`[locale]/admin/shop/orders/[id]/ShippingActions.tsx around
lines 96 - 116, The buttons are only disabled by isPending (from useTransition)
which doesn't cover the async fetch in runAction; add a local isSubmitting state
(e.g., const [isSubmitting, setIsSubmitting] = useState(false)), set
isSubmitting = true immediately before starting the fetch in runAction and
ensure setIsSubmitting(false) in all exit paths (after handling success, in the
catch block, and finally after startTransition finishes), keep startTransition
solely for router.refresh(), and update button props to disabled={isSubmitting
|| isPending} so the UI is disabled for the full request duration; reference
runAction, startTransition, isPending, setError, and the fetch/response handling
to locate where to set/reset isSubmitting.
♻️ Duplicate comments (1)
frontend/lib/services/shop/admin-order-lifecycle.ts (1)

233-242: ⚠️ Potential issue | 🟠 Major

Confirm retries still cannot backfill a missing audit row for non-shipping orders.

The repairOnly path returns before writeAdminAudit() whenever shipment repair is ineligible. If the first confirm already moved the order to PAID and the audit insert failed, every retry on a digital / non–Nova Poshta / refund-contained order becomes a clean no-op and the confirm entry is lost permanently. getAdminOrderTimeline() only reads canonical admin history from adminAuditLog, so the order history stays incomplete.

Also applies to: 297-305, 359-369

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/lib/services/shop/admin-order-lifecycle.ts` around lines 233 - 242,
The early return guarded by repairOnly + canRepairConfirmedOrderSideEffects
prevents writeAdminAudit() from ever being called, which lets retries
permanently miss inserting the confirm audit row for non-shipping/digital
orders; update the confirm/repair flow in admin-order-lifecycle.ts so
writeAdminAudit() is invoked (or at least attempted) before returning when
repairOnly is true but canRepairConfirmedOrderSideEffects(...) is false, and
apply the same change to the other early-return locations (the blocks around the
other two instances noted); reference canRepairConfirmedOrderSideEffects and
writeAdminAudit so you either move/write the audit insertion prior to the early
return or add a conditional call that ensures adminAuditLog is backfilled on
retries even if side-effect repair is ineligible.
🧹 Nitpick comments (1)
frontend/app/[locale]/admin/shop/orders/[id]/page.tsx (1)

359-395: Share the shipping-action visibility matrix with ShippingActions.tsx.

This predicate now exists twice: once here for showShippingActions, and again in frontend/app/[locale]/admin/shop/orders/[id]/ShippingActions.tsx for the button list. Once one side changes, the page can hide the whole shipping card while the client component would still consider actions visible, or vice versa. Please derive both from one helper.

Also applies to: 509-513

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/app/`[locale]/admin/shop/orders/[id]/page.tsx around lines 359 -
395, Extract the shipping-visibility predicate into a single shared helper and
have both consumers use it: create a pure function (e.g.,
deriveShippingVisibility or export shippingControlsEnabled) that accepts
AdminOrderDetail (and any small flags) and returns the same visibility matrix
currently produced by shippingControlsEnabled; replace the duplicate logic in
ShippingActions.tsx (the showShippingActions predicate) and the local
shippingControlsEnabled usage with imports from that shared helper so both
server/page and client/component derive visibility from the exact same
implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@frontend/app/`[locale]/admin/shop/orders/[id]/page.tsx:
- Around line 845-861: The account snapshot rendering is incorrectly gated on
customerSummary.accountEmail so admins don't see a customerAccountName-only
snapshot; update the conditional for the registered-account block (the JSX that
currently checks order.userId and customerSummary.accountEmail) to render when
order.userId and (customerSummary.accountName or customerSummary.accountEmail)
are present, then keep the existing rendering logic inside: display
customerSummary.accountName ?? customerSummary.accountEmail as the main line and
only render the secondary email line when customerSummary.accountEmail exists;
refer to the JSX using order.userId, customerSummary.accountName, and
customerSummary.accountEmail to locate the change.

In `@frontend/app/`[locale]/admin/shop/orders/[id]/RefundButton.tsx:
- Around line 53-55: The refund POST is awaited before calling startTransition
so useTransition's isPending doesn't cover the network request, allowing
duplicate clicks; update the handler (the function that calls fetch/post to
"/refund" — referenced by startTransition, isPending, useTransition and
error/setError) to call startTransition(() => { /* perform the await fetch/post
and subsequent setError or navigation inside this callback */ }) so the awaited
POST runs inside the transition, ensuring isPending becomes true for the
duration of the request and the button disabled logic (disabled || isPending)
correctly blocks duplicate submissions; preserve existing error handling by
calling setError inside the transition callback on failure.

In `@frontend/lib/services/shop/admin-order-lifecycle.ts`:
- Around line 445-453: The branch that calls cancelMonobankUnpaidPayment(...)
can let provider/domain errors escape raw; wrap that call in a try/catch and
convert any thrown error into an AdminOrderLifecycleActionError so the lifecycle
route can normalize it (e.g., catch errors around cancelMonobankUnpaidPayment in
the current.paymentProvider === 'monobank' branch, and rethrow new
AdminOrderLifecycleActionError({ code: <appropriate_code>, message:
error.message, cause: error }) or similar), ensuring the lifecycleError flow
receives a standardized error instead of a raw exception.
- Around line 427-435: The cancel/complete final-state branches (e.g., the
isFinalCanceled() branch and the corresponding completed/delivered branches)
mutate the order before calling writeAdminAudit(), which can leave order state
changed without a persisted audit if writeAdminAudit() throws; update these
handlers to either persist the audit inside the same DB transaction as the order
mutation or add an idempotent repair path that retries writing the missing audit
using a stable dedupe key (e.g., include order.id + action +
final-state-timestamp as the dedupe key) so repeated retries short-circuit
correctly; specifically modify the code paths that call writeAdminAudit() after
changing status (references: isFinalCanceled, writeAdminAudit, the cancel and
complete handlers) to ensure the audit insert is atomic or
retriable/deduplicated.

In `@frontend/lib/tests/shop/admin-order-detail-action-errors.test.ts`:
- Around line 40-47: The mocked localization strings contain mojibake for the
ellipsis; update the values for 'shop.orders.detail.paymentControls.refunding'
and 'shop.orders.detail.paymentControls.cancelingPayment' to use the real
catalog ellipsis character (…) instead of the garbled sequence (…) so the
mocked copy matches the real catalog and loading-state assertions will use the
correct text.

In `@frontend/messages/en.json`:
- Around line 952-958: The shipmentStatuses mapping in frontend/messages/en.json
currently contains a "created" key that doesn't exist in the schema; replace
that key with "processing" so the map matches shipping_shipments.status values
(queued | processing | succeeded | failed | needs_attention) and ensure the
label uses "Needs attention" for needs_attention; update the "created" entry
under "shipmentStatuses" to "processing" with the same localized string.

In `@frontend/messages/pl.json`:
- Around line 937-952: Replace the mojibake values under the shippingStatuses
and shipmentStatuses keys with correct Polish strings: change
shippingStatuses."shipped" from "WysÅ‚ane" to "Wysłane"; change
shipmentStatuses."succeeded" from "ZakoÅ„czono powodzeniem" to "Zakończono
powodzeniem"; change shipmentStatuses."failed" from "Błąd" to "Błąd". Keep all
other keys (pending, queued, creatingLabel, labelCreated, delivered, cancelled,
needsAttention, shipmentStatuses.queued, created, needsAttention) unchanged.

---

Outside diff comments:
In `@frontend/app/`[locale]/admin/shop/orders/[id]/ShippingActions.tsx:
- Around line 96-116: The buttons are only disabled by isPending (from
useTransition) which doesn't cover the async fetch in runAction; add a local
isSubmitting state (e.g., const [isSubmitting, setIsSubmitting] =
useState(false)), set isSubmitting = true immediately before starting the fetch
in runAction and ensure setIsSubmitting(false) in all exit paths (after handling
success, in the catch block, and finally after startTransition finishes), keep
startTransition solely for router.refresh(), and update button props to
disabled={isSubmitting || isPending} so the UI is disabled for the full request
duration; reference runAction, startTransition, isPending, setError, and the
fetch/response handling to locate where to set/reset isSubmitting.

---

Duplicate comments:
In `@frontend/lib/services/shop/admin-order-lifecycle.ts`:
- Around line 233-242: The early return guarded by repairOnly +
canRepairConfirmedOrderSideEffects prevents writeAdminAudit() from ever being
called, which lets retries permanently miss inserting the confirm audit row for
non-shipping/digital orders; update the confirm/repair flow in
admin-order-lifecycle.ts so writeAdminAudit() is invoked (or at least attempted)
before returning when repairOnly is true but
canRepairConfirmedOrderSideEffects(...) is false, and apply the same change to
the other early-return locations (the blocks around the other two instances
noted); reference canRepairConfirmedOrderSideEffects and writeAdminAudit so you
either move/write the audit insertion prior to the early return or add a
conditional call that ensures adminAuditLog is backfilled on retries even if
side-effect repair is ineligible.

---

Nitpick comments:
In `@frontend/app/`[locale]/admin/shop/orders/[id]/page.tsx:
- Around line 359-395: Extract the shipping-visibility predicate into a single
shared helper and have both consumers use it: create a pure function (e.g.,
deriveShippingVisibility or export shippingControlsEnabled) that accepts
AdminOrderDetail (and any small flags) and returns the same visibility matrix
currently produced by shippingControlsEnabled; replace the duplicate logic in
ShippingActions.tsx (the showShippingActions predicate) and the local
shippingControlsEnabled usage with imports from that shared helper so both
server/page and client/component derive visibility from the exact same
implementation.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9168aca8-9f67-49d0-bb11-2562dcd04e50

📥 Commits

Reviewing files that changed from the base of the PR and between ab45a85 and 7e3454b.

📒 Files selected for processing (22)
  • frontend/app/[locale]/admin/shop/orders/[id]/CancelPaymentButton.tsx
  • frontend/app/[locale]/admin/shop/orders/[id]/RefundButton.tsx
  • frontend/app/[locale]/admin/shop/orders/[id]/ShippingActions.tsx
  • frontend/app/[locale]/admin/shop/orders/[id]/lifecycle/route.ts
  • frontend/app/[locale]/admin/shop/orders/[id]/page.tsx
  • frontend/app/[locale]/admin/shop/products/[id]/edit/page.tsx
  • frontend/app/[locale]/admin/shop/products/page.tsx
  • frontend/db/queries/shop/admin-orders.ts
  • frontend/lib/services/shop/admin-order-lifecycle.ts
  • frontend/lib/tests/shop/admin-order-detail-action-errors.test.ts
  • frontend/lib/tests/shop/admin-order-detail-customer-summary.test.ts
  • frontend/lib/tests/shop/admin-order-detail-history.test.ts
  • frontend/lib/tests/shop/admin-order-detail-operational-actions.test.ts
  • frontend/lib/tests/shop/admin-order-detail-query-shaping.test.ts
  • frontend/lib/tests/shop/admin-order-lifecycle-actions.test.ts
  • frontend/lib/tests/shop/admin-order-lifecycle-route.test.ts
  • frontend/lib/tests/shop/admin-order-timeline-query.test.ts
  • frontend/lib/tests/shop/admin-orders-messages.test.ts
  • frontend/lib/tests/shop/admin-orders-query-filters.test.ts
  • frontend/messages/en.json
  • frontend/messages/pl.json
  • frontend/messages/uk.json
✅ Files skipped from review due to trivial changes (3)
  • frontend/lib/tests/shop/admin-orders-query-filters.test.ts
  • frontend/lib/tests/shop/admin-orders-messages.test.ts
  • frontend/lib/tests/shop/admin-order-detail-history.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
  • frontend/app/[locale]/admin/shop/orders/[id]/CancelPaymentButton.tsx
  • frontend/app/[locale]/admin/shop/products/page.tsx
  • frontend/app/[locale]/admin/shop/orders/[id]/lifecycle/route.ts
  • frontend/db/queries/shop/admin-orders.ts

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
frontend/app/[locale]/admin/shop/orders/[id]/page.tsx (1)

822-843: ⚠️ Potential issue | 🟡 Minor

Account email may display twice when name is missing.

When order.userId exists and only accountEmail is present (no accountName), line 828 renders accountEmail as the primary, and line 831-835 renders it again as secondary since the condition only checks customerSummary.accountEmail.

🔧 Suggested fix to show secondary email only when both exist
                     </div>
-                      {customerSummary.accountEmail ? (
+                      {customerSummary.accountName &&
+                      customerSummary.accountEmail ? (
                         <div className="text-muted-foreground text-xs break-all">
                           {customerSummary.accountEmail}
                         </div>
                       ) : null}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/app/`[locale]/admin/shop/orders/[id]/page.tsx around lines 822 -
843, The UI can render the same email twice when order.userId exists but
accountName is missing; update the rendering logic in the component that uses
order.userId, customerSummary.accountName and customerSummary.accountEmail so
the secondary email block is only shown when both accountName and accountEmail
exist (i.e., render primary as accountName ?? accountEmail, but render the
secondary/email-as-subtext only when accountName && accountEmail is truthy).
🧹 Nitpick comments (1)
frontend/lib/tests/shop/admin-order-lifecycle-actions.test.ts (1)

74-95: Consider extracting the as any type assertions into a typed test helper.

The insertOrder function uses as any to bypass strict typing. While acceptable for tests, a typed partial insert helper could improve maintainability if the schema evolves.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/lib/tests/shop/admin-order-lifecycle-actions.test.ts` around lines
74 - 95, The test helper currently bypasses the DB type system with "as any";
replace that with a properly typed helper by declaring an InsertOrderParams (or
use Partial<Order> from your schema) and updating the insertOrder function to
accept InsertOrderParams, merge defaults, and pass the resulting object to
db.insert(orders).values(...) without "as any" (refer to symbols db.insert,
orders, toDbMoney, and the insertOrder helper) so tests stay type-safe as the
schema evolves.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@frontend/app/`[locale]/admin/shop/orders/[id]/page.tsx:
- Around line 822-843: The UI can render the same email twice when order.userId
exists but accountName is missing; update the rendering logic in the component
that uses order.userId, customerSummary.accountName and
customerSummary.accountEmail so the secondary email block is only shown when
both accountName and accountEmail exist (i.e., render primary as accountName ??
accountEmail, but render the secondary/email-as-subtext only when accountName &&
accountEmail is truthy).

---

Nitpick comments:
In `@frontend/lib/tests/shop/admin-order-lifecycle-actions.test.ts`:
- Around line 74-95: The test helper currently bypasses the DB type system with
"as any"; replace that with a properly typed helper by declaring an
InsertOrderParams (or use Partial<Order> from your schema) and updating the
insertOrder function to accept InsertOrderParams, merge defaults, and pass the
resulting object to db.insert(orders).values(...) without "as any" (refer to
symbols db.insert, orders, toDbMoney, and the insertOrder helper) so tests stay
type-safe as the schema evolves.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 14e27ddd-fd90-49e8-9de4-71169d19f028

📥 Commits

Reviewing files that changed from the base of the PR and between 7e3454b and f3bd4fd.

📒 Files selected for processing (12)
  • frontend/app/[locale]/admin/shop/orders/[id]/RefundButton.tsx
  • frontend/app/[locale]/admin/shop/orders/[id]/ShippingActions.tsx
  • frontend/app/[locale]/admin/shop/orders/[id]/page.tsx
  • frontend/app/[locale]/admin/shop/orders/[id]/shippingActionVisibility.ts
  • frontend/lib/services/shop/admin-order-lifecycle.ts
  • frontend/lib/tests/shop/admin-order-detail-action-errors.test.ts
  • frontend/lib/tests/shop/admin-order-detail-customer-summary.test.ts
  • frontend/lib/tests/shop/admin-order-detail-operational-actions.test.ts
  • frontend/lib/tests/shop/admin-order-lifecycle-actions.test.ts
  • frontend/lib/tests/shop/admin-order-lifecycle-route.test.ts
  • frontend/messages/en.json
  • frontend/messages/pl.json
✅ Files skipped from review due to trivial changes (3)
  • frontend/lib/tests/shop/admin-order-detail-action-errors.test.ts
  • frontend/lib/tests/shop/admin-order-lifecycle-route.test.ts
  • frontend/messages/pl.json
🚧 Files skipped from review as they are similar to previous changes (2)
  • frontend/lib/tests/shop/admin-order-detail-operational-actions.test.ts
  • frontend/messages/en.json

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

♻️ Duplicate comments (2)
frontend/app/[locale]/admin/shop/orders/[id]/page.tsx (2)

475-479: ⚠️ Potential issue | 🟠 Major

Don't hide order cancel behind the payment control.

getAdminOrderLifecycleAvailability() already decides whether cancel is allowed. Adding !paymentEnabled.cancelPayment here suppresses that valid lifecycle action for pending Monobank orders, even though the lifecycle service coverage in frontend/lib/tests/shop/admin-order-lifecycle-actions.test.ts still exercises action: 'cancel' for that state. The payment-side cancel button is additive; it does not replace canceling/restocking the order.

🩹 Proposed fix
   const visibleLifecycle = {
     confirm: enabled.confirm,
-    cancel: enabled.cancel && !paymentEnabled.cancelPayment,
+    cancel: enabled.cancel,
     complete: enabled.complete && !shippingEnabled.markDelivered,
   };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/app/`[locale]/admin/shop/orders/[id]/page.tsx around lines 475 -
479, The cancel lifecycle action is being incorrectly suppressed in the
visibleLifecycle object by checking paymentEnabled.cancelPayment; remove the
paymentEnabled.cancelPayment check so visibleLifecycle.cancel uses only the
decision from getAdminOrderLifecycleAvailability() (i.e., keep
visibleLifecycle.cancel: enabled.cancel). Update the assignment that defines
visibleLifecycle (referencing visibleLifecycle, enabled, and
paymentEnabled.cancelPayment) to stop gating cancel on
paymentEnabled.cancelPayment so order cancel/restock remains available when
getAdminOrderLifecycleAvailability() allows it.

164-185: ⚠️ Potential issue | 🟡 Minor

Keep shipping labels humanized for non-mapped values.

These helpers still fall back to detailValue(), so any provider/method you don't explicitly map is rendered as the raw enum again. frontend/lib/tests/shop/admin-order-lifecycle-actions.test.ts already models non-nova_poshta provider values, so this page can still hit that path.

♻️ Proposed fix
 function humanizeShippingProvider(
   value: string | null,
   t: (key: string) => string
 ): string {
   if (value === 'nova_poshta') return t('shippingProviders.novaPoshta');
-  return detailValue(value);
+  return humanizeCode(value);
 }
 
 function humanizeShippingMethod(
   value: string | null,
   t: (key: string) => string
 ): string {
   switch (value) {
     case 'NP_WAREHOUSE':
       return t('shippingMethods.novaPoshtaWarehouse');
     case 'NP_LOCKER':
       return t('shippingMethods.novaPoshtaLocker');
     case 'NP_COURIER':
       return t('shippingMethods.novaPoshtaCourier');
     default:
-      return detailValue(value);
+      return humanizeCode(value);
   }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/app/`[locale]/admin/shop/orders/[id]/page.tsx around lines 164 -
185, humanizeShippingProvider and humanizeShippingMethod currently fall back to
detailValue(value) which returns the raw enum; instead try to resolve a
translation key for unknown values before falling back. Update
humanizeShippingProvider to return t(`shippingProviders.${value}`) (guarding for
null) when value isn't 'nova_poshta' and update humanizeShippingMethod to
attempt t(`shippingMethods.${value}`) for any unmapped case (e.g., default
branch) before calling detailValue(value); reference the functions
humanizeShippingProvider, humanizeShippingMethod and the fallback detailValue to
locate the spots to change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@frontend/app/`[locale]/admin/shop/orders/[id]/page.tsx:
- Around line 579-623: The lifecycle action buttons (the forms rendered when
visibleLifecycle.confirm/cancel/complete are true) use hard-coded low-contrast
text classes like text-emerald-100 / text-amber-100 / text-sky-100 which become
unreadable on light theme; update each button's className to use a theme-aware
text color (for example replace text-emerald-100 with text-emerald-700
dark:text-emerald-100, text-amber-100 with text-amber-700 dark:text-amber-100,
and text-sky-100 with text-sky-700 dark:text-sky-100) so the same buttons keep
good contrast in light and dark modes while preserving their colored accents.

In `@frontend/lib/tests/shop/admin-order-detail-customer-summary.test.ts`:
- Line 6: Tests mutate process.env.CSRF_SECRET in beforeEach but never restore
it; capture the original value (e.g., const _origCsrf = process.env.CSRF_SECRET)
before mutating in the beforeEach that touches process.env.CSRF_SECRET and add
an afterEach that restores it (if _origCsrf is undefined, delete
process.env.CSRF_SECRET, otherwise set it back) so the mutation in this suite
(see beforeEach that sets process.env.CSRF_SECRET) does not leak to other tests.

In `@frontend/lib/tests/shop/admin-order-lifecycle-actions.test.ts`:
- Around line 11-32: The test helper ensureAdminUser currently inserts a
hard-coded ADMIN_USER_ID ('admin-1') with onConflictDoNothing(), which leaves
stale rows in shared DB and makes tests order-dependent; change the setup so
each test gets a fresh admin or the row is deterministically repaired: either
generate a unique admin id per test (replace ADMIN_USER_ID usage) or perform an
upsert/repair before use (e.g., use users.insert(...).onConflictDoUpdate(...) or
delete any existing users where id = ADMIN_USER_ID then insert) so
ensureAdminUser/ADMIN_USER_ID no longer silently accept stale data when calling
the tested service.

---

Duplicate comments:
In `@frontend/app/`[locale]/admin/shop/orders/[id]/page.tsx:
- Around line 475-479: The cancel lifecycle action is being incorrectly
suppressed in the visibleLifecycle object by checking
paymentEnabled.cancelPayment; remove the paymentEnabled.cancelPayment check so
visibleLifecycle.cancel uses only the decision from
getAdminOrderLifecycleAvailability() (i.e., keep visibleLifecycle.cancel:
enabled.cancel). Update the assignment that defines visibleLifecycle
(referencing visibleLifecycle, enabled, and paymentEnabled.cancelPayment) to
stop gating cancel on paymentEnabled.cancelPayment so order cancel/restock
remains available when getAdminOrderLifecycleAvailability() allows it.
- Around line 164-185: humanizeShippingProvider and humanizeShippingMethod
currently fall back to detailValue(value) which returns the raw enum; instead
try to resolve a translation key for unknown values before falling back. Update
humanizeShippingProvider to return t(`shippingProviders.${value}`) (guarding for
null) when value isn't 'nova_poshta' and update humanizeShippingMethod to
attempt t(`shippingMethods.${value}`) for any unmapped case (e.g., default
branch) before calling detailValue(value); reference the functions
humanizeShippingProvider, humanizeShippingMethod and the fallback detailValue to
locate the spots to change.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0cfbe26b-8db5-46cf-92f5-9001c0cb8b66

📥 Commits

Reviewing files that changed from the base of the PR and between f3bd4fd and 17e71ea.

📒 Files selected for processing (3)
  • frontend/app/[locale]/admin/shop/orders/[id]/page.tsx
  • frontend/lib/tests/shop/admin-order-detail-customer-summary.test.ts
  • frontend/lib/tests/shop/admin-order-lifecycle-actions.test.ts

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@frontend/app/`[locale]/admin/shop/orders/[id]/page.tsx:
- Around line 630-637: The lifecycle error alert uses a low-contrast static
class (text-amber-100) that’s unreadable in light theme; update the JSX that
renders the alert for lifecycleErrorKey (the paragraph using
t(lifecycleErrorKey)) to use a theme-aware color, e.g. replace the static
text-amber-100 with a light-theme color plus dark override such as
text-amber-700 dark:text-amber-100 so the alert has good contrast in both light
and dark modes.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 05c62a44-1b87-49bb-a792-3658d84ee62e

📥 Commits

Reviewing files that changed from the base of the PR and between 17e71ea and 2c3f8d3.

📒 Files selected for processing (3)
  • frontend/app/[locale]/admin/shop/orders/[id]/page.tsx
  • frontend/lib/tests/shop/admin-order-detail-customer-summary.test.ts
  • frontend/lib/tests/shop/admin-order-lifecycle-actions.test.ts
✅ Files skipped from review due to trivial changes (1)
  • frontend/lib/tests/shop/admin-order-lifecycle-actions.test.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant