-
Notifications
You must be signed in to change notification settings - Fork 0
fix: Checkout skips voucher-required ticket validation #13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds validation to prevent checkout when tickets requiring vouchers are in the cart but the voucher has been removed. The changes implement checkout-time re-validation to catch cases where a user adds a voucher-required ticket to their cart, then removes the voucher before completing checkout.
Changes:
- Added validation in checkout service to verify tickets requiring vouchers still have an appropriate voucher at checkout time
- Modified
_revalidate_stockand_revalidate_ticket_stockto accept and validate voucher parameter - Added test case to verify checkout rejection when voucher-required tickets are present without a voucher
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/django_program/registration/services/checkout.py |
Added voucher parameter to stock validation functions and implemented voucher requirement checks during checkout |
tests/test_registration/test_checkout_service.py |
Added test case for voucher-required ticket validation, but contains a critical bug calling non-existent method |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| CartService.apply_voucher(cart, "HIDDEN") | ||
| CartService.add_ticket(cart, hidden_ticket, qty=1) | ||
| CartService.remove_voucher(cart) |
Copilot
AI
Feb 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method CartService.remove_voucher does not exist in the codebase. The CartService class only has the following methods: get_or_create_cart, add_ticket, add_addon, remove_item, update_quantity, apply_voucher, get_summary, and get_summary_from_items. This test will fail when executed. To remove a voucher from a cart, you should directly set cart.voucher = None and save the cart.
| CartService.remove_voucher(cart) | |
| cart.voucher = None | |
| cart.save(update_fields=["voucher", "updated_at"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@codex why did you recommend this patch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question — that recommendation was to fix a test bug, not to change product behavior.
Why I recommended that patch
At line 362, the test calls CartService.remove_voucher(cart), but CartService does not define a remove_voucher method, so the test would fail with an attribute error before reaching the checkout assertion. CartService includes methods like get_or_create_cart, add_ticket, add_addon, remove_item, update_quantity, apply_voucher, get_summary, and get_summary_from_items, but no remove_voucher. tests/test_registration/test_checkout_service.pyL360-L365 src/django_program/registration/services/cart.pyL59-L60 src/django_program/registration/services/cart.pyL103-L104 src/django_program/registration/services/cart.pyL166-L167 src/django_program/registration/services/cart.pyL219-L220 src/django_program/registration/services/cart.pyL248-L249 src/django_program/registration/services/cart.pyL288-L289 src/django_program/registration/services/cart.pyL317-L318 src/django_program/registration/services/cart.pyL333-L334
The suggested replacement (cart.voucher = None + save(update_fields=["voucher", "updated_at"])) was the minimal way to simulate “voucher removed after adding ticket,” which is exactly what that test is trying to validate at checkout time. This also mirrors how voucher assignment is persisted in apply_voucher (same fields saved), just in reverse direction. tests/test_registration/test_checkout_service.pyL340-L368 src/django_program/registration/services/cart.pyL312-L314
Commands I used to verify
sed -n '320,390p' tests/test_registration/test_checkout_service.pyrg -n "class CartService|def remove_voucher|def get_or_create_cart|def apply_voucher|def get_summary_from_items" src testssed -n '50,360p' src/django_program/registration/services/cart.pynl -ba tests/test_registration/test_checkout_service.py | sed -n '346,374p'nl -ba src/django_program/registration/services/cart.py | sed -n '50,360p'python - <<'PY' ... duckduckgo query ... PY(web search attempt failed due proxy/tunnel 403)
If you want, I can also propose a cleaner API-level follow-up (adding CartService.remove_voucher) so tests and application code avoid direct model mutation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its wild to me you'd introduce a method that doesn't even exist
Codex generated this pull request, but encountered an unexpected error after generation. This is a placeholder PR message.
Codex Task