Skip to content

fix(booking): validate phone numbers on the guest and custom-field paths (backport #335) - #336

Merged
harshtandiya merged 1 commit into
mainfrom
claude/backport-pr-335-main-0dzidi
Aug 12, 2026
Merged

fix(booking): validate phone numbers on the guest and custom-field paths (backport #335)#336
harshtandiya merged 1 commit into
mainfrom
claude/backport-pr-335-main-0dzidi

Conversation

@harshtandiya

Copy link
Copy Markdown
Collaborator

Manual backport of #335 to main, after the automated backport action failed to cherry-pick.

Why the action failed

The squashed commit (23644b7) touches two unrelated things: the phone-validation fix, and the E2E fixture change that stamps an explicit team on every Buzz Event / Event Host. That second part depends on team-based multi-tenancy (#312), which lives only on developmain has no Buzz Team doctype and no ensureTeamMembership helper.

Cherry-picking the whole commit conflicts in e2e/tests/auth.setup.ts and e2e/tests/check-in.setup.ts, and the hunks that do auto-merge are worse than the conflicts: they quietly introduce ensureTestTeam, team fields and a call to a helper that does not exist on main, so the E2E suite would fail to compile.

What this PR carries

Only the fix and its tests — the E2E team fixtures are dropped, since there is nothing on main for them to bind to:

  • buzz/api/booking/guests.py — validate the phone branch of send_booking_otp with validate_phone_number_with_country_code. Validate only, never rewrite, so the OTP cache key stays identical on both the send and verify sides.
  • buzz/api/booking/services.py — one validate_phone_fields, called from process() before build_booking(), covering booking-level and attendee-level Phone custom fields. Running it early matters: verify_guest_otp deletes the code from the cache with no rollback, so validating later cost the guest a valid OTP and left Attendee Ticket Add-on rows behind. verify_guest also validates the guest phone at submit, covering the direct-API path.
  • dashboard/src/components/BookingForm.vue — swap the guest phone FormControl for the existing PhoneInput, and surface OTP-send errors under the field instead of in a toast.
  • dashboard/src/components/PhoneInput.vue — optional error prop rendering frappe-ui's ErrorMessage. Existing callers pass nothing and are unchanged.
  • buzz/api/booking/test_booking.pyTestSendGuestBookingOtp and TestBookingPhoneCustomFields, unchanged from fix(booking): validate phone numbers on the guest and custom-field paths #335 and free of any team references.
  • e2e/tests/guest-booking.spec.ts — the too-short-number case, which needs no team.

Verification

  • Cherry-pick conflicts resolved by restoring main's version of e2e/helpers/frappe.ts and the six *.setup.ts fixtures; the remaining diff is 6 files, +155/−11, and applies with no fuzz.
  • python -m py_compile clean on the three changed Python files; imports the fix relies on (validate_phone_number_with_country_code, validate_custom_fields, watch in BookingForm.vue) all already exist on main.
  • Test helpers the new cases use (set_event, booking_request, free_ticket_type, the Additional Field child doctype) are present on main.
  • bench run-tests and Playwright were not run here — this sandbox has no bench site. CI on this PR is the real check.

Fixes#325 on the main line.


Generated by Claude Code

…ths (backport #335)
(cherry picked from commit 23644b7)
The e2e team fixtures from the original PR are dropped: main has no Buzz
Team doctype (team-based multi-tenancy, #312, is develop-only), so
`ensureTestTeam` and the `team` stamps have nothing to bind to here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EePTfzyKVxj7bcpbF9roWx
@greptile-apps

Copy link
Copy Markdown
Contributor

Greptile Summary

This backport adds server-side validation for guest OTP phone numbers and Phone custom fields before booking side effects, and updates the guest form to use the shared phone input with inline errors.

  • Validates phone identifiers before issuing or consuming guest OTPs.
  • Validates booking- and attendee-level Phone custom fields before building a booking.
  • Adds backend and end-to-end coverage for invalid phone numbers.
  • Replaces the guest phone control with PhoneInput and displays OTP-send failures beneath it.

Confidence Score: 4/5

The PR should not merge until Phone custom-field validation preserves each field's application scope, otherwise legitimate bookings can be rejected when fieldnames overlap.

The new fieldname-only phone map is shared across booking and attendee payloads even though custom-field definitions and persistence are scope-specific, allowing a Phone definition in one scope to misclassify a same-named non-Phone field in another.

Files Needing Attention: buzz/api/booking/services.py

Important Files Changed

FilenameOverview
buzz/api/booking/guests.pyAdds phone-format validation before OTP generation while preserving the submitted identifier used by the cache key.
buzz/api/booking/services.pyMoves Phone custom-field validation ahead of booking side effects and validates guest phones at submission, but conflates custom-field scopes in one fieldname map.
buzz/api/booking/test_booking.pyAdds coverage for OTP phone validation, booking and attendee Phone fields, and preserving an OTP when early validation fails.
dashboard/src/components/BookingForm.vueUses PhoneInput for guest phone OTP entry and routes phone OTP-send failures to an inline field error.
dashboard/src/components/PhoneInput.vueAdds an optional inline ErrorMessage without changing existing callers that omit the new prop.
e2e/tests/guest-booking.spec.tsAdds an end-to-end assertion that a too-short guest phone is rejected without opening the OTP dialog.

Fix All in Claude CodeFix All in Codex

Prompt To Fix All With AI
### Issue 1
buzz/api/booking/services.py:172-175
**Custom-field scopes are conflated**
When an event has a Phone field in one scope and a same-named non-Phone field in another, the fieldname-only map applies phone validation to both booking and attendee payloads, causing a valid non-phone value to fail the booking.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "fix(booking): validate phone numbers on ..." | Re-trigger Greptile

Comment on lines 172 to 175
filters={"event": self.request.event, "enabled": 1, "fieldtype": "Phone"},
fields=["fieldname", "label"],
)
return {field.fieldname: field.label for field in phone_fields}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1Custom-field scopes are conflated

When an event has a Phone field in one scope and a same-named non-Phone field in another, the fieldname-only map applies phone validation to both booking and attendee payloads, causing a valid non-phone value to fail the booking.

Prompt To Fix With AI
This is a comment left during a code review.
Path: buzz/api/booking/services.py
Line: 172-175
Comment:
**Custom-field scopes are conflated**
When an event has a Phone field in one scope and a same-named non-Phone field in another, the fieldname-only map applies phone validation to both booking and attendee payloads, causing a valid non-phone value to fail the booking.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude CodeFix in Codex

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Real but pre-existing, and deliberately not fixed here.

phone_field_labels is byte-identical to what is already on maingit show origin/main:buzz/api/booking/services.py has the same unfiltered query, and attendee_row already passed that map to validate_custom_fields. So an event carrying a Phone field and a same-named non-Phone field in another scope already mis-validates the attendee payload today; this PR extends the same map to the booking payload rather than introducing the conflation.

It is reachable — BuzzCustomField.validate scrubs fieldname from label with no uniqueness constraint on (event, fieldname), so two fields named "Contact Number" under different applied_to values collide. But the fix belongs on develop, where the code now lives, not in a backport whose whole job is to land #335's behaviour on main unchanged. Diverging the two branches here would make the next backport conflict for no gain.

Leaving as-is; worth a separate issue against develop if the collision is considered worth closing.


Generated by Claude Code

@harshtandiyaharshtandiya added the backport This PR / Issue is a backport for a branch other than develop label Aug 11, 2026
@harshtandiya
harshtandiya merged commit 3688fb0 into mainAug 12, 2026
9 checks passed
@harshtandiya
harshtandiya deleted the claude/backport-pr-335-main-0dzidi branch August 12, 2026 10:10
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backportThis PR / Issue is a backport for a branch other than develop

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@harshtandiya