(SP: 3) [SHOP] complete cleanup across merchandising, admin operations, shipment visibility, and public runtime safety#439
Merged
ViktorSvertoka merged 7 commits intodevelopfrom Apr 1, 2026
Conversation
…ped checkout surfaces
…ke coverage and cart env contract follow-up
liudmylasovetovs
requested review from
AM1007,
LesiaUKR and
ViktorSvertoka
as code owners
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📝 WalkthroughWalkthroughAdds an admin-facing Nova Poshta shipping editor (client form + PATCH API + server service), threads shipmentStatus/trackingNumber through order models and UI, canonicalizes public catalog queries and storefront categories, extends admin audit logging for refund/cancel/edit flows, updates runtime/env reads, and adds extensive tests. Changes
Sequence DiagramsequenceDiagram
participant Admin as Admin (Browser)
participant Form as ShippingEditForm (Client)
participant API as PATCH /api/.../shipping (Server)
participant Service as applyAdminOrderShippingEdit (Service)
participant DB as Database (orders, orderShipping, npCities/npWarehouses)
participant Audit as AdminAudit (writeAdminAudit)
Admin->>Form: Open editor & submit shipping changes
Form->>Form: Validate inputs & block duplicate submits
Form->>API: PATCH JSON + x-csrf-token (same-origin, credentials)
API->>API: Enforce same-origin, require admin auth, validate CSRF, validate payload
API->>Service: invoke applyAdminOrderShippingEdit(orderId, shipping, actorUserId, requestId)
Service->>DB: Lock order row, load current shipping snapshot
Service->>DB: Resolve city/warehouse data and compare snapshots
alt snapshots identical
Service-->>API: return { changed: false, order: { id, shippingMethodCode } }
else snapshots differ
Service->>DB: update orders row, upsert orderShipping
Service->>Audit: writeAdminAudit(edit_shipping, payload, dedupeSeed)
Service-->>API: return { changed: true, order: { id, shippingMethodCode } }
end
API-->>Form: HTTP 200 JSON (success/changed)
Form->>Form: clear submission state
Form->>Admin: trigger router.refresh() in startTransition
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 docstrings
🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b40d4a62e
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
frontend/lib/services/shop/shipping/admin-edit.ts (1)
143-148: JSON.stringify comparison is order-dependent.
snapshotsEqualrelies onJSON.stringifyfor deep equality. This works correctly here becausebuildNextComparableandtoComparableSnapshotconstruct objects with consistent property order. However, this is fragile if the structure changes.Consider using a deep-equal utility if this comparison becomes more complex in the future.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/lib/services/shop/shipping/admin-edit.ts` around lines 143 - 148, snapshotsEqual uses JSON.stringify which is order-dependent and fragile; replace the stringify comparison in snapshotsEqual with a proper deep-equality check (e.g., import and call a known deep-equal utility such as lodash/isEqual or fast-deep-equal) and ensure it handles the nullable left parameter (call deepEqual(left, right) or treat null appropriately). Update the imports and change the function body in snapshotsEqual to use the chosen deep-equal function instead of JSON.stringify to make comparisons robust against property order changes.frontend/app/[locale]/admin/shop/orders/[id]/ShippingEditForm.tsx (2)
113-154: Consider adding client-side required field validation.The form submits directly without checking if required fields (cityRef, recipientFullName, recipientPhone) are populated. While the server validates the payload, providing immediate client-side feedback improves UX:
- Add
requiredattribute to mandatory inputs, or- Check values before fetch and set a local error.
This prevents unnecessary network round-trips for obviously invalid submissions.
🤖 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]/ShippingEditForm.tsx around lines 113 - 154, The onSubmit handler sends the PATCH without validating required fields, so add client-side validation in onSubmit before calling fetch: check cityRef, recipientFullName, and recipientPhone (and warehouseRef when isWarehouseMethod is true) and if any are missing call setError(...) with a user-friendly message and setIsSubmitting(false) and return; alternatively add HTML required attributes on the corresponding inputs to enforce browser validation, but ensure onSubmit still guards against empty values by validating and returning early to avoid the network request (update the onSubmit function and any related UI input elements that bind cityRef, recipientFullName, recipientPhone, warehouseRef).
342-350: Addaria-describedbyto link submit button to error message.The error message uses
role="alert"which announces changes, but linking it to the submit button viaaria-describedbywould provide better context for screen reader users when they focus the button after an error:♿ Proposed accessibility improvement
<button type="submit" disabled={isSubmitting || isPending} aria-busy={isSubmitting || isPending} + aria-describedby={error ? errorId : undefined} className="rounded-lg border border-emerald-500/30 bg-emerald-500/5 px-3 py-2 text-sm font-medium text-emerald-700 transition-colors hover:bg-emerald-500/10 disabled:cursor-not-allowed disabled:opacity-50 dark:text-emerald-100" >🤖 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]/ShippingEditForm.tsx around lines 342 - 350, The submit button currently shown in the <button ...> markup (controlled by isSubmitting and isPending and rendering tEditor('save')/tEditor('saving')) should include an aria-describedby that points to the error message element so screen readers can associate the button with the alert; add a stable id (e.g., shippingErrorId or similar) to the element that renders the error message (the element with role="alert") and conditionally add aria-describedby={errorId} to the submit button when an error exists so the button references that role="alert" node for context.frontend/app/api/shop/admin/orders/[id]/shipping/route.ts (1)
282-290: Consider extracting duplicated actor ID extraction logic.The pattern for extracting
actorUserId(lines 285-288) is duplicated from the POST handler (lines 127-130):actorUserId: typeof adminUser?.id === 'string' && adminUser.id.trim().length > 0 ? adminUser.id : null,Consider extracting this to a small helper function to reduce duplication and ensure consistent behavior:
function extractActorUserId(adminUser: { id?: string } | null): string | null { return typeof adminUser?.id === 'string' && adminUser.id.trim().length > 0 ? adminUser.id : null; }This is a minor DRY improvement and not blocking.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/api/shop/admin/orders/`[id]/shipping/route.ts around lines 282 - 290, The duplicated actor ID extraction logic used when calling applyAdminOrderShippingEdit and in the POST handler should be moved into a small helper to avoid repetition and ensure consistent behavior; add a function like extractActorUserId(adminUser: { id?: string } | null): string | null that returns adminUser.id when typeof adminUser?.id === 'string' and adminUser.id.trim().length > 0, otherwise null, then replace the inline ternary in both callers (the place constructing the actorUserId for applyAdminOrderShippingEdit and the POST handler) with a call to extractActorUserId(adminUser).frontend/lib/tests/shop/admin-shipping-edit.test.ts (1)
47-67: Consider extracting test fixture factory or using a test helper module.The
seedEditableOrderfunction usesas anyassertions (lines 55, 67, 85, 116, 128) to bypass TypeScript when inserting test data. While this works for tests, it means the test won't catch schema drift.If the schema changes (e.g., a required field is added), these tests will pass locally but the actual service may fail. Consider:
- Using a shared test factory that mirrors the actual insertion logic, or
- Importing and using the same insert helpers used by the production code.
This is a trade-off between test isolation and catching schema mismatches early.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/lib/tests/shop/admin-shipping-edit.test.ts` around lines 47 - 67, The test uses unsafe "as any" casts when inserting fixtures (see seedEditableOrder and the npCities/npWarehouses inserts) which hides schema drift; replace these casts by creating properly typed fixture objects or a shared test factory that returns the correct types (or import and reuse the production insert helpers) and use those typed objects in db.insert calls; update all occurrences of "as any" referenced in seedEditableOrder (lines near the npCities/npWarehouses inserts and the other spots called out in the review) so the test will fail when required schema fields change.frontend/messages/pl.json (1)
896-914: Unicode escapes are inconsistent with the rest of the file.The
shippingEditorsection (lines 896-914) uses Unicode escapes like\u00f3(ó),\u0142(ł),\u0119(ę), etc., while the rest of the Polish translations use UTF-8 characters directly. This works correctly but creates inconsistency that makes the file harder to maintain.Consider normalizing to UTF-8 characters for consistency:
- "subtitle": "Zapisuj tylko zweryfikowane referencje Nova Poshta i dane odbiorcy dla tego zam\u00f3wienia.", + "subtitle": "Zapisuj tylko zweryfikowane referencje Nova Poshta i dane odbiorcy dla tego zamówienia.",This is a minor maintainability concern and not blocking.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/messages/pl.json` around lines 896 - 914, The shippingEditor block contains escaped Unicode sequences (e.g., "\u00f3", "\u0142", "\u0119") which are inconsistent with the rest of the file; replace those escapes in the "shippingEditor" object (keys like heading, subtitle, addressLine1, addressLine2, currentCity, currentPickupPoint, save, saving and each entry under errors: network, security, invalid, notAllowed, adminDisabled, generic) with their direct UTF-8 Polish characters (ó, ł, ę, ś, etc.) and save the JSON file as UTF-8 to maintain consistent encoding.
🤖 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/api/shop/admin/orders/`[id]/cancel-payment/route.ts:
- Around line 110-135: The audit write using writeAdminAudit (with
orderIdForLog, requestId and payload built from result) must be guarded so
failures don't turn a successful cancel into a 5xx; wrap the writeAdminAudit
invocation in a try/catch (or fire-and-forget Promise.catch) and on error log
the failure with contextual identifiers (orderIdForLog, requestId, action:
'cancel_payment') but do not rethrow or change the response flow so the cancel
result is returned to the client even if the audit fails.
In `@frontend/app/api/shop/admin/orders/`[id]/refund/route.ts:
- Around line 166-187: The current awaited call to writeAdminAudit can cause the
whole refund endpoint to fail if audit persistence errors; change this so
refundOrder's success isn't blocked by audit logging: after calling refundOrder
(the successful path that uses orderSummary and orderIdForLog), invoke
writeAdminAudit in a non-fatal way—either fire-and-forget (do not await) or wrap
the await in a try/catch that catches/logs any error (use requestId,
orderIdForLog, adminUser context) and does not rethrow; ensure the catch logs
the failure and moves on so the endpoint returns the successful refund response
even if writeAdminAudit fails.
---
Nitpick comments:
In `@frontend/app/`[locale]/admin/shop/orders/[id]/ShippingEditForm.tsx:
- Around line 113-154: The onSubmit handler sends the PATCH without validating
required fields, so add client-side validation in onSubmit before calling fetch:
check cityRef, recipientFullName, and recipientPhone (and warehouseRef when
isWarehouseMethod is true) and if any are missing call setError(...) with a
user-friendly message and setIsSubmitting(false) and return; alternatively add
HTML required attributes on the corresponding inputs to enforce browser
validation, but ensure onSubmit still guards against empty values by validating
and returning early to avoid the network request (update the onSubmit function
and any related UI input elements that bind cityRef, recipientFullName,
recipientPhone, warehouseRef).
- Around line 342-350: The submit button currently shown in the <button ...>
markup (controlled by isSubmitting and isPending and rendering
tEditor('save')/tEditor('saving')) should include an aria-describedby that
points to the error message element so screen readers can associate the button
with the alert; add a stable id (e.g., shippingErrorId or similar) to the
element that renders the error message (the element with role="alert") and
conditionally add aria-describedby={errorId} to the submit button when an error
exists so the button references that role="alert" node for context.
In `@frontend/app/api/shop/admin/orders/`[id]/shipping/route.ts:
- Around line 282-290: The duplicated actor ID extraction logic used when
calling applyAdminOrderShippingEdit and in the POST handler should be moved into
a small helper to avoid repetition and ensure consistent behavior; add a
function like extractActorUserId(adminUser: { id?: string } | null): string |
null that returns adminUser.id when typeof adminUser?.id === 'string' and
adminUser.id.trim().length > 0, otherwise null, then replace the inline ternary
in both callers (the place constructing the actorUserId for
applyAdminOrderShippingEdit and the POST handler) with a call to
extractActorUserId(adminUser).
In `@frontend/lib/services/shop/shipping/admin-edit.ts`:
- Around line 143-148: snapshotsEqual uses JSON.stringify which is
order-dependent and fragile; replace the stringify comparison in snapshotsEqual
with a proper deep-equality check (e.g., import and call a known deep-equal
utility such as lodash/isEqual or fast-deep-equal) and ensure it handles the
nullable left parameter (call deepEqual(left, right) or treat null
appropriately). Update the imports and change the function body in
snapshotsEqual to use the chosen deep-equal function instead of JSON.stringify
to make comparisons robust against property order changes.
In `@frontend/lib/tests/shop/admin-shipping-edit.test.ts`:
- Around line 47-67: The test uses unsafe "as any" casts when inserting fixtures
(see seedEditableOrder and the npCities/npWarehouses inserts) which hides schema
drift; replace these casts by creating properly typed fixture objects or a
shared test factory that returns the correct types (or import and reuse the
production insert helpers) and use those typed objects in db.insert calls;
update all occurrences of "as any" referenced in seedEditableOrder (lines near
the npCities/npWarehouses inserts and the other spots called out in the review)
so the test will fail when required schema fields change.
In `@frontend/messages/pl.json`:
- Around line 896-914: The shippingEditor block contains escaped Unicode
sequences (e.g., "\u00f3", "\u0142", "\u0119") which are inconsistent with the
rest of the file; replace those escapes in the "shippingEditor" object (keys
like heading, subtitle, addressLine1, addressLine2, currentCity,
currentPickupPoint, save, saving and each entry under errors: network, security,
invalid, notAllowed, adminDisabled, generic) with their direct UTF-8 Polish
characters (ó, ł, ę, ś, etc.) and save the JSON file as UTF-8 to maintain
consistent encoding.
🪄 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: 0f070133-f8b6-4e0c-bf21-251bb5cb408b
📒 Files selected for processing (47)
frontend/app/[locale]/admin/shop/orders/[id]/ShippingEditForm.tsxfrontend/app/[locale]/admin/shop/orders/[id]/page.tsxfrontend/app/[locale]/shop/cart/capabilities.tsfrontend/app/[locale]/shop/cart/page.tsxfrontend/app/[locale]/shop/checkout/success/MonobankRedirectStatus.tsxfrontend/app/[locale]/shop/checkout/success/page.tsxfrontend/app/[locale]/shop/orders/[id]/page.tsxfrontend/app/[locale]/shop/orders/page.tsxfrontend/app/[locale]/shop/page.tsxfrontend/app/[locale]/shop/products/[slug]/page.tsxfrontend/app/[locale]/shop/products/page.tsxfrontend/app/api/shop/admin/orders/[id]/cancel-payment/route.tsfrontend/app/api/shop/admin/orders/[id]/refund/route.tsfrontend/app/api/shop/admin/orders/[id]/shipping/route.tsfrontend/app/api/shop/catalog/route.tsfrontend/components/shop/ProductFilters.tsxfrontend/db/queries/shop/products.tsfrontend/lib/config/catalog.tsfrontend/lib/env/server-env.tsfrontend/lib/env/shop-legal.tsfrontend/lib/services/orders/payment-intent.tsfrontend/lib/services/orders/summary.tsfrontend/lib/services/shop/shipping/admin-edit.tsfrontend/lib/shop/catalog-query.tsfrontend/lib/shop/data.tsfrontend/lib/shop/guest-shipment-status.tsfrontend/lib/tests/shop/admin-csrf-contract.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-payment-audit-routes.test.tsfrontend/lib/tests/shop/admin-order-timeline-query.test.tsfrontend/lib/tests/shop/admin-shipping-edit-route.test.tsfrontend/lib/tests/shop/admin-shipping-edit.test.tsfrontend/lib/tests/shop/catalog-merchandising-cleanup.test.tsfrontend/lib/tests/shop/catalog-query-sorting.test.tsfrontend/lib/tests/shop/checkout-success-monobank-shipment.test.tsfrontend/lib/tests/shop/checkout-success-page-access.test.tsfrontend/lib/tests/shop/order-status-token.test.tsfrontend/lib/tests/shop/public-cart-env-contract.test.tsfrontend/lib/tests/shop/public-shop-runtime-cache-smoke.test.tsfrontend/lib/types/shop.tsfrontend/lib/validation/shop-admin-shipping.tsfrontend/lib/validation/shop.tsfrontend/messages/en.jsonfrontend/messages/pl.jsonfrontend/messages/uk.json
… shipping edit form validation
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
frontend/app/[locale]/admin/shop/orders/[id]/ShippingEditForm.tsx (2)
146-165: Submit trimmed values to maintain data consistency.Validation uses trimmed values (lines 119-122), but the request body uses the original untrimmed values. This could store data with leading/trailing whitespace, causing display inconsistencies or lookup issues.
Proposed fix to use trimmed values in request body
body: JSON.stringify({ provider: 'nova_poshta', methodCode, selection: { - cityRef, + cityRef: trimmedCityRef, ...(isWarehouseMethod - ? { warehouseRef } - : { addressLine1, addressLine2 }), + ? { warehouseRef: trimmedWarehouseRef } + : { addressLine1: addressLine1.trim(), addressLine2: addressLine2.trim() }), }, recipient: { - fullName: recipientFullName, - phone: recipientPhone, + fullName: trimmedRecipientFullName, + phone: trimmedRecipientPhone, ...(recipientEmail.trim().length > 0 ? { email: recipientEmail } : {}),🤖 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]/ShippingEditForm.tsx around lines 146 - 165, The request body is sending untrimmed form fields while validation used trimmed variants; update ShippingEditForm.tsx so the body uses the trimmed values for recipientEmail, recipientComment, recipientFullName, recipientPhone and addressLine1/addressLine2/warehouseRef where applicable (reuse the existing trimmed variables or create trimmed counterparts) instead of the raw state values referenced by methodCode/isWarehouseMethod so submitted data has leading/trailing whitespace removed.
274-279: Consider adding conditionalrequiredattribute for courier address.For consistency with the server-side validation that requires
addressLine1for courier delivery, this input should have therequiredattribute when the courier method is selected. This provides browser-native validation feedback.Proposed fix
<input id="shipping-address-line-1" value={addressLine1} onChange={event => setAddressLine1(event.target.value)} + required className="border-border bg-background text-foreground w-full rounded-lg border px-3 py-2 text-sm" />🤖 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]/ShippingEditForm.tsx around lines 274 - 279, The shipping address input for addressLine1 lacks a conditional required attribute; add browser-side validation by computing a boolean (e.g., isCourier = deliveryMethod === 'courier' or shippingMethod === 'COURIER' based on your component state) and set required={isCourier} on the <input id="shipping-address-line-1"> that uses value={addressLine1} and onChange={event => setAddressLine1(event.target.value)} so the field is required only when courier delivery is selected to match server-side validation.frontend/lib/tests/shop/admin-shipping-edit-form.test.ts (1)
72-110: LGTM, but consider adding a test for courieraddressLine1requirement.This test correctly verifies
warehouseRefis required for warehouse methods. Once the client-side validation is updated to requireaddressLine1for courier delivery (as noted in the form review), consider adding a parallel test case.Example test to add after fixing form validation
it('requires addressLine1 locally for courier methods before sending PATCH', async () => { const fetchMock = vi.fn(); vi.stubGlobal('fetch', fetchMock); const { ShippingEditForm } = await import('@/app/[locale]/admin/shop/orders/[id]/ShippingEditForm'); const { container } = render( createElement(ShippingEditForm, { orderId: '550e8400-e29b-41d4-a716-446655440102', csrfToken: 'csrf-token', initialShipping: { methodCode: 'NP_COURIER', cityRef: 'city-ref', cityLabel: null, warehouseRef: null, warehouseLabel: null, addressLine1: '', addressLine2: null, recipientFullName: 'Test User', recipientPhone: '+380501112233', recipientEmail: null, recipientComment: null, }, }) ); const form = container.querySelector('form'); if (!form) throw new Error('ShippingEditForm form not rendered'); fireEvent.submit(form); await waitFor(() => { expect(screen.getByRole('alert')).toHaveTextContent( 'shop.orders.detail.shippingEditor.errors.invalid' ); }); expect(fetchMock).not.toHaveBeenCalled(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/lib/tests/shop/admin-shipping-edit-form.test.ts` around lines 72 - 110, Add a new test mirroring the existing warehouse test to assert client-side validation requires addressLine1 for courier deliveries: create a test in admin-shipping-edit-form.test.ts that imports ShippingEditForm, stubs fetch, renders ShippingEditForm with methodCode 'NP_COURIER' and addressLine1 set to an empty string, submits the form, waits for the same alert text 'shop.orders.detail.shippingEditor.errors.invalid', and asserts fetch was not called; this will ensure the form validation (ShippingEditForm) enforces addressLine1 before sending the PATCH.
🤖 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]/ShippingEditForm.tsx:
- Around line 124-133: Client validation misses requiring addressLine1 for
courier deliveries: update the hasRequiredFields check in ShippingEditForm.tsx
to also require trimmedAddressLine1 when the delivery method is courier (i.e.
when isWarehouseMethod is false / method === 'NP_COURIER'). Modify the boolean
expression that builds hasRequiredFields (alongside trimmedCityRef,
trimmedRecipientFullName, trimmedRecipientPhone, trimmedWarehouseRef) to include
trimmedAddressLine1.length > 0 for non-warehouse/courier flows, and ensure
setError(tEditor('errors.invalid')) remains used when the check fails.
---
Nitpick comments:
In `@frontend/app/`[locale]/admin/shop/orders/[id]/ShippingEditForm.tsx:
- Around line 146-165: The request body is sending untrimmed form fields while
validation used trimmed variants; update ShippingEditForm.tsx so the body uses
the trimmed values for recipientEmail, recipientComment, recipientFullName,
recipientPhone and addressLine1/addressLine2/warehouseRef where applicable
(reuse the existing trimmed variables or create trimmed counterparts) instead of
the raw state values referenced by methodCode/isWarehouseMethod so submitted
data has leading/trailing whitespace removed.
- Around line 274-279: The shipping address input for addressLine1 lacks a
conditional required attribute; add browser-side validation by computing a
boolean (e.g., isCourier = deliveryMethod === 'courier' or shippingMethod ===
'COURIER' based on your component state) and set required={isCourier} on the
<input id="shipping-address-line-1"> that uses value={addressLine1} and
onChange={event => setAddressLine1(event.target.value)} so the field is required
only when courier delivery is selected to match server-side validation.
In `@frontend/lib/tests/shop/admin-shipping-edit-form.test.ts`:
- Around line 72-110: Add a new test mirroring the existing warehouse test to
assert client-side validation requires addressLine1 for courier deliveries:
create a test in admin-shipping-edit-form.test.ts that imports ShippingEditForm,
stubs fetch, renders ShippingEditForm with methodCode 'NP_COURIER' and
addressLine1 set to an empty string, submits the form, waits for the same alert
text 'shop.orders.detail.shippingEditor.errors.invalid', and asserts fetch was
not called; this will ensure the form validation (ShippingEditForm) enforces
addressLine1 before sending the PATCH.
🪄 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: 78a8792a-64e6-422c-8001-d1fabac99dec
📒 Files selected for processing (5)
frontend/app/[locale]/admin/shop/orders/[id]/ShippingEditForm.tsxfrontend/app/api/shop/admin/orders/[id]/cancel-payment/route.tsfrontend/app/api/shop/admin/orders/[id]/refund/route.tsfrontend/lib/tests/shop/admin-order-payment-audit-routes.test.tsfrontend/lib/tests/shop/admin-shipping-edit-form.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/lib/tests/shop/admin-order-payment-audit-routes.test.ts
…ress requirements and trimmed payload submission
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/app/[locale]/admin/shop/orders/[id]/ShippingEditForm.tsx (1)
129-139: Previous concern addressed; minor enhancement opportunity remains.The
addressLine1requirement for courier delivery has been correctly added (line 134). However, client-side validation is less strict than server-side: the server enforcesfullName.min(2),addressLine1.min(3), phone regex (/^(?:\+380\d{9}|0\d{9})$/), and email format, while the client only checks for non-empty values. Users could pass client validation but receive server-side errors.Consider aligning client validation with server constraints for a smoother UX, or accept that server validation provides the definitive check.
🤖 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]/ShippingEditForm.tsx around lines 129 - 139, The client-side validation in ShippingEditForm.tsx (the hasRequiredFields check and subsequent setError/tEditor usage) is weaker than server rules; update the validation logic around the hasRequiredFields calculation to mirror server constraints: require trimmedRecipientFullName.length >= 2, require trimmedAddressLine1.length >= 3 when isCourierMethod, validate trimmedRecipientPhone against the server regex /^(?:\+380\d{9}|0\d{9})$/, and validate trimmedRecipientEmail using a basic email format check before allowing submit; keep trimming and existing warehouse/courier conditional checks and use setError(tEditor('errors.invalid')) if any of these stricter checks fail.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/app/`[locale]/admin/shop/orders/[id]/ShippingEditForm.tsx:
- Around line 129-139: The client-side validation in ShippingEditForm.tsx (the
hasRequiredFields check and subsequent setError/tEditor usage) is weaker than
server rules; update the validation logic around the hasRequiredFields
calculation to mirror server constraints: require
trimmedRecipientFullName.length >= 2, require trimmedAddressLine1.length >= 3
when isCourierMethod, validate trimmedRecipientPhone against the server regex
/^(?:\+380\d{9}|0\d{9})$/, and validate trimmedRecipientEmail using a basic
email format check before allowing submit; keep trimming and existing
warehouse/courier conditional checks and use setError(tEditor('errors.invalid'))
if any of these stricter checks fail.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2e64839a-cca0-48ef-9f6e-33b41ec895f8
📒 Files selected for processing (2)
frontend/app/[locale]/admin/shop/orders/[id]/ShippingEditForm.tsxfrontend/lib/tests/shop/admin-shipping-edit-form.test.ts
✅ Files skipped from review due to trivial changes (1)
- frontend/lib/tests/shop/admin-shipping-edit-form.test.ts
ViktorSvertoka
approved these changes
Apr 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR completes Phase 6 (P1 — completion and cleanup) for the Shop module after launch blockers were closed.
It finalizes remaining cleanup work across merchandising, admin product/order operations, guest shipment visibility, controlled post-order shipping edits, and public runtime/cache safety. The goal was to remove weak or decorative behavior, tighten operational correctness, expand audit/history visibility, and add explicit runtime/cache protection for public shop surfaces without widening scope beyond the planned cleanup items.
Related Issue
Issue: #<issue_number>
Changes
/shop,/shop/products,/shop/products/[slug], and/shop/cart, plus a narrow follow-up to align the public cart env path with thereadServerEnv()server env contractDatabase Changes (if applicable)
How Has This Been Tested?
Screenshots (if applicable)
Checklist
Before submitting
Reviewers
Summary by CodeRabbit
New Features
Improvements
Tests