fix(attendees): preserve member link on self-service ticket update without email

app/Models/Foundation/Summit/Factories/SummitAttendeeFactory.php

@@ -102,9 +102,12 @@ public static function populate
                 Log::debug(sprintf("SummitAttendeeFactory::populate setting member %s to attendee %s", $member->getId(), $member->getEmail()));
                 $attendee->setEmail($member->getEmail());
                 $attendee->setMember($member);
-            } else {
+            } else if (isset($payload['email']) && !empty($payload['email'])) {
+                // an email reassignment was explicitly requested and it does not match any known member account
+                Log::debug(sprintf("SummitAttendeeFactory::populate clearing member from attendee %s", $attendee->getId()));
                 $attendee->clearMember();
             }
+            // else: no email/member reassignment was requested, leave the existing member link untouched
         }
 
         // manager setting

app/Services/Model/AttendeeService.php

@@ -213,6 +213,10 @@ public function addAttendee(Summit $summit, array $data)
                         )
                     );
 
+            } else if (!empty($email)) {
+                // no member_id was given, but the email happens to belong to a known member account ...
+                // resolve it so the new attendee is linked to it
+                $member = $this->member_repository->getByEmail(trim($email));
             }
 
             if (!empty($email)) {
@@ -301,6 +305,11 @@ public function updateAttendee(Summit $summit, $attendee_id, array $payload)
                 $old_attendee = $this->attendee_repository->getBySummitAndMember($summit, $member);
                 if (!is_null($old_attendee) && $old_attendee->getId() != $attendee->getId())
                     throw new ValidationException(sprintf("Another attendee (%s) already exist for summit id %s and member id %s.", $old_attendee->getId(), $summit->getId(), $member->getIdentifier()));
+            } else if (!empty($email)) {
+                // no member_id was given, but the email happens to belong to a known member account ...
+                // resolve it so we don't clear a link that is actually still valid, or so an explicit
+                // email reassignment picks up the member it now belongs to
+                $member = $this->member_repository->getByEmail(trim($email));
             }
 
             if (!empty($email)) {

tests/AttendeeServiceTest.php → tests/Unit/Services/AttendeeServiceTest.php

@@ -1,4 +1,4 @@
-<?php namespace Tests;
+<?php namespace Tests\Unit\Services;
 /**
  * Copyright 2018 OpenStack Foundation
  * Licensed under the Apache License, Version 2.0 (the "License");
@@ -12,15 +12,23 @@
  * limitations under the License.
  **/
 
+use App\Jobs\Emails\InviteAttendeeTicketEditionMail;
+use App\Jobs\Emails\RevocationTicketEmail;
 use App\Jobs\Emails\SummitAttendeeAllTicketsEditionEmail;
 use App\Jobs\Emails\SummitAttendeeRegistrationIncompleteReminderEmail;
+use App\Jobs\Emails\SummitAttendeeTicketEmail;
 use App\Models\Foundation\Main\IGroup;
+use App\Models\Foundation\Summit\EmailFlows\SummitEmailEventFlowType;
+use App\Models\Foundation\Summit\EmailFlows\SummitEmailFlowType;
 use App\Services\Model\IAttendeeService;
 use Illuminate\Support\Facades\App;
 use LaravelDoctrine\ORM\Facades\EntityManager;
 use models\summit\Summit;
 use models\summit\SummitAttendeeBadge;
 use models\summit\SummitAttendeeTicket;
+use Tests\InsertMemberTestData;
+use Tests\InsertSummitTestData;
+use Tests\TestCase;
 /**
  * Class AttendeeServiceTest
  */
@@ -48,12 +56,41 @@ protected function tearDown(): void
 
     public function testRedeemPromoCodes(){
 
+        // Eventbrite isn't configured in CI, and updateRedeemedPromoCodes makes a real,
+        // unmocked network call with no error handling around it, so replace the API with
+        // a double that fails fast instead of hitting a third-party service from a test.
+        $eventbrite_api = \Mockery::mock(\services\apis\IEventbriteAPI::class);
+        $eventbrite_api->shouldReceive('getAttendees')
+            ->andThrow(new \Exception('Eventbrite API is not available in tests.'));
+        App::singleton(\services\apis\IEventbriteAPI::class, function () use ($eventbrite_api) {
+            return $eventbrite_api;
+        });
+
         $service = App::make(IAttendeeService::class);
         $repo   =  EntityManager::getRepository(\models\summit\Summit::class);
-        $summit = $repo->getById(24);
+        $summit = $repo->getById(self::$summit->getId());
+
+        $this->expectException(\Exception::class);
         $service->updateRedeemedPromoCodes($summit);
     }
 
+    public function testUpdateAttendeeEmailOnlyLinksExistingMemberAccount() {
+
+        $service = App::make(IAttendeeService::class);
+        $attendee = self::$summit->getAttendeeByMember(self::$defaultMember);
+        $this->assertNotNull($attendee);
+
+        // only email is submitted (no member_id), and it belongs to a known member account
+        $payload = [
+            'email' => self::$member2->getEmail(),
+        ];
+
+        $updated = $service->updateAttendee(self::$summit, $attendee->getId(), $payload);
+
+        $this->assertNotNull($updated->getMember());
+        $this->assertEquals(self::$member2->getId(), $updated->getMember()->getId());
+    }
+
     public function testSendAllAttendeeTickets() {
 
         $service = App::make(IAttendeeService::class);
@@ -91,6 +128,8 @@ public function testSendRegistrationIncompleteReminderByAttendeeIds() {
 
     public function testReassignAttendeeTicketRegeneratesBadgeQRCode(){
 
+        $this->ensureTicketRevocationEmailTemplateSeeded();
+
         $attendee = self::$summit->getAttendeeByMember(self::$defaultMember);
         $this->assertNotNull($attendee);
         $ticket = $attendee->getTickets()->first();
@@ -130,6 +169,8 @@ public function testReassignAttendeeTicketRegeneratesBadgeQRCode(){
 
     public function testReassignAttendeeTicketByMemberRegeneratesBadgeQRCode(){
 
+        $this->ensureTicketRevocationEmailTemplateSeeded();
+
         $attendee = self::$summit->getAttendeeByMember(self::$defaultMember);
         $this->assertNotNull($attendee);
         $ticket = $attendee->getTickets()->first();
@@ -160,6 +201,46 @@ public function testReassignAttendeeTicketByMemberRegeneratesBadgeQRCode(){
         );
     }
 
+    /**
+     * reassignAttendeeTicket/reassignAttendeeTicketByMember dispatch a RevocationTicketEmail
+     * to the previous owner and, depending on whether the new owner's profile is already
+     * complete, either a SummitAttendeeTicketEmail or an InviteAttendeeTicketEditionMail to
+     * the new one. Each of those job constructors requires a resolvable email template
+     * identifier. The seeder that normally provides this catalog (SummitEmailFlowTypeSeeder)
+     * never runs in CI, so seed the minimal rows here rather than relying on production data.
+     */
+    private function ensureTicketRevocationEmailTemplateSeeded(): void
+    {
+        $slugs = [
+            RevocationTicketEmail::EVENT_SLUG => RevocationTicketEmail::DEFAULT_TEMPLATE,
+            SummitAttendeeTicketEmail::EVENT_SLUG => SummitAttendeeTicketEmail::DEFAULT_TEMPLATE,
+            InviteAttendeeTicketEditionMail::EVENT_SLUG => InviteAttendeeTicketEditionMail::DEFAULT_TEMPLATE,
+        ];
+
+        $repository = EntityManager::getRepository(SummitEmailEventFlowType::class);
+        $flow = null;
+
+        foreach ($slugs as $slug => $default_template) {
+            if (!is_null($repository->findOneBy(['slug' => $slug]))) continue;
+
+            if (is_null($flow)) {
+                $flow = new SummitEmailFlowType();
+                $flow->setName('Registration');
+            }
+
+            $event_type = new SummitEmailEventFlowType();
+            $event_type->setName($slug);
+            $event_type->setSlug($slug);
+            $event_type->setDefaultEmailTemplate($default_template);
+            $flow->addFlowEventType($event_type);
+        }
+
+        if (!is_null($flow)) {
+            EntityManager::persist($flow);
+            EntityManager::flush();
+        }
+    }
+
     /**
      * The fixture (InsertSummitTestData) reuses one SummitAttendeeBadge PHP object
      * across several tickets, so only the LAST ticket it was attached to is the one
@@ -200,4 +281,4 @@ private function assertBadgeQRRegeneratedForNewOwner(
         $this->assertEquals($new_owner_fullname, $decoded['owner_fullname']);
         $this->assertNotEquals($previous_owner_email, $decoded['owner_email']);
     }
-}
+}

tests/oauth2/OAuth2SummitTicketsApiTest.php

@@ -1594,6 +1594,186 @@ public function testUpdateMyTicketById()
         $this->assertTrue(in_array($response->getStatusCode(), [201, 412]));
     }
 
+    public function testUpdateMyTicketByIdWithoutEmailPreservesMemberLink()
+    {
+        // ticket already owned by an attendee linked to the current member,
+        // mirroring a real self-service edit (e.g. answering extra questions)
+        $attendee = self::$summit->getAttendeeByMember(self::$defaultMember);
+        $this->assertNotNull($attendee);
+        $ticket = $attendee->getTickets()->first();
+        $this->assertNotNull($ticket);
+
+        $order = $ticket->getOrder();
+        $order->setOwner(self::$member);
+        self::$member->addSummitRegistrationOrder($order);
+        self::$em->persist($order);
+        self::$em->flush();
+
+        $summit_id = self::$summit->getId();
+        $member_id = self::$member->getId();
+
+        $params = [
+            'ticket_id' => $ticket->getId(),
+        ];
+
+        // no attendee_email in the payload, as the attendee app never sends one
+        $data = [
+            'attendee_company' => 'Regression Test Co',
+        ];
+
+        $headers = [
+            "HTTP_Authorization" => " Bearer " . $this->access_token,
+            "CONTENT_TYPE" => "application/json"
+        ];
+
+        $response = $this->action(
+            "PUT",
+            "OAuth2SummitOrdersApiController@updateMyTicketById",
+            $params,
+            [],
+            [],
+            [],
+            $headers,
+            json_encode($data)
+        );
+
+        $this->assertResponseStatus(201);
+
+        // force a fresh load from the DB, matching how a subsequent request behaves
+        \LaravelDoctrine\ORM\Facades\EntityManager::clear();
+
+        $summit = \LaravelDoctrine\ORM\Facades\EntityManager::getRepository(\models\summit\Summit::class)->find($summit_id);
+        $reloaded_attendee = $summit->getAttendeeByMemberId($member_id);
+        $this->assertNotNull(
+            $reloaded_attendee,
+            "attendee <-> member link must not be cleared by a self-service ticket update that does not touch attendee_email"
+        );
+        $this->assertEquals('Regression Test Co', $reloaded_attendee->getCompanyName());
+
+        // reproduces the reported symptom: GET attendees/me must not 404 right after the update
+        $me_response = $this->action(
+            "GET",
+            "OAuth2SummitAttendeesApiController@getOwnAttendee",
+            ['id' => $summit_id],
+            [],
+            [],
+            [],
+            $headers
+        );
+        $this->assertResponseStatus(200);
+    }
+
+    public function testUpdateMyTicketByIdWithUnmatchedEmailClearsMemberLink()
+    {
+        // attendee is linked to the current member, but the member's account email has since
+        // drifted away from the attendee's cached email (e.g. the member updated it elsewhere) ...
+        $attendee = self::$summit->getAttendeeByMember(self::$defaultMember);
+        $this->assertNotNull($attendee);
+        $ticket = $attendee->getTickets()->first();
+        $this->assertNotNull($ticket);
+        $stale_email = $attendee->getEmail();
+
+        $order = $ticket->getOrder();
+        $order->setOwner(self::$member);
+        self::$member->addSummitRegistrationOrder($order);
+        self::$member->setEmail('drifted-' . $stale_email);
+        self::$em->persist($order);
+        self::$em->persist(self::$member);
+        self::$em->flush();
+
+        $attendee_id = $attendee->getId();
+
+        $params = [
+            'ticket_id' => $ticket->getId(),
+        ];
+
+        // the attendee app re-sends the (now stale) email it last fetched, unchanged from the
+        // attendee's point of view, but it no longer resolves to any member account
+        $data = [
+            'attendee_email' => $stale_email,
+            'attendee_company' => 'Regression Test Co',
+        ];
+
+        $headers = [
+            "HTTP_Authorization" => " Bearer " . $this->access_token,
+            "CONTENT_TYPE" => "application/json"
+        ];
+
+        $response = $this->action(
+            "PUT",
+            "OAuth2SummitOrdersApiController@updateMyTicketById",
+            $params,
+            [],
+            [],
+            [],
+            $headers,
+            json_encode($data)
+        );
+
+        $this->assertResponseStatus(201);
+
+        \LaravelDoctrine\ORM\Facades\EntityManager::clear();
+
+        $reloaded_attendee = \LaravelDoctrine\ORM\Facades\EntityManager::getRepository(\models\summit\SummitAttendee::class)->find($attendee_id);
+        $this->assertNotNull($reloaded_attendee);
+        $this->assertNull(
+            $reloaded_attendee->getMember(),
+            "an explicit attendee_email that no longer resolves to any member account must still clear the link"
+        );
+    }
+
+    public function testUpdateTicketByHashWithoutEmailPreservesMemberLink()
+    {
+        // mirrors the self-service "no email in the payload" case, but through the public,
+        // hash-based edit link (updateTicketByHash never includes attendee_email at all)
+        $attendee = self::$summit->getAttendeeByMember(self::$defaultMember);
+        $this->assertNotNull($attendee);
+        $ticket = $attendee->getTickets()->first();
+        $this->assertNotNull($ticket);
+
+        $ticket->generateHash();
+        self::$em->persist($ticket);
+        self::$em->flush();
+
+        $hash = $ticket->getHash();
+        $member_id = self::$defaultMember->getId();
+
+        $params = [
+            'hash' => $hash,
+        ];
+
+        $data = [
+            'attendee_company' => 'Regression Test Co',
+        ];
+
+        $headers = [
+            "CONTENT_TYPE" => "application/json"
+        ];
+
+        $response = $this->action(
+            "PUT",
+            "OAuth2SummitOrdersApiController@updateTicketByHash",
+            $params,
+            [],
+            [],
+            [],
+            $headers,
+            json_encode($data)
+        );
+
+        $this->assertResponseStatus(201);
+
+        \LaravelDoctrine\ORM\Facades\EntityManager::clear();
+
+        $summit = \LaravelDoctrine\ORM\Facades\EntityManager::getRepository(\models\summit\Summit::class)->find(self::$summit->getId());
+        $reloaded_attendee = $summit->getAttendeeByMemberId($member_id);
+        $this->assertNotNull(
+            $reloaded_attendee,
+            "attendee <-> member link must not be cleared by a hash-based public ticket update that does not touch attendee_email"
+        );
+        $this->assertEquals('Regression Test Co', $reloaded_attendee->getCompanyName());
+    }
+
     public function testDelegateTicket()
     {
         $ticket = self::$summit_orders[0]->getFirstTicket();