Repository navigation
feat(cash-sessions): per-staff shifts - #1395
Merged
Merged
Conversation
Allow 4-6 digit PINs for all staff roles to support tile-based PIN quick user switching on the lock/login screen while keeping all approval paths strictly restricted to active owner/manager PINs. Fixes #1338
- Use non-blocking per-request rate limit on POST /api/auth/pin-login while preserving per-user lockout - Serialize per-user PIN verification with withPinVerificationLock and audit missing credential requests - Re-read bill/order state and compute charge breakdowns inside the write transaction on PATCH /api/bills/:id/charges - Guard GET /api/settings/auto_lock_minutes with requireAnyPermission across staff roles - Pass explicit Bearer token to POST /auth/logout before clearing storage on lock/logout - Exclude autoLockMinutes from localStorage persistence, reset on session changes, and guard stale save rollbacks - Update kitchenDeliveryOverrideInvalid across all 24 locales to use Staff Approval PIN wording
…c.ts` line 270 to assert `page.getByText('Invalid Staff Approval PIN')` instead of `'Invalid manager PIN'`, matching the updated `pos.kitchenDeliveryOverrideInvalid` translation (`"Invalid Staff Approval PIN. Please try again."`) rendered by `PaymentModal.tsx` when `POST /api/bills/:id/payments` returns 403 `Invalid manager PIN`. The downstream `End-to-End Playwright Tests` and `Required Checks Gate` CI failures were caused by this Shard 1/4 E2E failure. Verification run and passed: - `npm run lint` - `npm run build && npm run build:frontend` - `npm run i18n:check` - `cd frontend && npx playwright test --config=playwright.config.ts --project=chromium e2e/approval-manager-selection.spec.ts e2e/pin-quick-switch.spec.ts` (4 passed) - `cd frontend && npx playwright test --config=playwright.config.ts --project=chromium e2e/kitchen-delivery-payment-retry.spec.ts e2e/kitchen/kitchen-delivery-billing-gate.spec.ts` (5 passed) - `node scripts/ci/check-changed-format.cjs --base 1278f91` - `git diff --check`
…heck (`test:orders-search`): - **Invariant**: Frontend components imported by `OrdersPage` must resolve `OrderDetailPanel` through the `@/components/orders/OrderDetailPanel` module specifier rather than a relative `./OrderDetailPanel` path, so Node-based `OrdersPage` unit tests that stub `@/components/orders/OrderDetailPanel` and provide a minimal `react` hook stub do not load the real `OrderDetailPanel` module and its `@radix-ui/react-slot` (`React.forwardRef`) dependency at module-load time. - **Root-cause fix**: Updated `frontend/src/components/orders/CorrectPaymentMethodModal.tsx` line 13 to import `normalizeBillPaymentDetails` from `'@/components/orders/OrderDetailPanel'` instead of `'./OrderDetailPanel'`. **Verification**: - `npm run test:orders-search` (passed) - `npm run test:payment-method-correction` (passed) - `npm run test:card-transaction-id` (passed) - `npm run test:ui-regressions-621-623-626` (passed) - `npm run test:password-policy` (passed) - `npm run lint` (passed) - `node scripts/ci/check-changed-format.cjs --base 1278f91` (passed) - `npm run build:frontend` (passed) - `git diff --check` (passed)
Contributor
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
 |
PR Summary by QodoAdd per-staff cash shifts, payment corrections, and PIN quick switch
AI Description
Diagram
High-Level Assessment
Files changed (81)
|
khaira777
changed the title
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
…e_shift_per_staff` is disabled (default), `getOpenSession` must preserve the store-wide open-session lookup (`SELECT * FROM cash_sessions WHERE status = 'open' LIMIT 1`), and when `one_shift_per_staff` is enabled, `getOpenSession` must look up only the specified actor's open session (`SELECT * FROM cash_sessions WHERE status = 'open' AND opened_by = ? LIMIT 1`) or return `undefined` when no actor user ID is provided. - **Root Cause:** [`getOpenSession`](file:///Users/gurkiratkhaira/.no-mistakes/worktrees/de2296f2e6f8/01M61KJ2A25N0GR48JQ5KGCY4B/main/services/shift-session-gate.ts#L48-L62) in [`main/services/shift-session-gate.ts`](file:///Users/gurkiratkhaira/.no-mistakes/worktrees/de2296f2e6f8/01M61KJ2A25N0GR48JQ5KGCY4B/main/services/shift-session-gate.ts) changed the store-wide open-session SQL query to `SELECT * FROM cash_sessions WHERE status = 'open' ORDER BY id ASC LIMIT 1` and kept an unrequired fallback to that query when `userId` was empty in per-staff mode. In [`tests/diagnostics-event.test.ts`](file:///Users/gurkiratkhaira/.no-mistakes/worktrees/de2296f2e6f8/01M61KJ2A25N0GR48JQ5KGCY4B/tests/diagnostics-event.test.ts#L1186-L1234) (section `4ab`), the test intercepts `SELECT * FROM cash_sessions WHERE status = 'open' LIMIT 1` to inject a synthetic database lookup failure during `POST /api/refunds`; because the SQL string no longer matched, the lookup returned `undefined` and triggered a `cash_session_required` validation rejection instead of preserving the database failure. ### Fix - Updated [`getOpenSession`](file:///Users/gurkiratkhaira/.no-mistakes/worktrees/de2296f2e6f8/01M61KJ2A25N0GR48JQ5KGCY4B/main/services/shift-session-gate.ts#L48-L62) in [`main/services/shift-session-gate.ts`](file:///Users/gurkiratkhaira/.no-mistakes/worktrees/de2296f2e6f8/01M61KJ2A25N0GR48JQ5KGCY4B/main/services/shift-session-gate.ts) to use `SELECT * FROM cash_sessions WHERE status = 'open' LIMIT 1` when `one_shift_per_staff` is off and removed the fallback query when `one_shift_per_staff` is on and `userId` is empty. ### Verification - `npm run test:diagnostics` (passed: all 7 suites including `tests/diagnostics-event.test.ts` with 471 passed, 0 failed) - `npm run lint` (passed) - `npm run build` (passed) - `npm run test:cash-sessions` (passed) - `npm run test:cash-closures` (passed) - `npm run test:upgrade-path` (passed) - `git diff --check` (passed)
khaira777
changed the title
khaira777
changed the title
… add v3.14.0 combined acceptance test
khaira777
changed the title
khaira777
changed the title
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 #1339
Intent
Implement opt-in one cash shift per staff member (feat(cash): opt-in one cash shift per staff member), stacked on top of paid-bill payment-method correction while its cash shift and business day remain open:
409session_already_openwhen opening a second shift).(opened_by) WHERE status = 'open'). Day close waits until all shifts are closed and adds them all up.getOpenSession(db, userId)). If the actor has no open shift, existing rules apply (blocked with409only whenrequire_open_shiftis on, otherwiseNO_CASH_SESSION_ID).409while more than one shift is open.cash_sessions_one_openin a non-destructive migration with a per-user partial unique index(opened_by) WHERE status = 'open'and enforce "only one open overall" in code when the setting is off. Upgrade from a DB with one open shift keeps it open and valid.useCashSession.ts,ShiftOpenModal,ShiftCloseModal,StatusBar, Settings toggle,docs/architecture/business-time.md,docs/reference/roles-and-permissions.md,docs/reference/api.md(and user docs warning if present), and extendtests/cash-sessions.test.tsandtests/upgrade-path.test.ts.What Changed
add_per_staff_cash_sessions) to replacecash_sessions_one_openwith a per-user partial unique indexcash_sessions_one_open_per_useron(opened_by) WHERE status = 'open', seed the opt-inone_shift_per_staffsetting ('false'by default), and block disablingone_shift_per_staff(409 multiple_shifts_open) while multiple shifts are open.getOpenSession(db, userId)) across bill payments, payment-method corrections, refunds, expenses, drawer movements, and printer drawer kicks so cash activity attaches to the actor's open shift whenone_shift_per_staffis enabled, whilePOST /api/cash-closureswaits for all open shifts on or before the business date to close and sums per-staff shift opening floats into the day'sopening_floatmovement.GET /api/cash-sessions/open, optionalsession_id/user_idtargeting on shift and movement routes with owner/manager vs. own-shift access checks, per-shift?session_idscoping onGET /api/reports/x-report, and frontend/i18n support inuseCashSession,useCashClose,ShiftCloseModal,CashCloseModal,PosTopbar,StatusBar, andPaymentMethodsSettings.Risk Assessment
✅ Low: The per-staff cash shifts implementation, migration v120, actor-scoped cash routing, shift-scoped X-report closure metadata, payment-method correction integration, UI controls, documentation, and behavioral test coverage are complete and consistent with the stated acceptance criteria.
Testing
Ran focused backend suites (
npm run test:cash-sessions,npm run test:upgrade-path,npm run test:cash-closures,tests/payment-method-correction.test.ts,npm run test:expenses,npm run test:refunds), compiled backend and static frontend (npm run build,npm run build:frontend), verified translations and docs (npm run i18n:check,check-docs.cjs), and drove the real running server and Chromium browser end-to-end across all per-staff shift scenarios with screenshots and API evidence captured in the run evidence directory.npm run test:cash-sessions,npm run test:expenses, andnpm run test:refundsnpm run test:upgrade-pathandnode tests/run-electron-node-test.cjs tests/payment-method-correction.test.tsEvidence: Live API & SQLite verification payload for per-staff shifts, attribution, close boundaries, shift-scoped X-report closure metadata, and day close reconciliation
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:cash-sessions,npm run test:expenses, andnpm run test:refundsnpm run test:upgrade-pathandnode tests/run-electron-node-test.cjs tests/payment-method-correction.test.tsnpm run test:cash-sessionsnpm run test:upgrade-pathnpm run test:cash-closuresnode tests/run-electron-node-test.cjs tests/payment-method-correction.test.tsnpm run test:expensesnpm run test:refundsnpm run build && npm run build:frontendnpm run i18n:checknode scripts/docs/check-docs.cjs --links --index --policyLive Playwright + Express server verification driving Settings toggle, concurrent per-staff shift open/close, actor-scoped cash attribution, shift-scoped X-report filter & closure metadata, and day close reconciliation✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.