Skip to content

Commit 9bc5b97

Browse files
committed
fix(promo-codes): harden reservation backfill per Codex review
Three no-behavior-change safety nits from the step-1 audit: - Version20260415191522 backfill now uses GREATEST(QtyUsed, VALUES(QtyUsed)) on the ON DUPLICATE KEY path so a re-run after live saga writes can never clobber a newer counter with a stale historical count. - Version20260415191522.down() is now a documented no-op. The previous TRUNCATE would have silently zeroed live counters once step 2 lands. Roll back Version20260415191521 if a clean slate is needed. - ISummitPromoCodeMemberReservationRepository docblock is reworded so the 'outer lock' statement is explicit as a CALLER PRECONDITION, not something the repository guarantees or enforces. Migration down/up round-trip re-verified on docker MySQL. Remaining step-1 concern — undo idempotency of the entity's decrement helper — is deferred to step 2, where the saga caller is the right place to guard duplicate undo invocations (mirror the 'redeem' flag pattern already used by ApplyPromoCodeTask::undo).
1 parent b1f3abe commit 9bc5b97

2 files changed

Lines changed: 14 additions & 10 deletions

File tree

app/Models/Foundation/Summit/Repositories/ISummitPromoCodeMemberReservationRepository.php

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,12 @@ interface ISummitPromoCodeMemberReservationRepository extends IBaseRepository
2424
/**
2525
* Look up the per-member reservation row for a given promo code.
2626
*
27-
* Callers invoking this from the reservation path must already hold an
28-
* exclusive row lock on the parent SummitRegistrationPromoCode (via
29-
* ISummitRegistrationPromoCodeRepository::getByValueExclusiveLock). That
30-
* outer lock is what serializes concurrent access to this row — no
31-
* separate PESSIMISTIC_WRITE is taken here.
27+
* CALLER PRECONDITION (not enforced here): when invoked from the order
28+
* reservation write path, the caller must already hold an exclusive row
29+
* lock on the parent SummitRegistrationPromoCode via
30+
* ISummitRegistrationPromoCodeRepository::getByValueExclusiveLock. That
31+
* outer lock is what serializes concurrent writes; this method does not
32+
* (and cannot) take or verify the lock itself.
3233
*
3334
* @param SummitRegistrationPromoCode $code
3435
* @param Member $member

database/migrations/model/Version20260415191522.php

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,16 +56,19 @@ public function up(Schema $schema): void
5656
AND o.Status IN ('Reserved', 'Paid', 'Confirmed')
5757
AND t.Status != 'Cancelled'
5858
GROUP BY t.PromoCodeID, o.OwnerID
59-
ON DUPLICATE KEY UPDATE QtyUsed = VALUES(QtyUsed), LastEdited = NOW()
59+
ON DUPLICATE KEY UPDATE
60+
QtyUsed = GREATEST(QtyUsed, VALUES(QtyUsed)),
61+
LastEdited = NOW()
6062
SQL
6163
);
6264
}
6365

6466
public function down(Schema $schema): void
6567
{
66-
$builder = new Builder($schema);
67-
if ($builder->hasTable('SummitPromoCodeMemberReservation')) {
68-
$this->addSql('TRUNCATE TABLE SummitPromoCodeMemberReservation');
69-
}
68+
// No-op: re-running the backfill is safe (GREATEST preserves live
69+
// counter values), but truncating the table on rollback would silently
70+
// zero live reservations after the saga starts writing. The safer
71+
// inverse is simply "nothing"; roll back Version20260415191521 to drop
72+
// the table entirely if you need a clean slate.
7073
}
7174
}

0 commit comments

Comments
 (0)