Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
romanetar wants to merge 6 commits into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e59e1d9
fix(attendees): preserve member link on self-service ticket update wi…
romanetar Aug 18, 2026
58679f8
test(attendees): move AttendeeServiceTest under Unit/Services so CI r…
romanetar Aug 20, 2026
232f043
fix(attendees): resolve email-to-member link in the service layer, no…
romanetar Aug 20, 2026
e03e6da
test(attendees): cover member-link clearing/preservation on ticket up…
romanetar Aug 20, 2026
2591144
test(attendees): fix 3 CI failures surfaced by running AttendeeServic…
romanetar Aug 20, 2026
3b60305
test(attendees): seed the other two email templates the reassignment …
romanetar Aug 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion app/Models/Foundation/Summit/Factories/SummitAttendeeFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'])) {
Comment thread
romanetar marked this conversation as resolved.
// 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();
Comment thread
romanetar marked this conversation as resolved.
}
// else: no email/member reassignment was requested, leave the existing member link untouched
}

// manager setting
Expand Down
9 changes: 9 additions & 0 deletions app/Services/Model/AttendeeService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down Expand Up @@ -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));

@smarcet smarcet Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@romanetar A member resolved through this new email branch skips the duplicate-attendee guard that the sibling member_id branch enforces a few lines above (getBySummitAndMember + ValidationException, lines 305-307).

Concrete failure: member M is linked to attendee B, and B's stored email has drifted from M's current account email — a durable state, since nothing updates linked attendees' emails when a member's email changes (updateAttendeesByMemberId and assocSummitOrders only touch attendees with no member set). An admin then updates a different attendee A with M's current email: getBySummitAndEmail finds nothing (B holds the old address), M is resolved here, populate sets the link, and the flush hits the unique index SummitAttendee_Member_Summit (MemberID, SummitID) (Version20191229173636.php:36). That surfaces as an unhandled DB exception → JSON 500 (app/Exceptions/Handler.php:70-73), where the member_id branch yields a clean 412.

Suggested fix: after getByEmail resolves a member, run the same $old_attendee = getBySummitAndMember(...) check as the member_id branch and throw the same ValidationException. The new branch in addAttendee (line 219) has the same gap.

}

if (!empty($email)) {
Expand Down
87 changes: 84 additions & 3 deletions tests/AttendeeServiceTest.php → tests/Unit/Services/AttendeeServiceTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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");
Expand All @@ -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
*/
Expand Down Expand Up @@ -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() {
Comment thread
romanetar marked this conversation as resolved.

$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);
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -200,4 +281,4 @@ private function assertBadgeQRRegeneratedForNewOwner(
$this->assertEquals($new_owner_fullname, $decoded['owner_fullname']);
$this->assertNotEquals($previous_owner_email, $decoded['owner_email']);
}
}
}
180 changes: 180 additions & 0 deletions tests/oauth2/OAuth2SummitTicketsApiTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

@smarcet smarcet Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@romanetar This test fails in CI (OAuth2Tests job, run 32398352529) because its premise — the member's account email drifts while the access token still carries the old one — is unrealizable through an authenticated request, by design.

ProtectedApiTestCase::setUp bakes the member's original email into the mocked token claims (tests/ProtectedApiTestCase.php:372 → :189). On the PUT, ResourceServerContext::getCurrentUser(update_member_fields=true) runs syncMemberFields, which resets Member.email back to the claim email (app/Models/OAuth2/ResourceServerContext.php:427), flushed and committed before the controller runs (DoctrineTransactionService.php:150-151). By the time SummitOrderService::updateTicketById calls getByEmail($stale_email), the drift has been undone, the member resolves, and the link is preserved — so the assertNull at line 1719 fails. The production clear-on-unmatched-email branch itself is fine; the test just can't reach it as written.

Suggested fix: after drifting the member's email, sync the mocked claims too — self::$service->setUserEmail(self::$member->getEmail()); — so the auth-side re-sync becomes a no-op and getByEmail($stale_email) genuinely returns null. Alternatively, drift a member other than the authenticated one.

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();
Expand Down
Loading