Repository navigation
fix(frontend): expose LAN addresses for customer display pairing - #1163
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:
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 |
|
@greptile review |
[Medium impact] The PR appears safe to merge; no actionable issues were found.
|
Intent
Fix issue #1147 so customer-display Settings exposes usable LAN addresses for pairing from another device. Reuse the existing authenticated /api/pos-info response, which reports the actual API-serving port, and do not use KDS-gated KDS info, a hardcoded port, a new public endpoint, a new dependency, or a Next.js API handler. Show the primary LAN address prominently and allow selecting other discovered IP and mDNS addresses plus a clearly labelled this-computer-only local preview. Both the display URL and pairing link must use the selected origin. Address selection occurs before pairing and switching origins must preserve the same pair token and device identity; do not mint another token. Preserve fragment-scoped access, one-time token visibility, reload behavior, and revocation. If no LAN address exists, state that clearly and offer local preview. If network lookup fails, show an explicit error and retry; never silently present localhost as LAN. Load network information once when the settings card opens and only again on explicit retry. Preserve the #1149 authentication and clipboard safeguards when rebasing. Do not duplicate the separately owned #1141 clipboard work or redo the merged #1105 pairing repair. Update canonical docs only if necessary and maintain translation completeness. Keep the desktop static-export boundary. The approved issue is #1147 and the resulting PR must close it.
What Changed
CustomerDisplayCardto fetch POS network addresses once fromGET /api/pos-info, defaulting to the primary LAN origin while allowing selection of alternate interface IPs, mDNS addresses, or a labelled local preview origin for both the display URL and the one-time#token=pairing link without minting additional tokens./api/pos-infolookup states explicitly with local-preview fallback messaging and an on-demand retry action, instead of silently presentingwindow.location.originas a LAN address.docs/architecture/frontend.md, and added Playwright E2E coverage for customer display pairing address selection and fallback states.Risk Assessment
✅ Low: The change is well-bounded to CustomerDisplayCard, its translations, documentation, and E2E tests, and the Round 1 fix cleanly resolved the DOM/locator collisions with existing E2E suites.
Testing
Built the backend and static desktop frontend, ran focused backend and translation suites (
test:customer-display-config,test:customer-display-settings-route,test:customer-display-pairing,test:api-path-casing-auth,test:translations), executed Playwright E2E specs (customer-display-pairing-addresses.spec.ts,customer-display-pairing.spec.ts, andsettings-request-budget.spec.ts) on isolated ports 43001-43003, and drove the live product on ports 43011-43013 to capture six visual screenshots covering primary LAN selection with the actual serving port, origin switching with token preservation, network lookup failure and retry, no-LAN fallback, unauthenticated display board pairing/reload, and revocation; physical cross-device testing on a second LAN device remains manual verification as no second hardware device is attached to this runner.Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed ✅
frontend/src/components/settings/CustomerDisplayCard.tsx:257- Rendering the pairing-address radio option origins inside<code>tags inCustomerDisplayCard.tsx:257,CustomerDisplayCard.tsx:279, andCustomerDisplayCard.tsx:297breaks existing E2E selectors on the Kitchen Display settings tab (tab=kds): (1) infrontend/e2e/customer-display-pairing.spec.ts:265-268(from fix(frontend): allow unauthenticated customer display pairing and guard clipboard copy #1149),card.locator('code')now matches 4+ elements instead of the 2 copyable URL<code>elements (CustomerDisplayCard.tsx:480andCustomerDisplayCard.tsx:322), failingexpect(codeElements).toHaveCount(2)and shiftingcodeElements.nth(1)frompairingLink.urltoprimaryOrigin(and in loopback-only environments where/api/pos-infohas no LAN IP,CustomerDisplayCard.tsx:311keepsPair displaydisabled infrontend/e2e/customer-display-pairing.spec.ts:128and:262until the local preview radio is checked); (2) infrontend/e2e/settings-request-budget.spec.ts:1261(called from:1286on the Kitchen Display tab),page.getByText('http://10.42.0.25:3001', { exact: true }).locator('xpath=..')now matches both the KDS QR pairing card link parent andCustomerDisplayCard'sotherOriginsradio label (CustomerDisplayCard.tsx:279), causingexpect(localCard).toContainText(englishLocalHint)atfrontend/e2e/settings-request-budget.spec.ts:1264to fail with a Playwright strict-mode violation. Use<span dir="ltr" className="... font-mono ...">for the radio option origin sublabels atCustomerDisplayCard.tsx:257,:279, and:297, scopelocalCardinfrontend/e2e/settings-request-budget.spec.ts:1261topage.getByRole('link', { name: localAddress, exact: true }).locator('xpath=..'), and ensure an address radio option is selected infrontend/e2e/customer-display-pairing.spec.ts:24when opening the tab.🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
npm run build && npm run build:frontendnpm run test:customer-display-confignpm run test:customer-display-settings-routenpm run test:customer-display-pairingnpm run test:api-path-casing-authnpm run test:translationsCI=1 E2E_BASE_URL=http://127.0.0.1:43001 E2E_KDS_BASE_URL=http://127.0.0.1:43002 E2E_SERVER_APP_BASE_URL=http://127.0.0.1:43003 npx --prefix frontend playwright test --config=frontend/playwright.config.ts --project=chromium frontend/e2e/customer-display-pairing-addresses.spec.ts frontend/e2e/customer-display-pairing.spec.ts -g "customer display|Customer display|Network pairing cards" frontend/e2e/settings-request-budget.spec.tsLive Playwright Chromium verification against tests/e2e-server.cjs on isolated ports 43011/43012/43013 capturing 6 visual evidence screenshots✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.