Skip to content

fix(registration): release applied promo code usage when a later code in the same order is rejected #622

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
romanetar wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(registration): release applied promo code usage when a later code in the same order is rejected #622

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d717bfa
fix(registration): release applied promo code usage when a later code…
romanetar Oct 7, 2026
b538ceb
fix(registration): keep releasing the remaining promo codes when one …
romanetar Oct 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 78 additions & 19 deletions app/Services/Model/Imp/SummitOrderService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -772,6 +772,17 @@ final class ApplyPromoCodeTask extends AbstractTask
*/
private $owner;

/**
* Usages applied (and committed) by this task instance, one entry per promo code.
* Filled by applyUsages() right after each code's transaction commits and consumed
* by undo(), which releases each entry exactly once. Tracking it on the instance
* (formerState is only updated when run() completes) lets run() compensate the
* codes it already applied when a later one is rejected.
*
* @var array<int, array{code: string, qty: int}>
*/
private $applied = [];

/**
* ApplyPromoCodeTask constructor.
* @param Summit $summit
Expand Down Expand Up @@ -815,6 +826,37 @@ public function run(array $formerState): array
$this->summit = $summit_repository->getById($this->summit->getId());
$owner_company_name = TaskUtils::getOwnerCompanyName($this->summit, $this->payload);

// Saga::run() only marks a task as "ran" AFTER run() returns. If code A is applied
// (its own transaction already committed) and code B is then rejected, the exception
// propagates before markAsRan(), so Saga::abort() never calls this task's undo() and
// code A's usage would leak. Release what this run applied before rethrowing.
try {
$this->applyUsages($promo_codes_usage, $owner_email, $owner_company_name);
} catch (\Exception $ex) {
try {
$this->undo();
} catch (\Throwable $undo_ex) {
// never mask the exception that rejected the reservation
Log::error(sprintf("ApplyPromoCodeTask::run undo failed: %s", $undo_ex->getMessage()));
Log::error($undo_ex);
}
throw $ex;
}

return $this->formerState;
}

/**
* Applies each promo code usage on its own transaction, recording it in $this->applied
* as soon as it commits.
*
* @param array $promo_codes_usage
* @param string $owner_email
* @param string|null $owner_company_name
* @throws \Exception
*/
private function applyUsages(array $promo_codes_usage, string $owner_email, ?string $owner_company_name): void
{
foreach ($promo_codes_usage as $promo_code_value => $info) {

$this->tx_service->transaction(function () use ($owner_email, $owner_company_name, $promo_code_value, $info) {
Expand Down Expand Up @@ -859,45 +901,62 @@ public function run(array $formerState): array
}, 30);

});
// mark a done
$promo_codes_usage[$promo_code_value]['redeem'] = true;
// mark as done: visible to undo() from now on
$this->applied[] = ['code' => $promo_code_value, 'qty' => intval($info['qty'])];
}
// update state
$this->formerState['promo_codes_usage'] = $promo_codes_usage;

return $this->formerState;
}

