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,25 @@ 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 (!is_null($member)) {
+                    $old_attendee = $this->attendee_repository->getBySummitAndMember($summit, $member);
+
+                    if (!is_null($old_attendee))
+                        throw new ValidationException
+                        (
+                            sprintf
+                            (
+                                "attendee already exist for summit id %s and member id %s",
+                                $summit->getId(),
+                                $member->getIdentifier()
+                            )
+                        );
+                }
             }
 
             if (!empty($email)) {
@@ -301,6 +320,17 @@ 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 (!is_null($member)) {
+                    $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()));
+                }
             }
 
             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,25 @@
  * 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\exceptions\ValidationException;
 use models\summit\Summit;
+use models\summit\SummitAttendee;
 use models\summit\SummitAttendeeBadge;
 use models\summit\SummitAttendeeTicket;
+use Tests\InsertMemberTestData;
+use Tests\InsertSummitTestData;
+use Tests\TestCase;
 /**
  * Class AttendeeServiceTest
  */
@@ -48,12 +58,93 @@ 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());
+    }
+
+    /**
+     * @see https://github.com/OpenStackweb/summit-api/pull/588#discussion_r3897380061
+     * member2 is already linked to a different attendee in this summit, whose stored email
+     * has drifted away from member2's current account email - so the sibling
+     * getBySummitAndEmail check (keyed on email) can't catch the collision. Resolving member2
+     * via the email fallback branch must still raise the same clean ValidationException the
+     * member_id branch raises, instead of letting the flush fail on the unique
+     * (MemberID, SummitID) index.
+     */
+    private function linkOtherAttendeeToMember2WithDriftedEmail(): void
+    {
+        $other_attendee = new SummitAttendee();
+        $other_attendee->setMember(self::$member2);
+        $other_attendee->setEmail('drifted-' . self::$member2->getEmail());
+        $other_attendee->setFirstName(self::$member2->getFirstName());
+        $other_attendee->setSurname(self::$member2->getLastName());
+
+        self::$summit->addAttendee($other_attendee);
+        self::$em->persist($other_attendee);
+        self::$em->flush();
+    }
+
+    public function testUpdateAttendeeEmailResolvingToAlreadyLinkedMemberThrowsValidationException()
+    {
+        $this->linkOtherAttendeeToMember2WithDriftedEmail();
+
+        $service = App::make(IAttendeeService::class);
+        $attendee = self::$summit->getAttendeeByMember(self::$defaultMember);
+        $this->assertNotNull($attendee);
+
+        $payload = [
+            'email' => self::$member2->getEmail(),
+        ];
+
+        $this->expectException(ValidationException::class);
+        $service->updateAttendee(self::$summit, $attendee->getId(), $payload);
+    }
+
+    public function testAddAttendeeEmailResolvingToAlreadyLinkedMemberThrowsValidationException()
+    {
+        $this->linkOtherAttendeeToMember2WithDriftedEmail();
+
+        $service = App::make(IAttendeeService::class);
+
+        $payload = [
+            'email' => self::$member2->getEmail(),
+        ];
+
+        $this->expectException(ValidationException::class);
+        $service->addAttendee(self::$summit, $payload);
+    }
+
     public function testSendAllAttendeeTickets() {
 
         $service = App::make(IAttendeeService::class);
@@ -91,6 +182,8 @@ public function testSendRegistrationIncompleteReminderByAttendeeIds() {
 
     public function testReassignAttendeeTicketRegeneratesBadgeQRCode(){
 
+        $this->ensureTicketRevocationEmailTemplateSeeded();
+
         $attendee = self::$summit->getAttendeeByMember(self::$defaultMember);
         $this->assertNotNull($attendee);
         $ticket = $attendee->getTickets()->first();
@@ -130,6 +223,8 @@ public function testReassignAttendeeTicketRegeneratesBadgeQRCode(){
 
     public function testReassignAttendeeTicketByMemberRegeneratesBadgeQRCode(){
 
+        $this->ensureTicketRevocationEmailTemplateSeeded();
+
         $attendee = self::$summit->getAttendeeByMember(self::$defaultMember);
         $this->assertNotNull($attendee);
         $ticket = $attendee->getTickets()->first();
@@ -160,6 +255,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 +335,4 @@ private function assertBadgeQRRegeneratedForNewOwner(
         $this->assertEquals($new_owner_fullname, $decoded['owner_fullname']);
         $this->assertNotEquals($previous_owner_email, $decoded['owner_email']);
     }
-}
+}