fix: prevent booking after event start date #135
Closed
Conversation
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
📝 WalkthroughWalkthroughThe PR introduces a booking closure system enabling per-event Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client (Vue)
participant API as API
participant Event as BuzzEvent
participant Settings as BuzzSettings
participant Booking as EventBooking
Client->>API: GET event booking data
API->>Event: is_booking_open
Event->>Event: Check booking_closes_on?
alt Per-event date set
Event->>Event: Compare current date ≤ booking_closes_on
else Fall back to global
Event->>Settings: allow_booking_before_event_start_days
Settings-->>Event: days value
Event->>Event: Calculate cutoff date & compare
end
Event-->>API: is_booking_open result
API-->>Client: booking_allowed in response
Client->>Client: bookingAllowed = data.booking_allowed
alt bookingAllowed is true
Client->>Client: Render booking form
Client->>Booking: POST EventBooking (submit)
Booking->>Booking: validate_booking_open()
Booking->>Event: is_booking_open
Event-->>Booking: booking status
alt Bookings still open
Booking-->>Client: Validation success
else Bookings closed
Booking-->>Client: Error - bookings closed
end
else bookingAllowed is false
Client->>Client: Render "Bookings Closed" message
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@buzz/api.py`:
- Around line 120-124: In get_event_booking_data, you're assigning the method
object event_doc.is_booking_open instead of its boolean result; update the code
in get_event_booking_data to call the method (event_doc.is_booking_open()) and
assign that boolean to data.booking_allowed so serialization and booking checks
work correctly; locate the assignment to data.booking_allowed in the
get_event_booking_data function and replace the method reference with the method
call on the event_doc instance.
In `@buzz/events/doctype/buzz_event/buzz_event.py`:
- Around line 201-204: Clamp the booking cutoff to non-negative so a negative
settings.allow_booking_before_event_start_days cannot extend booking past the
event; when you read settings = frappe.get_cached_doc("Buzz Settings") compute
cutoff_days = max(0, settings.allow_booking_before_event_start_days or 0) (or
validate earlier in the Buzz Settings model) before calling
add_days(self.start_date, -cutoff_days), keeping the rest of the logic with
cutoff_date, getdate and current_date unchanged.
In `@buzz/ticketing/doctype/event_booking/event_booking.py`:
- Around line 42-44: The booking-closure check is calling the method object
instead of invoking it, so change places where event.is_booking_open is used
(notably in validate_booking_open called from validate and the block around
lines 293-297) to call the method as event.is_booking_open() and use its boolean
result to enforce closure (raise the existing validation error/throw when
False). Locate and update any conditional(s) that currently reference
event.is_booking_open to call the method and keep the existing error
message/exception behavior.
In `@dashboard/src/pages/BookTickets.vue`:
- Around line 73-76: The assignment eventBookingData.bookingAllowed =
data.booking_allowed can flip the UI to “Bookings Closed” when the API omits or
returns null; change it to use a nullish fallback so the existing default is
preserved (e.g., use the nullish coalescing pattern when setting bookingAllowed
or explicitly check for undefined/null before assignment in the BookTickets.vue
code that assigns eventBookingData.bookingAllowed).
🧹 Nitpick comments (1)
buzz/events/doctype/buzz_settings/buzz_settings.py (1)
18-25: Add server-side bounds validation for booking cutoff days.The new setting is only constrained at the DocType level; adding a server-side guard keeps behavior consistent with other “*_before_event_start_days” settings.
♻️ Suggested validation hook
def validate(self): """Validate the settings.""" self.validate_transfer_days() + self.validate_booking_days() + +def validate_booking_days(self): + """Validate that booking cutoff days is a reasonable value.""" + if self.allow_booking_before_event_start_days is None: + return + if self.allow_booking_before_event_start_days < 0: + frappe.throw(_("Allow Booking Before Event Start Days cannot be negative.")) + elif self.allow_booking_before_event_start_days > 365: + frappe.throw(_("Allow Booking Before Event Start Days cannot be more than 365 days."))
Comment on lines
120
to
124
| def get_event_booking_data(event_route: str) -> dict: | ||
| data = frappe._dict() | ||
| event_doc = frappe.get_cached_doc("Buzz Event", {"route": event_route}) | ||
|
|
||
| data.booking_allowed = event_doc.is_booking_open | ||
| # Ticket Types |
There was a problem hiding this comment.
Call is_booking_open() instead of passing the method object.
event_doc.is_booking_open is a method on Buzz Event; assigning it directly will either serialize incorrectly or always truthy. This prevents proper booking closure and can break JSON serialization.
🐛 Fix
- data.booking_allowed = event_doc.is_booking_open
+ data.booking_allowed = event_doc.is_booking_open()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def get_event_booking_data(event_route: str) -> dict: | |
| data = frappe._dict() | |
| event_doc = frappe.get_cached_doc("Buzz Event", {"route": event_route}) | |
| data.booking_allowed = event_doc.is_booking_open | |
| # Ticket Types | |
| def get_event_booking_data(event_route: str) -> dict: | |
| data = frappe._dict() | |
| event_doc = frappe.get_cached_doc("Buzz Event", {"route": event_route}) | |
| data.booking_allowed = event_doc.is_booking_open() | |
| # Ticket Types |
🤖 Prompt for AI Agents
In `@buzz/api.py` around lines 120 - 124, In get_event_booking_data, you're
assigning the method object event_doc.is_booking_open instead of its boolean
result; update the code in get_event_booking_data to call the method
(event_doc.is_booking_open()) and assign that boolean to data.booking_allowed so
serialization and booking checks work correctly; locate the assignment to
data.booking_allowed in the get_event_booking_data function and replace the
method reference with the method call on the event_doc instance.
Comment on lines
+201
to
+204
| settings = frappe.get_cached_doc("Buzz Settings") | ||
| cutoff_days = settings.allow_booking_before_event_start_days or 0 | ||
| cutoff_date = add_days(self.start_date, -cutoff_days) | ||
| return current_date <= getdate(cutoff_date) |
There was a problem hiding this comment.
Guard against negative allow_booking_before_event_start_days.
A negative value would extend booking after the event start, breaking the intended rule. Consider clamping to >= 0 here (or validating in Buzz Settings).
✅ Suggested defensive clamp
- cutoff_days = settings.allow_booking_before_event_start_days or 0
+ cutoff_days = max(int(settings.allow_booking_before_event_start_days or 0), 0)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| settings = frappe.get_cached_doc("Buzz Settings") | |
| cutoff_days = settings.allow_booking_before_event_start_days or 0 | |
| cutoff_date = add_days(self.start_date, -cutoff_days) | |
| return current_date <= getdate(cutoff_date) | |
| settings = frappe.get_cached_doc("Buzz Settings") | |
| cutoff_days = max(int(settings.allow_booking_before_event_start_days or 0), 0) | |
| cutoff_date = add_days(self.start_date, -cutoff_days) | |
| return current_date <= getdate(cutoff_date) |
🤖 Prompt for AI Agents
In `@buzz/events/doctype/buzz_event/buzz_event.py` around lines 201 - 204, Clamp
the booking cutoff to non-negative so a negative
settings.allow_booking_before_event_start_days cannot extend booking past the
event; when you read settings = frappe.get_cached_doc("Buzz Settings") compute
cutoff_days = max(0, settings.allow_booking_before_event_start_days or 0) (or
validate earlier in the Buzz Settings model) before calling
add_days(self.start_date, -cutoff_days), keeping the rest of the logic with
cutoff_date, getdate and current_date unchanged.
Comment on lines
42
to
44
| def validate(self): | ||
| self.validate_booking_open() | ||
| self.validate_ticket_availability() |
There was a problem hiding this comment.
is_booking_open must be called to enforce booking closure.
event.is_booking_open is a method, so the current check always evaluates truthy and never blocks bookings.
🐛 Fix
- if not event.is_booking_open:
+ if not event.is_booking_open():
frappe.throw(frappe._("Bookings are closed for {0}").format(event.title))Also applies to: 293-297
🤖 Prompt for AI Agents
In `@buzz/ticketing/doctype/event_booking/event_booking.py` around lines 42 - 44,
The booking-closure check is calling the method object instead of invoking it,
so change places where event.is_booking_open is used (notably in
validate_booking_open called from validate and the block around lines 293-297)
to call the method as event.is_booking_open() and use its boolean result to
enforce closure (raise the existing validation error/throw when False). Locate
and update any conditional(s) that currently reference event.is_booking_open to
call the method and keep the existing error message/exception behavior.
Comment on lines
73
to
76
| eventBookingData.customFields = data.custom_fields || []; | ||
| eventBookingData.paymentGateways = data.payment_gateways || []; | ||
| eventBookingData.bookingAllowed = data.booking_allowed; | ||
| }, |
There was a problem hiding this comment.
Guard against missing booking_allowed to avoid false “closed” state.
If the API omits the field or returns null, the UI flips to “Bookings Closed.” A nullish fallback keeps the default behavior intact.
🐛 Suggested fix
- eventBookingData.bookingAllowed = data.booking_allowed;
+ eventBookingData.bookingAllowed = data.booking_allowed ?? true;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| eventBookingData.customFields = data.custom_fields || []; | |
| eventBookingData.paymentGateways = data.payment_gateways || []; | |
| eventBookingData.bookingAllowed = data.booking_allowed; | |
| }, | |
| eventBookingData.customFields = data.custom_fields || []; | |
| eventBookingData.paymentGateways = data.payment_gateways || []; | |
| eventBookingData.bookingAllowed = data.booking_allowed ?? true; | |
| }, |
🤖 Prompt for AI Agents
In `@dashboard/src/pages/BookTickets.vue` around lines 73 - 76, The assignment
eventBookingData.bookingAllowed = data.booking_allowed can flip the UI to
“Bookings Closed” when the API omits or returns null; change it to use a nullish
fallback so the existing default is preserved (e.g., use the nullish coalescing
pattern when setting bookingAllowed or explicitly check for undefined/null
before assignment in the BookTickets.vue code that assigns
eventBookingData.bookingAllowed).
Contributor
Author
Sign up for free
to 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.
Stops users from booking tickets after event has started.
Two options for organizers
If both are set, the per-event date is used.
Works for max cases: close booking on start date, X days before start, or for multi-day events - any
day between start and end.
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.