/**
* Releases the usages applied by this task instance. Idempotent: each entry is dropped
* from $applied once released, so the local release done by run() on failure plus a later
* Saga::abort() can not release the same usage twice. A release that fails is logged and
* kept in $applied, so it does not block the remaining codes and a later undo() retries
* only the entries that did not go through.
*/
public function undo()
{
Log::debug
(
sprintf
(
"ApplyPromoCodeTask::undo: compensating transaction former state %s payload %s",
json_encode($this->formerState),
"ApplyPromoCodeTask::undo: compensating transaction applied %s payload %s",
json_encode($this->applied),
json_encode($this->payload)
)
);

$promo_codes_usage = $this->formerState['promo_codes_usage'];
$owner_email = $this->payload['owner_email'];

foreach ($promo_codes_usage as $code => $info) {
Log::debug(sprintf("ApplyPromoCodeTask::undo undoing promo code %s info %s owner_email %s", $code, json_encode($info), $owner_email));
// a failing release must not block the release of the remaining codes:
// failed entries are kept so a later undo() can retry them
$remaining = [];

$this->tx_service->transaction(function () use ($code, $info, $owner_email) {
$promo_code = $this->promo_code_repository->getByValueExclusiveLock($this->summit, $code);
if (is_null($promo_code)) return;
foreach ($this->applied as $entry) {
$code = $entry['code'];
Log::debug(sprintf("ApplyPromoCodeTask::undo undoing promo code %s qty %s owner_email %s", $code, $entry['qty'], $owner_email));

if (!isset($info['redeem'])) return;
try {
// reload the code: instances from the failed transaction are detached
// once its rollback cleared the EntityManager
$this->tx_service->transaction(function () use ($code, $entry, $owner_email) {
$promo_code = $this->promo_code_repository->getByValueExclusiveLock($this->summit, $code);
if (is_null($promo_code)) return;

$this->lock_service->lock('promocode.' . $promo_code->getId() . '.usage.lock', function () use ($promo_code, $info, $owner_email) {
$promo_code->removeUsage(intval($info['qty']), $owner_email);
}, 30);
$this->lock_service->lock('promocode.' . $promo_code->getId() . '.usage.lock', function () use ($promo_code, $entry, $owner_email) {
$promo_code->removeUsage($entry['qty'], $owner_email);
}, 30);

});
});
} catch (\Throwable $ex) {
Log::warning(sprintf(
"ApplyPromoCodeTask::undo failed to release %s × %s: %s",
$code, $entry['qty'], $ex->getMessage()
));
$remaining[] = $entry;
}
}

$this->applied = $remaining;
}
}

Expand Down
134 changes: 134 additions & 0 deletions tests/SummitOrderServiceTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@
use models\main\ITagRepository;
use App\Models\Foundation\Summit\PromoCodes\PromoCodesConstants;
use models\summit\SpeakersRegistrationDiscountCode;
use models\summit\SummitRegistrationDiscountCode;
use models\summit\SummitRegistrationPromoCode;
use models\summit\PresentationSpeaker;
use models\summit\ISummitAttendeeRepository;
use models\summit\ISummitAttendeeTicketRepository;
use models\summit\ISummitRegistrationPromoCodeRepository;
Expand Down Expand Up @@ -762,6 +765,137 @@ public function testReserveRejectedByPromoCodeLeavesNoOrderAndRestoresStock()
$this->assertEquals($initial_quantity_sold, $reFetchedTicketType->getQuantitySold());
}

/**
* Each promo code is applied on its own transaction. When a later code of the same
* order is rejected, the usage already applied for the first one must be released
* together with the deleted order.
*/
public function testReserveRejectedOnSecondPromoCodeReleasesFirstCodeUsage()
{
$this->openRegistrationPeriod();
$service = App::make(ISummitOrderService::class);

$regular_code = new SummitRegistrationDiscountCode();
$regular_code->setCode('TEST_DC_' . uniqid());
$regular_code->setAmount(15);
$regular_code->setQuantityAvailable(10);
$regular_code->addAllowedTicketType(self::$default_ticket_type);
self::$summit->addPromoCode($regular_code);

// the buyer is not assigned to this speakers code: ApplyPromoCodeTask rejects it
$speakers_code = new SpeakersRegistrationDiscountCode();
$speakers_code->setCode('TEST_SPK_DC_' . uniqid());
$speakers_code->setType(PromoCodesConstants::SpeakerSummitRegistrationPromoCodeTypes[0]);
$speakers_code->setAmount(100);
$speakers_code->addAllowedTicketType(self::$default_ticket_type);
self::$summit->addPromoCode($speakers_code);

self::$em->persist(self::$summit);
self::$em->flush();

$regular_code_id = $regular_code->getId();
$ticket_type_id = self::$default_ticket_type->getId();
$initial_quantity_used = $regular_code->getQuantityUsed();

$payload = [
"owner_email" => sprintf('not-a-speaker-%s@test.com', uniqid()),
"owner_first_name" => "Not",
"owner_last_name" => "Speaker",
"owner_company" => "Pumant",
"tickets" => [
["type_id" => $ticket_type_id, "promo_code" => $regular_code->getCode()],
["type_id" => $ticket_type_id, "promo_code" => $speakers_code->getCode()],
],
];

try {
$service->reserve(null, self::$summit, $payload);
$this->fail('Expected ValidationException was not thrown');
} catch (ValidationException $ex) {
$this->assertStringContainsString('is not valid for', $ex->getMessage());
}

self::$em->clear();
$re_fetched_code = self::$em->find(SummitRegistrationPromoCode::class, $regular_code_id);
$this->assertEquals($initial_quantity_used, $re_fetched_code->getQuantityUsed());
}

