fix: prevent event booking after event ends#110
fix: prevent event booking after event ends#110imgullu786 wants to merge 3 commits intoBuildWithHussain:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds a new runtime validation to EventBooking that prevents creating or modifying bookings for events that have already ended by computing the event end moment (end_date + end_time, default 23:59:59), handling timezones when present, and raising an error if now is after that moment. Called at the start of validate. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
Fix all issues with AI Agents 🤖
In @buzz/ticketing/doctype/event_booking/event_booking.py:
- Line 46: Move the call to self.validate_event_dates() to the very start of the
validate() method so event date validation happens before any ticket
availability checks, amount calculations, or tax application; locate the
validate() function in event_booking.py and relocate the existing
self.validate_event_dates() invocation to the top of that method (before calls
that perform availability checks, compute amounts, or apply taxes) to fail fast
when an event has already ended.
- Around line 179-192: validate_event_dates currently calls frappe.db.get_value
for end_date/end_time; instead, read these fields from the cached event document
that apply_taxes_if_applicable() populates on the Booking object (e.g., check
self.event_doc or self._event and use its get("end_date") and get("end_time")),
and only fall back to frappe.db.get_value if no cached document exists; keep the
same logic that builds time_part and compares now_datetime() to the combined
datetime and raise the same frappe.throw when the event has ended.
- Around line 186-188: The comparison mixes a timezone-aware now_datetime() with
a naive get_datetime(...) result causing a TypeError; replace
get_datetime(f"{event_end_date} {time_part}") with a timezone-aware parse (e.g.
get_datetime_in_timezone(f"{event_end_date} {time_part}",
<appropriate_timezone>)) so event_end_datetime is timezone-aware, or
alternatively convert now_datetime() to naive before comparing (e.g.
now_datetime().replace(tzinfo=None)); update the code around event_end_datetime
and the if now_datetime() > event_end_datetime check to use the chosen
consistent timezone-aware or naive approach.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
buzz/ticketing/doctype/event_booking/event_booking.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Server
🔇 Additional comments (2)
buzz/ticketing/doctype/event_booking/event_booking.py (2)
8-8: LGTM! Imports are appropriate for the date validation logic.The
get_datetimeandnow_datetimeutilities are correctly imported and used in the new validation method.
184-184: Verify behavior for events without an end date.The validation is skipped entirely when
event_end_dateis not set, allowing indefinite bookings for such events. Please confirm this is the intended behavior, as some events may require an end date to prevent open-ended registrations.
5a412af to
91e53b1
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @buzz/ticketing/doctype/event_booking/event_booking.py:
- Around line 195-197: The comparison in the else branch uses now_datetime()
(timezone-aware) against event_end_naive (naive), causing a TypeError; convert
now_datetime() to a naive datetime before comparing (e.g., strip tzinfo as done
in the if branch) so that the comparison between now and event_end_naive is
between two naive datetimes; update the else block where now_datetime() is
compared to event_end_naive and ensure you reuse the same naive conversion
helper or logic used earlier to keep behavior consistent.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
buzz/ticketing/doctype/event_booking/event_booking.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Server
🔇 Additional comments (2)
buzz/ticketing/doctype/event_booking/event_booking.py (2)
8-8: LGTM! Imports are appropriate for timezone-aware validation.The imported utilities are correctly used in the new validation method to handle timezone-aware datetime comparisons.
41-41: LGTM! Fail-fast validation placement is optimal.Calling the validation at the start of
validate()prevents unnecessary ticket availability checks and amount calculations when the event has already ended.
| else: | ||
| if now_datetime() > event_end_naive: | ||
| frappe.throw(_("Cannot book tickets. This event has already ended."), title=_("Event Ended")) |
There was a problem hiding this comment.
Fix TypeError: comparing timezone-aware and naive datetimes.
The else branch at line 196 compares now_datetime() (which returns a timezone-aware datetime in the system timezone) with event_end_naive (which is naive). Python raises TypeError when comparing timezone-aware and naive datetimes.
To fix this, make now_datetime() naive before comparison by stripping its timezone info, similar to the approach in the if branch.
🔎 Proposed fix to use naive datetime comparison
else:
- if now_datetime() > event_end_naive:
+ if now_datetime().replace(tzinfo=None) > event_end_naive:
frappe.throw(_("Cannot book tickets. This event has already ended."), title=_("Event Ended"))🤖 Prompt for AI Agents
In @buzz/ticketing/doctype/event_booking/event_booking.py around lines 195 -
197, The comparison in the else branch uses now_datetime() (timezone-aware)
against event_end_naive (naive), causing a TypeError; convert now_datetime() to
a naive datetime before comparing (e.g., strip tzinfo as done in the if branch)
so that the comparison between now and event_end_naive is between two naive
datetimes; update the else block where now_datetime() is compared to
event_end_naive and ensure you reuse the same naive conversion helper or logic
used earlier to keep behavior consistent.
There was a problem hiding this comment.
@imgullu786 Can you confirm if this is not the case?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
Adds validation to Event booking once the event end date/time has passed.
Closes #91
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.