fix(registration): vet question eligibility before ambiguous-column skip on ticket import

Author: JpMaxMan

app/Services/Model/Imp/SummitOrderService.php

@@ -4785,28 +4785,20 @@ private function resolveExtraQuestionColumns(Summit $summit, SummitAttendee $att
             }
 
             // match by name and by label ( the ticket csv export emits question labels as
-            // column names ); when the two lookups disagree the column is ambiguous — one
-            // question's name is another question's label — skip it rather than silently
-            // filing the answer under the wrong question
-            $question = $summit->getOrderExtraQuestionByName($question_name);
-            $question_by_label = $summit->getOrderExtraQuestionByLabel($question_name);
-            if (!is_null($question) && !is_null($question_by_label) && $question_by_label->getId() !== $question->getId()) {
-                Log::warning
-                (
-                    sprintf
-                    (
-                        "SummitOrderService::resolveExtraQuestionColumns column %s is ambiguous on summit %s ( name of question %s, label of question %s ), skipping it",
-                        $question_name,
-                        $summit->getId(),
-                        $question->getId(),
-                        $question_by_label->getId()
-                    )
-                );
-                continue;
-            }
-            if (is_null($question))
-                $question = $question_by_label;
-            if (is_null($question)) {
+            // column names ); each candidate is vetted for usage / attendee eligibility
+            // before deciding ambiguity, so a question that could never take this answer
+            // ( order scoped or not allowed for the attendee ) can not shadow a legitimate
+            // match; only when two eligible questions remain — one question's name is
+            // another question's label — is the column truly ambiguous and skipped, rather
+            // than silently filing the answer under the wrong question
+            $candidates = [];
+            foreach ([$summit->getOrderExtraQuestionByName($question_name),
+                      $summit->getOrderExtraQuestionByLabel($question_name)] as $candidate) {
+                if (is_null($candidate)) continue;
+                $candidates[$candidate->getId()] = $candidate;
+            }
+
+            if (count($candidates) === 0) {
                 Log::warning
                 (
                     sprintf
@@ -4819,31 +4811,55 @@ private function resolveExtraQuestionColumns(Summit $summit, SummitAttendee $att
                 continue;
             }
 
-            if ($question->getUsage() === SummitOrderExtraQuestionTypeConstants::OrderQuestionUsage) {
-                Log::warning
-                (
-                    sprintf
+            $eligible = [];
+            foreach ($candidates as $candidate) {
+                if ($candidate->getUsage() === SummitOrderExtraQuestionTypeConstants::OrderQuestionUsage) {
+                    Log::warning
                     (
-                        "SummitOrderService::resolveExtraQuestionColumns question %s is order scoped, can not be answered per attendee, skipping it",
-                        $question_name
-                    )
-                );
-                continue;
+                        sprintf
+                        (
+                            "SummitOrderService::resolveExtraQuestionColumns question %s ( column %s ) is order scoped, can not be answered per attendee, skipping it",
+                            $candidate->getId(),
+                            $question_name
+                        )
+                    );
+                    continue;
+                }
+                if (!$attendee->isAllowedQuestion($candidate)) {
+                    Log::warning
+                    (
+                        sprintf
+                        (
+                            "SummitOrderService::resolveExtraQuestionColumns question %s ( column %s ) is not allowed for attendee %s, skipping it",
+                            $candidate->getId(),
+                            $question_name,
+                            $attendee->getEmail()
+                        )
+                    );
+                    continue;
+                }
+                $eligible[$candidate->getId()] = $candidate;
             }
 
-            if (!$attendee->isAllowedQuestion($question)) {
+            if (count($eligible) === 0) continue;
+
+            if (count($eligible) > 1) {
                 Log::warning
                 (
                     sprintf
                     (
-                        "SummitOrderService::resolveExtraQuestionColumns question %s is not allowed for attendee %s, skipping it",
+                        "SummitOrderService::resolveExtraQuestionColumns column %s is ambiguous on summit %s ( name of question %s, label of question %s ), skipping it",
                         $question_name,
-                        $attendee->getEmail()
+                        $summit->getId(),
+                        array_keys($eligible)[0],
+                        array_keys($eligible)[1]
                     )
                 );
                 continue;
             }
 
+            $question = reset($eligible);
+
             if ($question->allowsValues()) {
                 $value = $this->resolveListQuestionValue($question, $value);
                 if (is_null($value)) continue;

tests/SummitOrderServiceTest.php

@@ -993,6 +993,49 @@ public function testImportTicketDataSkipsAmbiguousExtraQuestionColumn()
         $this->assertCount(0, $attendee->getExtraQuestionAnswers());
     }
 
+    public function testImportTicketDataIneligibleQuestionDoesNotMakeColumnAmbiguous()
+    {
+        Queue::fake();
+
+        // question A's NAME equals question B's LABEL, but B is order scoped so it was
+        // never a real candidate — the column must resolve to A, not be skipped as
+        // ambiguous ( ambiguity is decided among eligible questions only )
+        $question_a = $this->insertOrderExtraQuestion
+        (
+            'Shirt',
+            ExtraQuestionTypeConstants::TextQuestionType,
+            [],
+            SummitOrderExtraQuestionTypeConstants::TicketQuestionUsage,
+            'Question A Label'
+        );
+        $question_b = $this->insertOrderExtraQuestion
+        (
+            'B_INTERNAL_NAME',
+            ExtraQuestionTypeConstants::TextQuestionType,
+            [],
+            SummitOrderExtraQuestionTypeConstants::OrderQuestionUsage,
+            'Shirt'
+        );
+
+        $ticket = $this->getUnassignedTicket();
+
+        $csv_content = <<<CSV
+number,attendee_email,attendee_first_name,attendee_last_name,extra_question:Shirt
+{$ticket->getNumber()},new.attendee@nowhere.com,New,Attendee,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_a);
+        $this->assertNotNull($answer);
+        $this->assertEquals('Large', $answer->getValue());
+        $this->assertNull($attendee->getExtraQuestionAnswerByQuestion($question_b));
+    }
+
     public function testTicketCSVExportMultiValueAnswerRoundTripsThroughImport()
     {
         Queue::fake();