fix(promo-codes): restrict implicit "apply to all" to Audience=All only#540
fix(promo-codes): restrict implicit "apply to all" to Audience=All only#540caseylocker wants to merge 2 commits into
Conversation
When SummitRegistrationPromoCode::allowed_ticket_types is empty, canBeAppliedTo() previously returned true for any ticket type — including WithPromoCode-audience tickets that are deliberately hidden from the public. This change tightens the implicit-sweep branch to require Audience = All; non-All audiences must be opted in via explicit allowed_ticket_types membership. Adds 11 unit tests (4 audience values × empty/explicit) and a smoke test confirming DomainAuthorizedSummitRegistrationDiscountCode's override still applies to its explicitly-allowed WithPromoCode tickets. Refs: ClickUp 86b9vrpxp; parent feature SDS doc/promo-codes-for-early-registration-access.md
…case testNonEmptyAllowedTicketTypesNotContainingTicketReturnsFalse previously only ran for Audience=All. Convert it to a DataProvider so a regression that accidentally returns true on the absent-from-list path for WithInvitation / WithoutInvitation / WithPromoCode also gets caught. Also tighten the stale "returns true when allowed_ticket_types is empty" comment in DomainAuthorizedSummitRegistrationDiscountCode::addTicketTypeRule to reflect the post-fix conditional, and refresh the SDS line-number cites to match the current method body. Refs: ClickUp 86b9vrpxp; Codex review of 8defe88.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/summit-api/openapi/pr-540/ This page is automatically updated on each push to this PR. |
ref: https://app.clickup.com/t/86b9vrpxp
Summary
Tightens
SummitRegistrationPromoCode::canBeAppliedTo()so the implicit "apply to all" branch (emptyallowed_ticket_types) only matches ticket types withAudience = All. Other audiences (WithInvitation,WithoutInvitation,WithPromoCode) must be opted in via explicitallowed_ticket_typesmembership.doc/promo-codes-for-early-registration-access.md(merged via PR feat(promo-codes): domain-authorized promo codes for early registration access #525)doc/promo-code-apply-to-all-audience-restriction.mdWhat changed
app/Models/Foundation/Summit/Registration/PromoCodes/SummitRegistrationPromoCode.php:483-495— empty-collection branch returns$ticketType->getAudience() === SummitTicketType::Audience_All. Subclass overrides (SummitRegistrationDiscountCode::canBeAppliedTo,DomainAuthorizedSummitRegistrationDiscountCode::canBeAppliedTo) delegate to the patched base — no override changes needed.tests/Unit/Services/PromoCodeCanBeAppliedToAudienceTest.php— 11 new unit tests, dataProvider over all 4 audiences × {empty, present, absent} + a smoke test for the domain-authorized override.doc/promo-code-apply-to-all-audience-restriction.md— dedicated SDS.app/Models/Foundation/Summit/Registration/PromoCodes/DomainAuthorizedSummitRegistrationDiscountCode.php:73— tightened a stale comment now that the empty branch is audience-conditional.Caller audit
canBeAppliedTo()is the single gate for every production path:RegularPromoCodeTicketTypesStrategy::applyPromo2TicketType()line 82SummitOrderService.php:812and:1093(order/checkout pipeline)RegularTicketTypePromoCodeValidationStrategy.php:91No code path independently treats empty
allowed_ticket_typesas "applies to all audiences" for promo codes. The out-of-scope siblingSummitAttendee.php:1367is a different domain (invitations).Behavior change (intentional)
Existing discount codes with empty
allowed_ticket_typesthat today implicitly coverWithInvitation/WithoutInvitationtickets will stop covering them after deploy. This is called out in the ticket: "Existing discount codes with 'Apply to all' checked continue to work correctly for Audience = All ticket types — no regression."AC5 wording note
The ClickUp acceptance criterion #5 says "submitting
apply_to_all_ticket_types = trueresolves to Audience = All only, regardless of frontend." No such payload field exists in this API — it's a conceptual name. The actual enforcement iscanBeAppliedTo(), the single gate every consumer flows through. A hostile client cannot bypass it because there's noapply_to_all_*field to honor or ignore.Test plan
./vendor/bin/phpunit tests/Unit/Services/PromoCodeCanBeAppliedToAudienceTest.php— 11 tests, 11 assertions passtests/Unit/Services/passes (53 tests, 77 assertions, no deprecations)tests/oauth2/OAuth2SummitOrdersApiTest.php,OAuth2SummitTicketTypesApiTest.php,OAuth2PromoCodesApiTest.php. The existingtestGetAllowedTicketTypesApplyingRegularDiscountCodeToAllOfTheminOAuth2SummitTicketTypesApiTest.php:218may need updating since it removes types 1/2/3 fromallowed_ticket_typesand asserts the discount applies — that's exactly the path we tightened. Confirm the assertion is still meaningful after the fix and update if needed.SUMMIT_DISCOUNT_CODEwith empty rules, verify it does NOT apply to aWithPromoCodeticket type; add the WithPromoCode type toallowed_ticket_types, verify it then applies.