fix(promo): SDS-aligned polish for promo-code discovery flow - #148
Conversation
Previously, switching to a non-qualifying ticket silently called removePromoCode, hiding the rejection from the user. Now always re-validates against the new ticket so the backend rejection surfaces as INVALID and the user can decide to Remove or pick another ticket.
The early-auto-apply effect required no tickets to be visible (!hasTickets), preventing auto-apply when public tickets were already in the list. Per the registration-lite SDS, auto-apply fires whenever a single auto_apply discovered code exists and no code is currently applied. Drops the hasTickets condition from the gate and deps; updates the stale concurrency comment.
When the suggestion banner is visible, clicking Apply with a non- qualifying ticket selected used to fail INVALID against that ticket — misleading after the UI just promised "you qualify". Three coordinated changes implement the "Apply switches you" UX: - On Apply of the discovered code with a non-qualifying ticket, deselect first. The qualifying ticket isn't in the dropdown list yet; it shows up only after the code-filtered refetch lands inside applyPromoCode. - Once that refresh settles (applyingCode clears), the post-apply effect picks the first ticket the code applies to. Falls back to the only-ticket case when nothing qualifies. - The suggestion banner stays visible across non-qualifying ticket picks, so the "Apply will switch you" affordance doesn't disappear the moment the user picks a different ticket. Exposes applyingCode and isCodeValidForTicket from usePromoCode so the ticket-type caller can drive both the gate and the qualifying- first pick. Manual (non-discovered) codes are unaffected — they still validate against the current ticket; backend decides.
The post-apply auto-select scanned originalTicketTypes (unfiltered Redux list), so a discovered code could pick a ticket outside its sales window. The dropdown is built from allowedTicketTypes, so the user couldn't see or change the selection. Scan allowedTicketTypes instead.
Removes an implicit dependency on applyPromoCode's brief optimistic promoCode=truthy window. Cleanup now expresses the policy directly: hook errors take precedence over the unapplied warning.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughUpdated promo code hook and components so discovered codes are revalidated on ticket change, early auto-apply uses date-filtered allowedTicketTypes (selects qualifying tickets after applying), registration form clears unapplied warnings on validation errors, PromoCodeInput shows alternate suggestion copy, and unit + e2e tests were added/updated. ChangesPromo Code Auto-Apply with Date Filtering
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
…uto-apply contract c60b30e changed onTicketSelected to re-validate the discovered code on a ticket switch rather than removePromoCode'ing it; a8590e2 dropped the hasTickets gate from the early-auto-apply effect to align with the SDS. The three tests that asserted the old contract were still in the suite and failed. Updates them to the new contract.
…qualify The "You qualify for the following promo code:" banner stayed visible across non-qualifying ticket picks so the "Apply will switch you" affordance persists, but it then claimed the user qualified while a non-qualifying ticket was selected. Keeps the personal phrasing on qualifying picks and swaps to "Following promo code applies to a different ticket. Apply to switch." when not.
|
Early auto-apply can blank the ticket catalog when the code-filtered list comes back empty With the If the code's allowed tickets are all outside their sales window — the exact config 8b9b814 fixes the auto-select for — the refetch returns an empty allowed list and:
Blast radius: every authenticated visitor who qualifies for that code, on every load, for as long as the misconfiguration lasts — and it's silent (the widget thinks the apply worked, nothing reaches Sentry). Pre-PR, the Suggested guard: after auto-apply settles, if the refreshed (date-filtered) ticket list is empty, auto-remove the code and fall back to the public list — optionally keeping the rejection message visible so the user knows why. Repro sketch (e2e): single discovered |
|
Good catch, mechanism checks out: applyPromoCode → getTicketTypes with filter=promo_code==X replaces the ticketTypes slice, so an empty code-filtered response does blank the public catalog. Confirmed in actions.js + reducer.js. Want to go with a pre-check rather than the post-apply rollback you sketched. The hook receives the date-filtered ticket list from the form already; the early-auto-apply effect can bail when no ticket in that list satisfies isCodeValidForTicket. That way we never enter the broken state, no catalog flash, nothing applied, nothing to surface or roll back. The user just sees the public list. Post-apply rollback (silent or with a notice) either flashes "no tickets" briefly or makes a state we deliberately created disappear back, which is its own UX problem. Pre-check sidesteps both. It does add a precondition the SDS does not spell out, that the code is applicable to a currently-purchasable ticket. Want to make sure you're OK with that read before I push. If you want the operator visibility piece (silent misconfig in prod), I can add a Sentry breadcrumb when the gate skips. Separate concern from the user-facing fix. |
|
Pre-check shipped in bd853eb 🎉 Gate sits in the early-auto-apply effect:
Carve-out: when allowedTicketTypes is empty (code-only events), we still fire. The code may be the gateway to private/code-only tickets that only surface after the code-filtered refetch, and that's the case the pre-a8590e2 !hasTickets gate was originally optimizing for. Coverage:
Still leaves the case where discovery returns a code that unlocks nothing for anyone (allowed_ticket_types empty in both directions). That is strictly a backend hygiene problem and falls through to the existing INVALID empty-list path. Happy to add a separate handler if you want it covered explicitly, but I'd argue discovery should not surface those at all. Thanks for the catch on this one, would have been a nasty silent prod regression. |
bd853eb to
b6835cd
Compare
|
You're right, I shouldn't have assumed the API contract without verifying it. Confirmed in RegularPromoCodeTicketTypesStrategy::getTicketTypes() that filter=promo_code==X augments the catalog rather than replacing it, so the "catalog blanking" couldn't happen, and the pre-check blocks the WithPromoCode gateway case exactly as you flagged (hidden tickets only revealed by the filtered call would never match the visible-tickets predicate). Reset the branch back to b6835cd and force-pushed. The unit regression and the e2e from bd853eb went with it since they were mocking the wrong API semantics. Thanks for the double-check. |
2 participants
ref: https://app.clickup.com/t/86ba9qt03
Per-commit summary
1.
c60b30e— Re-validate the discovered code when the user switches ticketsINVALID, so they can choose Remove or pick a different ticket.2.
a8590e2— Fire early auto-apply per the SDS rule!hasTickets). If public tickets were already visible, auto-apply never fired.auto_applycode is discovered and none is applied. ThehasTicketsgate is gone.3.
a718823— Auto-switch to the qualifying ticket on ApplyINVALIDagainst that ticket. Misleading after the UI just said "you qualify".applyingCodeandisCodeValidForTicketfromusePromoCodeso the ticket-type caller can drive both the gate and the qualifying pick.4.
8b9b814— Auto-select only from tickets the user can actually buy (+ new e2e spec)allowedTicketTypes) — same list the dropdown uses.5.
98a7ba5— Make the "you didn't click Apply" warning yield to a real validation error (+ new e2e spec)applyPromoCodebriefly sets ReduxpromoCodeto truthy before clearing it on failure — a race window inside the action.applyPromoCode(e.g. "only set state on success") would silently leave the stale warning masking the actual "invalid code" message.validationErrordirectly. Same behavior today, robust to future refactors of the apply action.Summary by CodeRabbit
New Features
Bug Fixes
Tests
Localization