/**
* A speaker whose reservation was rejected because of another ticket's promo code
* must keep their speakers code unredeemed, so they can reserve again right away.
*/
public function testReserveRejectedOnSecondPromoCodeLeavesSpeakersCodeRedeemable()
{
$this->openRegistrationPeriod();
$service = App::make(ISummitOrderService::class);

$buyer_email = self::$defaultSpeaker->getEmail();

$speakers_code = new SpeakersRegistrationDiscountCode();
$speakers_code->setCode('TEST_SPK_DC_' . uniqid());
$speakers_code->setType(PromoCodesConstants::SpeakerSummitRegistrationPromoCodeTypes[0]);
$speakers_code->setAmount(100);
$speakers_code->addAllowedTicketType(self::$default_ticket_type);
$speakers_code->assignSpeaker(self::$defaultSpeaker);
self::$summit->addPromoCode($speakers_code);

// expired: PreProcessReservationTask does not check the validity period, so it is
// ApplyPromoCodeTask that rejects it, after the speakers code was already applied
$now = new \DateTime('now', new \DateTimeZone('UTC'));
$expired_code = new SummitRegistrationDiscountCode();
$expired_code->setCode('TEST_DC_' . uniqid());
$expired_code->setAmount(15);
$expired_code->setQuantityAvailable(10);
$expired_code->setValidSinceDate((clone $now)->sub(new \DateInterval('P10D')));
$expired_code->setValidUntilDate((clone $now)->sub(new \DateInterval('P5D')));
$expired_code->addAllowedTicketType(self::$default_ticket_type);
self::$summit->addPromoCode($expired_code);

self::$em->persist(self::$summit);
self::$em->flush();

$speakers_code_id = $speakers_code->getId();
$speaker_id = self::$defaultSpeaker->getId();
$ticket_type_id = self::$default_ticket_type->getId();

$payload = [
"owner_email" => $buyer_email,
"owner_first_name" => "Sebastian",
"owner_last_name" => "Marcet",
"owner_company" => "Pumant",
"tickets" => [
["type_id" => $ticket_type_id, "promo_code" => $speakers_code->getCode()],
["type_id" => $ticket_type_id, "promo_code" => $expired_code->getCode()],
],
];

try {
$service->reserve(null, self::$summit, $payload);
$this->fail('Expected ValidationException was not thrown');
} catch (ValidationException $ex) {
$this->assertStringContainsString('is not a valid code', $ex->getMessage());
}

self::$em->clear();
$re_fetched_code = self::$em->find(SummitRegistrationPromoCode::class, $speakers_code_id);
$speaker = self::$em->find(PresentationSpeaker::class, $speaker_id);
$assignment = $re_fetched_code->getSpeakerAssignment($speaker);
$this->assertNotNull($assignment);
$this->assertNull($assignment->getRedeemedAt());

// the speaker can retry right away with their own code
self::$summit = self::$summit_repository->find(self::$summit->getId());
$retry_payload = $payload;
$retry_payload["tickets"] = [
["type_id" => $ticket_type_id, "promo_code" => $speakers_code->getCode()],
];
$order = $service->reserve(null, self::$summit, $retry_payload);
$this->assertNotNull($order);

// release the seat so the fixture teardown can delete the ticket type
$service->cancel(self::$summit, $order->getHash());
}

/**
* ReserveTicketsTask rejects orders mixing ticket types of different currencies.
*/
Expand Down
Loading
Loading