From 0a4b3ba188b478d03d7bbbcc4eae70ac69396e78 Mon Sep 17 00:00:00 2001 From: romanetar Date: Mon, 2 Nov 2026 14:28:15 +0100 Subject: [PATCH 1/5] fix(attendees): stop member/attendee email conflicts from failing sync jobs and ticket assignment When a member's email changes to the email of a member-less attendee of the same summit, UpdateAttendeeInfo, MemberAssocSummitOrders and ticket assignment collided on the SummitAttendee unique keys. - updateAttendeesByMemberId: flush the rename of the member-less attendee before the member's attendee takes the email, so the merge commits. When the merge does not apply (email not verified, or the email belongs to another member) keep the attendee email, log a warning and still update the other fields. - Share getLinkableByMember between the job and assocSummitOrders so a member-less attendee is not linked to a member that already has an attendee in that summit. - ownerAssignTicket: throw ValidationException (412) instead of failing with a 500 in that state. - Add summit:repair-attendee-member-email-pairs to list the existing pairs and dispatch UpdateAttendeeInfo for the ones with a verified email. - Add regression tests and run them in the integration-tests matrix. --- .github/workflows/push.yml | 1 + .../RepairAttendeeMemberEmailPairsCommand.php | 121 +++++++++ app/Console/Kernel.php | 1 + .../ISummitAttendeeRepository.php | 9 + .../DoctrineSummitAttendeeRepository.php | 31 +++ app/Services/Model/AttendeeService.php | 89 ++++--- app/Services/Model/Imp/MemberService.php | 11 +- app/Services/Model/Imp/SummitOrderService.php | 27 ++ tests/AttendeeMemberEmailConflictTest.php | 232 ++++++++++++++++++ tests/oauth2/OAuth2SummitTicketsApiTest.php | 107 ++++++++ 10 files changed, 590 insertions(+), 39 deletions(-) create mode 100644 app/Console/Commands/RepairAttendeeMemberEmailPairsCommand.php create mode 100644 tests/AttendeeMemberEmailConflictTest.php diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index c4323141e3..b5e5755091 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -82,6 +82,7 @@ jobs: - { name: "SpeakerSubmitterPublishedFilter", filter: "tests/ProcessSpeakersEmailRequestJobTest.php tests/ProcessSubmittersEmailRequestJobTest.php tests/SpeakerRepositoryTest.php tests/SubmitterRepositoryTest.php tests/SpeakerServiceOriginalFilterTest.php tests/SubmitterServiceOriginalFilterTest.php" } - { name: "SpeakerBulkEmailChunkResume", filter: "tests/SpeakerServiceBulkSendChunkingTest.php tests/SpeakerServiceResumeSendEmailsTest.php tests/ProcessSpeakersEmailRequestJobResumeTest.php tests/ProcessSpeakersEmailRequestJobFailedHookTest.php" } - { name: "AttendeeBulkEmailChunkResume", filter: "tests/AttendeeServiceBulkSendChunkingTest.php tests/AttendeeServiceResumeSendEmailsTest.php tests/ProcessAttendeesEmailRequestJobResumeTest.php tests/ProcessAttendeesEmailRequestJobFailedHookTest.php tests/SummitAttendeeAnnouncementEmailTest.php tests/DoctrineSummitAttendeeRepositoryTest.php tests/SummitExtraQuestionsMemoizationTest.php" } + - { name: "AttendeeMemberEmailConflict", filter: "tests/AttendeeMemberEmailConflictTest.php" } - { name: "SummitJsonGenerator", filter: "tests/SummitJsonGeneratorCommandTest.php" } - { name: "Repositories", filter: "tests/Repositories/" } - { name: "Services", filter: "tests/Unit/Services/" } diff --git a/app/Console/Commands/RepairAttendeeMemberEmailPairsCommand.php b/app/Console/Commands/RepairAttendeeMemberEmailPairsCommand.php new file mode 100644 index 0000000000..cd98e4042d --- /dev/null +++ b/app/Console/Commands/RepairAttendeeMemberEmailPairsCommand.php @@ -0,0 +1,121 @@ +select(<<<'SQL' +SELECT + a.SummitID AS summit_id, + m.ID AS member_id, + m.EmailVerified AS email_verified, + a.ID AS attendee_id, + a.Email AS attendee_email, + b.ID AS memberless_attendee_id, + b.Email AS member_email +FROM SummitAttendee a +JOIN Member m ON m.ID = a.MemberID +JOIN SummitAttendee b + ON b.SummitID = a.SummitID AND b.MemberID IS NULL AND b.Email = m.Email +WHERE NOT (a.Email <=> m.Email) +ORDER BY a.SummitID, m.EmailVerified DESC, m.ID +SQL + ); + } + + /** + * @return int + */ + public function handle(): int + { + $pairs = $this->getPairs(); + + $groups = []; + foreach ($pairs as $pair) { + $key = sprintf("%s|%s", $pair->summit_id, $pair->email_verified); + $groups[$key] = ($groups[$key] ?? 0) + 1; + } + ksort($groups); + + $this->info(sprintf("%s pairs", count($pairs))); + $rows = []; + foreach ($groups as $key => $count) { + [$summit_id, $verified] = explode('|', $key); + $rows[] = [$summit_id, $verified ? 'yes' : 'no', $count]; + } + $this->table(['summit_id', 'member email verified', 'pairs'], $rows); + + if (!$this->option('dispatch')) { + $this->line('Dry run, nothing dispatched. Use --dispatch to enqueue UpdateAttendeeInfo for the verified ones.'); + return 0; + } + + // one job per member: it updates the member's attendees in every summit + $member_ids = []; + foreach ($pairs as $pair) { + if (!$pair->email_verified) continue; + $member_ids[(int)$pair->member_id] = true; + } + $member_ids = array_keys($member_ids); + + $limit = $this->option('limit'); + if (!is_null($limit) && $limit !== '') + $member_ids = array_slice($member_ids, 0, max(0, intval($limit))); + + foreach ($member_ids as $member_id) { + Log::info(sprintf("RepairAttendeeMemberEmailPairsCommand dispatching UpdateAttendeeInfo member id %s", $member_id)); + JobDispatcher::withDbFallback( + job: new UpdateAttendeeInfo($member_id), + logContext: ['member_id' => $member_id] + ); + } + + $this->info(sprintf("dispatched UpdateAttendeeInfo for %s members", count($member_ids))); + return 0; + } +} diff --git a/app/Console/Kernel.php b/app/Console/Kernel.php index b0d15b21ae..8c3ffe0e65 100644 --- a/app/Console/Kernel.php +++ b/app/Console/Kernel.php @@ -42,6 +42,7 @@ class Kernel extends ConsoleKernel \App\Console\Commands\SummitEmailFlowTypeSeederCommand::class, \App\Console\Commands\PresentationMaterialsCreateMUXAssetsCommand::class, \App\Console\Commands\RecalculateAttendeesStatusCommand::class, + \App\Console\Commands\RepairAttendeeMemberEmailPairsCommand::class, \App\Console\Commands\EnableMP4SupportAtMUXCommand::class, \App\Console\Commands\SummitMediaUploadMigratePrivateToPublicStorage::class, \App\Console\Commands\PresentationMediaUploadsRegenerateTemporalLinks::class, diff --git a/app/Models/Foundation/Summit/Repositories/ISummitAttendeeRepository.php b/app/Models/Foundation/Summit/Repositories/ISummitAttendeeRepository.php index e3d9237cac..7441faff08 100644 --- a/app/Models/Foundation/Summit/Repositories/ISummitAttendeeRepository.php +++ b/app/Models/Foundation/Summit/Repositories/ISummitAttendeeRepository.php @@ -73,6 +73,15 @@ public function getByEmail(string $email); */ public function getByEmailAndMemberNotSet(string $email); + /** + * Member-less attendees with the member's email that can be linked to the member, that is, the + * ones in summits where the member has no attendee yet (SummitAttendee_Member_Summit allows one). + * Logs a warning for each skipped summit. + * @param Member $member + * @return SummitAttendee[] + */ + public function getLinkableByMember(Member $member): array; + /** * @param Summit $summit * @param null|string $external_id diff --git a/app/Repositories/Summit/DoctrineSummitAttendeeRepository.php b/app/Repositories/Summit/DoctrineSummitAttendeeRepository.php index dc7dfe05d0..e0468ebba9 100644 --- a/app/Repositories/Summit/DoctrineSummitAttendeeRepository.php +++ b/app/Repositories/Summit/DoctrineSummitAttendeeRepository.php @@ -430,6 +430,37 @@ public function getByEmailAndMemberNotSet(string $email) return $query->getQuery()->getResult(); } + /** + * @param Member $member + * @return SummitAttendee[] + */ + public function getLinkableByMember(Member $member): array + { + $res = []; + foreach ($this->getByEmailAndMemberNotSet($member->getEmail()) as $attendee) { + if (!$attendee instanceof SummitAttendee) continue; + $summit = $attendee->getSummit(); + $member_attendee = $summit->getAttendeeByMember($member); + if (!is_null($member_attendee)) { + Log::warning + ( + sprintf + ( + "DoctrineSummitAttendeeRepository::getLinkableByMember member %s already has attendee %s in summit %s, skipping member-less attendee %s with email %s.", + $member->getId(), + $member_attendee->getId(), + $summit->getId(), + $attendee->getId(), + $attendee->getEmail() + ) + ); + continue; + } + $res[] = $attendee; + } + return $res; + } + /** * @param Summit $summit * @param string $email diff --git a/app/Services/Model/AttendeeService.php b/app/Services/Model/AttendeeService.php index b0dee3d57f..d34183f2e6 100644 --- a/app/Services/Model/AttendeeService.php +++ b/app/Services/Model/AttendeeService.php @@ -982,49 +982,77 @@ public function updateAttendeesByMemberId(int $member_id): void $summit = $attendee->getSummit(); if (!$summit instanceof Summit) continue; - // check if we have an attendee without member set and with same email - $attendee_without_member = $summit->getAttendeeByEmailAndMemberNotSet($email); + // check if the email is already taken by another attendee of the summit + $attendee_with_email = $summit->getAttendeeByEmail($email); + $update_email = true; - if (!is_null($attendee_without_member) && $member->isEmailVerified()) { + if (!is_null($attendee_with_email) && $attendee_with_email->getId() != $attendee->getId()) { - Log::debug - ( - sprintf + $attendee_without_member = $summit->getAttendeeByEmailAndMemberNotSet($email); + + if (!is_null($attendee_without_member) && $member->isEmailVerified()) { + + Log::debug ( - "AttendeeService::updateAttendeesByMemberId found attendee %s with email %s and member not set ( marking it for deletion ).", - $attendee_without_member->getId(), - $attendee_without_member->getEmail() - ) - ); + sprintf + ( + "AttendeeService::updateAttendeesByMemberId found attendee %s with email %s and member not set ( marking it for deletion ).", + $attendee_without_member->getId(), + $attendee_without_member->getEmail() + ) + ); - // mark the attendee to be deleted bc its dupe + // mark the attendee to be deleted bc its dupe - $attendees_2_delete[] = $attendee_without_member->getId(); + $attendees_2_delete[] = $attendee_without_member->getId(); - // overwrite email to avoid FK exception - $attendee_without_member->setEmail($attendee_without_member->getEmail() . '-deleted'); + // overwrite email to avoid unique key exception + $attendee_without_member->setEmail($attendee_without_member->getEmail() . '-deleted'); + // flush now: Doctrine runs the UPDATEs in the order the entities were loaded, + // and the member's attendee was loaded first, so it would take the email + // while this row still has it. + $this->attendee_repository->add($attendee_without_member, true); - // check if we have ticket and re-assign them - if ($attendee_without_member->hasTickets()) { - foreach ($attendee_without_member->getTickets() as $ticket) { - Log::debug - ( - sprintf + // check if we have ticket and re-assign them + if ($attendee_without_member->hasTickets()) { + foreach ($attendee_without_member->getTickets() as $ticket) { + Log::debug ( - "AttendeeService::updateAttendeesByMemberId re-assigning ticket %s from attendee %s to attendee %s", - $ticket->getId(), - $attendee_without_member->getId(), - $attendee->getId() - ) - ); - $attendee->addTicket($ticket); + sprintf + ( + "AttendeeService::updateAttendeesByMemberId re-assigning ticket %s from attendee %s to attendee %s", + $ticket->getId(), + $attendee_without_member->getId(), + $attendee->getId() + ) + ); + $attendee->addTicket($ticket); + } } + } else { + $update_email = false; + Log::warning + ( + sprintf + ( + "AttendeeService::updateAttendeesByMemberId can not set email %s to attendee %s of member %s in summit %s: it belongs to attendee %s (member %s, email verified %s), keeping attendee email.", + $email, + $attendee->getId(), + $member_id, + $summit->getId(), + $attendee_with_email->getId(), + $attendee_with_email->hasMember() ? $attendee_with_email->getMemberId() : 'none', + $member->isEmailVerified() ? 'yes' : 'no' + ) + ); } } $attendee->setFirstName($fname); $attendee->setSurname($lname); - $attendee->setEmail($email); + + if ($update_email) + $attendee->setEmail($email); // company logic @@ -1076,11 +1104,10 @@ public function updateAttendeesByMemberId(int $member_id): void $email = $member->getEmail(); $company_name = $member->getCompany(); - $attendees = $this->attendee_repository->getByEmailAndMemberNotSet($member->getEmail()); + $attendees = $this->attendee_repository->getLinkableByMember($member); if (!is_null($attendees)) { foreach ($attendees as $attendee) { - if (!$attendee instanceof SummitAttendee) continue; Log::debug ( diff --git a/app/Services/Model/Imp/MemberService.php b/app/Services/Model/Imp/MemberService.php index 60a57cff05..c8e2545097 100644 --- a/app/Services/Model/Imp/MemberService.php +++ b/app/Services/Model/Imp/MemberService.php @@ -41,7 +41,6 @@ use models\summit\ISpeakerRegistrationRequestRepository; use models\summit\ISummitAttendeeRepository; use models\summit\Presentation; -use models\summit\SummitAttendee; use models\summit\SummitOrder; use function Psy\debug; @@ -766,13 +765,9 @@ public function assocSummitOrders(int $member_id):void{ $this->tx_service->transaction(function() use($member_id){ // associate attendees/tickets $member = $this->member_repository->getById($member_id); - $attendees = $this->attendee_repository->getByEmailAndMemberNotSet($member->getEmail()); - if(!is_null($attendees)) { - foreach ($attendees as $attendee) { - if (!$attendee instanceof SummitAttendee) continue; - Log::debug(sprintf("MemberService::assocSummitOrders got attendee %s for member %s", $attendee->getId(), $member_id)); - $attendee->setMember($member); - } + foreach ($this->attendee_repository->getLinkableByMember($member) as $attendee) { + Log::debug(sprintf("MemberService::assocSummitOrders got attendee %s for member %s", $attendee->getId(), $member_id)); + $attendee->setMember($member); } }); } diff --git a/app/Services/Model/Imp/SummitOrderService.php b/app/Services/Model/Imp/SummitOrderService.php index 9f490a9551..eed233b6be 100644 --- a/app/Services/Model/Imp/SummitOrderService.php +++ b/app/Services/Model/Imp/SummitOrderService.php @@ -2262,6 +2262,33 @@ public function ownerAssignTicket(Member $current_user, int $order_id, int $tick $attendee = $summit->getAttendeeByMember($member); } + if (!is_null($member) && !is_null($attendee)) { + // the member can have only one attendee per summit + $member_attendee = $summit->getAttendeeByMember($member); + if (!is_null($member_attendee) && $member_attendee->getId() != $attendee->getId()) { + Log::warning + ( + sprintf + ( + "SummitOrderService::ownerAssignTicket email %s belongs to member %s that already has attendee %s in summit %s, attendee found by email is %s.", + $email, + $member->getId(), + $member_attendee->getId(), + $summit->getId(), + $attendee->getId() + ) + ); + throw new ValidationException + ( + sprintf + ( + "Email %s belongs to a member that already has an attendee in this summit.", + $email + ) + ); + } + } + if (is_null($attendee)) { // if attendee did not exists... create a new one Log::debug diff --git a/tests/AttendeeMemberEmailConflictTest.php b/tests/AttendeeMemberEmailConflictTest.php new file mode 100644 index 0000000000..60a2e328ad --- /dev/null +++ b/tests/AttendeeMemberEmailConflictTest.php @@ -0,0 +1,232 @@ +em(); + $this->old_email = self::$member->getEmail(); + + $member_attendee = self::$summit->getAttendeeByMember(self::$member); + $memberless_attendee = self::$summit->getAttendeeByEmailAndMemberNotSet(self::MemberlessEmail); + $this->assertNotNull($member_attendee); + $this->assertNotNull($memberless_attendee); + + $note = new SummitAttendeeNote('Note on the member-less attendee', $memberless_attendee); + $memberless_attendee->addNote($note); + $em->persist($note); + $em->flush(); + + $this->member_attendee_id = $member_attendee->getId(); + $this->memberless_attendee_id = $memberless_attendee->getId(); + $this->note_id = $note->getId(); + } + + protected function tearDown(): void + { + self::clearSummitTestData(); + self::clearMemberTestData(); + parent::tearDown(); + } + + private function em(): \Doctrine\ORM\EntityManagerInterface + { + $em = Registry::getManager(SilverstripeBaseModel::EntityManager); + if (!$em->isOpen()) { + $em = Registry::resetManager(SilverstripeBaseModel::EntityManager); + } + return $em; + } + + private function db(): Connection + { + return $this->em()->getConnection(); + } + + /** + * the member email changes to the member-less attendee's one and the first name changes too + */ + private function changeMemberEmail(bool $verified): void + { + $em = $this->em(); + $member = $em->getRepository(Member::class)->find(self::$member->getId()); + $member->setEmail(self::MemberlessEmail); + $member->setEmailVerified($verified); + $member->setFirstName(self::NewFirstName); + $em->flush(); + $em->clear(); + } + + private function attendeeRow(int $id): array|false + { + return $this->db()->fetchAssociative( + 'SELECT ID, Email, FirstName, MemberID FROM SummitAttendee WHERE ID = ?', + [$id] + ); + } + + private function ticketsCount(int $attendee_id): int + { + return (int)$this->db()->fetchOne('SELECT COUNT(ID) FROM SummitAttendeeTicket WHERE OwnerID = ?', [$attendee_id]); + } + + private function assertMemberAttendeeKeptOldEmail(): void + { + $row = $this->attendeeRow($this->member_attendee_id); + $this->assertEquals($this->old_email, $row['Email']); + $this->assertEquals(self::NewFirstName, $row['FirstName']); + $this->assertEquals(self::$member->getId(), $row['MemberID']); + } + + public function testUpdateAttendeesByMemberIdVerifiedEmailMergesMemberlessAttendee(): void + { + $this->changeMemberEmail(true); + + App::make(IAttendeeService::class)->updateAttendeesByMemberId(self::$member->getId()); + + $rows = $this->db()->fetchAllAssociative( + 'SELECT ID, MemberID, FirstName FROM SummitAttendee WHERE SummitID = ? AND Email = ?', + [self::$summit->getId(), self::MemberlessEmail] + ); + $this->assertCount(1, $rows); + $this->assertEquals($this->member_attendee_id, $rows[0]['ID']); + $this->assertEquals(self::$member->getId(), $rows[0]['MemberID']); + $this->assertEquals(self::NewFirstName, $rows[0]['FirstName']); + // its own 5 plus the member-less attendee's 1 + $this->assertEquals(6, $this->ticketsCount($this->member_attendee_id)); + + $this->assertFalse($this->attendeeRow($this->memberless_attendee_id)); + $this->assertEquals( + 0, + (int)$this->db()->fetchOne('SELECT COUNT(ID) FROM SummitAttendeeNote WHERE ID = ?', [$this->note_id]) + ); + } + + public function testUpdateAttendeesByMemberIdUnverifiedEmailKeepsBothAttendees(): void + { + $this->changeMemberEmail(false); + + App::make(IAttendeeService::class)->updateAttendeesByMemberId(self::$member->getId()); + + $this->assertMemberAttendeeKeptOldEmail(); + + $row = $this->attendeeRow($this->memberless_attendee_id); + $this->assertEquals(self::MemberlessEmail, $row['Email']); + $this->assertNull($row['MemberID']); + $this->assertEquals(1, $this->ticketsCount($this->memberless_attendee_id)); + } + + public function testUpdateAttendeesByMemberIdEmailOwnedByAnotherMemberLeavesThatAttendeeUnchanged(): void + { + // the attendee that has the email belongs to another member + $this->db()->executeStatement( + 'UPDATE SummitAttendee SET MemberID = ? WHERE ID = ?', + [self::$member2->getId(), $this->memberless_attendee_id] + ); + $before = $this->attendeeRow($this->memberless_attendee_id); + + $this->changeMemberEmail(true); + + App::make(IAttendeeService::class)->updateAttendeesByMemberId(self::$member->getId()); + + $this->assertMemberAttendeeKeptOldEmail(); + $this->assertEquals($before, $this->attendeeRow($this->memberless_attendee_id)); + $this->assertEquals(1, $this->ticketsCount($this->memberless_attendee_id)); + } + + public function testAssocSummitOrdersDoesNotLinkMemberlessAttendeeToMemberWithAttendee(): void + { + $this->changeMemberEmail(true); + + App::make(IMemberService::class)->assocSummitOrders(self::$member->getId()); + + $row = $this->attendeeRow($this->memberless_attendee_id); + $this->assertNotFalse($row); + $this->assertNull($row['MemberID']); + $this->assertEquals($this->old_email, $this->attendeeRow($this->member_attendee_id)['Email']); + } + + public function testRepairCommandDispatchesOnlyVerifiedMembers(): void + { + $this->changeMemberEmail(true); + Bus::fake(); + + // dry run: lists the pair, dispatches nothing + Artisan::call('summit:repair-attendee-member-email-pairs'); + $this->assertStringContainsString('1 pairs', Artisan::output()); + Bus::assertNotDispatched(UpdateAttendeeInfo::class); + + Artisan::call('summit:repair-attendee-member-email-pairs', ['--dispatch' => true]); + $member_id = self::$member->getId(); + Bus::assertDispatched(UpdateAttendeeInfo::class, function (UpdateAttendeeInfo $job) use ($member_id) { + return (fn() => $this->member_id)->call($job) == $member_id; + }); + Bus::assertDispatchedTimes(UpdateAttendeeInfo::class, 1); + } + + public function testRepairCommandSkipsUnverifiedMembers(): void + { + $this->changeMemberEmail(false); + Bus::fake(); + + Artisan::call('summit:repair-attendee-member-email-pairs', ['--dispatch' => true]); + + $this->assertStringContainsString('1 pairs', Artisan::output()); + Bus::assertNotDispatched(UpdateAttendeeInfo::class); + } +} diff --git a/tests/oauth2/OAuth2SummitTicketsApiTest.php b/tests/oauth2/OAuth2SummitTicketsApiTest.php index 555617c4ad..04b119edfd 100644 --- a/tests/oauth2/OAuth2SummitTicketsApiTest.php +++ b/tests/oauth2/OAuth2SummitTicketsApiTest.php @@ -21,6 +21,7 @@ use Mockery; use models\summit\IPaymentConstants; use models\summit\PaymentGatewayProfileFactory; +use models\summit\SummitAttendee; /** * Class OAuth2SummitTicketsApiTest @@ -1542,6 +1543,112 @@ public function testAssignAttendee() return $ticket; } + /** + * member2 has its own attendee in the summit and its email changes to the email of the + * member-less attendee, so assigning a ticket to that email would link two attendees + * of the same summit to member2. + */ + private function assignTicketToEmailOfMemberOwningAnotherAttendee(bool $member_email_verified): void + { + $memberless_email = 'memberless@test.com'; + + $ticket = self::$summit_orders[0]->getFirstTicket(); + $order = $ticket->getOrder(); + $order->setOwner(self::$member); + self::$member->addSummitRegistrationOrder($order); + + $member2_attendee = new SummitAttendee(); + $member2_attendee->setMember(self::$member2); + $member2_attendee->setEmail(self::$member2->getEmail()); + $member2_attendee->setFirstName(self::$member2->getFirstName()); + $member2_attendee->setSurname(self::$member2->getLastName()); + self::$summit->addAttendee($member2_attendee); + + $member2_original_email = self::$member2->getEmail(); + $member2_original_verified = self::$member2->isEmailVerified(); + self::$member2->setEmail($memberless_email); + self::$member2->setEmailVerified($member_email_verified); + self::$em->persist($order); + self::$em->flush(); + + try { + $this->assertAssignAttendeeRejected($order, $ticket, $member2_attendee, $memberless_email); + } finally { + // Member.Email is unique and this class does not clear the members between tests; + // plain SQL because the entity manager is closed after a failed request + self::$em->getConnection()->executeStatement( + 'UPDATE Member SET Email = ?, EmailVerified = ? WHERE ID = ?', + [$member2_original_email, (int)$member2_original_verified, self::$member2->getId()] + ); + } + } + + private function assertAssignAttendeeRejected($order, $ticket, SummitAttendee $member2_attendee, string $memberless_email): void + { + + $memberless_attendee = self::$summit->getAttendeeByEmailAndMemberNotSet($memberless_email); + $this->assertNotNull($memberless_attendee); + $memberless_attendee_id = $memberless_attendee->getId(); + $db = self::$em->getConnection(); + $member2_attendee_email = $db->fetchOne('SELECT Email FROM SummitAttendee WHERE ID = ?', [$member2_attendee->getId()]); + $attendees_count = $db->fetchOne('SELECT COUNT(ID) FROM SummitAttendee WHERE SummitID = ?', [self::$summit->getId()]); + $tickets_count = $db->fetchOne('SELECT COUNT(ID) FROM SummitAttendeeTicket WHERE OwnerID = ?', [$memberless_attendee_id]); + + $response = $this->action( + "PUT", + "OAuth2SummitOrdersApiController@assignAttendee", + [ + 'order_id' => $order->getId(), + 'ticket_id' => $ticket->getId(), + ], + [], + [], + [], + [ + "HTTP_Authorization" => " Bearer " . $this->access_token, + "CONTENT_TYPE" => "application/json" + ], + json_encode([ + 'attendee_email' => $memberless_email, + 'attendee_first_name' => 'Assigned', + 'attendee_last_name' => 'Attendee', + ]) + ); + + $this->assertResponseStatus(412); + + // nothing changed + $row = $db->fetchAssociative( + 'SELECT Email, MemberID, FirstName FROM SummitAttendee WHERE ID = ?', + [$memberless_attendee_id] + ); + $this->assertEquals($memberless_email, $row['Email']); + $this->assertNull($row['MemberID']); + $this->assertNotEquals('Assigned', $row['FirstName']); + $this->assertEquals( + $tickets_count, + $db->fetchOne('SELECT COUNT(ID) FROM SummitAttendeeTicket WHERE OwnerID = ?', [$memberless_attendee_id]) + ); + $this->assertEquals( + $attendees_count, + $db->fetchOne('SELECT COUNT(ID) FROM SummitAttendee WHERE SummitID = ?', [self::$summit->getId()]) + ); + $this->assertEquals( + $member2_attendee_email, + $db->fetchOne('SELECT Email FROM SummitAttendee WHERE ID = ?', [$member2_attendee->getId()]) + ); + } + + public function testAssignAttendeeEmailOfMemberWithAnotherAttendeeVerified() + { + $this->assignTicketToEmailOfMemberOwningAnotherAttendee(true); + } + + public function testAssignAttendeeEmailOfMemberWithAnotherAttendeeNotVerified() + { + $this->assignTicketToEmailOfMemberOwningAnotherAttendee(false); + } + public function testRemoveAttendee() { $assigned_ticket = $this->testAssignAttendee(); From 90084f6a8e1d61869b334702a952c908262fd88c Mon Sep 17 00:00:00 2001 From: romanetar Date: Mon, 2 Nov 2026 14:44:47 +0100 Subject: [PATCH 2/5] fix(attendees): let the repair command resume after a failed member A limited run selected the same first members again if their job kept failing, so later pairs were never dispatched. Order the members by id and add --after-member-id, and print the last dispatched member id. --- .../RepairAttendeeMemberEmailPairsCommand.php | 12 +++++++++- tests/AttendeeMemberEmailConflictTest.php | 23 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/app/Console/Commands/RepairAttendeeMemberEmailPairsCommand.php b/app/Console/Commands/RepairAttendeeMemberEmailPairsCommand.php index cd98e4042d..dcc36a46e6 100644 --- a/app/Console/Commands/RepairAttendeeMemberEmailPairsCommand.php +++ b/app/Console/Commands/RepairAttendeeMemberEmailPairsCommand.php @@ -37,7 +37,7 @@ class RepairAttendeeMemberEmailPairsCommand extends Command /** * @var string */ - protected $signature = 'summit:repair-attendee-member-email-pairs {--dispatch : enqueue UpdateAttendeeInfo for each member of a pair with a verified email} {--limit= : dispatch at most this many members}'; + protected $signature = 'summit:repair-attendee-member-email-pairs {--dispatch : enqueue UpdateAttendeeInfo for each member of a pair with a verified email} {--limit= : dispatch at most this many members} {--after-member-id= : only dispatch members with an id greater than this one, to resume after the last dispatched member}'; /** * @var string @@ -102,6 +102,14 @@ public function handle(): int $member_ids[(int)$pair->member_id] = true; } $member_ids = array_keys($member_ids); + // stable order, so --after-member-id can resume past a member whose job keeps failing + sort($member_ids); + + $after_member_id = $this->option('after-member-id'); + if (!is_null($after_member_id) && $after_member_id !== '') { + $after_member_id = intval($after_member_id); + $member_ids = array_values(array_filter($member_ids, fn(int $id) => $id > $after_member_id)); + } $limit = $this->option('limit'); if (!is_null($limit) && $limit !== '') @@ -116,6 +124,8 @@ public function handle(): int } $this->info(sprintf("dispatched UpdateAttendeeInfo for %s members", count($member_ids))); + if (count($member_ids)) + $this->line(sprintf("last member id dispatched: %s (resume with --after-member-id=%s)", end($member_ids), end($member_ids))); return 0; } } diff --git a/tests/AttendeeMemberEmailConflictTest.php b/tests/AttendeeMemberEmailConflictTest.php index 60a2e328ad..4555e8ae7f 100644 --- a/tests/AttendeeMemberEmailConflictTest.php +++ b/tests/AttendeeMemberEmailConflictTest.php @@ -229,4 +229,27 @@ public function testRepairCommandSkipsUnverifiedMembers(): void $this->assertStringContainsString('1 pairs', Artisan::output()); Bus::assertNotDispatched(UpdateAttendeeInfo::class); } + + public function testRepairCommandResumesAfterMemberId(): void + { + $this->changeMemberEmail(true); + Bus::fake(); + + // the member is at or before the cursor: skipped + Artisan::call('summit:repair-attendee-member-email-pairs', [ + '--dispatch' => true, + '--after-member-id' => self::$member->getId(), + ]); + Bus::assertNotDispatched(UpdateAttendeeInfo::class); + + Artisan::call('summit:repair-attendee-member-email-pairs', [ + '--dispatch' => true, + '--after-member-id' => self::$member->getId() - 1, + ]); + Bus::assertDispatchedTimes(UpdateAttendeeInfo::class, 1); + $this->assertStringContainsString( + sprintf('--after-member-id=%s', self::$member->getId()), + Artisan::output() + ); + } } From 8fc66e1e3b7eaba9e82bcc382dff157ab0cc54c6 Mon Sep 17 00:00:00 2001 From: romanetar Date: Mon, 2 Nov 2026 14:44:48 +0100 Subject: [PATCH 3/5] fix(attendees): list the pair ids in the repair command report The dry run only showed counts. List the summit, member, attendee and member-less attendee ids of each pair, without emails. --- .../RepairAttendeeMemberEmailPairsCommand.php | 17 ++++++++++++++--- tests/AttendeeMemberEmailConflictTest.php | 12 ++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/app/Console/Commands/RepairAttendeeMemberEmailPairsCommand.php b/app/Console/Commands/RepairAttendeeMemberEmailPairsCommand.php index dcc36a46e6..b9f7b8ec64 100644 --- a/app/Console/Commands/RepairAttendeeMemberEmailPairsCommand.php +++ b/app/Console/Commands/RepairAttendeeMemberEmailPairsCommand.php @@ -55,9 +55,7 @@ private function getPairs(): array m.ID AS member_id, m.EmailVerified AS email_verified, a.ID AS attendee_id, - a.Email AS attendee_email, - b.ID AS memberless_attendee_id, - b.Email AS member_email + b.ID AS memberless_attendee_id FROM SummitAttendee a JOIN Member m ON m.ID = a.MemberID JOIN SummitAttendee b @@ -90,6 +88,19 @@ public function handle(): int } $this->table(['summit_id', 'member email verified', 'pairs'], $rows); + // ids only, no emails: enough to inspect the attendees and to follow up on the unverified ones + $detail = []; + foreach ($pairs as $pair) { + $detail[] = [ + $pair->summit_id, + $pair->member_id, + $pair->email_verified ? 'yes' : 'no', + $pair->attendee_id, + $pair->memberless_attendee_id, + ]; + } + $this->table(['summit_id', 'member_id', 'member email verified', 'attendee_id', 'member-less attendee_id'], $detail); + if (!$this->option('dispatch')) { $this->line('Dry run, nothing dispatched. Use --dispatch to enqueue UpdateAttendeeInfo for the verified ones.'); return 0; diff --git a/tests/AttendeeMemberEmailConflictTest.php b/tests/AttendeeMemberEmailConflictTest.php index 4555e8ae7f..d824e9cb12 100644 --- a/tests/AttendeeMemberEmailConflictTest.php +++ b/tests/AttendeeMemberEmailConflictTest.php @@ -230,6 +230,18 @@ public function testRepairCommandSkipsUnverifiedMembers(): void Bus::assertNotDispatched(UpdateAttendeeInfo::class); } + public function testRepairCommandListsPairIdentifiers(): void + { + $this->changeMemberEmail(false); + + Artisan::call('summit:repair-attendee-member-email-pairs'); + $output = Artisan::output(); + + $this->assertStringContainsString((string)$this->member_attendee_id, $output); + $this->assertStringContainsString((string)$this->memberless_attendee_id, $output); + $this->assertStringNotContainsString(self::MemberlessEmail, $output); + } + public function testRepairCommandResumesAfterMemberId(): void { $this->changeMemberEmail(true); From 04fbe0d9d47b71b1158171aae37807249a669a52 Mon Sep 17 00:00:00 2001 From: romanetar Date: Mon, 2 Nov 2026 14:44:48 +0100 Subject: [PATCH 4/5] fix(attendees): use a bounded temporary email for the merged duplicate email + "-deleted" can exceed SummitAttendee.Email (varchar 100). Use a value that is unique per attendee and always fits. --- app/Services/Model/AttendeeService.php | 5 +++-- tests/AttendeeMemberEmailConflictTest.php | 23 +++++++++++++++++++++-- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/app/Services/Model/AttendeeService.php b/app/Services/Model/AttendeeService.php index d34183f2e6..4d19aa1118 100644 --- a/app/Services/Model/AttendeeService.php +++ b/app/Services/Model/AttendeeService.php @@ -1006,8 +1006,9 @@ public function updateAttendeesByMemberId(int $member_id): void $attendees_2_delete[] = $attendee_without_member->getId(); - // overwrite email to avoid unique key exception - $attendee_without_member->setEmail($attendee_without_member->getEmail() . '-deleted'); + // overwrite email to avoid unique key exception: unique per attendee and + // always fits SummitAttendee.Email (varchar 100), unlike email + suffix + $attendee_without_member->setEmail(sprintf("%s-deleted", $attendee_without_member->getId())); // flush now: Doctrine runs the UPDATEs in the order the entities were loaded, // and the member's attendee was loaded first, so it would take the email // while this row still has it. diff --git a/tests/AttendeeMemberEmailConflictTest.php b/tests/AttendeeMemberEmailConflictTest.php index d824e9cb12..79185d8e05 100644 --- a/tests/AttendeeMemberEmailConflictTest.php +++ b/tests/AttendeeMemberEmailConflictTest.php @@ -101,11 +101,11 @@ private function db(): Connection /** * the member email changes to the member-less attendee's one and the first name changes too */ - private function changeMemberEmail(bool $verified): void + private function changeMemberEmail(bool $verified, string $email = self::MemberlessEmail): void { $em = $this->em(); $member = $em->getRepository(Member::class)->find(self::$member->getId()); - $member->setEmail(self::MemberlessEmail); + $member->setEmail($email); $member->setEmailVerified($verified); $member->setFirstName(self::NewFirstName); $em->flush(); @@ -264,4 +264,23 @@ public function testRepairCommandResumesAfterMemberId(): void Artisan::output() ); } + + public function testUpdateAttendeesByMemberIdMergesMemberlessAttendeeWithEmailNearColumnLimit(): void + { + // SummitAttendee.Email is varchar(100): email + "-deleted" would not fit + $long_email = str_repeat('a', 98 - strlen('@test.com')) . '@test.com'; + $this->assertEquals(98, strlen($long_email)); + $this->db()->executeStatement( + 'UPDATE SummitAttendee SET Email = ? WHERE ID = ?', + [$long_email, $this->memberless_attendee_id] + ); + $this->changeMemberEmail(true, $long_email); + + App::make(IAttendeeService::class)->updateAttendeesByMemberId(self::$member->getId()); + + $this->assertFalse($this->attendeeRow($this->memberless_attendee_id)); + $row = $this->attendeeRow($this->member_attendee_id); + $this->assertEquals($long_email, $row['Email']); + $this->assertEquals(6, $this->ticketsCount($this->member_attendee_id)); + } } From 9c0884878d856e105817f87b1c5478e77418cf97 Mon Sep 17 00:00:00 2001 From: romanetar Date: Mon, 2 Nov 2026 14:44:48 +0100 Subject: [PATCH 5/5] fix(attendees): keep raw emails out of the new warning logs The ids already identify the conflict. --- app/Repositories/Summit/DoctrineSummitAttendeeRepository.php | 5 ++--- app/Services/Model/AttendeeService.php | 3 +-- app/Services/Model/Imp/SummitOrderService.php | 3 +-- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/app/Repositories/Summit/DoctrineSummitAttendeeRepository.php b/app/Repositories/Summit/DoctrineSummitAttendeeRepository.php index e0468ebba9..1b876a1689 100644 --- a/app/Repositories/Summit/DoctrineSummitAttendeeRepository.php +++ b/app/Repositories/Summit/DoctrineSummitAttendeeRepository.php @@ -446,12 +446,11 @@ public function getLinkableByMember(Member $member): array ( sprintf ( - "DoctrineSummitAttendeeRepository::getLinkableByMember member %s already has attendee %s in summit %s, skipping member-less attendee %s with email %s.", + "DoctrineSummitAttendeeRepository::getLinkableByMember member %s already has attendee %s in summit %s, skipping member-less attendee %s.", $member->getId(), $member_attendee->getId(), $summit->getId(), - $attendee->getId(), - $attendee->getEmail() + $attendee->getId() ) ); continue; diff --git a/app/Services/Model/AttendeeService.php b/app/Services/Model/AttendeeService.php index 4d19aa1118..b54175e79e 100644 --- a/app/Services/Model/AttendeeService.php +++ b/app/Services/Model/AttendeeService.php @@ -1036,8 +1036,7 @@ public function updateAttendeesByMemberId(int $member_id): void ( sprintf ( - "AttendeeService::updateAttendeesByMemberId can not set email %s to attendee %s of member %s in summit %s: it belongs to attendee %s (member %s, email verified %s), keeping attendee email.", - $email, + "AttendeeService::updateAttendeesByMemberId can not update the email of attendee %s of member %s in summit %s: the new email belongs to attendee %s (member %s, email verified %s), keeping attendee email.", $attendee->getId(), $member_id, $summit->getId(), diff --git a/app/Services/Model/Imp/SummitOrderService.php b/app/Services/Model/Imp/SummitOrderService.php index eed233b6be..11e5ae1d3f 100644 --- a/app/Services/Model/Imp/SummitOrderService.php +++ b/app/Services/Model/Imp/SummitOrderService.php @@ -2270,8 +2270,7 @@ public function ownerAssignTicket(Member $current_user, int $order_id, int $tick ( sprintf ( - "SummitOrderService::ownerAssignTicket email %s belongs to member %s that already has attendee %s in summit %s, attendee found by email is %s.", - $email, + "SummitOrderService::ownerAssignTicket the email belongs to member %s that already has attendee %s in summit %s, attendee found by email is %s.", $member->getId(), $member_attendee->getId(), $summit->getId(),