fix(registration): delete rejected reservation orders on saga rollback

app/Services/Model/Imp/SummitOrderService.php

@@ -153,7 +153,14 @@ private function markAsRan(AbstractTask $task)
     private function abort()
     {
         foreach (array_reverse($this->already_run_tasks) as $task) {
-            $task->undo();
+            // a failing compensation must neither mask the exception that aborted the saga
+            // nor prevent the remaining tasks (ticket stock, member quota) from being undone
+            try {
+                $task->undo();
+            } catch (\Throwable $ex) {
+                Log::error(sprintf("Saga::abort undo failed for %s: %s", get_class($task), $ex->getMessage()));
+                Log::error($ex);
+            }
         }
     }
 }
@@ -689,7 +696,21 @@ public function undo()
             return;
         }
 
-        $this->tx_service->transaction(function () use ($order) {
+        $order_id = $order->getId();
+
+        $this->tx_service->transaction(function () use ($order_id) {
+            // The order cached in formerState (and $this->summit) belong to the transaction that
+            // created them. When a later task fails, the root transaction rollback clears the
+            // EntityManager, so they are detached by now and can not be removed: reload them.
+            $order_repository = App::make(ISummitOrderRepository::class);
+            $order = $order_repository->getByIdExclusiveLock($order_id);
+            if (!$order instanceof SummitOrder) {
+                Log::warning(sprintf("ReserveOrderTask::undo: order id %s not found, nothing to compensate", $order_id));
+                return;
+            }
+            // re attach
+            $summit = App::make(ISummitRepository::class)->getById($this->summit->getId());
+
             Log::info(sprintf("ReserveOrderTask::undo: removing reserved order id %s number %s",
                 $order->getId(), $order->getNumber()));
 
@@ -703,8 +724,8 @@ public function undo()
                 }
             }
 
-            $this->summit->removeOrder($order);
-            App::make(ISummitOrderRepository::class)->delete($order);
+            $summit->removeOrder($order);
+            $order_repository->delete($order);
         });
     }
 }

tests/SummitOrderServiceTest.php

@@ -42,6 +42,8 @@
 use models\main\ICompanyRepository;
 use models\main\IMemberRepository;
 use models\main\ITagRepository;
+use App\Models\Foundation\Summit\PromoCodes\PromoCodesConstants;
+use models\summit\SpeakersRegistrationDiscountCode;
 use models\summit\ISummitAttendeeRepository;
 use models\summit\ISummitAttendeeTicketRepository;
 use models\summit\ISummitRegistrationPromoCodeRepository;
@@ -710,6 +712,56 @@ public function testReserveFailsWhenTicketTypeSoldOut()
         $this->assertEquals($initial_order_count, self::$summit->getOrders()->count());
     }
 
