Uh oh!
There was an error while loading. Please reload this page.
refactor(api): split tickets into windows, services and typed responses - #301
Merged
Conversation
Contributor
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Contributor
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
harshtandiyaforce-pushed
the
refactor/api-forms
branch
from
July 29, 2026 16:38
09ba65c to
6bdca6fCompareharshtandiyaforce-pushed
the
refactor/api-tickets
branch
from
July 29, 2026 16:38
10feeae to
0a2790cCompareContributor
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Contributor
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
harshtandiyaforce-pushed
the
refactor/api-tickets
branch
from
July 29, 2026 16:45
0a2790c to
af8bbadCompareharshtandiyaforce-pushed
the
refactor/api-forms
branch
from
July 29, 2026 16:45
6bdca6f to
f56fad3CompareContributor
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
harshtandiyaforce-pushed
the
refactor/api-tickets
branch
from
July 29, 2026 16:53
af8bbad to
dfe6e8fCompareharshtandiyaforce-pushed
the
refactor/api-forms
branch
from
July 29, 2026 16:53
f56fad3 to
381c7ebCompareContributor
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
harshtandiyaforce-pushed
the
refactor/api-tickets
branch
from
July 29, 2026 17:00
dfe6e8f to
481ea93Compareharshtandiyaforce-pushed
the
refactor/api-forms
branch
from
July 29, 2026 17:00
381c7eb to
5b3381fCompareContributor
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
tickets/__init__.py held 324 lines: four endpoints, three near-identical
window checks, three trivial wrappers around them, 66 lines of inline email
HTML, and a get_ticket_details that assembled its payload inline.
The three window checks differed only in which Buzz Settings field they read,
so they collapse into windows.is_window_open(event, cutoff_fieldname) with
the three field names as constants. booking/ imports that directly now
instead of the wrappers. Each check also caught bare Exception and logged it
as an error; that guard was dead for the case it looked like it covered,
since get_ticket_details already loads the Buzz Event before any window is
consulted, so a dangling event reference raises there first.
can_transfer_ticket, can_change_add_ons and can_request_cancellation lose
their @frappe.whitelist(): the dashboard only ever read them off the ticket
and booking detail payloads. Those payloads now carry plain booleans rather
than {"can_transfer": true, "event_id": ...} wrappers, so TicketDetails.vue
and BookingDetails.vue unwrap one level less. That also fixes an
inconsistency where TicketDetails flattened can_transfer_ticket in its
transform but read can_change_add_ons nested.
TicketService owns the ticket-scoped work; the add-on and cancellation paths
key off other doctypes and stay module functions. The transfer emails move to
emails.py untouched, including their catch — a failed send must not undo the
transfer. Ten named errors replace the bare throws.
The three writes are now methods=["POST"], as in forms: frappe skips CSRF
outside UNSAFE_HTTP_METHODS and only auto-commits on POST/PUT.
Also drops the dead downloadTicket stub in TicketDetails.vue. It referenced
buzz.api.download_ticket, which does not exist, was never called, and left
downloadingTicket permanently false behind the Download button's :loading.
get_ticket_details was diffed against a copy of the pre-change module over
six live tickets: identical once the can_* wrappers are normalised, except
that zoom_reference_doctype and zoom_reference_name are now always present
as null rather than absent. Nothing in the dashboard reads either.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>TicketDetails.vue and BookingDetails.vue had no end-to-end coverage at all, which is awkward for a change that reshapes exactly what they read. The spec asserts the payload the page itself fetched, via waitForResponse on get_ticket_details, rather than issuing a second call. The page already holds a valid session and CSRF token; a fresh APIRequestContext under the attendee's storage state would be signing with the shared Administrator token. It asserts on the API rather than on the Transfer and Edit buttons because those gate on doc.booking_status, which TicketDetails derives client-side from data.booking — and get_ticket_details only returns a booking to the user who owns it. A booking seeded over REST is owned by Administrator, so the buttons stay hidden for a seeded attendee no matter how the windows resolve. Asserting the flags at the wire is what this PR actually changed. Seeding runs its own project, since get_ticket_details refuses anyone but the attendee. Cleanup also removes bookings for the event: this setup no longer creates any, but a leftover one blocks the event delete on re-runs. tickets is added to the chromium project's testIgnore, or the spec would run twice, the second time under the shared session. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The window tests move an event's start date back and forth, and they were doing it on the test-route Buzz Event that test_forms and test_sponsorships also read. IntegrationTestCase rolls the database back but does not clear the document cache (frappe/tests/classes/integration_test_case.py:117 rolls the connections and nothing else). Writing a field, clearing the cache and then reading it leaves the cache holding a value the rollback has since removed from the row — visible to any later test module in the same process. The tests now build their own event, so the rollback takes the whole document with it. Buzz Settings is a single and cannot be owned per test, so its cache entry is dropped on cleanup instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
harshtandiyaforce-pushed
the
refactor/api-tickets
branch
from
July 29, 2026 17:17
481ea93 to
0d32345CompareContributor
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Uh oh!
There was an error while loading. Please reload this page.
Contributor
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin main
git worktree add -d .worktree/backport-301-to-main origin/main
cd .worktree/backport-301-to-main
git switch --create backport-301-to-main
git cherry-pick -x e218c1ea0d1e62013a7f1d956e9591e49034b1a9 |
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Stacked on #300 — read that first. Base
refactor/api-forms, repoint todevelopas the stack lands.PR 5 of the
buzz/apirestructure.tickets/__init__.py324 lines → 24, overwindows.py(18),services.py(190),emails.py(71),schemas.py,exceptions.py.The three window checks differed only in which Buzz Settings field they read, so they collapse into
windows.is_window_open(event, cutoff_fieldname). Each also caught bareExceptionand logged it; that guard was dead for the case it appeared to cover, sinceget_ticket_detailsloads the Buzz Event before any window is consulted, so a dangling event reference raises there first.Payload reshape.
can_transfer_ticket/can_change_add_ons/can_request_cancellationlose their@frappe.whitelist()— the dashboard only read them off the detail payloads — and now travel as plain booleans instead of{"can_transfer": true, "event_id": ...}.TicketDetails.vueandBookingDetails.vueunwrap one level less. This also fixes an existing inconsistency: TicketDetails flattenedcan_transfer_ticketin its transform but readcan_change_add_onsnested.Touches
booking/— three lines, becauseget_booking_detailsemits the same flags andcreate_add_on_doclives here. PR 6 rewrites that file anyway; the plan's "disjoint folders" claim does not quite hold for this pair.The three writes are
methods=["POST"], same reasoning as #300.Also deletes the dead
downloadTicketstub: it referencedbuzz.api.download_ticket, which does not exist, was never called, and leftdownloadingTicketpermanently false behind the Download button's:loading.Not changed: the transfer emails move to
emails.pyuntouched, catch included — a failed send must not undo the transfer.get_ticket_detailsstill refuses a booking owner who is not the attendee, whiletransfer_ticketallows them; that asymmetry is noted in the plan, not fixed here.Checks: 26 Python tests (new file), 5 Playwright in a new
tickets-*project pair, ruff andvue-tscclean.get_ticket_detailsdiffed against a copy of the pre-change module over six live tickets — identical once thecan_*wrappers are normalised, exceptzoom_reference_doctype/zoom_reference_nameare now always present asnullrather than absent, and nothing in the dashboard reads either. Only thetickets-*projects were run; no other spec touches the two pages this changes.🤖 Generated with Claude Code