Skip to content

Commit 05bcafe

Browse files
committed
fix(promo-codes): step-3 review follow-ups
Two findings from Codex audit of 999eec1: 1. BUG — ApplyPromoCodeTask's pointer comment referenced tests/Unit/Services/ApplyPromoCodeTaskConcurrencyTest, a file deleted in the same commit. Repoint at the new location, tests/Unit/Services/PreProcessReservationTaskConcurrencyTest. (Patch applied by Codex.) 2. CONCERN — coverage parity lost the "single order qty > limit with no prior reservation row" case from the deleted ApplyPromoCodeTaskQuantityPerAccountTest::testRejectsWhenOrderExceedsLimit. Restore that branch via a new PreProcessReservation test: testSingleOrderExceedingLimitRejects (limit=1, prior=null, qty=2 → reject; repo `add` must never be called). Now 7/7 passing in PreProcessReservationTaskConcurrencyTest.
1 parent 999eec1 commit 05bcafe

2 files changed

Lines changed: 25 additions & 2 deletions

File tree

app/Services/Model/Imp/SummitOrderService.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -820,8 +820,8 @@ public function run(array $formerState): array
820820
// PESSIMISTIC_WRITE row lock on the promo code, BEFORE ReserveOrderTask
821821
// commits tickets. A post-facto count here (as this task did prior to
822822
// the TOCTOU fix) cannot distinguish the current order's freshly-
823-
// committed tickets from a concurrent order's — see smarcet's
824-
// reproduction in tests/Unit/Services/ApplyPromoCodeTaskConcurrencyTest.
823+
// committed tickets from a concurrent order's — see the reproduced
824+
// scenario now covered in tests/Unit/Services/PreProcessReservationTaskConcurrencyTest.
825825

826826
Log::debug(sprintf("adding %s usage to promo code %s", $qty, $promo_code->getId()));
827827

tests/Unit/Services/PreProcessReservationTaskConcurrencyTest.php

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,29 @@ public function testSecondReserveRejectsOverLimit(): void
131131
]);
132132
}
133133

134+
/**
135+
* A single first-time request with qty > limit must be rejected
136+
* even when there is no prior reservation row. Limit=1, qty=2,
137+
* check: 0 + 2 > 1 true → reject. The repo's `add` must NOT be
138+
* called since we never reach the persist branch.
139+
*/
140+
public function testSingleOrderExceedingLimitRejects(): void
141+
{
142+
[$promoCode, $owner, $promoRepo, $reservationRepo, $tx] =
143+
$this->buildCollaborators(limit: 1, priorReservation: null);
144+
145+
$reservationRepo->shouldReceive('add')->never();
146+
147+
$task = $this->buildTask(owner: $owner, promoRepo: $promoRepo, reservationRepo: $reservationRepo, tx: $tx);
148+
149+
$this->expectException(ValidationException::class);
150+
$this->expectExceptionMessageMatches('/reached the maximum of 1/');
151+
152+
$this->invokeReserve($task, [
153+
'DOMAIN-CODE-1' => ['qty' => 2, 'types' => [42]],
154+
]);
155+
}
156+
134157
/**
135158
* Limit=2, prior qty_used=1 (from a previous Task A), this
136159
* request qty=1. Check: 1 + 1 > 2 false → pass, increment by 1.

0 commit comments

Comments
 (0)