+    /**
+     * ApplyPromoCodeTask (last saga task) rejects a speakers promo code that is not
+     * assigned to the buyer. The saga compensation must then delete the order that
+     * ReserveOrderTask already committed and restore the ticket type stock, and the
+     * caller must see the original ValidationException, not a detached-entity error.
+     */
+    public function testReserveRejectedByPromoCodeLeavesNoOrderAndRestoresStock()
+    {
+        $this->openRegistrationPeriod();
+        $service = App::make(ISummitOrderService::class);
+
+        $promo_code = new SpeakersRegistrationDiscountCode();
+        $promo_code->setCode('TEST_SPK_DC_' . uniqid());
+        $promo_code->setType(PromoCodesConstants::SpeakerSummitRegistrationPromoCodeTypes[0]);
+        $promo_code->setAmount(100);
+        $promo_code->addAllowedTicketType(self::$default_ticket_type);
+        self::$summit->addPromoCode($promo_code);
+        self::$em->persist(self::$summit);
+        self::$em->flush();
+
+        $ticket_type_id = self::$default_ticket_type->getId();
+        $initial_quantity_sold = self::$default_ticket_type->getQuantitySold();
+        $initial_order_count = self::$summit->getOrders()->count();
+        $buyer_email = sprintf('not-a-speaker-%s@test.com', uniqid());
+
+        $payload = [
+            "owner_email" => $buyer_email,
+            "owner_first_name" => "Not",
+            "owner_last_name" => "Speaker",
+            "owner_company" => "Pumant",
+            "tickets" => [
+                ["type_id" => $ticket_type_id, "promo_code" => $promo_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();
+        self::$summit = self::$summit_repository->find(self::$summit->getId());
+        $this->assertEquals($initial_order_count, self::$summit->getOrders()->count());
+
+        $reFetchedTicketType = self::$em->find(SummitTicketType::class, $ticket_type_id);
+        $this->assertEquals($initial_quantity_sold, $reFetchedTicketType->getQuantitySold());
+    }
+
     /**
      * ReserveTicketsTask rejects orders mixing ticket types of different currencies.
      */

tests/Unit/Services/SagaCompensationTest.php

@@ -19,6 +19,7 @@
 use libs\utils\ITransactionService;
 use Mockery;
 use models\main\Member;
+use models\summit\ISummitRepository;
 use models\summit\Summit;
 use models\summit\SummitAttendee;
 use models\summit\SummitAttendeeTicket;
@@ -119,16 +120,24 @@ public function testUndoDeletesOrderAndDetachesTicketsFromAttendees(): void
         $order->shouldReceive('getTickets')->andReturn([$ticket1, $ticket2, $ticket3]);
 
         $summit = Mockery::mock(Summit::class);
+        $summit->shouldReceive('getId')->andReturn(77);
         $summit->shouldReceive('removeOrder')->once()->with($order);
 
         $owner = Mockery::mock(Member::class);
 
+        // undo() reloads the order and the summit: the instances cached by run() are
+        // detached once the failing task's root transaction cleared the EntityManager.
         $order_repo = Mockery::mock(ISummitOrderRepository::class);
+        $order_repo->shouldReceive('getByIdExclusiveLock')->once()->with(9001)->andReturn($order);
         $order_repo->shouldReceive('delete')->once()->with($order);
 
-        // Bind the repo into the container so App::make() inside undo() resolves it.
+        $summit_repo = Mockery::mock(ISummitRepository::class);
+        $summit_repo->shouldReceive('getById')->once()->with(77)->andReturn($summit);
+
+        // Bind the repos into the container so App::make() inside undo() resolves them.
         $container = \Illuminate\Support\Facades\Facade::getFacadeApplication();
         $container->instance(ISummitOrderRepository::class, $order_repo);
+        $container->instance(ISummitRepository::class, $summit_repo);
 
         $tx_service = Mockery::mock(ITransactionService::class);
         $tx_service->shouldReceive('transaction')->once()->andReturnUsing(function ($fn) {
@@ -179,6 +188,49 @@ public function undo() { /* never runs — it threw in run() */ }
         );
     }
 
+    /**
+     * A compensation that itself fails must neither mask the exception that aborted
+     * the saga nor stop the remaining compensations (ticket stock, member quota)
+     * from running.
+     */
+    public function testSagaAbortKeepsUndoingAndRethrowsOriginalWhenAnUndoFails(): void
+    {
+        $order_of_calls = [];
+
+        $first = new RecordingTask('first', $order_of_calls);
+        $broken_undo = new class extends AbstractTask {
+            public function run(array $formerState): array
+            {
+                return $formerState;
+            }
+            public function undo()
+            {
+                throw new \LogicException('undo failure');
+            }
+        };
+        $failing = new class extends AbstractTask {
+            public function run(array $formerState): array
+            {
+                throw new \RuntimeException('downstream failure');
+            }
+            public function undo() { /* never runs — it threw in run() */ }
+        };
+
+        $saga = Saga::start()
+            ->addTask($first)
+            ->addTask($broken_undo)
+            ->addTask($failing);
+
+        try {
+            $saga->run();
+            $this->fail('Expected saga to propagate the downstream exception');
+        } catch (\RuntimeException $ex) {
+            $this->assertSame('downstream failure', $ex->getMessage());
+        }
+
+        $this->assertSame(['run:first', 'undo:first'], $order_of_calls);
+    }
+
     /**
      * Construct a ReserveOrderTask with only the fields undo() needs. run() is
      * not exercised here, so most collaborators can be plain Mockery doubles.