Repository navigation
fix(tables): preserve reserved destinations - #1143
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe move-order route now reloads and checks the target table during the transaction. It rejects held or reserved destinations. Integration and end-to-end tests verify conflict responses, preserved reservation and source-table state, and a successful move after the reservation is released. ChangesReserved table move
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Linked Issues check |  |
Issue [#1061] requires reserved-table moves to be rejected with HTTP 409 unless an explicit override flag is provided. The reported route change rejects reserved targets even when override or force fl… |
Implement and test the explicit override flag required by issue [#1061], or update the linked issue requirement before merging. The override path must define authorization and preserve the existing non-override 409 behavior. |
| Docstring Coverage | |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
| Description check | |
The description provides detailed intent, implementation summary, scope-related behavior, testing commands, results, and evidence. However, it does not follow the repository template and omits or does… | Update the description to include every template section. State the related issue, intended scope and exclusions, database/migration/user-visible/offline impact, cross-cutting surface statuses, documentation status, and completed contributo… |
-
Check name Status Explanation Out of Scope Changes check The reported changes are limited to the move-order route and backend and browser tests for reserved, stale-read, existing guard, and available-table move behavior. These changes support issue [ #1061].…Title check The title clearly and concisely describes the main change: preserving reserved table destinations during order moves.
Full details: Linked Issues check
-
Explanation
Issue [
#1061] requires reserved-table moves to be rejected with HTTP 409 unless an explicit override flag is provided. The reported route change rejects reserved targets even when override or force flags are supplied, and the added tests assert that behavior. The reported implementation satisfies the 409 protection and preserves state, but it does not satisfy the issue's explicit override exception. The route re-check and retained inactive, held, and occupied guards support the other stated requirements.
Full details: Description check
-
Explanation
The description provides detailed intent, implementation summary, scope-related behavior, testing commands, results, and evidence. However, it does not follow the repository template and omits or does not explicitly complete the Related work, Scope, Compatibility & data safety, Cross-cutting surfaces, Developer documentation, and Contributor checklist sections.
Resolution
Update the description to include every template section. State the related issue, intended scope and exclusions, database/migration/user-visible/offline impact, cross-cutting surface statuses, documentation status, and completed contributor checklist items. Keep the existing testing commands and results under the required Verification section.
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
|
@greptileai review |
 [Medium impact] The PR appears safe to merge; no new actionable issues were found.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
@greptileai review |
fc67076 to
649d20e
Compare
|
@greptileai review |
|
@greptileai review |
|
@greptileai review |
1 participant
Intent
PR 6 - Preserve reserved table destinations (Fixes #1061)
Branch:
codex/reserved-table-movesTitle:
fix(tables): preserve reserved destinationsRequirements:
Acceptance: reserved moves fail without side effects; valid available-table moves succeed; a destination changed after the initial read is checked again.
What Changed
POST /api/tables/:id/move-ordertransaction and reject moves with409 Target table is reserved(alongside active, held, and occupied checks) before mutating the order or either table.reservedandheld409error messages inMoveTableModalinstead of falling back to the generic occupied-table message.POST /api/tables/:id/move-orderresponse and error status codes indocs/reference/api.md, and add backend and E2E tests covering reserved destination rejection and post-read status changes.Risk Assessment
✅ Low: The change cleanly enforces the reserved destination table check with a 409 response inside the authoritative move transaction before any order or table mutations, surfaces the conflict message in MoveTableModal, and adds thorough integration and E2E coverage without side effects.
Testing
Ran the backend integration and UI unit suites (
npm run test:tables-string-idsandnpm run test:held-orders), built the backend and static frontend bundle, and drove the live product in headless Chromium via Playwright (frontend/e2e/table-move-live.spec.tsand a targeted live capture) on isolated standalone server ports, gathering 10 visual screenshots and JSON API/database verification artifacts confirming all reserved, stale-read, mid-transaction, no-override, inactive/held/occupied guard, and available-table move scenarios pass.Evidence: Live API and database state verification for reserved, held, occupied, inactive, and available table moves
{ "guards": { "inactiveMove": { "status": 400, "data": { "error": "Target table is not active" } }, "heldMove": { "status": 409, "data": { "error": "Target table has a held order" } }, "occupiedMove": { "status": 409, "data": { "error": "Target table already has an active order" } } }, "reservedMoveRejection": { "reserveTableStatus": 200, "directReservedMove": { "status": 409, "data": { "error": "Target table is reserved" } }, "overrideReservedMove": { "status": 409, "data": { "error": "Target table is reserved" } }, "orderAfterReject": { "id": 1, "table_id": "tbl-41dc0d5f", "status": "pending" }, "sourceTableAfterReject": { "id": "tbl-41dc0d5f", "status": "occupied", "activeOrderId": 1 }, "targetTableAfterReject": { "id": "tbl-9817cac2", "status": "reserved", "activeOrder": null, "reservation_customer_id": "e2e-customer", "reservation_customer_name": "E2E Guest", "reservation_customer_phone": "+66800000001" } }, "availableMoveAfterRelease": { "orderAfterSuccess": { "id": 1, "table_id": "tbl-9817cac2" }, "sourceTableAfterSuccess": { "id": "tbl-41dc0d5f", "status": "available", "activeOrder": null }, "targetTableAfterSuccess": { "id": "tbl-9817cac2", "status": "occupied", "activeOrderId": 1 } } }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:tables-string-idsnpm run test:held-ordersE2E_BASE_URL=http://127.0.0.1:4301 E2E_KDS_BASE_URL=http://127.0.0.1:4302 E2E_SERVER_APP_BASE_URL=http://127.0.0.1:4303 EVIDENCE_DIR=~/.no-mistakes/evidence/01M4YJ4Q7A41SYZBNS5JHF6296 npx --prefix frontend playwright test --config=frontend/playwright.config.ts --project=chromium frontend/e2e/table-move-live.spec.tsLive Playwright & API verification against standalone server on ports 4401-4403 capturing 09-reserved-table-move-rejected.png, 10-reserved-table-released-move-completed.png, and reserved-table-moves-live-verification.json✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.