Repository navigation
feat(orders): multi-counter ordering and payment - #1411
Merged
Merged
Conversation
Contributor
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
 |
PR Summary by QodoAdd multi-counter ordering and payment workflow
AI Description
Diagram
High-Level Assessment
Files changed (55)
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
khaira777
changed the title
Add multi-counter ordering and payment workflow with owner-managed counter configuration, device counter role selection, and real orders and bills persisted under awaiting_payment checkout stage. Prepayment preserves dine-in table occupancy and keeps kitchen status pending. Shared transactional release gates enforce full payment across bills and scheduled instant before kitchen release. Expose LAN waiting queue with cursor pagination, typed/QR ticket lookup, and optional thermal ticket slip printing.
…eQL finding (main/routes/orders.ts): - Invariant: When constructing SQL LIKE patterns with an ESCAPE clause, all wildcards and the escape character itself must be escaped in user input. - Root Cause: Line 1630 used search.replace(/[%_]/g, '\\$&') without escaping backslashes, triggering CodeQL alert "Incomplete string escaping or encoding" (CVE/CWE-116). - Fix: Updated the regex to search.replace(/[\\%_]/g, '\\$&') in main/routes/orders.ts so backslashes, percent signs, and underscores are safely escaped. 2. TypeScript build failures in frontend (Lint & Build Validation, Native Electron, Playwright Shards 1-4, End-to-End Playwright Tests, Required Checks Gate): - Invariant: Variables mutated in asynchronous test step callbacks and used across steps or teardown must remain safely typed without triggering strict null checks or unreachable-code narrowing. - Root Cause: In frontend/e2e/multi-counter-ordering.spec.ts, orderingCounter, paymentCounter, combinedCounter, createdTable, and limitedProduct were initialized to null. TypeScript does not track assignments across inner async closures, causing TS18047 ('possibly null') in steps and TS2339 (property on type 'never') in teardown. - Fix: Initialized the counter, table, and product state objects to { id: 0 } and updated teardown guards to check (id > 0), following the codebase pattern established in frontend/e2e/table-move-live.spec.ts. Verification: - npm run lint (0 errors) - npm run build (passed) - npm run build:frontend (passed cleanly, including full TypeScript compilation and static page generation) - npm run docs:check (passed) - npm run i18n:check (passed) - git diff --check (passed) - node tests/multi-counter-ordering.test.ts (126/126 assertions passed) - npm run test:upgrade-path (passed) - npm run audit:db (passed) - npm run test:receipt-column-oracle (passed) - npm run test:url-allowlist (passed)
…wright E2E (Shard 2/4)" and dependent checks ("End-to-End Playwright Tests", "Required Checks Gate"): 1. Invariant: End-to-end test fixtures must clean up all created records and restore modified settings in their teardown hooks so that subsequent test suites in the same worker or shard execute against an isolated, predictable baseline state. 2. Root Cause: In `frontend/e2e/multi-counter-ordering.spec.ts`, test variables (`orderingCounter`, `paymentCounter`, `combinedCounter`, and `limitedProduct`) were typed as `{ id: number }` and initialized to `{ id: 0 }`. In the `finally` teardown block, their cleanup checks used `if (id > 0)`. Because counters and products in FloCafe have string IDs (e.g. `cnt_...`, `prd_...`), the condition `'cnt_...' > 0` evaluated to `false`, causing the `DELETE` API requests to be completely skipped. Consequently, `limitedProduct` ('Limited Pie') remained in `e2e-category` alongside the seeded 'E2E Coffee' product, and `multi_counter_enabled` remained set to `'true'`. When `frontend/e2e/order-attempt-storage.spec.ts` executed subsequently in Shard 2, `addProductToCart(page)` called `page.getByTestId('pos-product-card').click()`, which threw a strict mode violation error because two product cards matched instead of one. 3. Fix: In `frontend/e2e/multi-counter-ordering.spec.ts`: - Typed and initialized `orderingCounter`, `paymentCounter`, `combinedCounter`, and `limitedProduct` with `{ id: '' }` string IDs. - Stored `dineInOrderId` from Scenario 5 so that the dine-in order is marked completed to release table occupancy. - Corrected teardown guards to check truthy IDs (`if (orderingCounter.id)`, `if (paymentCounter.id)`, `if (combinedCounter.id)`, `if (limitedProduct.id)`), properly deactivating the table via `POST /api/tables/:id/deactivate`. - Restored `multi_counter_enabled` back to `'false'` via `PUT /api/settings/multi_counter_enabled` in the teardown block. 4. Verification: - `npm run lint` (0 errors) - `npm run build` (passed) - `npm run build:frontend` (passed) - `git diff --check` (clean) - `npm run docs:check` (passed) - `npm run i18n:check` (passed) - `npx tsx tests/multi-counter-ordering.test.ts` (126/126 passed) - `npm run test:upgrade-path` (passed) - `npm run audit:db` (passed) - `npm run test:receipt-column-oracle` (passed) - `npm run test:url-allowlist` (passed) - `cd frontend && npx playwright test e2e/multi-counter-ordering.spec.ts e2e/order-attempt-storage.spec.ts` (9/9 passed in 25.4s)
khaira777
force-pushed
the
fm/flocafe-361-multi-counter-ordering
branch
from
October 30, 2026 03:14
268aade to
774da1a
Compare
…e Test Suite (Shard 1/4), Core Test Suite (Shard 2/4), Lint & Build Validation (Linux), and Required Checks Gate: 1. Finding ci-1: Core Test Suite (Shard 1/4) (`test:issue-zero-balance-settlement`) - Invariant: Syncing unpaid bills when order items or allocations change must not mark an order completed or release a table while uncancelled order items remain active. - Root Cause: In `main/routes/bills.ts:2546`, `reconcileOrderSettlement(db, bill.order_id, now());` was erroneously called inside `syncUnpaidBillsForOrder` whenever an individual bill's balance dropped to 0. When an item on a split check was cancelled while another discounted item was active on a sibling zero-closed check, `reconcileOrderSettlement` marked the order completed prematurely, causing subsequent item restoration to fail with 400 ("Cannot restore items on completed or cancelled orders"). - Fix: Removed `reconcileOrderSettlement(db, bill.order_id, now());` from `syncUnpaidBillsForOrder` in `main/routes/bills.ts`. Bill syncing now updates check totals and zero-closes individual checks without prematurely settling or completing the parent order. 2. Findings ci-2 & ci-3: Core Test Suite (Shard 2/4) (`test:lint-engine`) & Lint & Build Validation (Linux) (`npm run lint:budget`) - Invariant: The backend codebase must not exceed the recorded lint warning budget (1000 warnings) and must not contain unused variables or preventable loose types on code paths introduced or modified by this change. - Root Cause: `orderFullyPaid` was destructured but unused in `completeExactBalanceSettlement` (`main/routes/bills.ts:3704`), and multiple `as any` casts were added in `main/printers/thermal.ts` and `main/routes/printers.ts` around ticket printing, pushing backend warnings to 1002 (exceeding the 1000 warning budget limit). - Fix: - In `main/routes/bills.ts:3700`, removed unused `orderFullyPaid` from `reconcileOrderSettlement` destructuring. - In `main/printers/thermal.ts`, extended `DispatchResult` and introduced `PaymentTicketPrintResult` with optional `bytes?: number[] | Buffer`, `preview?: boolean`, and `connection_type?: string`, removing `as any` casts. - In `main/routes/printers.ts`, strongly typed `order.items` and removed `as any` on printer and print result properties. Backend warnings dropped to 991 / 1000. 3. Finding ci-4: Required Checks Gate - Unblocked by resolving findings ci-1, ci-2, and ci-3. Verification: - `npm run test:issue-zero-balance-settlement` (112/112 passed) - `npm run test:lint-engine` (passed) - `npm run lint:budget` (passed: 991 backend warnings [budget: 1000], 0 frontend warnings [budget: 5]) - `npx tsx tests/multi-counter-ordering.test.ts` (168/168 passed) - `npm run lint` (0 errors, 991 backend warnings, 0 frontend warnings) - `npm run build` (passed cleanly) - `npm run test:upgrade-path` (passed) - `npm run audit:db` (passed) - `npm run docs:check` (passed) - `npm run i18n:check` (passed) - `npm run test:receipt-column-oracle` (passed) - `npm run test:url-allowlist` (passed) - `git diff --check` (clean, 0 whitespace errors)
…t & Build Validation (Linux)" and "Required Checks Gate": 1. Finding ci-1: "Lint & Build Validation (Linux)" (check_id: github-check-run:123874513474) - Invariant: All source files touched or added by the branch must conform to repository formatting rules enforced by Oxfmt (`node scripts/ci/check-changed-format.cjs`). - Scope & Enumeration: Examined all 13 root files and 10 frontend files modified against base `53b1f6beab4cace6c63e9e08e0db0c804be4f006`. Only `main/printers/thermal.ts` contained formatting divergence from Oxfmt rules in `printPaymentTicketDetailed` (ternary expression indentation across lines 1534-1558). - Root Cause: Multi-line indentation in the return object assignment inside `printPaymentTicketDetailed` failed `oxfmt --check` in `scripts/ci/check-changed-format.cjs`, exiting with code 1 and failing the workflow step. - Fix: Formatted `main/printers/thermal.ts` using `oxfmt`, adjusting the ternary assignment indentation to match the canonical Oxfmt style. 2. Finding ci-2: "Required Checks Gate" (check_id: github-check-run:123876623625) - Invariant: The required checks gate must succeed once all prerequisite check runs complete without failure. - Root Cause: Failed as a downstream gate consequence of the `Lint & Build Validation (Linux)` job failure. - Resolution: Resolved by fixing the formatting check failure in finding ci-1. Verification performed locally: - `node scripts/ci/check-changed-format.cjs --base 53b1f6b` (passed, checked 13 root and 10 frontend files) - `npm run lint:backend` (0 errors, 991 warnings [budget: 1000]) - `npx tsc --noEmit` (passed cleanly) - `npm run lint` (passed cleanly across backend and frontend) - `npm run lint:budget` (passed: 991 backend warnings [budget: 1000], 0 frontend warnings [budget: 5]) - `npm run test:script-coverage` (passed: 273 reachable scripts verified) - `npm run build` (passed cleanly, TypeScript backend compiled to dist/) - `npm run build:frontend` (passed cleanly, Next.js static export succeeded) - `npm run test:issue-zero-balance-settlement` (passed: 112/112 assertions passed) - `npm run test:lint-engine` (passed) - `npx tsx tests/multi-counter-ordering.test.ts` (passed: 168/168 assertions passed) - `npm run test:upgrade-path` (passed) - `npm run audit:db` (passed) - `npm run docs:check` (passed) - `npm run i18n:check` (passed) - `npm run test:receipt-column-oracle` (passed: 12/12 passed) - `npm run test:url-allowlist` (passed) - `git diff --check` (clean, 0 whitespace errors) - `git status` (clean diff limited exclusively to `main/printers/thermal.ts`)
17 tasks done
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
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.
Fixes #361
api_response:
body: "## Intent\n\nImplement FloCafe issue #361 (
Feature: Multi-counter ordering flow (create here, pay there)), building on the scheduled-orders release foundation already merged onmain(#360/#1408):\nProvide owner-managed configuration, disabled by default:\n- Named counters have ordering, payment, or combined roles.\n- Each main POS device selects its counter; remember that selection locally for that store/server.\n- Ordering counters default to building an order and choosing Send to payment. Payment counters open the shared waiting queue. Combined counters retain normal checkout and can explicitly choose Send to payment.\n- Counter roles guide the interface. Existing staff permissions remain authoritative; authorized staff can switch views. Do not add order ownership restrictions.\n- Support takeaway and dine-in. Scheduling and multi-counter checkout can therefore combine for takeaway only.\n- Store real orders and bills, not held-order JSON. Persistcheckout_stage: building | awaiting_payment | paidseparately from kitchen progress, together with the order's dispatch policy.\n- Sending to payment saves the quoted order and exposes a shared LAN queue showing ticket number and total.\n- Retrieve tickets by queue selection, typed ticket number, or QR scanner input. Show a number and QR and provide optional printing through existing printer infrastructure. Camera scanning is excluded.\n- QR content identifies the ticket; it grants no authentication or payment authority.\n- Release multi-counter orders only after full payment across all applicable bills. For scheduled takeaway orders, require both full payment and the scheduled instant.\n- Reuse existing payment idempotency and stale-edit protection. Concurrent counters must not duplicate charges, stock movements, or kitchen release.\n- Treat the new workflow as prepayment without weakening ordinary postpaid settlement guards. Payment settlement must not complete kitchen work or prematurely free a dine-in table.\n- Stock-release failure after payment leaves a clearly visible paid-but-blocked ticket. Recovery is explicit retry or the existing refund/cancellation path.\n- Use one shared, transactional release operation for scheduling and multi-counter checkout. All entrypoints must enforce its gates. After successful release, existing kitchen, inventory, audit, and printing behavior resumes.\n\n## What Changed\n\n- Added counter management schema, owner settings (/api/counters), and local POS device counter selection supportingordering,payment, andcombinedroles.\n- Introduced ordercheckout_stageanddispatch_policyfields to support "Send to payment" deferral, QR payment ticket confirmation and printing, and shared waiting queue retrieval (/api/orders/awaiting-payment,/api/orders/by-ticket/:ticket,PaymentQueueDrawer).\n- Integrated post-payment release inreconcileOrderSettlementandreleaseScheduledOrder, releasing kitchen items and deducting stock upon full payment without prematurely completing active orders or releasing occupied dine-in tables.\n\n## Risk Assessment\n\n✅ Low: The multi-counter ordering implementation strictly satisfies all acceptance criteria, resolves previous review decisions, preserves core architectural invariants, and introduces no reachable defects or unrequested complexity.\n\n## Testing\n\nAll 7 multi-counter end-to-end and adversarial scenarios were driven live against the running FloCafe product stack in Playwright and backend integration runners, verifying owner configuration, ticket generation with QR, auto-opening payment queue, dine-in table occupancy preservation, and stock recovery, producing 8 full-page visual evidence screenshots.\n\n- Live validation: ✅ go - 7 of 7 scenarios driven live against the product\n\n| Scenario | Result | Live | Evidence |\n| --- | --- | --- | --- |\n| Owner configures multi-counter mode and manages named counters with ordering, payment,"truncated: true
original_length: 9562