Skip to content

Commit 8843b6c

Browse files
committed
fix: refuse an answer submitted to an image or video block
A display-only block was skipped during validation whether or not an answer came with it, so a hand-made request could store an answer against a block that has nowhere to show it. An absent answer is still expected -- and still must not count as a missing required one -- but a present answer is now refused. The submit view never sends one for these blocks, since they emit no value, so this changes nothing for a form filled in normally. Adds tests for both halves, and for the image and video settings accepted by areExtraSettingsValid, which were not exercised before. Signed-off-by: global-prog <raqeeb@uosamarra.edu.iq>
1 parent e52688b commit 8843b6c

3 files changed

Lines changed: 86 additions & 2 deletions

File tree

lib/Service/SubmissionService.php

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -577,9 +577,14 @@ public function validateSubmission(array $questions, array $answers, string $for
577577
$questionId = $question['id'];
578578
$questionAnswered = array_key_exists($questionId, $answers);
579579

580-
// Display-only blocks are never answered, so they must not be treated as an
581-
// unanswered mandatory question.
580+
// Display-only blocks take no answer. An absent answer is therefore expected and
581+
// must not count as an unanswered mandatory question -- but a present one is
582+
// refused outright rather than skipped, since nothing else would stop it being
583+
// stored against a block that has nowhere to show it.
582584
if (in_array($question['type'], Constants::ANSWER_TYPES_DISPLAY_ONLY, true)) {
585+
if ($questionAnswered) {
586+
throw new \InvalidArgumentException(sprintf('Question "%s" does not take an answer.', $question['text']));
587+
}
583588
continue;
584589
}
585590

tests/Unit/Service/FormsServiceTest.php

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1392,6 +1392,37 @@ public function testAreExtraSettingsValid(array $extraSettings, string $question
13921392

13931393
public static function dataAreExtraSettingsValid() {
13941394
return [
1395+
'valid-image-settings' => [
1396+
'extraSettings' => [
1397+
'url' => 'https://example.com/picture.png',
1398+
'alt' => 'A picture',
1399+
],
1400+
'questionType' => Constants::ANSWER_TYPE_IMAGE,
1401+
'expected' => true
1402+
],
1403+
'valid-video-settings' => [
1404+
'extraSettings' => [
1405+
'url' => 'https://example.com/clip',
1406+
],
1407+
'questionType' => Constants::ANSWER_TYPE_VIDEO,
1408+
'expected' => true
1409+
],
1410+
'invalid-image-key' => [
1411+
// A block has no answer, so the answer-shaping settings of other types
1412+
// must not be accepted on it.
1413+
'extraSettings' => [
1414+
'shuffleOptions' => true,
1415+
],
1416+
'questionType' => Constants::ANSWER_TYPE_IMAGE,
1417+
'expected' => false
1418+
],
1419+
'invalid-video-type' => [
1420+
'extraSettings' => [
1421+
'url' => ['not', 'a', 'string'],
1422+
],
1423+
'questionType' => Constants::ANSWER_TYPE_VIDEO,
1424+
'expected' => false
1425+
],
13951426
'empty-extra-settings' => [
13961427
'extraSettings' => [],
13971428
'questionType' => Constants::ANSWER_TYPE_LONG,

tests/Unit/Service/SubmissionServiceTest.php

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -805,6 +805,54 @@ private function setUpCsvTest(array $questions, array $submissions, string $csvT
805805
// Data for validation of Submissions
806806
public static function dataValidateSubmission() {
807807
return [
808+
'display-only-block-not-answered' => [
809+
// Questions
810+
[
811+
['id' => 1, 'type' => 'image', 'text' => 'picture', 'isRequired' => false],
812+
['id' => 2, 'type' => 'short', 'text' => 'q2', 'isRequired' => true],
813+
],
814+
// Answers
815+
[
816+
'2' => ['answer'],
817+
],
818+
// Expected Result
819+
null,
820+
],
821+
'display-only-block-marked-required' => [
822+
// Questions -- a block can never be answered, so a stray required flag must
823+
// not make the whole form impossible to submit.
824+
[
825+
['id' => 1, 'type' => 'video', 'text' => 'clip', 'isRequired' => true],
826+
],
827+
// Answers
828+
[],
829+
// Expected Result
830+
null,
831+
],
832+
'display-only-image-answered' => [
833+
// Questions
834+
[
835+
['id' => 1, 'type' => 'image', 'text' => 'picture', 'isRequired' => false],
836+
],
837+
// Answers
838+
[
839+
'1' => ['anything'],
840+
],
841+
// Expected Result
842+
'Question "picture" does not take an answer.',
843+
],
844+
'display-only-video-answered' => [
845+
// Questions
846+
[
847+
['id' => 1, 'type' => 'video', 'text' => 'clip', 'isRequired' => false],
848+
],
849+
// Answers
850+
[
851+
'1' => ['anything'],
852+
],
853+
// Expected Result
854+
'Question "clip" does not take an answer.',
855+
],
808856
'required-not-answered' => [
809857
// Questions
810858
[

0 commit comments

Comments
 (0)