From 68dc17423b3e2f03de5774d482ba136a19899e2c Mon Sep 17 00:00:00 2001 From: JpMaxMan Date: Thu, 2 Jul 2026 18:08:28 -0500 Subject: [PATCH 1/7] feat(registration): add order extra question answers to ticket CSV import Adds extra_question:{question name} column support to the ticket data import, template endpoint and OpenAPI docs. Answers are upserted through the existing ExtraQuestionAnswerHolder persistence path; unknown questions, order-scoped questions, disallowed questions, empty values and locked answers are logged and skipped without failing the row. List type questions accept value name/label/id ('|' separated for CheckBoxList) and store value ids. Co-Authored-By: Claude Fable 5 --- .../OAuth2SummitTicketApiController.php | 12 +- app/Services/Model/ISummitOrderService.php | 3 + app/Services/Model/Imp/SummitOrderService.php | 184 +++++++++++ tests/SummitOrderServiceTest.php | 299 ++++++++++++++++++ 4 files changed, 495 insertions(+), 3 deletions(-) diff --git a/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitTicketApiController.php b/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitTicketApiController.php index d04c74c8e..b77584a41 100644 --- a/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitTicketApiController.php +++ b/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitTicketApiController.php @@ -745,7 +745,7 @@ public function ingestExternalTicketData($summit_id) path: '/api/v1/summits/{id}/tickets/csv/template', operationId: 'getTicketImportTemplate', summary: 'Get ticket import template', - description: 'Returns a CSV template for importing ticket data', + description: 'Returns a CSV template for importing ticket data. Includes one column per summit badge feature name plus one extra_question:{question name} column per summit order extra question (Ticket/Both usage).', security: [['summit_tickets_oauth2' => [ SummitScopes::WriteSummitData, SummitScopes::WriteRegistrationData, @@ -788,7 +788,8 @@ public function getImportTicketDataTemplate($summit_id) * ticket_promo_code (optional) * badge_type_id (optional) * badge_type_name (optional) - * badge_features (optional) + * badge_features (optional, one col per badge feature name) + * extra_question:{question name} (optional, one col per order extra question - Ticket/Both usage) */ $summit = SummitFinderStrategyFactory::build($this->summit_repository, $this->getResourceServerContext())->find($summit_id); @@ -816,6 +817,11 @@ public function getImportTicketDataTemplate($summit_id) $row[$featuresType->getName()] = ''; } + // order extra questions for summit ( ticket / attendee scoped ones ) + foreach ($summit->getOrderExtraQuestionsByUsage(SummitOrderExtraQuestionTypeConstants::TicketQuestionUsage) as $question) { + $row[sprintf('%s%s', ISummitOrderService::ExtraQuestionColumnPrefix, $question->getName())] = ''; + } + $template = [ $row ]; @@ -835,7 +841,7 @@ public function getImportTicketDataTemplate($summit_id) path: '/api/v1/summits/{id}/tickets/csv', operationId: 'importTicketData', summary: 'Import ticket data from CSV', - description: 'Imports ticket data from a CSV file', + description: 'Imports ticket data from a CSV file. Supported columns: id, number, attendee_email, attendee_first_name, attendee_last_name, attendee_tags, attendee_company, attendee_company_id, ticket_type_name, ticket_type_id, promo_code_id, promo_code, ticket_promo_code, badge_type_id, badge_type_name, one column per badge feature name (1/0) and one extra_question:{question name} column per order extra question (Ticket/Both usage; for list type questions use the value name/label, "|" separated for multi value).', security: [['summit_tickets_oauth2' => [ SummitScopes::WriteSummitData, SummitScopes::WriteRegistrationData, diff --git a/app/Services/Model/ISummitOrderService.php b/app/Services/Model/ISummitOrderService.php index c16b67385..273acbe70 100644 --- a/app/Services/Model/ISummitOrderService.php +++ b/app/Services/Model/ISummitOrderService.php @@ -25,6 +25,9 @@ */ interface ISummitOrderService extends IProcessPaymentService { + // ticket data import csv column naming convention for order extra question answers + const ExtraQuestionColumnPrefix = 'extra_question:'; + /** * @param Member|null $owner * @param Summit $summit diff --git a/app/Services/Model/Imp/SummitOrderService.php b/app/Services/Model/Imp/SummitOrderService.php index bff004ce2..6b29c4b8c 100644 --- a/app/Services/Model/Imp/SummitOrderService.php +++ b/app/Services/Model/Imp/SummitOrderService.php @@ -26,6 +26,8 @@ use App\Jobs\ProcessTicketDataImport; use App\Jobs\SendAttendeeInvitationEmail; use App\Jobs\Utils\JobDispatcher; +use App\Models\Foundation\ExtraQuestions\ExtraQuestionType; +use App\Models\Foundation\ExtraQuestions\ExtraQuestionTypeConstants; use App\Models\Foundation\Summit\Factories\SummitOrderFactory; use App\Models\Foundation\Summit\Registration\IBuildDefaultPaymentGatewayProfileStrategy; use App\Models\Foundation\Summit\Registration\PromoCodes\PromoCodesUtils; @@ -4284,6 +4286,7 @@ public function importTicketData(Summit $summit, UploadedFile $csv_file): void * badge_type_id (optional) * badge_type_name (optional) * one col per feature + * extra_question:{question name} (optional, one col per order extra question - Ticket/Both usage) */ // validate format with col names @@ -4583,6 +4586,11 @@ public function processTicketData(int $summit_id, string $filename) Log::debug(sprintf("SummitOrderService::processTicketData - got ticket %s (%s)", $ticket->getId(), $ticket->getNumber())); + // extra questions ( extra_question:{question name} columns ) + $answers_owner = !is_null($attendee) ? $attendee : ($ticket->hasOwner() ? $ticket->getOwner() : null); + if (!is_null($answers_owner)) + $this->upsertAttendeeExtraQuestionAnswers($summit, $answers_owner, $row); + // badge data if (!$badge_data_present) { Log::warning("SummitOrderService::processTicketData badge data is not present stop current row processing."); @@ -4657,6 +4665,182 @@ public function processTicketData(int $summit_id, string $filename) $this->download_strategy->delete($path); } + /** + * Upserts attendee extra question answers from a ticket data import row + * (one column per question, "extra_question:{question name}" naming convention). + * Unknown question names, order-scoped questions, disallowed questions and empty values are + * skipped, never fail the row. Mandatory-question completeness is not enforced ( admin bulk load ). + * Former answers not present on the row are carried over on a best effort basis: the shared + * persistence path ( ExtraQuestionAnswerHolder::hadCompletedExtraQuestions ) rebuilds the full + * answers set and drops answers whose question no longer exists or is no longer allowed for the + * attendee, same as the admin attendee update flow. + * @param Summit $summit + * @param SummitAttendee $attendee + * @param array $row + * @throws ValidationException + */ + private function upsertAttendeeExtraQuestionAnswers(Summit $summit, SummitAttendee $attendee, array $row): void + { + $new_answers = []; + + foreach ($row as $col => $value) { + + if (!str_starts_with(strval($col), self::ExtraQuestionColumnPrefix)) continue; + + $question_name = trim(substr($col, strlen(self::ExtraQuestionColumnPrefix))); + $value = trim(strval($value)); + + if ($value === '') { + Log::debug + ( + sprintf + ( + "SummitOrderService::upsertAttendeeExtraQuestionAnswers question %s has an empty value for attendee %s, skipping it", + $question_name, + $attendee->getEmail() + ) + ); + continue; + } + + $question = $summit->getOrderExtraQuestionByName($question_name); + if (is_null($question)) { + Log::warning + ( + sprintf + ( + "SummitOrderService::upsertAttendeeExtraQuestionAnswers question %s does not exist on summit %s, skipping it", + $question_name, + $summit->getId() + ) + ); + continue; + } + + if ($question->getUsage() === SummitOrderExtraQuestionTypeConstants::OrderQuestionUsage) { + Log::warning + ( + sprintf + ( + "SummitOrderService::upsertAttendeeExtraQuestionAnswers question %s is order scoped, can not be answered per attendee, skipping it", + $question_name + ) + ); + continue; + } + + if (!$attendee->isAllowedQuestion($question)) { + Log::warning + ( + sprintf + ( + "SummitOrderService::upsertAttendeeExtraQuestionAnswers question %s is not allowed for attendee %s, skipping it", + $question_name, + $attendee->getEmail() + ) + ); + continue; + } + + if ($question->allowsValues()) { + // list type questions store the value id(s) ( comma separated when multi value ) + // csv cells accept the value name/label ( "|" separated for multi value ) or the raw value id + $value_ids = []; + foreach (explode('|', $value) as $v) { + $v = trim($v); + if ($v === '') continue; + $question_value = $question->getValueByName($v); + if (is_null($question_value)) + $question_value = $question->getValueByLabel($v); + if (is_null($question_value) && is_numeric($v)) + $question_value = $question->getValueById(intval($v)); + if (is_null($question_value)) { + Log::warning + ( + sprintf + ( + "SummitOrderService::upsertAttendeeExtraQuestionAnswers value %s does not exist on question %s, skipping it", + $v, + $question_name + ) + ); + continue; + } + $value_ids[] = $question_value->getId(); + } + if (count($value_ids) === 0) continue; + // only CheckBoxList questions admit multiple selected values + if (count($value_ids) > 1 && $question->getType() !== ExtraQuestionTypeConstants::CheckBoxListQuestionType) { + Log::warning + ( + sprintf + ( + "SummitOrderService::upsertAttendeeExtraQuestionAnswers question %s admits a single value, got %s, skipping it", + $question_name, + count($value_ids) + ) + ); + continue; + } + $value = implode(ExtraQuestionType::QuestionChoicesCharSeparator, $value_ids); + } + + $former_answer = $attendee->getExtraQuestionAnswerByQuestion($question); + $former_value = is_null($former_answer) ? '' : $former_answer->getValue(); + if (!empty($former_value) && $former_value != $value && !$attendee->canChangeAnswerValue($question)) { + Log::warning + ( + sprintf + ( + "SummitOrderService::upsertAttendeeExtraQuestionAnswers answer for question %s can not be changed by this time for attendee %s, skipping it", + $question_name, + $attendee->getEmail() + ) + ); + continue; + } + + $new_answers[$question->getId()] = $value; + } + + if (count($new_answers) === 0) return; + + if (!$attendee->hasAllowedExtraQuestions()) { + Log::warning + ( + sprintf + ( + "SummitOrderService::upsertAttendeeExtraQuestionAnswers attendee %s does not have allowed extra questions, skipping answers", + $attendee->getEmail() + ) + ); + return; + } + + // upsert semantics: the persistence path rebuilds the full answers set, so carry over + // the former answers not overridden by the csv row ( best effort, see docblock ) + $extra_questions = []; + foreach ($attendee->getExtraQuestionAnswers() as $former_answer) { + $question_id = $former_answer->getQuestionId(); + if (!array_key_exists($question_id, $new_answers)) + $extra_questions[] = ['question_id' => $question_id, 'answer' => $former_answer->getValue()]; + } + foreach ($new_answers as $question_id => $value) + $extra_questions[] = ['question_id' => $question_id, 'answer' => $value]; + + Log::debug + ( + sprintf + ( + "SummitOrderService::upsertAttendeeExtraQuestionAnswers attendee %s answers %s", + $attendee->getEmail(), + json_encode($extra_questions) + ) + ); + + $attendee->hadCompletedExtraQuestions($extra_questions); + } + /** * @param Summit $summit * @param array $payload diff --git a/tests/SummitOrderServiceTest.php b/tests/SummitOrderServiceTest.php index b75299c49..470ec60a7 100644 --- a/tests/SummitOrderServiceTest.php +++ b/tests/SummitOrderServiceTest.php @@ -14,6 +14,8 @@ use App\Jobs\Emails\Registration\Reminders\SummitOrderReminderEmail; use App\Jobs\Emails\Registration\Reminders\SummitTicketReminderEmail; +use App\Models\Foundation\ExtraQuestions\ExtraQuestionTypeConstants; +use App\Models\Foundation\ExtraQuestions\ExtraQuestionTypeValue; use App\Models\Foundation\Main\IGroup; use App\Models\Foundation\Summit\Registration\IBuildDefaultPaymentGatewayProfileStrategy; use App\Models\Foundation\Summit\Repositories\ISummitAttendeeBadgePrintRuleRepository; @@ -29,6 +31,7 @@ use App\Services\Model\SummitOrderService; use App\Services\Utils\ILockManagerService; use Illuminate\Support\Facades\App; +use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Queue; use libs\utils\ITransactionService; use Mockery; @@ -43,7 +46,11 @@ use models\summit\Summit; use models\summit\SummitAttendee; use models\summit\SummitAttendeeTicket; +use models\summit\SummitBadgeFeatureType; use models\summit\SummitOrder; +use models\summit\SummitOrderExtraQuestionAnswer; +use models\summit\SummitOrderExtraQuestionType; +use models\summit\SummitOrderExtraQuestionTypeConstants; use models\summit\SummitTicketType; /** @@ -373,4 +380,296 @@ public function testAutoAssignDifferentPrePaidTicketsUntilEmpty() { $this->assertTrue(str_starts_with($ex->getMessage(), 'No more available PrePaid Tickets for Promo Code')); } } + + /** + * @param string $csv_content + * @return ISummitOrderService + */ + private function buildTicketDataImportService(string $csv_content): ISummitOrderService + { + $upload_strategy = Mockery::mock(IFileUploadStrategy::class); + + $download_strategy = Mockery::mock(IFileDownloadStrategy::class); + $download_strategy->shouldReceive('exists')->andReturn(true); + $download_strategy->shouldReceive('get')->andReturn($csv_content); + $download_strategy->shouldReceive('getDriver')->andReturn('mock'); + $download_strategy->shouldReceive('delete'); + + return new SummitOrderService( + App::make(ISummitTicketTypeRepository::class), + App::make(IMemberRepository::class), + App::make(ISummitRegistrationPromoCodeRepository::class), + App::make(\models\summit\ISummitPromoCodeMemberReservationRepository::class), + App::make(ISummitAttendeeRepository::class), + App::make(ISummitOrderRepository::class), + App::make(ISummitAttendeeTicketRepository::class), + App::make(ISummitAttendeeBadgeRepository::class), + App::make(ISummitRepository::class), + App::make(ISummitAttendeeBadgePrintRuleRepository::class), + App::make(IMemberService::class), + App::make(IBuildDefaultPaymentGatewayProfileStrategy::class), + $upload_strategy, + $download_strategy, + App::make(ICompanyRepository::class), + App::make(ITagRepository::class), + App::make(ISummitRefundRequestRepository::class), + App::make(ICompanyService::class), + App::make(ITicketFinderStrategyFactory::class), + App::make(ITransactionService::class), + App::make(ILockManagerService::class) + ); + } + + /** + * @param string $name + * @param string $type + * @param array $values + * @return SummitOrderExtraQuestionType + */ + private function insertOrderExtraQuestion + ( + string $name, + string $type = ExtraQuestionTypeConstants::TextQuestionType, + array $values = [] + ): SummitOrderExtraQuestionType + { + $question = new SummitOrderExtraQuestionType(); + $question->setName($name); + $question->setLabel($name); + $question->setType($type); + $question->setUsage(SummitOrderExtraQuestionTypeConstants::TicketQuestionUsage); + + foreach ($values as $value_name) { + $value = new ExtraQuestionTypeValue(); + $value->setValue($value_name); + $value->setLabel($value_name); + $question->addValue($value); + } + + self::$summit->addOrderExtraQuestion($question); + self::$em->persist(self::$summit); + self::$em->flush(); + + return $question; + } + + /** + * @param SummitAttendee $attendee + * @param SummitOrderExtraQuestionType $question + * @param string $value + */ + private function insertExtraQuestionAnswer(SummitAttendee $attendee, SummitOrderExtraQuestionType $question, string $value): void + { + $answer = new SummitOrderExtraQuestionAnswer(); + $answer->setQuestion($question); + $answer->setValue($value); + $attendee->addExtraQuestionAnswer($answer); + self::$em->persist($attendee); + self::$em->flush(); + } + + /** + * @return SummitAttendeeTicket + */ + private function getUnassignedTicket(): SummitAttendeeTicket + { + foreach (self::$summit->getOrders() as $order) { + foreach ($order->getTickets() as $ticket) { + if (!$ticket->hasOwner()) return $ticket; + } + } + $this->fail('no unassigned ticket available on test fixture'); + } + + /** + * @return SummitAttendee + */ + private function getDefaultAttendee(): SummitAttendee + { + $attendee = App::make(ISummitAttendeeRepository::class) + ->getBySummitAndEmail(self::$summit, self::$defaultMember->getEmail()); + $this->assertNotNull($attendee); + return $attendee; + } + + public function testImportTicketDataSetsExtraQuestionAnswerOnNewAttendee() + { + Queue::fake(); + + $question = $this->insertOrderExtraQuestion('Dietary Requirements'); + $ticket = $this->getUnassignedTicket(); + + $csv_content = <<getNumber()},new.attendee@nowhere.com,New,Attendee,Vegan +CSV; + + $service = $this->buildTicketDataImportService($csv_content); + $service->processTicketData(self::$summit->getId(), 'tickets.csv'); + + $attendee = App::make(ISummitAttendeeRepository::class) + ->getBySummitAndEmail(self::$summit, 'new.attendee@nowhere.com'); + + $this->assertNotNull($attendee); + $answer = $attendee->getExtraQuestionAnswerByQuestion($question); + $this->assertNotNull($answer); + $this->assertEquals('Vegan', $answer->getValue()); + } + + public function testImportTicketDataUpdatesExtraQuestionAnswerOnExistingAttendee() + { + Queue::fake(); + + // background import runs without an authenticated admin, so answer updates + // are gated by the summit level setting + self::$summit->setAllowUpdateAttendeeExtraQuestions(true); + self::$em->persist(self::$summit); + self::$em->flush(); + + // SummitAttendee::canChangeAnswerValue caches per attendee id for 60 secs, + // drop any stale entry from a previous run ( attendee ids are reused across DB re-seeds ) + Cache::flush(); + + $question = $this->insertOrderExtraQuestion('Dietary Requirements'); + $attendee = $this->getDefaultAttendee(); + $this->insertExtraQuestionAnswer($attendee, $question, 'Meat'); + + $ticket = $attendee->getTickets()->first(); + + $csv_content = <<getNumber()},{$attendee->getEmail()},Vegan +CSV; + + $service = $this->buildTicketDataImportService($csv_content); + $service->processTicketData(self::$summit->getId(), 'tickets.csv'); + + $answer = $attendee->getExtraQuestionAnswerByQuestion($question); + $this->assertNotNull($answer); + $this->assertEquals('Vegan', $answer->getValue()); + } + + public function testImportTicketDataSkipsUnknownExtraQuestion() + { + Queue::fake(); + + $this->insertOrderExtraQuestion('Dietary Requirements'); + $ticket = $this->getUnassignedTicket(); + + $csv_content = <<getNumber()},new.attendee@nowhere.com,New,Attendee,Some Value +CSV; + + $service = $this->buildTicketDataImportService($csv_content); + $service->processTicketData(self::$summit->getId(), 'tickets.csv'); + + // row is still processed ( attendee created and ticket assigned ), unknown question is skipped + $attendee = App::make(ISummitAttendeeRepository::class) + ->getBySummitAndEmail(self::$summit, 'new.attendee@nowhere.com'); + + $this->assertNotNull($attendee); + $this->assertCount(0, $attendee->getExtraQuestionAnswers()); + } + + public function testImportTicketDataIgnoresEmptyExtraQuestionValue() + { + Queue::fake(); + + $question = $this->insertOrderExtraQuestion('Dietary Requirements'); + $attendee = $this->getDefaultAttendee(); + $this->insertExtraQuestionAnswer($attendee, $question, 'Vegan'); + + $ticket = $attendee->getTickets()->first(); + + $csv_content = <<getNumber()},{$attendee->getEmail()}, +CSV; + + $service = $this->buildTicketDataImportService($csv_content); + $service->processTicketData(self::$summit->getId(), 'tickets.csv'); + + // former answer is preserved, empty values never clear answers + $answer = $attendee->getExtraQuestionAnswerByQuestion($question); + $this->assertNotNull($answer); + $this->assertEquals('Vegan', $answer->getValue()); + } + + public function testImportTicketDataListQuestionStoresValueIds() + { + Queue::fake(); + + $question = $this->insertOrderExtraQuestion + ( + 'T-Shirt Size', + ExtraQuestionTypeConstants::CheckBoxListQuestionType, + ['Small', 'Large'] + ); + + $ticket = $this->getUnassignedTicket(); + + $csv_content = <<getNumber()},new.attendee@nowhere.com,New,Attendee,Small|Large +CSV; + + $service = $this->buildTicketDataImportService($csv_content); + $service->processTicketData(self::$summit->getId(), 'tickets.csv'); + + $attendee = App::make(ISummitAttendeeRepository::class) + ->getBySummitAndEmail(self::$summit, 'new.attendee@nowhere.com'); + + $this->assertNotNull($attendee); + $answer = $attendee->getExtraQuestionAnswerByQuestion($question); + $this->assertNotNull($answer); + + $expected_value = implode(',', [ + $question->getValueByName('Small')->getId(), + $question->getValueByName('Large')->getId(), + ]); + $this->assertEquals($expected_value, $answer->getValue()); + } + + public function testImportTicketDataBadgeFeaturesStillClearedAndReSet() + { + Queue::fake(); + + $feature1 = new SummitBadgeFeatureType(); + $feature1->setName('FEATURE 1'); + self::$summit->addFeatureType($feature1); + + $feature2 = new SummitBadgeFeatureType(); + $feature2->setName('FEATURE 2'); + self::$summit->addFeatureType($feature2); + + $question = $this->insertOrderExtraQuestion('Dietary Requirements'); + + $attendee = $this->getDefaultAttendee(); + $ticket = $attendee->getTickets()->first(); + $badge = $ticket->getBadge(); + $badge->addFeature($feature1); + self::$em->persist(self::$summit); + self::$em->flush(); + + $csv_content = <<getNumber()},{$attendee->getEmail()},BADGE TYPE1,0,1,Vegan +CSV; + + $service = $this->buildTicketDataImportService($csv_content); + $service->processTicketData(self::$summit->getId(), 'tickets.csv'); + + // badge features are cleared and re set from the csv columns + $feature_names = []; + foreach ($badge->getFeatures() as $feature) { + $feature_names[] = $feature->getName(); + } + $this->assertEquals(['FEATURE 2'], $feature_names); + + // extra question answer is set on the same row + $answer = $attendee->getExtraQuestionAnswerByQuestion($question); + $this->assertNotNull($answer); + $this->assertEquals('Vegan', $answer->getValue()); + } } From 2b4196d23c3ced96ec51dfe7e79946cb2733b6fb Mon Sep 17 00:00:00 2001 From: JpMaxMan Date: Thu, 2 Jul 2026 21:04:42 -0500 Subject: [PATCH 2/7] test(registration): fix badge-features import test fixture assumptions The import re-reads tickets with HINT_REFRESH, so it sees DB state: the fixture's assigned tickets share one badge entity whose FK points at only the last of them. Use an unassigned ticket, which gets its own DB-consistent badge from SummitTicketType::applyTo. Co-Authored-By: Claude Fable 5 --- tests/SummitOrderServiceTest.php | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/tests/SummitOrderServiceTest.php b/tests/SummitOrderServiceTest.php index 470ec60a7..402c8a5fb 100644 --- a/tests/SummitOrderServiceTest.php +++ b/tests/SummitOrderServiceTest.php @@ -645,29 +645,35 @@ public function testImportTicketDataBadgeFeaturesStillClearedAndReSet() $question = $this->insertOrderExtraQuestion('Dietary Requirements'); - $attendee = $this->getDefaultAttendee(); - $ticket = $attendee->getTickets()->first(); - $badge = $ticket->getBadge(); - $badge->addFeature($feature1); + // use an unassigned ticket: SummitTicketType::applyTo auto-creates a badge per ticket with + // a DB-consistent one-to-one. The fixture's assigned tickets share a single badge entity + // whose ticket FK can only point at one of them, and the import re-reads the ticket with + // HINT_REFRESH ( getByNumberExclusiveLock ), so DB state is what the service sees. + $ticket = $this->getUnassignedTicket(); + $ticket->getBadge()->addFeature($feature1); self::$em->persist(self::$summit); self::$em->flush(); $csv_content = <<getNumber()},{$attendee->getEmail()},BADGE TYPE1,0,1,Vegan +number,attendee_email,attendee_first_name,attendee_last_name,badge_type_name,FEATURE 1,FEATURE 2,extra_question:Dietary Requirements +{$ticket->getNumber()},new.attendee@nowhere.com,New,Attendee,BADGE TYPE1,0,1,Vegan CSV; $service = $this->buildTicketDataImportService($csv_content); $service->processTicketData(self::$summit->getId(), 'tickets.csv'); // badge features are cleared and re set from the csv columns + // ( re-read the badge from the ticket, the import may have refreshed the association ) $feature_names = []; - foreach ($badge->getFeatures() as $feature) { + foreach ($ticket->getBadge()->getFeatures() as $feature) { $feature_names[] = $feature->getName(); } $this->assertEquals(['FEATURE 2'], $feature_names); // extra question answer is set on the same row + $attendee = App::make(ISummitAttendeeRepository::class) + ->getBySummitAndEmail(self::$summit, 'new.attendee@nowhere.com'); + $this->assertNotNull($attendee); $answer = $attendee->getExtraQuestionAnswerByQuestion($question); $this->assertNotNull($answer); $this->assertEquals('Vegan', $answer->getValue()); From 8cdce730c208dc42952e78deb7e4bbbad6ecdc40 Mon Sep 17 00:00:00 2001 From: JpMaxMan Date: Thu, 2 Jul 2026 22:16:28 -0500 Subject: [PATCH 3/7] fix(registration): normalize list answer ordering + guard import row on answer persistence Per CodeRabbit review: sort list-question value ids so the same selection in a different order is not treated as a changed answer, and catch ValidationException from the extra-question persistence path so one bad payload cannot strand the remaining import rows. Co-Authored-By: Claude Fable 5 --- app/Services/Model/Imp/SummitOrderService.php | 26 +++++++++++++++++-- tests/SummitOrderServiceTest.php | 10 ++++--- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/app/Services/Model/Imp/SummitOrderService.php b/app/Services/Model/Imp/SummitOrderService.php index 6b29c4b8c..982ec7c83 100644 --- a/app/Services/Model/Imp/SummitOrderService.php +++ b/app/Services/Model/Imp/SummitOrderService.php @@ -4677,7 +4677,6 @@ public function processTicketData(int $summit_id, string $filename) * @param Summit $summit * @param SummitAttendee $attendee * @param array $row - * @throws ValidationException */ private function upsertAttendeeExtraQuestionAnswers(Summit $summit, SummitAttendee $attendee, array $row): void { @@ -4782,11 +4781,20 @@ private function upsertAttendeeExtraQuestionAnswers(Summit $summit, SummitAttend ); continue; } + sort($value_ids); $value = implode(ExtraQuestionType::QuestionChoicesCharSeparator, $value_ids); } $former_answer = $attendee->getExtraQuestionAnswerByQuestion($question); $former_value = is_null($former_answer) ? '' : $former_answer->getValue(); + + // list answers compare as sets: the same selection in a different order is not a change + if ($question->allowsValues() && !empty($former_value)) { + $former_value_ids = array_map('intval', explode(ExtraQuestionType::QuestionChoicesCharSeparator, $former_value)); + sort($former_value_ids); + if ($former_value_ids === $value_ids) $value = $former_value; + } + if (!empty($former_value) && $former_value != $value && !$attendee->canChangeAnswerValue($question)) { Log::warning ( @@ -4838,7 +4846,21 @@ private function upsertAttendeeExtraQuestionAnswers(Summit $summit, SummitAttend ) ); - $attendee->hadCompletedExtraQuestions($extra_questions); + try { + $attendee->hadCompletedExtraQuestions($extra_questions); + } catch (ValidationException $ex) { + // never abort the import on a bad extra-question payload: the remaining + // rows must still be processed ( the queue job does not retry the file ) + Log::warning + ( + sprintf + ( + "SummitOrderService::upsertAttendeeExtraQuestionAnswers attendee %s could not persist answers: %s", + $attendee->getEmail(), + $ex->getMessage() + ) + ); + } } /** diff --git a/tests/SummitOrderServiceTest.php b/tests/SummitOrderServiceTest.php index 402c8a5fb..7c4d632f9 100644 --- a/tests/SummitOrderServiceTest.php +++ b/tests/SummitOrderServiceTest.php @@ -609,9 +609,10 @@ public function testImportTicketDataListQuestionStoresValueIds() $ticket = $this->getUnassignedTicket(); + // reversed token order on purpose: stored value ids are normalized ( sorted ) $csv_content = <<getNumber()},new.attendee@nowhere.com,New,Attendee,Small|Large +{$ticket->getNumber()},new.attendee@nowhere.com,New,Attendee,Large|Small CSV; $service = $this->buildTicketDataImportService($csv_content); @@ -624,11 +625,12 @@ public function testImportTicketDataListQuestionStoresValueIds() $answer = $attendee->getExtraQuestionAnswerByQuestion($question); $this->assertNotNull($answer); - $expected_value = implode(',', [ + $expected_value_ids = [ $question->getValueByName('Small')->getId(), $question->getValueByName('Large')->getId(), - ]); - $this->assertEquals($expected_value, $answer->getValue()); + ]; + sort($expected_value_ids); + $this->assertEquals(implode(',', $expected_value_ids), $answer->getValue()); } public function testImportTicketDataBadgeFeaturesStillClearedAndReSet() From da8b3d4e2953a21cb03bddffc49a917a319d027b Mon Sep 17 00:00:00 2001 From: JpMaxMan Date: Fri, 3 Jul 2026 13:24:49 -0500 Subject: [PATCH 4/7] refactor(registration): review fixes for extra-question CSV import Per review: process badge data before extra questions and flush + evict the badge-features result cache so same-row badge/feature grants are visible to the question permission gates; split the upsert into resolve/merge helpers; ctype_digit for raw value ids; targeted cache forget in tests. Adds regression tests for the same-row feature grant (new + existing attendee), order-scoped skip, locked-answer skip, and badge creation for badge-less tickets (folds in #568). Co-Authored-By: Claude Fable 5 --- .../Registration/Attendees/SummitAttendee.php | 16 + app/Services/Model/Imp/SummitOrderService.php | 345 +++++++++++------- tests/SummitOrderServiceTest.php | 193 +++++++++- 3 files changed, 410 insertions(+), 144 deletions(-) diff --git a/app/Models/Foundation/Summit/Registration/Attendees/SummitAttendee.php b/app/Models/Foundation/Summit/Registration/Attendees/SummitAttendee.php index 2111180c6..9fd768724 100644 --- a/app/Models/Foundation/Summit/Registration/Attendees/SummitAttendee.php +++ b/app/Models/Foundation/Summit/Registration/Attendees/SummitAttendee.php @@ -1342,6 +1342,22 @@ public function getAllowedBadgeFeatures(bool $exclude_deactivated_tickets = true return $native_query->enableResultCache(600 , sprintf("ATTENDEES_%s_BADGE_FEATURES",$this->id))->getResult(); } + /** + * Drops the getAllowedBadgeFeatures result cache entry for this attendee, so permission + * checks that depend on badge features ( isAllowedQuestion ) re-evaluate against current + * state after the attendee's badge/features change mid-request. + */ + public function evictAllowedBadgeFeaturesCache(): void + { + try { + $result_cache = $this->getEM()->getConfiguration()->getResultCache(); + if (!is_null($result_cache)) + $result_cache->deleteItem(sprintf("ATTENDEES_%s_BADGE_FEATURES", $this->id)); + } catch (\Exception $ex) { + Log::warning($ex); + } + } + public function buildExtraQuestionAnswer(): ExtraQuestionAnswer { return new SummitOrderExtraQuestionAnswer(); diff --git a/app/Services/Model/Imp/SummitOrderService.php b/app/Services/Model/Imp/SummitOrderService.php index 982ec7c83..790fc7c72 100644 --- a/app/Services/Model/Imp/SummitOrderService.php +++ b/app/Services/Model/Imp/SummitOrderService.php @@ -75,6 +75,7 @@ use models\summit\SummitBadgeType; use models\summit\SummitBadgeViewType; use models\summit\SummitOrder; +use models\summit\SummitOrderExtraQuestionType; use models\summit\SummitOrderExtraQuestionTypeConstants; use models\summit\SummitRegistrationInvitation; use models\summit\SummitRegistrationPromoCode; @@ -4586,78 +4587,80 @@ public function processTicketData(int $summit_id, string $filename) Log::debug(sprintf("SummitOrderService::processTicketData - got ticket %s (%s)", $ticket->getId(), $ticket->getNumber())); - // extra questions ( extra_question:{question name} columns ) - $answers_owner = !is_null($attendee) ? $attendee : ($ticket->hasOwner() ? $ticket->getOwner() : null); - if (!is_null($answers_owner)) - $this->upsertAttendeeExtraQuestionAnswers($summit, $answers_owner, $row); + // badge data is processed BEFORE extra questions: the extra-question permission + // gates ( SummitAttendee::isAllowedQuestion ) read the attendee badge features + // through native SQL, so this row's own badge/feature grants must be applied + // ( and flushed, see upsertAttendeeExtraQuestionAnswers ) first + if ($badge_data_present) { - // badge data - if (!$badge_data_present) { - Log::warning("SummitOrderService::processTicketData badge data is not present stop current row processing."); - return; - } + $badge_type = null; - $badge_type = null; + if ($reader->hasColumn("badge_type_id")) { + Log::debug(sprintf("SummitOrderService::processTicketData trying to get badge type by id %s", $row['badge_type_id'])); + $badge_type = $summit->getBadgeTypeById(intval($row['badge_type_id'])); + } - if ($reader->hasColumn("badge_type_id")) { - Log::debug(sprintf("SummitOrderService::processTicketData trying to get badge type by id %s", $row['badge_type_id'])); - $badge_type = $summit->getBadgeTypeById(intval($row['badge_type_id'])); - } + if (is_null($badge_type) && $reader->hasColumn("badge_type_name")) { + Log::debug(sprintf("SummitOrderService::processTicketData trying to get badge type by name %s", $row['badge_type_name'])); + $badge_type = $summit->getBadgeTypeByName(trim($row['badge_type_name'])); + } - if (is_null($badge_type) && $reader->hasColumn("badge_type_name")) { - Log::debug(sprintf("SummitOrderService::processTicketData trying to get badge type by name %s", $row['badge_type_name'])); - $badge_type = $summit->getBadgeTypeByName(trim($row['badge_type_name'])); - } + if (!is_null($badge_type)) + Log::debug(sprintf("SummitOrderService::processTicketData - got badge type %s (%s)", $badge_type->getId(), $badge_type->getName())); - if (!is_null($badge_type)) - Log::debug(sprintf("SummitOrderService::processTicketData - got badge type %s (%s)", $badge_type->getId(), $badge_type->getName())); + if (!$ticket->hasBadge() && is_null($badge_type)) { + Log::warning("SummitOrderService::processTicketData ticket has no badge and badge type is null, skipping badge processing."); + } else { - if (!$ticket->hasBadge()) { - // create it - if (!is_null($badge_type)) { - Log::warning("SummitOrderService::processTicketData badge type is null stop current row processing."); - return; - } - Log::debug(sprintf("SummitOrderService::processTicketData - ticket %s (%s) has not badge ... creating it", $ticket->getId(), $ticket->getNumber())); - $badge = SummitBadgeType::buildBadgeFromType($badge_type); - $ticket->setBadge($badge); - } + if (!$ticket->hasBadge()) { + // create it + Log::debug(sprintf("SummitOrderService::processTicketData - ticket %s (%s) has not badge ... creating it", $ticket->getId(), $ticket->getNumber())); + $badge = SummitBadgeType::buildBadgeFromType($badge_type); + $ticket->setBadge($badge); + } - $badge = $ticket->getBadge(); + $badge = $ticket->getBadge(); - if (!is_null($badge_type)) - $badge->setType($badge_type); + if (!is_null($badge_type)) + $badge->setType($badge_type); - $clearedFeatures = false; - // check if we are setting any badge feature - Log::debug("SummitOrderService::processTicketData processing badge type features"); - foreach ($summit->getBadgeFeaturesTypes() as $featuresType) { - $feature_name = $featuresType->getName(); - Log::debug(sprintf("SummitOrderService::processTicketData processing badge type feature %s for ticket %s", $feature_name, $ticket->getId())); - if (!$reader->hasColumn($feature_name)) { - Log::debug(sprintf("SummitOrderService::processTicketData badge type feature %s does not exists as column", $feature_name)); - continue; - } + $clearedFeatures = false; + // check if we are setting any badge feature + Log::debug("SummitOrderService::processTicketData processing badge type features"); + foreach ($summit->getBadgeFeaturesTypes() as $featuresType) { + $feature_name = $featuresType->getName(); + Log::debug(sprintf("SummitOrderService::processTicketData processing badge type feature %s for ticket %s", $feature_name, $ticket->getId())); + if (!$reader->hasColumn($feature_name)) { + Log::debug(sprintf("SummitOrderService::processTicketData badge type feature %s does not exists as column", $feature_name)); + continue; + } - if (!$clearedFeatures) { - $badge->clearFeatures(); - $clearedFeatures = true; - } + if (!$clearedFeatures) { + $badge->clearFeatures(); + $clearedFeatures = true; + } - $mustAdd = intval($row[$feature_name]) === 1; - if (!$mustAdd) { - Log::debug(sprintf("SummitOrderService::processTicketData badge type feature %s not set for ticket %s", $feature_name, $ticket->getId())); - continue; - } - Log::debug(sprintf("SummitOrderService::processTicketData - ticket %s (%s) - trying to add new features to ticket badge (%s)", $ticket->getId(), $ticket->getNumber(), $feature_name)); - $feature = $summit->getFeatureTypeByName(trim($feature_name)); - if (is_null($feature)) { - Log::warning(sprintf("SummitOrderService::processTicketData feature %s does not exist on summit %s", $feature, $summit->getId())); - continue; + $mustAdd = intval($row[$feature_name]) === 1; + if (!$mustAdd) { + Log::debug(sprintf("SummitOrderService::processTicketData badge type feature %s not set for ticket %s", $feature_name, $ticket->getId())); + continue; + } + Log::debug(sprintf("SummitOrderService::processTicketData - ticket %s (%s) - trying to add new features to ticket badge (%s)", $ticket->getId(), $ticket->getNumber(), $feature_name)); + $feature = $summit->getFeatureTypeByName(trim($feature_name)); + if (is_null($feature)) { + Log::warning(sprintf("SummitOrderService::processTicketData feature %s does not exist on summit %s", $feature, $summit->getId())); + continue; + } + Log::debug(sprintf("SummitOrderService::processTicketData badge type feature %s set for ticket %s", $feature_name, $ticket->getId())); + $badge->addFeature($feature); + } } - Log::debug(sprintf("SummitOrderService::processTicketData badge type feature %s set for ticket %s", $feature_name, $ticket->getId())); - $badge->addFeature($feature); } + + // extra questions ( extra_question:{question name} columns ) + $answers_owner = !is_null($attendee) ? $attendee : ($ticket->hasOwner() ? $ticket->getOwner() : null); + if (!is_null($answers_owner)) + $this->upsertAttendeeExtraQuestionAnswers($summit, $answers_owner, $row); }); } @@ -4668,6 +4671,10 @@ public function processTicketData(int $summit_id, string $filename) /** * Upserts attendee extra question answers from a ticket data import row * (one column per question, "extra_question:{question name}" naming convention). + * The prefix is deliberate: on this import the bare-name column namespace already belongs to + * badge features, and a question named like a feature would be ambiguous at parse time — + * additive per-name column families should be namespaced ( badge feature columns stay bare + * for backward compatibility with existing CSVs ). * Unknown question names, order-scoped questions, disallowed questions and empty values are * skipped, never fail the row. Mandatory-question completeness is not enforced ( admin bulk load ). * Former answers not present on the row are carried over on a best effort basis: the shared @@ -4679,6 +4686,77 @@ public function processTicketData(int $summit_id, string $filename) * @param array $row */ private function upsertAttendeeExtraQuestionAnswers(Summit $summit, SummitAttendee $attendee, array $row): void + { + $has_extra_question_columns = false; + foreach ($row as $col => $value) { + if (str_starts_with(strval($col), self::ExtraQuestionColumnPrefix)) { + $has_extra_question_columns = true; + break; + } + } + if (!$has_extra_question_columns) return; + + // flush the row's own pending writes ( new attendee / ticket / badge features ) so the + // native-SQL permission gates below ( isAllowedQuestion / hasAllowedExtraQuestions ) see + // this row's badge/feature grants instead of stale pre-row state; the badge-features + // result cache may have been warmed earlier in this same row ( pre-grant ), drop it too + $this->attendee_repository->add($attendee, true); + $attendee->evictAllowedBadgeFeaturesCache(); + + $new_answers = $this->resolveExtraQuestionColumns($summit, $attendee, $row); + + if (count($new_answers) === 0) return; + + if (!$attendee->hasAllowedExtraQuestions()) { + Log::warning + ( + sprintf + ( + "SummitOrderService::upsertAttendeeExtraQuestionAnswers attendee %s does not have allowed extra questions, skipping answers", + $attendee->getEmail() + ) + ); + return; + } + + $extra_questions = $this->mergeWithFormerAnswers($attendee, $new_answers); + + Log::debug + ( + sprintf + ( + "SummitOrderService::upsertAttendeeExtraQuestionAnswers attendee %s answers %s", + $attendee->getEmail(), + json_encode($extra_questions) + ) + ); + + try { + $attendee->hadCompletedExtraQuestions($extra_questions); + } catch (ValidationException $ex) { + // never abort the import on a bad extra-question payload: the remaining + // rows must still be processed ( the queue job does not retry the file ) + Log::warning + ( + sprintf + ( + "SummitOrderService::upsertAttendeeExtraQuestionAnswers attendee %s could not persist answers: %s", + $attendee->getEmail(), + $ex->getMessage() + ) + ); + } + } + + /** + * Resolves the row's extra_question:* columns to [question_id => value], applying the + * empty-value, unknown-question, order-scoped, not-allowed and change-lock skips. + * @param Summit $summit + * @param SummitAttendee $attendee + * @param array $row + * @return array + */ + private function resolveExtraQuestionColumns(Summit $summit, SummitAttendee $attendee, array $row): array { $new_answers = []; @@ -4694,7 +4772,7 @@ private function upsertAttendeeExtraQuestionAnswers(Summit $summit, SummitAttend ( sprintf ( - "SummitOrderService::upsertAttendeeExtraQuestionAnswers question %s has an empty value for attendee %s, skipping it", + "SummitOrderService::resolveExtraQuestionColumns question %s has an empty value for attendee %s, skipping it", $question_name, $attendee->getEmail() ) @@ -4708,7 +4786,7 @@ private function upsertAttendeeExtraQuestionAnswers(Summit $summit, SummitAttend ( sprintf ( - "SummitOrderService::upsertAttendeeExtraQuestionAnswers question %s does not exist on summit %s, skipping it", + "SummitOrderService::resolveExtraQuestionColumns question %s does not exist on summit %s, skipping it", $question_name, $summit->getId() ) @@ -4721,7 +4799,7 @@ private function upsertAttendeeExtraQuestionAnswers(Summit $summit, SummitAttend ( sprintf ( - "SummitOrderService::upsertAttendeeExtraQuestionAnswers question %s is order scoped, can not be answered per attendee, skipping it", + "SummitOrderService::resolveExtraQuestionColumns question %s is order scoped, can not be answered per attendee, skipping it", $question_name ) ); @@ -4733,7 +4811,7 @@ private function upsertAttendeeExtraQuestionAnswers(Summit $summit, SummitAttend ( sprintf ( - "SummitOrderService::upsertAttendeeExtraQuestionAnswers question %s is not allowed for attendee %s, skipping it", + "SummitOrderService::resolveExtraQuestionColumns question %s is not allowed for attendee %s, skipping it", $question_name, $attendee->getEmail() ) @@ -4742,47 +4820,8 @@ private function upsertAttendeeExtraQuestionAnswers(Summit $summit, SummitAttend } if ($question->allowsValues()) { - // list type questions store the value id(s) ( comma separated when multi value ) - // csv cells accept the value name/label ( "|" separated for multi value ) or the raw value id - $value_ids = []; - foreach (explode('|', $value) as $v) { - $v = trim($v); - if ($v === '') continue; - $question_value = $question->getValueByName($v); - if (is_null($question_value)) - $question_value = $question->getValueByLabel($v); - if (is_null($question_value) && is_numeric($v)) - $question_value = $question->getValueById(intval($v)); - if (is_null($question_value)) { - Log::warning - ( - sprintf - ( - "SummitOrderService::upsertAttendeeExtraQuestionAnswers value %s does not exist on question %s, skipping it", - $v, - $question_name - ) - ); - continue; - } - $value_ids[] = $question_value->getId(); - } - if (count($value_ids) === 0) continue; - // only CheckBoxList questions admit multiple selected values - if (count($value_ids) > 1 && $question->getType() !== ExtraQuestionTypeConstants::CheckBoxListQuestionType) { - Log::warning - ( - sprintf - ( - "SummitOrderService::upsertAttendeeExtraQuestionAnswers question %s admits a single value, got %s, skipping it", - $question_name, - count($value_ids) - ) - ); - continue; - } - sort($value_ids); - $value = implode(ExtraQuestionType::QuestionChoicesCharSeparator, $value_ids); + $value = $this->resolveListQuestionValue($question, $value); + if (is_null($value)) continue; } $former_answer = $attendee->getExtraQuestionAnswerByQuestion($question); @@ -4792,7 +4831,8 @@ private function upsertAttendeeExtraQuestionAnswers(Summit $summit, SummitAttend if ($question->allowsValues() && !empty($former_value)) { $former_value_ids = array_map('intval', explode(ExtraQuestionType::QuestionChoicesCharSeparator, $former_value)); sort($former_value_ids); - if ($former_value_ids === $value_ids) $value = $former_value; + if (implode(ExtraQuestionType::QuestionChoicesCharSeparator, $former_value_ids) === $value) + $value = $former_value; } if (!empty($former_value) && $former_value != $value && !$attendee->canChangeAnswerValue($question)) { @@ -4800,7 +4840,7 @@ private function upsertAttendeeExtraQuestionAnswers(Summit $summit, SummitAttend ( sprintf ( - "SummitOrderService::upsertAttendeeExtraQuestionAnswers answer for question %s can not be changed by this time for attendee %s, skipping it", + "SummitOrderService::resolveExtraQuestionColumns answer for question %s can not be changed by this time for attendee %s, skipping it", $question_name, $attendee->getEmail() ) @@ -4811,23 +4851,76 @@ private function upsertAttendeeExtraQuestionAnswers(Summit $summit, SummitAttend $new_answers[$question->getId()] = $value; } - if (count($new_answers) === 0) return; + return $new_answers; + } - if (!$attendee->hasAllowedExtraQuestions()) { + /** + * Resolves a list-question csv cell ( value name / label / raw value id per token, "|" separated + * for multi value ) to the stored form: sorted value ids joined by the question-choices separator. + * Returns null when no token resolves or when multiple values hit a single-value question. + * @param SummitOrderExtraQuestionType $question + * @param string $value + * @return string|null + */ + private function resolveListQuestionValue(SummitOrderExtraQuestionType $question, string $value): ?string + { + $value_ids = []; + + foreach (explode('|', $value) as $v) { + $v = trim($v); + if ($v === '') continue; + $question_value = $question->getValueByName($v); + if (is_null($question_value)) + $question_value = $question->getValueByLabel($v); + if (is_null($question_value) && ctype_digit($v)) + $question_value = $question->getValueById(intval($v)); + if (is_null($question_value)) { + Log::warning + ( + sprintf + ( + "SummitOrderService::resolveListQuestionValue value %s does not exist on question %s, skipping it", + $v, + $question->getName() + ) + ); + continue; + } + $value_ids[] = $question_value->getId(); + } + + if (count($value_ids) === 0) return null; + + // only CheckBoxList questions admit multiple selected values + if (count($value_ids) > 1 && $question->getType() !== ExtraQuestionTypeConstants::CheckBoxListQuestionType) { Log::warning ( sprintf ( - "SummitOrderService::upsertAttendeeExtraQuestionAnswers attendee %s does not have allowed extra questions, skipping answers", - $attendee->getEmail() + "SummitOrderService::resolveListQuestionValue question %s admits a single value, got %s, skipping it", + $question->getName(), + count($value_ids) ) ); - return; + return null; } - // upsert semantics: the persistence path rebuilds the full answers set, so carry over - // the former answers not overridden by the csv row ( best effort, see docblock ) + sort($value_ids); + return implode(ExtraQuestionType::QuestionChoicesCharSeparator, $value_ids); + } + + /** + * Carries forward the attendee's former answers not overridden by the csv row ( the persistence + * path rebuilds the full answers set ) and appends the new ones, ready for + * hadCompletedExtraQuestions(). Carry-over is best effort, see upsertAttendeeExtraQuestionAnswers. + * @param SummitAttendee $attendee + * @param array $new_answers + * @return array + */ + private function mergeWithFormerAnswers(SummitAttendee $attendee, array $new_answers): array + { $extra_questions = []; + foreach ($attendee->getExtraQuestionAnswers() as $former_answer) { $question_id = $former_answer->getQuestionId(); if (!array_key_exists($question_id, $new_answers)) @@ -4836,31 +4929,7 @@ private function upsertAttendeeExtraQuestionAnswers(Summit $summit, SummitAttend foreach ($new_answers as $question_id => $value) $extra_questions[] = ['question_id' => $question_id, 'answer' => $value]; - Log::debug - ( - sprintf - ( - "SummitOrderService::upsertAttendeeExtraQuestionAnswers attendee %s answers %s", - $attendee->getEmail(), - json_encode($extra_questions) - ) - ); - - try { - $attendee->hadCompletedExtraQuestions($extra_questions); - } catch (ValidationException $ex) { - // never abort the import on a bad extra-question payload: the remaining - // rows must still be processed ( the queue job does not retry the file ) - Log::warning - ( - sprintf - ( - "SummitOrderService::upsertAttendeeExtraQuestionAnswers attendee %s could not persist answers: %s", - $attendee->getEmail(), - $ex->getMessage() - ) - ); - } + return $extra_questions; } /** diff --git a/tests/SummitOrderServiceTest.php b/tests/SummitOrderServiceTest.php index 7c4d632f9..b72440656 100644 --- a/tests/SummitOrderServiceTest.php +++ b/tests/SummitOrderServiceTest.php @@ -430,14 +430,15 @@ private function insertOrderExtraQuestion ( string $name, string $type = ExtraQuestionTypeConstants::TextQuestionType, - array $values = [] + array $values = [], + string $usage = SummitOrderExtraQuestionTypeConstants::TicketQuestionUsage ): SummitOrderExtraQuestionType { $question = new SummitOrderExtraQuestionType(); $question->setName($name); $question->setLabel($name); $question->setType($type); - $question->setUsage(SummitOrderExtraQuestionTypeConstants::TicketQuestionUsage); + $question->setUsage($usage); foreach ($values as $value_name) { $value = new ExtraQuestionTypeValue(); @@ -526,12 +527,12 @@ public function testImportTicketDataUpdatesExtraQuestionAnswerOnExistingAttendee self::$em->persist(self::$summit); self::$em->flush(); - // SummitAttendee::canChangeAnswerValue caches per attendee id for 60 secs, - // drop any stale entry from a previous run ( attendee ids are reused across DB re-seeds ) - Cache::flush(); - $question = $this->insertOrderExtraQuestion('Dietary Requirements'); $attendee = $this->getDefaultAttendee(); + + // SummitAttendee::canChangeAnswerValue caches per attendee id for 60 secs, + // drop any stale entry from a previous run ( attendee ids are reused across DB re-seeds ) + Cache::forget(sprintf("SummitAttendee.canChangeAnswerValue.%s", $attendee->getId())); $this->insertExtraQuestionAnswer($attendee, $question, 'Meat'); $ticket = $attendee->getTickets()->first(); @@ -680,4 +681,184 @@ public function testImportTicketDataBadgeFeaturesStillClearedAndReSet() $this->assertNotNull($answer); $this->assertEquals('Vegan', $answer->getValue()); } + + public function testImportTicketDataBadgeFeatureRestrictedQuestionAnsweredInSameRowAsFeatureGrant() + { + Queue::fake(); + + $feature = new SummitBadgeFeatureType(); + $feature->setName('VIP'); + self::$summit->addFeatureType($feature); + + $question = $this->insertOrderExtraQuestion('VIP Perk Choice'); + $question->addAllowedBadgeFeatureType($feature); + self::$em->persist(self::$summit); + self::$em->flush(); + + // badge feature and the answer to a question restricted to that same feature, + // both set on the same CSV row for a brand new attendee + $ticket = $this->getUnassignedTicket(); + + $csv_content = <<getNumber()},new.attendee@nowhere.com,New,Attendee,BADGE TYPE1,1,Yes +CSV; + + $service = $this->buildTicketDataImportService($csv_content); + $service->processTicketData(self::$summit->getId(), 'tickets.csv'); + + // the badge feature is granted in this same row + $feature_names = []; + foreach ($ticket->getBadge()->getFeatures() as $f) { + $feature_names[] = $f->getName(); + } + $this->assertEquals(['VIP'], $feature_names); + + // ... so the question restricted to that same feature is answerable too + $attendee = App::make(ISummitAttendeeRepository::class) + ->getBySummitAndEmail(self::$summit, 'new.attendee@nowhere.com'); + $this->assertNotNull($attendee); + $answer = $attendee->getExtraQuestionAnswerByQuestion($question); + $this->assertNotNull($answer); + $this->assertEquals('Yes', $answer->getValue()); + } + + public function testImportTicketDataBadgeFeatureRestrictedQuestionSameRowForExistingAttendee() + { + Queue::fake(); + + $feature = new SummitBadgeFeatureType(); + $feature->setName('VIP'); + self::$summit->addFeatureType($feature); + + $question = $this->insertOrderExtraQuestion('VIP Perk Choice'); + $question->addAllowedBadgeFeatureType($feature); + self::$em->persist(self::$summit); + self::$em->flush(); + + // existing attendee: the badge-features result cache gets warmed pre-grant earlier in the + // same row ( updateStatus during the reassign path ), so this exercises the cache eviction + $attendee = $this->getDefaultAttendee(); + $ticket = $attendee->getTickets()->first(); + + $csv_content = <<getNumber()},{$attendee->getEmail()},BADGE TYPE1,1,Yes +CSV; + + $service = $this->buildTicketDataImportService($csv_content); + $service->processTicketData(self::$summit->getId(), 'tickets.csv'); + + $answer = $attendee->getExtraQuestionAnswerByQuestion($question); + $this->assertNotNull($answer); + $this->assertEquals('Yes', $answer->getValue()); + } + + public function testImportTicketDataSkipsOrderScopedExtraQuestion() + { + Queue::fake(); + + $question = $this->insertOrderExtraQuestion + ( + 'Billing Notes', + ExtraQuestionTypeConstants::TextQuestionType, + [], + SummitOrderExtraQuestionTypeConstants::OrderQuestionUsage + ); + + $ticket = $this->getUnassignedTicket(); + + $csv_content = <<getNumber()},new.attendee@nowhere.com,New,Attendee,Some Value +CSV; + + $service = $this->buildTicketDataImportService($csv_content); + $service->processTicketData(self::$summit->getId(), 'tickets.csv'); + + // row still processes ( attendee created, ticket assigned ), order-scoped question is skipped + $attendee = App::make(ISummitAttendeeRepository::class) + ->getBySummitAndEmail(self::$summit, 'new.attendee@nowhere.com'); + $this->assertNotNull($attendee); + $this->assertNull($attendee->getExtraQuestionAnswerByQuestion($question)); + $this->assertCount(0, $attendee->getExtraQuestionAnswers()); + } + + public function testImportTicketDataPreservesLockedAnswerWhenUpdatesDisallowed() + { + Queue::fake(); + + // no authenticated admin during a queued import, so with the summit setting off + // an existing non-empty answer can not be changed + self::$summit->setAllowUpdateAttendeeExtraQuestions(false); + self::$em->persist(self::$summit); + self::$em->flush(); + + $question = $this->insertOrderExtraQuestion('Dietary Requirements'); + $attendee = $this->getDefaultAttendee(); + $this->insertExtraQuestionAnswer($attendee, $question, 'Meat'); + + Cache::forget(sprintf("SummitAttendee.canChangeAnswerValue.%s", $attendee->getId())); + + $ticket = $attendee->getTickets()->first(); + + $csv_content = <<getNumber()},{$attendee->getEmail()},Vegan +CSV; + + $service = $this->buildTicketDataImportService($csv_content); + $service->processTicketData(self::$summit->getId(), 'tickets.csv'); + + // the locked answer is preserved, the change is skipped + $answer = $attendee->getExtraQuestionAnswerByQuestion($question); + $this->assertNotNull($answer); + $this->assertEquals('Meat', $answer->getValue()); + } + + public function testImportTicketDataCreatesBadgeWhenTicketHasNone() + { + Queue::fake(); + + // every fixture ticket type carries a badge type, so SummitTicketType::applyTo + // auto-creates a badge at setTicketType time — build a ticket from a type with + // no badge type to get a genuinely badge-less ticket + $ticket_type = new SummitTicketType(); + $ticket_type->setName('NO BADGE TICKET TYPE'); + $ticket_type->setCost(100); + $ticket_type->setCurrency('USD'); + $ticket_type->setQuantity2Sell(10); + $ticket_type->setAudience(SummitTicketType::Audience_All); + self::$summit->addTicketType($ticket_type); + + $order = new SummitOrder(); + $order->setOwner(self::$defaultMember); + $order->setSummit(self::$summit); + self::$summit->addOrder($order); + + $ticket = new SummitAttendeeTicket(); + $ticket->setTicketType($ticket_type); + $ticket->activate(); + $order->addTicket($ticket); + $order->setPaid(); + $order->generateNumber(); + $ticket->generateNumber(); + $ticket->generateQRCode(); + + self::$em->persist(self::$summit); + self::$em->flush(); + + $this->assertFalse($ticket->hasBadge()); + + $csv_content = <<getNumber()},new.attendee@nowhere.com,New,Attendee,BADGE TYPE1 +CSV; + + $service = $this->buildTicketDataImportService($csv_content); + $service->processTicketData(self::$summit->getId(), 'tickets.csv'); + + $this->assertTrue($ticket->hasBadge()); + $this->assertEquals('BADGE TYPE1', $ticket->getBadge()->getType()->getName()); + } } From 7b3ad5018f16a25a6d65409f00220f1c5fd3b78e Mon Sep 17 00:00:00 2001 From: JpMaxMan Date: Fri, 3 Jul 2026 16:41:44 -0500 Subject: [PATCH 5/7] test(registration): badge-less ticket requires a summit without a default badge type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SummitTicketType::getBadgeType falls back to the summit default badge type, so on the main fixture summit applyTo always builds a badge and a badge-less ticket cannot be constructed. Build the scenario on the second fixture summit, which has no badge types — the real case the badge creation fix covers. Co-Authored-By: Claude Fable 5 --- tests/SummitOrderServiceTest.php | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/tests/SummitOrderServiceTest.php b/tests/SummitOrderServiceTest.php index b72440656..a15f30c99 100644 --- a/tests/SummitOrderServiceTest.php +++ b/tests/SummitOrderServiceTest.php @@ -47,6 +47,7 @@ use models\summit\SummitAttendee; use models\summit\SummitAttendeeTicket; use models\summit\SummitBadgeFeatureType; +use models\summit\SummitBadgeType; use models\summit\SummitOrder; use models\summit\SummitOrderExtraQuestionAnswer; use models\summit\SummitOrderExtraQuestionType; @@ -820,21 +821,27 @@ public function testImportTicketDataCreatesBadgeWhenTicketHasNone() { Queue::fake(); - // every fixture ticket type carries a badge type, so SummitTicketType::applyTo - // auto-creates a badge at setTicketType time — build a ticket from a type with - // no badge type to get a genuinely badge-less ticket + // on a summit with a default badge type a badge-less ticket can not exist: + // SummitTicketType::getBadgeType falls back to the summit default, so applyTo + // always builds a badge at setTicketType time. Use the second fixture summit + // ( no badge types, hence no default ) — the real world case this covers. + $badge_type = new SummitBadgeType(); + $badge_type->setName('VIP BADGE'); + $badge_type->setDescription('VIP BADGE'); + self::$summit2->addBadgeType($badge_type); // deliberately NOT the default + $ticket_type = new SummitTicketType(); $ticket_type->setName('NO BADGE TICKET TYPE'); $ticket_type->setCost(100); $ticket_type->setCurrency('USD'); $ticket_type->setQuantity2Sell(10); $ticket_type->setAudience(SummitTicketType::Audience_All); - self::$summit->addTicketType($ticket_type); + self::$summit2->addTicketType($ticket_type); $order = new SummitOrder(); $order->setOwner(self::$defaultMember); - $order->setSummit(self::$summit); - self::$summit->addOrder($order); + $order->setSummit(self::$summit2); + self::$summit2->addOrder($order); $ticket = new SummitAttendeeTicket(); $ticket->setTicketType($ticket_type); @@ -845,20 +852,20 @@ public function testImportTicketDataCreatesBadgeWhenTicketHasNone() $ticket->generateNumber(); $ticket->generateQRCode(); - self::$em->persist(self::$summit); + self::$em->persist(self::$summit2); self::$em->flush(); $this->assertFalse($ticket->hasBadge()); $csv_content = <<getNumber()},new.attendee@nowhere.com,New,Attendee,BADGE TYPE1 +{$ticket->getNumber()},new.attendee@nowhere.com,New,Attendee,VIP BADGE CSV; $service = $this->buildTicketDataImportService($csv_content); - $service->processTicketData(self::$summit->getId(), 'tickets.csv'); + $service->processTicketData(self::$summit2->getId(), 'tickets.csv'); $this->assertTrue($ticket->hasBadge()); - $this->assertEquals('BADGE TYPE1', $ticket->getBadge()->getType()->getName()); + $this->assertEquals('VIP BADGE', $ticket->getBadge()->getType()->getName()); } } From 670a9c72d08767702550c5697ac2440e2d6d9d39 Mon Sep 17 00:00:00 2001 From: JpMaxMan Date: Fri, 3 Jul 2026 18:09:24 -0500 Subject: [PATCH 6/7] test(registration): set support email on summit2 fixture path SummitAttendeeTicketEmail's constructor requires the summit support email; the second fixture summit never sets one, so the ticket reassignment path threw during the badge-less import test. Co-Authored-By: Claude Fable 5 --- tests/SummitOrderServiceTest.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/SummitOrderServiceTest.php b/tests/SummitOrderServiceTest.php index a15f30c99..330d6884a 100644 --- a/tests/SummitOrderServiceTest.php +++ b/tests/SummitOrderServiceTest.php @@ -825,6 +825,11 @@ public function testImportTicketDataCreatesBadgeWhenTicketHasNone() // SummitTicketType::getBadgeType falls back to the summit default, so applyTo // always builds a badge at setTicketType time. Use the second fixture summit // ( no badge types, hence no default ) — the real world case this covers. + + // the ticket-assignment email dispatched on the reassign path hard-requires a + // support email on the summit; summit2's fixture does not set one + self::$summit2->setSupportEmail('summit2@test.com'); + $badge_type = new SummitBadgeType(); $badge_type->setName('VIP BADGE'); $badge_type->setDescription('VIP BADGE'); From d55df72b07ecbd254cb2a9e88424015e5ac4f118 Mon Sep 17 00:00:00 2001 From: JpMaxMan Date: Mon, 6 Jul 2026 12:23:49 -0500 Subject: [PATCH 7/7] fix(registration): dedupe resolved list-question value ids on ticket import Duplicated CSV tokens (drag-fill artifacts, or the same choice spelled as name/label/raw id) resolve to the same value id: they falsely tripped the single-value guard on radio/combo questions and persisted duplicated id strings for CheckBoxList, which also resisted later correction under the answer-change lock. Adds both regression tests from review. Co-Authored-By: Claude Fable 5 --- app/Services/Model/Imp/SummitOrderService.php | 5 ++ tests/SummitOrderServiceTest.php | 64 +++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/app/Services/Model/Imp/SummitOrderService.php b/app/Services/Model/Imp/SummitOrderService.php index 790fc7c72..2a1d27b64 100644 --- a/app/Services/Model/Imp/SummitOrderService.php +++ b/app/Services/Model/Imp/SummitOrderService.php @@ -4889,6 +4889,11 @@ private function resolveListQuestionValue(SummitOrderExtraQuestionType $question $value_ids[] = $question_value->getId(); } + // duplicated tokens ( spreadsheet drag-fill artifacts, or the same choice spelled as + // name / label / raw id ) resolve to the same value id — dedupe before the single-value + // guard and before building the stored form + $value_ids = array_values(array_unique($value_ids)); + if (count($value_ids) === 0) return null; // only CheckBoxList questions admit multiple selected values diff --git a/tests/SummitOrderServiceTest.php b/tests/SummitOrderServiceTest.php index 330d6884a..daf42f230 100644 --- a/tests/SummitOrderServiceTest.php +++ b/tests/SummitOrderServiceTest.php @@ -635,6 +635,70 @@ public function testImportTicketDataListQuestionStoresValueIds() $this->assertEquals(implode(',', $expected_value_ids), $answer->getValue()); } + public function testImportTicketDataDuplicatedTokenOnSingleValueQuestionIsAccepted() + { + Queue::fake(); + + $question = $this->insertOrderExtraQuestion + ( + 'Meal Preference', + ExtraQuestionTypeConstants::RadioButtonListQuestionType, + ['Vegan', 'Meat'] + ); + + $ticket = $this->getUnassignedTicket(); + + // duplicated token ( e.g. spreadsheet drag-fill artifact ): one distinct value selected, + // must not be rejected by the single-value guard + $csv_content = <<getNumber()},new.attendee@nowhere.com,New,Attendee,Vegan|Vegan +CSV; + + $service = $this->buildTicketDataImportService($csv_content); + $service->processTicketData(self::$summit->getId(), 'tickets.csv'); + + $attendee = App::make(ISummitAttendeeRepository::class) + ->getBySummitAndEmail(self::$summit, 'new.attendee@nowhere.com'); + $this->assertNotNull($attendee); + + $answer = $attendee->getExtraQuestionAnswerByQuestion($question); + $this->assertNotNull($answer); + $this->assertEquals(strval($question->getValueByName('Vegan')->getId()), $answer->getValue()); + } + + public function testImportTicketDataDuplicatedTokenOnCheckBoxListStoresUniqueValueIds() + { + Queue::fake(); + + $question = $this->insertOrderExtraQuestion + ( + 'T-Shirt Size', + ExtraQuestionTypeConstants::CheckBoxListQuestionType, + ['Small', 'Large'] + ); + + $ticket = $this->getUnassignedTicket(); + + $csv_content = <<getNumber()},new.attendee@nowhere.com,New,Attendee,Large|Large +CSV; + + $service = $this->buildTicketDataImportService($csv_content); + $service->processTicketData(self::$summit->getId(), 'tickets.csv'); + + $attendee = App::make(ISummitAttendeeRepository::class) + ->getBySummitAndEmail(self::$summit, 'new.attendee@nowhere.com'); + $this->assertNotNull($attendee); + + $answer = $attendee->getExtraQuestionAnswerByQuestion($question); + $this->assertNotNull($answer); + // stored form is unique value ids: "5,5" would render duplicated labels ( getNiceValue ) + // and resist later correction under the answer-change lock ( "5,5" != "5" reads as a change ) + $this->assertEquals(strval($question->getValueByName('Large')->getId()), $answer->getValue()); + } + public function testImportTicketDataBadgeFeaturesStillClearedAndReSet() { Queue::fake();