fix(sponsors): never reject badge scan uploads over mandatory questions and block deleting extra questions

app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSponsorApiController.php

@@ -3684,8 +3684,8 @@ public function updateExtraQuestion($summit_id, $sponsor_id, $extra_question_id)
 
     #[OA\Delete(
         path: "/api/v1/summits/{id}/sponsors/{sponsor_id}/extra-questions/{extra_question_id}",
-        description: "required-groups " . IGroup::SuperAdmins . ", " . IGroup::Administrators . ", " . IGroup::SummitAdministrators . ", " . IGroup::Sponsors . ", " . IGroup::SponsorExternalUsers,
-        summary: 'Delete Sponsor Extra Question',
+        summary: 'Delete Sponsor Extra Question (always refused)',
+        description: "Always refused with 412: devices may hold answers for this question that the server has not seen yet. Edit the wording or turn off Mandatory instead. required-groups " . IGroup::SuperAdmins . ", " . IGroup::Administrators . ", " . IGroup::SummitAdministrators . ", " . IGroup::Sponsors . ", " . IGroup::SponsorExternalUsers,
         operationId: 'deleteSponsorExtraQuestion',
         tags: ['Sponsors'],
         x: [
@@ -3730,8 +3730,8 @@ public function updateExtraQuestion($summit_id, $sponsor_id, $extra_question_id)
         ],
         responses: [
             new OA\Response(
-                response: Response::HTTP_NO_CONTENT,
-                description: 'Extra question deleted successfully'
+                response: Response::HTTP_PRECONDITION_FAILED,
+                description: "Validation Error - always returned: questions can't be deleted once created, because devices may already be using them"
             ),
             new OA\Response(response: Response::HTTP_UNAUTHORIZED, description: "Unauthorized"),
             new OA\Response(response: Response::HTTP_NOT_FOUND, description: "Not Found"),
@@ -3979,8 +3979,8 @@ function ($value_id, $payload, $summit, $sponsor_id,  $extra_question_id) {
     use ParametrizedDeleteEntity;
     #[OA\Delete(
         path: "/api/v1/summits/{id}/sponsors/{sponsor_id}/extra-questions/{extra_question_id}/values/{value_id}",
-        description: "required-groups " . IGroup::SuperAdmins . ", " . IGroup::Administrators . ", " . IGroup::SummitAdministrators . ", " . IGroup::Sponsors . ", " . IGroup::SponsorExternalUsers,
-        summary: 'Delete Extra Question Value',
+        description: "Always refused with 412: devices may hold answers for this option that the server has not seen yet. Edit the wording instead. required-groups " . IGroup::SuperAdmins . ", " . IGroup::Administrators . ", " . IGroup::SummitAdministrators . ", " . IGroup::Sponsors . ", " . IGroup::SponsorExternalUsers,
+        summary: 'Delete Extra Question Value (always refused)',
         operationId: 'deleteSponsorExtraQuestionValue',
         tags: ['Sponsors'],
         x: [
@@ -4032,8 +4032,8 @@ function ($value_id, $payload, $summit, $sponsor_id,  $extra_question_id) {
         ],
         responses: [
             new OA\Response(
-                response: Response::HTTP_NO_CONTENT,
-                description: 'Question value deleted successfully'
+                response: Response::HTTP_PRECONDITION_FAILED,
+                description: "Validation Error - always returned: answer options can't be deleted once created, because devices may already be using them"
             ),
             new OA\Response(response: Response::HTTP_UNAUTHORIZED, description: "Unauthorized"),
             new OA\Response(response: Response::HTTP_NOT_FOUND, description: "Not Found"),

app/Services/Model/Imp/SponsorUserInfoGrantService.php

@@ -408,12 +408,8 @@ private function addBadgeScanLocked(
                     // extra questions
                     $extra_questions = $data['extra_questions'] ?? [];
 
-                    if (count($extra_questions)) {
-                        $res = $scan->hadCompletedExtraQuestions($extra_questions);
-                        if (!$res) {
-                            throw new ValidationException("You neglected to fill in all mandatory questions for the badge scan.");
-                        }
-                    }
+                    if (count($extra_questions))
+                        $this->applyExtraQuestionAnswers($scan, $extra_questions, 'addBadgeScan');
 
                     return $scan;
                 });
@@ -477,7 +473,6 @@ private function addBadgeScanLocked(
      * @param SponsorBadgeScan $existing
      * @param array $data
      * @return SponsorBadgeScan
-     * @throws ValidationException
      */
     private function mergeRetryIntoExistingScan(SponsorBadgeScan $existing, array $data): SponsorBadgeScan
     {
@@ -486,12 +481,82 @@ private function mergeRetryIntoExistingScan(SponsorBadgeScan $existing, array $d
             $existing->setNotes($notes);
 
         $extra_questions = $data['extra_questions'] ?? [];
-        if (count($extra_questions) && !$existing->hadCompletedExtraQuestions($extra_questions))
-            throw new ValidationException("You neglected to fill in all mandatory questions for the badge scan.");
+        if (count($extra_questions))
+            $this->applyExtraQuestionAnswers($existing, $extra_questions, 'mergeRetryIntoExistingScan');
 
         return $existing;
     }
 
+    /**
+     * "Mandatory" and the allowed options are instructions to the device, not
+     * server rules: a device can hold answers captured under a question setup
+     * the server no longer has (e.g. a mandatory Text question switched to a
+     * ComboBox, so a free-text answer matches no option). Save whatever answers
+     * were sent and never reject the upload or roll the scan back over them.
+     *
+     * hadCompletedExtraQuestions adds every answer before validating them and
+     * nothing is flushed yet, so catching its ValidationException keeps both
+     * the scan and the answers.
+     * @param SponsorBadgeScan $scan
+     * @param array $extra_questions
+     * @param string $caller
+     */
+    private function applyExtraQuestionAnswers(SponsorBadgeScan $scan, array $extra_questions, string $caller): void
+    {
+        try {
+            if (!$scan->hadCompletedExtraQuestions($extra_questions))
+                Log::warning(
+                    sprintf(
+                        "SponsorUserInfoGrantService::%s badge scan was saved with missing mandatory answers - %s",
+                        $caller,
+                        $this->describeScanForLog($scan)
+                    )
+                );
+        }
+        catch (ValidationException $ex) {
+            Log::warning(
+                sprintf(
+                    "SponsorUserInfoGrantService::%s badge scan was saved with answers that do not match the question setup (%s) - %s",
+                    $caller,
+                    $ex->getMessage(),
+                    $this->describeScanForLog($scan)
+                )
+            );
+        }
+    }
+
+    /**
+     * Ids support needs to find a scan saved with incomplete answers. A new
+     * scan has no id until the transaction flushes (logged as 0); sponsor,
+     * badge, member and scan date identify it.
+     * @param SponsorBadgeScan $scan
+     * @return string
+     */
+    private function describeScanForLog(SponsorBadgeScan $scan): string
+    {
+        $answered = [];
+        foreach ($scan->getExtraQuestionAnswers() as $answer) {
+            if ($answer->hasValue())
+                $answered[$answer->getQuestionId()] = true;
+        }
+
+        $unanswered_mandatory = [];
+        foreach ($scan->getExtraQuestions() as $question) {
+            if ($question->isMandatory() && !isset($answered[$question->getId()]))
+                $unanswered_mandatory[] = $question->getId();
+        }
+
+        return sprintf(
+            "scan %s sponsor %s badge %s member %s scan_date %s unanswered mandatory questions [%s]",
+            $scan->getId(),
+            $scan->getSponsor()->getId(),
+            $scan->getBadge()->getId(),
+            $scan->getUser()->getId(),
+            $scan->getScanDate()->getTimestamp(),
+            implode(",", $unanswered_mandatory)
+        );
+    }
+
     /**
      * @param Summit $summit
      * @param Member $current_member
@@ -521,12 +586,8 @@ public function updateBadgeScan(Summit $summit, Member $current_member, int $sca
             // extra questions
             $extra_questions = $payload['extra_questions'] ?? [];
 
-            if (count($extra_questions)) {
-                $res = $scan->hadCompletedExtraQuestions($extra_questions);
-                if (!$res) {
-                    throw new ValidationException("You neglected to fill in all mandatory questions for the badge scan.");
-                }
-            }
+            if (count($extra_questions))
+                $this->applyExtraQuestionAnswers($scan, $extra_questions, 'updateBadgeScan');
 
             return $scan;
         });

app/Services/Model/Imp/SummitSponsorService.php

@@ -1084,18 +1084,10 @@ public function updateSponsorExtraQuestion(Summit $summit, int $sponsor_id, int
      */
     public function deleteSponsorExtraQuestion(Summit $summit, int $sponsor_id, int $extra_question_id): void
     {
-        $this->tx_service->transaction(function () use ($summit, $sponsor_id, $extra_question_id) {
-            $summit_sponsor = $summit->getSummitSponsorById($sponsor_id);
-            if (is_null($summit_sponsor))
-                throw new EntityNotFoundException("Sponsor not found.");
-
-            $extra_question = $summit_sponsor->getExtraQuestionById($extra_question_id);
-
-            if(!$extra_question instanceof SummitSponsorExtraQuestionType)
-                throw new EntityNotFoundException("Sponsor extra question not found.");
-
-            $summit_sponsor->removeExtraQuestion($extra_question);
-        });
+        // Always refused: a device may hold answers for this question that the server has not seen yet.
+        throw new ValidationException(
+            "Questions can't be deleted once created, because devices may already be using them. You can edit the wording, turn off Mandatory, or contact support."
+        );
     }
 
     /**
@@ -1160,19 +1152,10 @@ public function updateExtraQuestionValue(Summit $summit, int $sponsor_id, int $q
      */
     public function deleteExtraQuestionValue(Summit $summit, int $sponsor_id, int $question_id, int $value_id): void
     {
-        $this->tx_service->transaction(function () use ($summit, $sponsor_id, $question_id, $value_id) {
-            $summit_sponsor = $summit->getSummitSponsorById($sponsor_id);
-            if (is_null($summit_sponsor))
-                throw new EntityNotFoundException("Sponsor not found.");
-
-            $extra_question = $summit_sponsor->getExtraQuestionById($question_id);
-
-            if(!$extra_question instanceof SummitSponsorExtraQuestionType)
-                throw new EntityNotFoundException("Sponsor extra question not found.");
-
-
-            parent::_deleteExtraQuestionValue($extra_question, $value_id);
-        });
+        // Always refused: a device may hold answers for this option that the server has not seen yet.
+        throw new ValidationException(
+            "Answer options can't be deleted once created, because devices may already be using them. You can edit the wording, or contact support."
+        );
     }
 
     /**

tests/oauth2/OAuth2SummitBadgeScanApiControllerTest.php

@@ -12,6 +12,8 @@
  * limitations under the License.
  **/
 
+use App\Models\Foundation\ExtraQuestions\ExtraQuestionTypeConstants;
+use App\Models\Foundation\ExtraQuestions\ExtraQuestionTypeValue;
 use App\Models\Foundation\Main\IGroup;
 use App\Models\Foundation\Summit\ExtraQuestions\SummitSponsorExtraQuestionType;
 use App\Models\Foundation\Summit\Repositories\ISponsorRepository;
@@ -1493,4 +1495,158 @@ public function testAddBadgeScanUniqueViolationAppliesNotesToTheWinningScan(){
         $this->assertEquals('hot lead, follow up', $this->reloadScan($winner_id)->getNotes(),
             "notes of the request that lost the race must be persisted on the winning scan");
     }
+
+    /**
+     * "Mandatory" is an instruction to the device, not a server rule: sets the
+     * sponsor's first two questions as mandatory and returns their ids.
+     * @return array
+     */
+    private function makeFirstTwoSponsorQuestionsMandatory(): array
+    {
+        $sponsor = self::$summit->getSummitSponsors()[0];
+        $ids = [];
+        foreach ([0, 1] as $idx) {
+            $question = $sponsor->getExtraQuestions()[$idx];
+            if (!$question instanceof SummitSponsorExtraQuestionType) self::fail();
+            $question->setMandatory(true);
+            $ids[] = $question->getId();
+        }
+        self::$em->persist($sponsor);
+        self::$em->flush();
+        return $ids;
+    }
+
+    /**
+     * A scan captured offline and answered before reconnecting reaches the
+     * server as a first POST that carries answers. A mandatory question left
+     * unanswered must neither reject it nor roll the scan back.
+     */
+    public function testAddBadgeScanWithMissingMandatoryAnswerIsSavedWithItsOtherAnswers(){
+        [$answered_id, ] = $this->makeFirstTwoSponsorQuestionsMandatory();
+
+        $service = App::make(ISponsorUserInfoGrantService::class);
+        $scan = $service->addBadgeScan(self::$summit, self::$member, $this->badgeScanPayload([
+            'extra_questions' => [
+                ['question_id' => $answered_id, 'answer' => 'None'],
+            ],
+        ]));
+
+        $answers = $this->reloadScan($scan->getId())->getExtraQuestionAnswers();
+        $this->assertCount(1, $answers);
+        $this->assertEquals($answered_id, $answers->first()->getQuestionId());
+    }
+
+    public function testAddBadgeScanRetryWithMissingMandatoryAnswerIsSaved(){
+        [$answered_id, ] = $this->makeFirstTwoSponsorQuestionsMandatory();
+
+        $service = App::make(ISponsorUserInfoGrantService::class);
+        $first = $service->addBadgeScan(self::$summit, self::$member, $this->badgeScanPayload());
+        $first_id = $first->getId();
+
+        $service->addBadgeScan(self::$summit, self::$member, $this->badgeScanPayload([
+            'extra_questions' => [
+                ['question_id' => $answered_id, 'answer' => 'None'],
+            ],
+        ]));
+
+        $answers = $this->reloadScan($first_id)->getExtraQuestionAnswers();
+        $this->assertCount(1, $answers);
+        $this->assertEquals($answered_id, $answers->first()->getQuestionId());
+    }
+
+    public function testUpdateBadgeScanWithMissingMandatoryAnswerIsSaved(){
+        [$answered_id, ] = $this->makeFirstTwoSponsorQuestionsMandatory();
+
+        $service = App::make(ISponsorUserInfoGrantService::class);
+        $scan = $service->addBadgeScan(self::$summit, self::$member, $this->badgeScanPayload());
+        $scan_id = $scan->getId();
+
+        $service->updateBadgeScan(self::$summit, self::$member, $scan_id, [
+            'extra_questions' => [
+                ['question_id' => $answered_id, 'answer' => 'None'],
+            ],
+        ]);
+
+        $answers = $this->reloadScan($scan_id)->getExtraQuestionAnswers();
+        $this->assertCount(1, $answers);
+        $this->assertEquals($answered_id, $answers->first()->getQuestionId());
+    }
+
+    /**
+     * The sponsor switches the first question (Text) to a mandatory ComboBox
+     * while devices still hold free-text answers for it. Returns its id.
+     * @return int
+     */
+    private function switchFirstSponsorQuestionToMandatoryComboBox(): int
+    {
+        $sponsor = self::$summit->getSummitSponsors()[0];
+        $question = $sponsor->getExtraQuestions()[0];
+        if (!$question instanceof SummitSponsorExtraQuestionType) self::fail();
+        $question->setType(ExtraQuestionTypeConstants::ComboBoxQuestionType);
+        $question->setMandatory(true);
+        $value = new ExtraQuestionTypeValue();
+        $value->setLabel('Yes');
+        $value->setValue('Yes');
+        $question->addValue($value);
+        self::$em->persist($sponsor);
+        self::$em->flush();
+        return $question->getId();
+    }
+
+    /**
+     * A free-text answer ("Yes") matches no option id of the ComboBox, which
+     * makes the question validation throw; it must neither reject the upload
+     * nor roll the scan back.
+     */
+    public function testAddBadgeScanWithAnswerNotMatchingTheQuestionOptionsIsSaved(){
+        $question_id = $this->switchFirstSponsorQuestionToMandatoryComboBox();
+
+        $service = App::make(ISponsorUserInfoGrantService::class);
+        $scan = $service->addBadgeScan(self::$summit, self::$member, $this->badgeScanPayload([
+            'extra_questions' => [
+                ['question_id' => $question_id, 'answer' => 'Yes'],
+            ],
+        ]));
+
+        $answers = $this->reloadScan($scan->getId())->getExtraQuestionAnswers();
+        $this->assertCount(1, $answers);
+        $this->assertEquals($question_id, $answers->first()->getQuestionId());
+        $this->assertEquals('Yes', $answers->first()->getValue());
+    }
+
+    public function testAddBadgeScanRetryWithAnswerNotMatchingTheQuestionOptionsIsSaved(){
+        $question_id = $this->switchFirstSponsorQuestionToMandatoryComboBox();
+
+        $service = App::make(ISponsorUserInfoGrantService::class);
+        $first = $service->addBadgeScan(self::$summit, self::$member, $this->badgeScanPayload());
+        $first_id = $first->getId();
+
+        $service->addBadgeScan(self::$summit, self::$member, $this->badgeScanPayload([
+            'extra_questions' => [
+                ['question_id' => $question_id, 'answer' => 'Yes'],
+            ],
+        ]));
+
+        $answers = $this->reloadScan($first_id)->getExtraQuestionAnswers();
+        $this->assertCount(1, $answers);
+        $this->assertEquals('Yes', $answers->first()->getValue());
+    }
+
+    public function testUpdateBadgeScanWithAnswerNotMatchingTheQuestionOptionsIsSaved(){
+        $question_id = $this->switchFirstSponsorQuestionToMandatoryComboBox();
+
+        $service = App::make(ISponsorUserInfoGrantService::class);
+        $scan = $service->addBadgeScan(self::$summit, self::$member, $this->badgeScanPayload());
+        $scan_id = $scan->getId();
+
+        $service->updateBadgeScan(self::$summit, self::$member, $scan_id, [
+            'extra_questions' => [
+                ['question_id' => $question_id, 'answer' => 'Yes'],
+            ],
+        ]);
+
+        $answers = $this->reloadScan($scan_id)->getExtraQuestionAnswers();
+        $this->assertCount(1, $answers);
+        $this->assertEquals('Yes', $answers->first()->getValue());
+    }
 }