refactor(api): give check-in a service class and typed responses - #297
refactor(api): give check-in a service class and typed responses#297harshtandiya wants to merge 5 commits into
Conversation
Both endpoints are now two lines over CheckinService, which owns the Frontdesk Manager guard, the ticket lookup and the cancelled/already-in checks. checkin_ticket no longer calls validate_ticket_for_checkin to reuse them. Errors carry their own copy: TicketNotFound (404), TicketCancelled and AlreadyCheckedIn (409), replacing three generic 417s. The scanner used to branch on substrings of the serialized error to pick a toast. That contract is gone — it reads err.messages[0] like the rest of the dashboard. One of its branches was already dead: nothing throws "not confirmed and cannot be used for check-in". Response gains one key: check_in_date is now always present, null before check-in. The scanner already declared it optional. Everything else is byte-identical, key order included. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
get_cached_doc already caches, so cached_property only adds a second layer that can go stale within a request. Plain properties read through the framework cache each time. The existence check moves to the constructor, which is where it belongs now that the property no longer runs it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Doctype controllers carry frappe's auto-generated field types, so the properties can name them. Imported under TYPE_CHECKING: no runtime import, so no risk of a cycle through buzz_event.py, which already imports from buzz.api.forms. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
The check-in refactor was only verified in-process: Python tests assert the model, and payloads were diffed by importing old and new modules side by side. Nothing exercised HTTP, the browser, or the components that read the response. Two things that only break in a browser are now covered: the response keys TicketDetailsModal renders, and the client-side role gate, which reads the User.roles child table out of get_user_info. Seeding needed two new pieces. createUserWithRoles and saveLoginState let a project run under an identity other than the shared one — check-in demands Frontdesk Manager, which CI's System Manager user does not have. The access-denied case gets its own roleless user rather than reusing Administrator, who holds every role and would never see that screen. Cleanup runs at the top of the setup, not in afterAll: the spec's context carries the front-desk session, while the helpers sign writes with the Administrator CSRF token. extractTicketId moves out of QRScanner.vue into utils so it can be tested. Camera capture stays uncovered; this is the only real logic behind it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Added E2E coverage ( The last three are the ones worth having: two assert that class-defined error copy reaches the toast now that the substring matching is gone, and the role test covers the client-side gate that reads the Three things worth knowing for the next spec:
Camera capture and |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
565c51a to
f516496
Compare
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
The check-in assertion raced the request it was meant to observe. It waited on the "Last Scan Result" panel, which is already on screen from the preceding validate call, so the Event Check In query ran before checkin_ticket had landed. CI was slow enough to expose it; locally it always passed. confirmCheckIn now waits for the response and for the modal to close, which is a state only check-in produces. Retries failed differently — the modal never opened — because the first attempt had by then consumed the shared ticket, leaving the retry to scan an already checked-in one. Each test creates its own ticket, so the pair of check-in tests no longer depend on running in order and a retry starts clean. The seeded ticket is gone; setup keeps the ticket type. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
1 participant
Stacked on #296 — read that first. Diff here is
checkin/plus its scanner composable.Both endpoints are two lines now;
CheckinServiceowns the Frontdesk Manager guard, the ticket lookup and the cancelled / already-checked-in checks.checkin_ticketno longer callsvalidate_ticket_for_checkinjust to reuse them.Errors replace three generic 417s:
TicketNotFound(404),TicketCancelledandAlreadyCheckedIn(409), each with its own copy.The scanner was matching on error substrings.
useTicketValidation.tspicked its toast byJSON.stringify(error).includes("Ticket not found")and friends — so the error text was an undeclared contract, and changing any message would have silently broken it. Now readserr.messages[0]like the rest of the dashboard. One branch was already dead: nothing in Python throws "not confirmed and cannot be used for check-in".One intentional wire change:
check_in_dateis now always present,nullbefore check-in, where it used to be absent.useTicketValidation.ts:20already declared itcheck_in_date?: string | null. Same forpayment_detailson the check-in response —TicketDetailsModal.vue:85guards it withv-if. Everything else byte-identical, key order included; diffed old vs new module in-process.Schema field types match what Frappe actually returns —
date,timedelta,datetime— sojson_handlerrenders them exactly as before ("10:00:00", not RFC-3339).Checks: 8 new tests, green. Full
bench run-tests --app buzz,ruff,pre-commit,yarn typecheckclean.Noticed, not fixed: only
docstatus == 2blocks check-in, so a draft ticket still scans through. Pre-existing; out of scope here.🤖 Generated with Claude Code