Skip to content

Commit fe32435

Browse files
caseylockerclaude
andcommitted
docs(promo-codes): add review follow-ups for Tasks 5 and 7
Task 5: accepted NITs for constant naming, interface gap, and pre-existing edge cases. Task 7: MUST-FIX — canBuyRegistrationTicketByType() missing WithPromoCode branch blocks checkout for promo-code-only tickets. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent a556412 commit fe32435

1 file changed

Lines changed: 5 additions & 2 deletions

File tree

doc/promo-codes-for-early-registration-access.md

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -440,7 +440,10 @@ Deviations from the SDS captured during implementation. Each entry is either **O
440440
- `php artisan clear-compiled && php artisan cache:clear`
441441

442442
**Review Follow-ups:**
443-
- None
443+
- [x] **Constant naming deviates from SDS spec (NIT — accepted):** SDS specifies `AUDIENCE_WITH_PROMO_CODE`; implementation uses `Audience_With_Promo_Code`. Follows existing codebase convention (`Audience_All`, `Audience_With_Invitation`, `Audience_Without_Invitation`). All consumers reference the constant rather than the string literal. No correctness risk.
444+
- [x] **`isPromoCodeOnly()` not declared in `ISummitTicketType` interface (NIT):** Method is only called on concrete `SummitTicketType` objects (via `getAllowedTicketTypes()` in the strategy), so no runtime failure. Future code working through the `ISummitTicketType` abstraction would need a cast. No current impact; worth adding to the interface in a follow-on cleanup.
445+
- [x] **`isInviteOnlyRegistration()` ignores `WithPromoCode` types (NIT — out of scope):** A summit with only `WithPromoCode` ticket types returns `false`. Pre-existing method not changed by this task; edge case is unlikely in practice. No action required here.
446+
- [x] **`getTicketTypeBySummit` by-ID endpoint exposes `WithPromoCode` metadata to any OAuth user (NIT — pre-existing pattern):** Requires `ReadSummitData` scope (the same scope the registration frontend uses), so any authenticated user who knows a ticket type ID can fetch its metadata. Identical behavior exists today for `WithInvitation` types. Primary public listing (`getAllBySummit`) correctly enforces `audience=All`. Not a new risk introduced by this task.
444447

445448
---
446449

@@ -523,7 +526,7 @@ Deviations from the SDS captured during implementation. Each entry is either **O
523526
- Test: `All` ticket type + promo code → IS returned with promo applied (existing behavior)
524527

525528
**Review Follow-ups:**
526-
- None
529+
- [ ] **`canBuyRegistrationTicketByType()` missing `WithPromoCode` branch (MUST-FIX):** `Summit::canBuyRegistrationTicketByType()` (`Summit.php:5523`) has no branch for `audience = WithPromoCode`. When a user without an invitation attempts to purchase a `WithPromoCode` ticket type at checkout, `PreProcessReservationTask` (`SummitOrderService.php:1224`) calls this method and receives `false`, throwing `ValidationException` — the order is rejected even with a valid qualifying promo code. Fix: add `if ($audience === SummitTicketType::Audience_With_Promo_Code) return true;` immediately after the `Audience_All` branch. Access control is already handled by the promo code's own `checkSubject()` / `canBeAppliedTo()` — the `audience` field governs visibility only, not purchase authorization.
527530

528531
---
529532

0 commit comments

Comments
 (0)