Repository navigation
fix(pos,billing): close post-merge review gaps - #1410
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
|
PR Summary by QodoFix checkout session races, payment authorization, and invoice charges
AI Description
Diagram
High-Level Assessment
Files changed (10)
|
Code Review by Qodo
1.
|
3efec60 to
224d902
Compare
- Guard async checkout continuations with session epoch and user identity, scoped retry records, and preserved caller auth headers - Recheck payments.take permission inside transactional payment-method correction - Include custom charge breakdown lines in archived daily invoice export - Allow all authenticated staff to read auto_lock_minutes idle-lock policy
…ard 2/4 (`e2e/order-attempt-storage.spec.ts:496` - "prepaid: losing the bill retry snapshot stops payment and reuses the original order key"). ### Invariant Analysis - **Invariant:** When local persistence of an in-flight checkout attempt fails after an upstream mutation has succeeded, the failure must be attributed to the corresponding stage (`retry_state`) and operation (`bill.generate` when persisting the generated bill, `payment.create` when persisting payment request changes on an existing bill), capturing the client diagnostic and safely halting checkout before payment. - **Root Cause:** In `frontend/src/app/(dashboard)/pos/page.tsx`, `requirePrepaidAttemptSaved` was called immediately following `POST /bills/generate` while `failureStage` was still `'bill_generation'`. When the test simulated storage quota exhaustion for records carrying a bill, `requirePrepaidAttemptSaved` threw while `failureStage === 'bill_generation'`, bypassing the error-handler branch that emits the `fc.client.bill.generate.retry_state_update` diagnostic capture (which requires `failureStage === 'retry_state'`). Furthermore, mutating `attempt.bill` before the retry-state block caused subsequent logic to classify the operation as `payment.create` instead of `bill.generate`. - **Fix:** In `frontend/src/app/(dashboard)/pos/page.tsx`, removed the premature `requirePrepaidAttemptSaved` call after bill generation. If the session changed during the request (`!isSessionCurrent()`), the attempt carrying the newly generated bill is saved under `originUserId` via `savePrepaidAttempt` before halting. On the normal active session continuation path, the retry-state update block executes with `attempt.bill` unset, correctly setting `failureOperation = 'bill.generate'` and `failureStage = 'retry_state'` before calling `requirePrepaidAttemptSaved(attempt, originUserId)`. - **Sibling Call Paths & Consumers:** - `POST /orders` (order creation): preserves recovery record under `originUserId` and halts if session changed; if storage fails, captures `fc.client.order.create.retry_state_update`. - `POST /bills/generate` (bill generation): preserves recovery record under `originUserId` and halts if session changed; if storage fails, captures `fc.client.bill.generate.retry_state_update`. - `POST /bills/:id/payments` (payment): preserves paid bill recovery record under `originUserId` and halts if session changed. ### Verification Run - `npm run clean`: Passed (ports 3001, 3002, 3003 free). - `node tests/run-electron-node-test.cjs tests/pin-quick-switch.test.ts`: Passed (all 8 sections, 100% green). - `node tests/run-electron-node-test.cjs tests/daily-invoices-export.test.ts`: Passed. - `node tests/run-electron-node-test.cjs tests/payment-method-correction.test.ts`: Passed. - `npm run build`: Passed. - `npm run build:frontend`: Passed. - `cd frontend && npx playwright test e2e/order-attempt-storage.spec.ts`: All 8 tests passed, including test at line 496. - `npm run lint`: Passed (backend 0 errors, frontend 0 errors). - `npm run format:check:changed -- --base b27799e`: Passed (10 files clean). - `npm run docs:check`: Passed. - `npm run i18n:check`: Passed
224d902 to
8463b5b
Compare
…e '@/lib/cart-price'` by replacing the frontend-only path alias `@/` with relative sibling imports `./` in `frontend/src/lib/cart-order-item.ts` and `frontend/src/lib/cart-price.ts`. Verified with `npm run test:diagnostics`, `npm run test:issue-788-order-attempt-storage`, `npm run test:cart-variant-identity`, `npm run test:pin-quick-switch`, `npm run test:cart-price-override`, `npm run test:daily-invoices-export`, `npm run test:payment-method-correction`, `npm run lint`, `npm run build:frontend`, and `npm run build`
…Test Suite (Shard 2/4)` (`test:whatsapp-service`): ### 1. Invariant Quiet hours configured through the end of the day (`quietHoursEnd: '23:59'`) or across the entire day (`00:00` to `23:59`) must include the final minute of the day (`23:59`), ensuring automated WhatsApp messages and retries are consistently blocked with reason `quiet_hours` across all 1,440 minutes of the day. Every place where this invariant must hold: - Window check function: `isTimeWithinQuietHours` in [shared/whatsapp-automation.ts](file:///Users/gurkiratkhaira/.no-mistakes/worktrees/de2296f2e6f8/01M64ZSQNNR1Q74M6KMYT8RX7B/shared/whatsapp-automation.ts#L569-L584), which is the single shared boundary evaluating whether an HH:MM timestamp falls within quiet hours. - Consumer: `isCurrentlyQuietHours` in [main/services/whatsapp.ts](file:///Users/gurkiratkhaira/.no-mistakes/worktrees/de2296f2e6f8/01M64ZSQNNR1Q74M6KMYT8RX7B/main/services/whatsapp.ts#L606-L620), which formats the current tenant time and delegates to `isTimeWithinQuietHours`. - Sibling call path 1: `sendMessageInternal` in [main/services/whatsapp.ts](file:///Users/gurkiratkhaira/.no-mistakes/worktrees/de2296f2e6f8/01M64ZSQNNR1Q74M6KMYT8RX7B/main/services/whatsapp.ts#L2367-L2377), enforcing quiet hours on automated message sends. - Sibling call path 2: `retryMessage` in [main/services/whatsapp.ts](file:///Users/gurkiratkhaira/.no-mistakes/worktrees/de2296f2e6f8/01M64ZSQNNR1Q74M6KMYT8RX7B/main/services/whatsapp.ts#L2540-L2560), which routes retried automated messages through `sendMessageInternal`. - Test suites: [tests/whatsapp-service.test.ts](file:///Users/gurkiratkhaira/.no-mistakes/worktrees/de2296f2e6f8/01M64ZSQNNR1Q74M6KMYT8RX7B/tests/whatsapp-service.test.ts#L464-L476) and [tests/reports-whatsapp.test.ts](file:///Users/gurkiratkhaira/.no-mistakes/worktrees/de2296f2e6f8/01M64ZSQNNR1Q74M6KMYT8RX7B/tests/reports-whatsapp.test.ts#L1400-L1425). ### 2. Root Cause In `shared/whatsapp-automation.ts`, `isTimeWithinQuietHours` evaluated same-day windows (`start < end`) as a strict half-open interval `currentHhMm >= start && currentHhMm < end`. Because standard 24-hour time pickers only go up to `23:59`, configuring full-day quiet hours (`quietHoursStart: '00:00'` to `quietHoursEnd: '23:59'`) or quiet hours through midnight (`22:00` to `23:59`) excluded minute `23:59`. When CI Shard 2/4 executed `tests/whatsapp-service.test.ts` at `2026-10-29T23:59:03Z`, `isCurrentlyQuietHours` returned `false`, allowing `whatsapp.retryMessage(autoFailedId)` to succeed instead of failing with `quiet_hours`. ### 3. Fix Applied - In [shared/whatsapp-automation.ts](file:///Users/gurkiratkhaira/.no-mistakes/worktrees/de2296f2e6f8/01M64ZSQNNR1Q74M6KMYT8RX7B/shared/whatsapp-automation.ts#L579): updated same-day window condition to `currentHhMm >= start && (end === '23:59' || currentHhMm < end)`, ensuring windows terminating at `23:59` include the entire last minute of the day. - In [tests/whatsapp-service.test.ts](file:///Users/gurkiratkhaira/.no-mistakes/worktrees/de2296f2e6f8/01M64ZSQNNR1Q74M6KMYT8RX7B/tests/whatsapp-service.test.ts#L470-L476): added regression assertions verifying `isTimeWithinQuietHours` evaluates to `true` for all 23:59 edge cases. ### 4. Verification - `npm run test:whatsapp-service`: Passed (all WhatsApp service API assertions passed). - `env SHARD_TOTAL=4 SHARD_INDEX=1 node scripts/ci/run-test-shard.cjs`: Passed (all 57 test suites in Shard 2/4 passed). - `npm run lint`: Passed (0 errors). - `npm run build`: Passed (TypeScript build completed cleanly). - `npm run docs:check`: Passed (markdownlint and doc integrity verified). - `npm run i18n:check`: Passed (translation integrity verified). - `git diff --check`: Passed (no whitespace errors)
Fixes #1409. Fix post-merge review defects across PRs #1392, #1394, and #1400: isolate POS checkout continuations and committed carts across session switches, recheck payments.take inside payment correction transaction, include custom charges in daily invoice export, and allow authenticated staff to read idle-lock policy.
What Changed
charges_breakdownand render itemised custom fee lines alongside standard service, packaging, and delivery charges.payments.takepermission inside payment method correction transactions and allowed all active authenticated staff members to read theauto_lock_minutesidle-lock setting.Risk Assessment
✅ Low: All post-merge review requirements across session boundary isolation, in-transaction permission re-checks, custom invoice charges, and idle-lock access are cleanly addressed with surgical diffs and executable tests.
Testing
Exercised all four core functional areas live against the running backend API and Next.js frontend in headless Chromium, capturing 4 browser UI screenshots, the rendered daily invoices HTML export, and 2 structured API response evidence files, all passing with zero regressions or findings.
Evidence: POS Owner Session with Cart Item
Evidence: POS Locked Session PIN User Picker
Evidence: Subsequent Identical Order Checkout Modal
Evidence: Subsequent Identical Order Paid Successfully
Evidence: Daily Invoices Export HTML with Custom Eco Fee
Rendered daily invoices HTML export containing itemised Eco Fee in invoice-totals section.Evidence: Scenario 1 Idle Lock Policy JSON Evidence
Log verifying authenticated server staff reading idle lock policy (200 OK) while unauthenticated requests return 401.Evidence: Scenario 2 Payment Correction Authorization JSON Evidence
Log demonstrating manager payment correction succeeding with payments.take (200 OK) and rejecting with 403 when payments.take is revoked.Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
npm run test:payment-method-correctionnpm run test:daily-invoices-exportnpm run test:pin-quick-switchcd frontend && npx playwright test e2e/pin-quick-switch.spec.ts --project=chromiumcd frontend && npx playwright test e2e/live-product-validation.spec.ts --project=chromium✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.