fix(tickets): adjust ticket type sold counters when a ticket's type is changed

app/Models/Foundation/Summit/Registration/Attendees/SummitAttendee.php

@@ -528,6 +528,17 @@ public function sendRevocationTicketEmail(SummitAttendeeTicket $ticket)
         if (!$ticket->hasOwner()) return;
 
         if ($ticket->getOwner()->getId() != $this->getId()) return;
+        $this->dispatchRevocationTicketEmail($ticket);
+    }
+
+    /**
+     * Enqueues the revocation email without checking the current ticket owner: use it once a
+     * reassignment is committed and $this is no longer the owner (the queue is not transactional,
+     * so enqueueing inside the transaction would notify the former owner even on rollback).
+     * @param SummitAttendeeTicket $ticket
+     */
+    public function dispatchRevocationTicketEmail(SummitAttendeeTicket $ticket): void
+    {
         $email = $this->getEmail();
         $key = md5($email);
         if (Cache::add(sprintf("%s_revoke_ticket", $key), true, 600)) {

app/Services/Model/Imp/SummitOrderService.php

@@ -3953,7 +3953,7 @@ public function updateTicketById(Member $current_user, int $ticket_id, array $pa
      */
     public function updateTicket(Summit $summit, int $order_id, int $ticket_id, array $payload): SummitAttendeeTicket
     {
-        list($ticket, $shouldSendInvitationEmail) = $this->tx_service->transaction(function () use ($summit, $order_id, $ticket_id, $payload) {
+        list($ticket, $shouldSendInvitationEmail, $revoked_owner) = $this->tx_service->transaction(function () use ($summit, $order_id, $ticket_id, $payload) {
             // lock and get the order
             $order = $this->order_repository->getByIdExclusiveLock($order_id);
 
@@ -4026,9 +4026,12 @@ public function updateTicket(Summit $summit, int $order_id, int $ticket_id, arra
             }
 
             $shouldSendInvitationEmail = false;
+            $revoked_owner = null;
             // we are doing a reassignment from owner to new owner
             if (!is_null($owner) && !is_null($new_owner) && $owner->getId() !== $new_owner->getId()) {
-                $owner->sendRevocationTicketEmail($ticket);
+                // the queue is not transactional: the revocation email is enqueued after commit (see below),
+                // so a rollback further down (e.g. sold out ticket type) does not notify the former owner
+                $revoked_owner = $owner;
                 $owner->removeTicket($ticket);
                 $owner->updateStatus();
             }
@@ -4057,8 +4060,31 @@ public function updateTicket(Summit $summit, int $order_id, int $ticket_id, arra
                 if (is_null($ticket_type))
                     throw new EntityNotFoundException("ticket type not found");
 
+                $old_ticket_type_id = $ticket->getTicketTypeId();
+
                 $ticket->upgradeTicketType($ticket_type);
 
+                // quantity_sold is a hand maintained counter: move the sale from the old type to the new one
+                if ($old_ticket_type_id !== $ticket_type->getId()) {
+                    // lock in ascending id order so two opposite moves can not deadlock
+                    $ids = [$old_ticket_type_id, $ticket_type->getId()];
+                    sort($ids);
+                    $locked_types = [];
+                    foreach ($ids as $id) {
+                        $locked_types[$id] = $this->ticket_type_repository->getByIdExclusiveLock($id, true);
+                    }
+
+                    // fails (and rolls back the whole update) if the new type has no seats left
+                    $locked_types[$ticket_type->getId()]->sell(1);
+
+                    try {
+                        $locked_types[$old_ticket_type_id]->restore(1);
+                    } catch (ValidationException $ex) {
+                        // old counter already out of sync (below the restored qty), do not block the admin
+                        Log::warning($ex);
+                    }
+                }
+
                 $shouldSendInvitationEmail = true;
             }
 
@@ -4074,9 +4100,13 @@ public function updateTicket(Summit $summit, int $order_id, int $ticket_id, arra
                 $ticket->setBadge($badge);
             }
 
-            return [$ticket, $shouldSendInvitationEmail];
+            return [$ticket, $shouldSendInvitationEmail, $revoked_owner];
         });
 
+        // the reassignment is committed: now it is safe to tell the former owner
+        if (!is_null($revoked_owner))
+            $revoked_owner->dispatchRevocationTicketEmail($ticket);
+
         if ($shouldSendInvitationEmail && $summit->isRegistrationSendTicketEmailAutomatically() && $ticket->hasOwner())
             $ticket->getOwner()->sendInvitationEmail($ticket);
 

tests/SummitOrderServiceTest.php

@@ -14,6 +14,7 @@
 
 use App\Jobs\Emails\Registration\Reminders\SummitOrderReminderEmail;
 use App\Jobs\Emails\Registration\Reminders\SummitTicketReminderEmail;
+use App\Jobs\Emails\RevocationTicketEmail;
 use App\Models\Foundation\ExtraQuestions\ExtraQuestionTypeConstants;
 use App\Models\Foundation\ExtraQuestions\ExtraQuestionTypeValue;
 use App\Models\Foundation\Main\IGroup;
@@ -1460,6 +1461,171 @@ public function testUpdateTicketBadgeTypeChangeDoesNotDuplicateBadge(){
         $this->assertEquals('NEW BADGE TYPE', $badge->getType()->getName());
     }
 
+    /**
+     * Resolves the real ticket of the default attendee (see the fixture note in
+     * testUpdateTicketReassignmentRegeneratesBadgeQRCode) and primes the sold counters:
+     * fixture tickets are created without SummitTicketType::sell(), so the old type is
+     * sold once (unless $sell_old_type is false, i.e. a counter already out of sync) to make a decrement observable.
+     * @return array [summit_id, order_id, ticket_id, old_type_id, new_type_id]
+     */
+    private function prepareTicketTypeChange(int $new_type_quantity_2_sell = 100, int $new_type_sold = 0, bool $sell_old_type = true): array
+    {
+        $attendee = self::$summit->getAttendeeByMember(self::$defaultMember);
+        $this->assertNotNull($attendee);
+        $badge_id = $attendee->getTickets()->first()->getBadge()->getId();
+
+        $summit_id = self::$summit->getId();
+
+        $real_ticket = EntityManager::getRepository(SummitAttendeeBadge::class)->find($badge_id)->getTicket();
+        $ticket_id   = $real_ticket->getId();
+        $order_id    = $real_ticket->getOrder()->getId();
+        $old_type    = $real_ticket->getTicketType();
+        $new_type    = self::$default_ticket_type_2;
+        $this->assertNotEquals($old_type->getId(), $new_type->getId());
+
+        if ($sell_old_type) $old_type->sell(1);
+        $new_type->setQuantity2Sell($new_type_quantity_2_sell);
+        if ($new_type_sold > 0) $new_type->sell($new_type_sold);
+        self::$em->persist($old_type);
+        self::$em->persist($new_type);
+        self::$em->flush();
+
+        $ids = [$summit_id, $order_id, $ticket_id, $old_type->getId(), $new_type->getId()];
+        EntityManager::clear();
+        return $ids;
+    }
+
+    public function testUpdateTicketTypeChangeMovesSoldCounter()
+    {
+        Queue::fake();
+        list($summit_id, $order_id, $ticket_id, $old_type_id, $new_type_id) = $this->prepareTicketTypeChange();
+
+        $summit = EntityManager::getRepository(Summit::class)->find($summit_id);
+        App::make(ISummitOrderService::class)
+            ->updateTicket($summit, $order_id, $ticket_id, ['ticket_type_id' => $new_type_id]);
+
+        EntityManager::clear();
+        $ticket = EntityManager::getRepository(SummitAttendeeTicket::class)->find($ticket_id);
+        $this->assertEquals($new_type_id, $ticket->getTicketTypeId());
+        $this->assertEquals(0, EntityManager::getRepository(SummitTicketType::class)->find($old_type_id)->getQuantitySold());
+        $this->assertEquals(1, EntityManager::getRepository(SummitTicketType::class)->find($new_type_id)->getQuantitySold());
+    }
+
+    public function testUpdateTicketTypeChangeWithOldCounterAlreadyAtZeroStillMovesTheTicket()
+    {
+        Queue::fake();
+        list($summit_id, $order_id, $ticket_id, $old_type_id, $new_type_id) = $this->prepareTicketTypeChange(100, 0, false);
+
+        $summit = EntityManager::getRepository(Summit::class)->find($summit_id);
+        App::make(ISummitOrderService::class)
+            ->updateTicket($summit, $order_id, $ticket_id, ['ticket_type_id' => $new_type_id]);
+
+        EntityManager::clear();
+        $ticket = EntityManager::getRepository(SummitAttendeeTicket::class)->find($ticket_id);
+        $this->assertEquals($new_type_id, $ticket->getTicketTypeId());
+        $this->assertEquals(0, EntityManager::getRepository(SummitTicketType::class)->find($old_type_id)->getQuantitySold());
+        $this->assertEquals(1, EntityManager::getRepository(SummitTicketType::class)->find($new_type_id)->getQuantitySold());
+    }
+
+    public function testUpdateTicketSameTypeLeavesSoldCounterUntouched()
+    {
+        Queue::fake();
+        list($summit_id, $order_id, $ticket_id, $old_type_id) = $this->prepareTicketTypeChange();
+
+        $summit = EntityManager::getRepository(Summit::class)->find($summit_id);
+        App::make(ISummitOrderService::class)
+            ->updateTicket($summit, $order_id, $ticket_id, ['ticket_type_id' => $old_type_id]);
+
+        EntityManager::clear();
+        $this->assertEquals(1, EntityManager::getRepository(SummitTicketType::class)->find($old_type_id)->getQuantitySold());
+    }
+
+    public function testUpdateTicketToSoldOutTypeFailsAndLeavesCountersUntouched()
+    {
+        Queue::fake();
+        list($summit_id, $order_id, $ticket_id, $old_type_id, $new_type_id) = $this->prepareTicketTypeChange(1, 1);
+
+        $summit = EntityManager::getRepository(Summit::class)->find($summit_id);
+        try {
+            App::make(ISummitOrderService::class)
+                ->updateTicket($summit, $order_id, $ticket_id, ['ticket_type_id' => $new_type_id]);
+            $this->fail('Moving a ticket to a sold out ticket type should be rejected.');
+        } catch (ValidationException $ex) {
+            // expected
+        }
+
+        EntityManager::clear();
+        $ticket = EntityManager::getRepository(SummitAttendeeTicket::class)->find($ticket_id);
+        $this->assertEquals($old_type_id, $ticket->getTicketTypeId());
+        $this->assertEquals(1, EntityManager::getRepository(SummitTicketType::class)->find($old_type_id)->getQuantitySold());
+        $this->assertEquals(1, EntityManager::getRepository(SummitTicketType::class)->find($new_type_id)->getQuantitySold());
+    }
+
+    /**
+     * The revocation email is deduplicated per attendee email with a 10 minutes cache key
+     * (SummitAttendee::dispatchRevocationTicketEmail); the test cache driver is not reset between
+     * tests, so the key must be cleared for the Queue assertions below to be meaningful.
+     */
+    private function forgetRevocationEmailDedup(string $email): void
+    {
+        Cache::forget(sprintf("%s_revoke_ticket", md5($email)));
+    }
+
+    public function testUpdateTicketReassignmentToSoldOutTypeDoesNotSendRevocationEmail()
+    {
+        Queue::fake();
+        $old_owner_email = self::$defaultMember->getEmail();
+        $new_owner_email = self::$member2->getEmail();
+        $this->forgetRevocationEmailDedup($old_owner_email);
+        list($summit_id, $order_id, $ticket_id, $old_type_id, $new_type_id) = $this->prepareTicketTypeChange(1, 1);
+
+        $summit = EntityManager::getRepository(Summit::class)->find($summit_id);
+        try {
+            App::make(ISummitOrderService::class)->updateTicket($summit, $order_id, $ticket_id, [
+                'attendee_email' => $new_owner_email,
+                'ticket_type_id' => $new_type_id,
+            ]);
+            $this->fail('Moving a ticket to a sold out ticket type should be rejected.');
+        } catch (ValidationException $ex) {
+            // expected
+        }
+
+        // the whole update rolled back: the former owner must not be told the ticket was revoked
+        Queue::assertNotPushed(RevocationTicketEmail::class);
+
+        EntityManager::clear();
+        $ticket = EntityManager::getRepository(SummitAttendeeTicket::class)->find($ticket_id);
+        $this->assertEquals($old_type_id, $ticket->getTicketTypeId());
+        $this->assertEquals($old_owner_email, $ticket->getOwner()->getEmail());
+    }
+
+    public function testUpdateTicketReassignmentWithTypeChangeSendsRevocationEmailAfterCommit()
+    {
+        Queue::fake();
+        $old_owner_email = self::$defaultMember->getEmail();
+        $new_owner_email = self::$member2->getEmail();
+        $this->forgetRevocationEmailDedup($old_owner_email);
+        list($summit_id, $order_id, $ticket_id, , $new_type_id) = $this->prepareTicketTypeChange();
+
+        $summit = EntityManager::getRepository(Summit::class)->find($summit_id);
+        App::make(ISummitOrderService::class)->updateTicket($summit, $order_id, $ticket_id, [
+            'attendee_email' => $new_owner_email,
+            'ticket_type_id' => $new_type_id,
+        ]);
+
+        // the email must go to the former owner even though, once committed, the ticket belongs to the new one
+        Queue::assertPushed(RevocationTicketEmail::class, function (RevocationTicketEmail $job) use ($old_owner_email) {
+            $prop = new \ReflectionProperty(\App\Jobs\Emails\AbstractEmailJob::class, 'to_email');
+            $prop->setAccessible(true);
+            return $prop->getValue($job) === $old_owner_email;
+        });
+
+        EntityManager::clear();
+        $ticket = EntityManager::getRepository(SummitAttendeeTicket::class)->find($ticket_id);
+        $this->assertEquals($new_type_id, $ticket->getTicketTypeId());
+        $this->assertEquals($new_owner_email, $ticket->getOwner()->getEmail());
+    }
+
     public function testAddTicketsCommitsAcrossNestedTransaction()
     {
         $service = App::make(ISummitOrderService::class);