(SP: 3) [SHOP] complete phase 3 admin operations surface and admin UI polish#418
(SP: 3) [SHOP] complete phase 3 admin operations surface and admin UI polish#418ViktorSvertoka merged 12 commits intodevelopfrom
Conversation
…aligned UI polish
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds 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
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
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
💡 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".
There was a problem hiding this comment.
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 | 🟡 MinorIndentation issue in
fulfillmentStagesobject requires correction.The
fulfillmentStagesobject (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.messagecontains a browser error string (e.g.,"Failed to fetch"or"NetworkError") rather than the expected code'NETWORK_ERROR'. This meansmapShippingErrorwill almost always hit thedefaultcase, returningt('errors.generic')instead oft('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:
paidLateMarchIdat line 93 uses April 30paidLateMarchIdat line 137 uses May 31aprilIdat line 141 uses June 2paidMidMarchId/paidLateMarchIdat lines 191/195 use June datesConsider renaming to reflect actual months or use neutral names like
paidLateId,paidMidIdfor 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, andfulfillmentStageshave extra leading indentation compared to sibling keys liketotal(line 901) andpaymentStatus(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.messagepassed tomapCancelPaymentErrorwill be browser error text, not a code. Consider using'NETWORK_ERROR'directly as suggested forRefundButton.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?.messagewhileRefundButtonusesjson?.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: EmptyafterEachcan be removed.The comment explains cleanup happens in
finallyblocks, but the emptyafterEachhook 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
fetchthrows (network failure),err.messagecontains browser error text like "Failed to fetch", not an error code. Passing this tomapRefundErrorwill always hit thedefaultcase.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
📒 Files selected for processing (31)
frontend/app/[locale]/admin/shop/orders/[id]/CancelPaymentButton.tsxfrontend/app/[locale]/admin/shop/orders/[id]/RefundButton.tsxfrontend/app/[locale]/admin/shop/orders/[id]/ShippingActions.tsxfrontend/app/[locale]/admin/shop/orders/[id]/lifecycle/route.tsfrontend/app/[locale]/admin/shop/orders/[id]/page.tsxfrontend/app/[locale]/admin/shop/orders/page.tsxfrontend/app/[locale]/admin/shop/products/[id]/edit/page.tsxfrontend/app/[locale]/admin/shop/products/_components/ProductForm.tsxfrontend/app/[locale]/admin/shop/products/new/page.tsxfrontend/app/[locale]/admin/shop/products/page.tsxfrontend/app/api/shop/admin/orders/route.tsfrontend/components/admin/shop/AdminPagination.tsxfrontend/components/admin/shop/AdminProductDeleteButton.tsxfrontend/components/admin/shop/AdminProductStatusToggle.tsxfrontend/components/tests/shop/admin-order-detail-action-errors.test.tsxfrontend/db/queries/shop/admin-orders.tsfrontend/lib/services/shop/admin-order-lifecycle.tsfrontend/lib/tests/shop/admin-edit-product-page-error-handling.test.tsfrontend/lib/tests/shop/admin-order-detail-customer-summary.test.tsfrontend/lib/tests/shop/admin-order-detail-history.test.tsfrontend/lib/tests/shop/admin-order-detail-operational-actions.test.tsfrontend/lib/tests/shop/admin-order-lifecycle-actions.test.tsfrontend/lib/tests/shop/admin-order-timeline-query.test.tsfrontend/lib/tests/shop/admin-orders-messages.test.tsfrontend/lib/tests/shop/admin-orders-page-filters.test.tsfrontend/lib/tests/shop/admin-orders-query-filters.test.tsfrontend/lib/tests/shop/admin-orders-route-filters.test.tsfrontend/lib/validation/shop-admin-orders.tsfrontend/messages/en.jsonfrontend/messages/pl.jsonfrontend/messages/uk.json
frontend/app/[locale]/admin/shop/orders/[id]/lifecycle/route.ts
Outdated
Show resolved
Hide resolved
…, lifecycle, and admin action UX
There was a problem hiding this comment.
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 | 🟠 MajorDisable shipping actions for the full request duration, not just for
router.refresh().The
disabled={isPending}check at line 202 only covers thestartTransition()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 | 🟠 MajorConfirm retries still cannot backfill a missing audit row for non-shipping orders.
The
repairOnlypath returns beforewriteAdminAudit()whenever shipment repair is ineligible. If the first confirm already moved the order toPAIDand 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 fromadminAuditLog, 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 withShippingActions.tsx.This predicate now exists twice: once here for
showShippingActions, and again infrontend/app/[locale]/admin/shop/orders/[id]/ShippingActions.tsxfor 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
📒 Files selected for processing (22)
frontend/app/[locale]/admin/shop/orders/[id]/CancelPaymentButton.tsxfrontend/app/[locale]/admin/shop/orders/[id]/RefundButton.tsxfrontend/app/[locale]/admin/shop/orders/[id]/ShippingActions.tsxfrontend/app/[locale]/admin/shop/orders/[id]/lifecycle/route.tsfrontend/app/[locale]/admin/shop/orders/[id]/page.tsxfrontend/app/[locale]/admin/shop/products/[id]/edit/page.tsxfrontend/app/[locale]/admin/shop/products/page.tsxfrontend/db/queries/shop/admin-orders.tsfrontend/lib/services/shop/admin-order-lifecycle.tsfrontend/lib/tests/shop/admin-order-detail-action-errors.test.tsfrontend/lib/tests/shop/admin-order-detail-customer-summary.test.tsfrontend/lib/tests/shop/admin-order-detail-history.test.tsfrontend/lib/tests/shop/admin-order-detail-operational-actions.test.tsfrontend/lib/tests/shop/admin-order-detail-query-shaping.test.tsfrontend/lib/tests/shop/admin-order-lifecycle-actions.test.tsfrontend/lib/tests/shop/admin-order-lifecycle-route.test.tsfrontend/lib/tests/shop/admin-order-timeline-query.test.tsfrontend/lib/tests/shop/admin-orders-messages.test.tsfrontend/lib/tests/shop/admin-orders-query-filters.test.tsfrontend/messages/en.jsonfrontend/messages/pl.jsonfrontend/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
…, action safety, and detail UI
There was a problem hiding this comment.
♻️ Duplicate comments (1)
frontend/app/[locale]/admin/shop/orders/[id]/page.tsx (1)
822-843:⚠️ Potential issue | 🟡 MinorAccount email may display twice when name is missing.
When
order.userIdexists and onlyaccountEmailis present (noaccountName), line 828 rendersaccountEmailas the primary, and line 831-835 renders it again as secondary since the condition only checkscustomerSummary.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 theas anytype assertions into a typed test helper.The
insertOrderfunction usesas anyto 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
📒 Files selected for processing (12)
frontend/app/[locale]/admin/shop/orders/[id]/RefundButton.tsxfrontend/app/[locale]/admin/shop/orders/[id]/ShippingActions.tsxfrontend/app/[locale]/admin/shop/orders/[id]/page.tsxfrontend/app/[locale]/admin/shop/orders/[id]/shippingActionVisibility.tsfrontend/lib/services/shop/admin-order-lifecycle.tsfrontend/lib/tests/shop/admin-order-detail-action-errors.test.tsfrontend/lib/tests/shop/admin-order-detail-customer-summary.test.tsfrontend/lib/tests/shop/admin-order-detail-operational-actions.test.tsfrontend/lib/tests/shop/admin-order-lifecycle-actions.test.tsfrontend/lib/tests/shop/admin-order-lifecycle-route.test.tsfrontend/messages/en.jsonfrontend/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
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
frontend/app/[locale]/admin/shop/orders/[id]/page.tsx (2)
475-479:⚠️ Potential issue | 🟠 MajorDon't hide order cancel behind the payment control.
getAdminOrderLifecycleAvailability()already decides whether cancel is allowed. Adding!paymentEnabled.cancelPaymenthere suppresses that valid lifecycle action for pending Monobank orders, even though the lifecycle service coverage infrontend/lib/tests/shop/admin-order-lifecycle-actions.test.tsstill exercisesaction: '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 | 🟡 MinorKeep 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.tsalready models non-nova_poshtaprovider 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
📒 Files selected for processing (3)
frontend/app/[locale]/admin/shop/orders/[id]/page.tsxfrontend/lib/tests/shop/admin-order-detail-customer-summary.test.tsfrontend/lib/tests/shop/admin-order-lifecycle-actions.test.ts
frontend/lib/tests/shop/admin-order-detail-customer-summary.test.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
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
📒 Files selected for processing (3)
frontend/app/[locale]/admin/shop/orders/[id]/page.tsxfrontend/lib/tests/shop/admin-order-detail-customer-summary.test.tsfrontend/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
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:
Related Issue
Issue: #<issue_number>
Changes
Implemented ADM-03 for Admin Orders:
status,dateFrom, anddateToCompleted Admin Orders UI polish:
Polished Admin Products admin UI:
Implemented ADM-04 for Admin Order Detail:
Implemented ADM-05 for Admin lifecycle actions:
confirm / cancel / completeadmin actionsMade the Admin Order Detail page the primary operational admin surface:
Database Changes (if applicable)
How Has This Been Tested?
Additional notes:
Screenshots (if applicable)
Checklist
Before submitting
Reviewers
Summary by CodeRabbit
New Features
Improvements
Tests