diff --git a/question/type/ddimageortext/edit_ddimageortext_form.php b/question/type/ddimageortext/edit_ddimageortext_form.php index 1c364f2305f..21a2f46cb17 100644 --- a/question/type/ddimageortext/edit_ddimageortext_form.php +++ b/question/type/ddimageortext/edit_ddimageortext_form.php @@ -205,9 +205,10 @@ class qtype_ddimageortext_edit_form extends qtype_ddtoimage_edit_form_base { public function validation($data, $files) { $errors = parent::validation($data, $files); if (!self::file_uploaded($data['bgimage'])) { - $errors["bgimage"] = get_string('formerror_nobgimage', 'qtype_'.$this->qtype()); + $errors['bgimage'] = get_string('formerror_nobgimage', 'qtype_'.$this->qtype()); } + $dropfound = false; $allchoices = array(); for ($i = 0; $i < $data['nodropzone']; $i++) { $ytoppresent = (trim($data['drops'][$i]['ytop']) !== ''); @@ -219,6 +220,7 @@ class qtype_ddimageortext_edit_form extends qtype_ddtoimage_edit_form_base { $imagechoicepresent = ($choice !== '0'); if ($imagechoicepresent) { + $dropfound = true; if (!$ytoppresent) { $errors["drops[$i]"] = get_string('formerror_noytop', 'qtype_ddimageortext'); } else if (!$ytopisint) { @@ -247,25 +249,41 @@ class qtype_ddimageortext_edit_form extends qtype_ddtoimage_edit_form_base { $allchoices[$choice] = $i; } else { if ($ytoppresent || $xleftpresent || $labelpresent) { + $dropfound = true; $errors["drops[$i]"] = get_string('formerror_noimageselected', 'qtype_ddimageortext'); } } } + if (!$dropfound) { + $errors['drops[0]'] = get_string('formerror_droprequired', 'qtype_ddimageortext'); + } + + $dragfound = false; for ($dragindex = 0; $dragindex < $data['noitems']; $dragindex++) { $label = $data['draglabel'][$dragindex]; if ($data['drags'][$dragindex]['dragitemtype'] == 'word') { + if ($label !== '') { + $dragfound = true; + } $allowedtags = '
'; $errormessage = get_string('formerror_disallowedtags', 'qtype_ddimageortext', s($allowedtags)); } else { + if (self::file_uploaded($data['dragitem'][$dragindex])) { + $dragfound = true; + } $allowedtags = ''; $errormessage = get_string('formerror_noallowedtags', 'qtype_ddimageortext'); } if ($label != strip_tags($label, $allowedtags)) { - $errors["drags[{$dragindex}]"] = $errormessage; + $errors["draglabel[{$dragindex}]"] = $errormessage; } } + if (!$dragfound) { + $errors['drags[0]'] = get_string('formerror_dragrequired', 'qtype_ddimageortext'); + } + return $errors; } } diff --git a/question/type/ddimageortext/lang/en/qtype_ddimageortext.php b/question/type/ddimageortext/lang/en/qtype_ddimageortext.php index 681281555a7..b55c0494a6c 100644 --- a/question/type/ddimageortext/lang/en/qtype_ddimageortext.php +++ b/question/type/ddimageortext/lang/en/qtype_ddimageortext.php @@ -39,6 +39,8 @@ $string['dropbackground'] = 'Background image for dragging markers onto'; $string['dropzone'] = 'Drop zone {$a}'; $string['dropzoneheader'] = 'Drop zones'; $string['formerror_disallowedtags'] = 'Only "{$a}" tags are allowed in this draggable text.'; +$string['formerror_dragrequired'] = 'You must add at least one draggable item to this question.'; +$string['formerror_droprequired'] = 'You must define at least one drop zone for this question.'; $string['formerror_noallowedtags'] = 'HTML tags are not allowed in this text which is the alt text for a draggable image.'; $string['formerror_noytop'] = 'You must provide a value for the y coordinate for the top left corner of this drop area. You can drag and drop the drop area above to set the coordinates or enter them manually here.'; $string['formerror_noxleft'] = 'You must provide a value for the x coordinate for the top left corner of this drop area. You can drag and drop the drop area above to set the coordinates or enter them manually here.'; diff --git a/question/type/ddimageortext/questionbase.php b/question/type/ddimageortext/questionbase.php index 4754bf5760c..499f2c17195 100644 --- a/question/type/ddimageortext/questionbase.php +++ b/question/type/ddimageortext/questionbase.php @@ -138,8 +138,13 @@ class qtype_ddtoimage_question_base extends qtype_gapselect_question_base { } public function get_random_guess_score() { - $accum = 0; + if (empty($this->places)) { + // Having no places would be nonsensical. However, it used to be possible + // to create questions like that, so avoid errors in this case. + return null; + } + $accum = 0; foreach ($this->places as $place) { foreach ($this->choices[$place->group] as $choice) { if ($choice->infinite) { diff --git a/question/type/ddimageortext/tests/behat/add.feature b/question/type/ddimageortext/tests/behat/add.feature index 1aa0de85545..c948c845b20 100644 --- a/question/type/ddimageortext/tests/behat/add.feature +++ b/question/type/ddimageortext/tests/behat/add.feature @@ -99,3 +99,16 @@ Feature: Test creating a drag and drop onto image question And I click on "Add" "button" in the "Choose a question type to add" "dialogue" And the following fields match these values: | id_shuffleanswers | 1 | + + @javascript @_file_upload + Scenario: Question must have at least one drag item and one drop zone + When I am on the "Course 1" "core_question > course question bank" page logged in as teacher + And I press "Create a new question ..." + And I set the field "Drag and drop onto image" to "1" + And I click on "Add" "button" in the "Choose a question type to add" "dialogue" + And I set the field "Question name" to "Test question" + And I set the field "Question text" to "Identify the features in this cross-section." + And I upload "question/type/ddimageortext/tests/fixtures/oceanfloorbase.jpg" file to "Background image" filemanager + And I press "Save changes" + Then I should see "You must add at least one draggable item to this question." + And I should see "You must define at least one drop zone for this question." diff --git a/question/type/ddimageortext/tests/form/edit_form_test.php b/question/type/ddimageortext/tests/form/edit_form_test.php index f5eebe03aec..920ee99012d 100644 --- a/question/type/ddimageortext/tests/form/edit_form_test.php +++ b/question/type/ddimageortext/tests/form/edit_form_test.php @@ -83,6 +83,13 @@ class edit_form_test extends \advanced_testcase { ['dragitemtype' => 'word'], ['dragitemtype' => 'word'], ], + 'dragitem' => [ + 0, + 0, + 0, + 0, + 0, + ], 'draglabel' => [ 'frog', 'toad', @@ -94,12 +101,12 @@ class edit_form_test extends \advanced_testcase { $errors = $form->validation($submitteddata, []); - $this->assertArrayNotHasKey('drags[0]', $errors); + $this->assertArrayNotHasKey('draglabel[0]', $errors); $this->assertEquals('HTML tags are not allowed in this text which is the alt text for a draggable image.', - $errors['drags[1]']); - $this->assertArrayNotHasKey('drags[2]', $errors); - $this->assertArrayNotHasKey('drags[3]', $errors); + $errors['draglabel[1]']); + $this->assertArrayNotHasKey('draglabel[2]', $errors); + $this->assertArrayNotHasKey('draglabel[3]', $errors); $this->assertEquals('Only "<br><sub><sup><b><i><strong><em><span>" ' . - 'tags are allowed in this draggable text.', $errors['drags[4]']); + 'tags are allowed in this draggable text.', $errors['draglabel[4]']); } } diff --git a/question/type/ddimageortext/tests/question_test.php b/question/type/ddimageortext/tests/question_test.php index 0b0af344833..bf7f521ad07 100644 --- a/question/type/ddimageortext/tests/question_test.php +++ b/question/type/ddimageortext/tests/question_test.php @@ -33,6 +33,7 @@ require_once($CFG->dirroot . '/question/type/ddimageortext/tests/helper.php'); * @package qtype_ddimageortext * @copyright 2009 The Open University * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers qtype_ddimageortext_question */ class question_test extends \basic_testcase { @@ -91,6 +92,17 @@ class question_test extends \basic_testcase { $this->assertEquals(0.5, $dd->get_random_guess_score()); } + public function test_get_random_guess_score_broken_question() { + // Before MDL-76298 was fixed, it was possible to create a question with + // no drop zones, and that caused a fatal division by zero error. Because + // people might have questions like that in their database, we need to + // check the calculation is robust to it. + /** @var \qtype_ddimageortext_question $dd */ + $dd = \test_question_maker::make_question('ddimageortext'); + $dd->places = []; + $this->assertNull($dd->get_random_guess_score()); + } + public function test_get_right_choice_for() { $dd = \test_question_maker::make_question('ddimageortext'); $dd->shufflechoices = false; diff --git a/question/type/ddmarker/edit_ddmarker_form.php b/question/type/ddmarker/edit_ddmarker_form.php index a30c81d87ef..e6cd7d0682d 100644 --- a/question/type/ddmarker/edit_ddmarker_form.php +++ b/question/type/ddmarker/edit_ddmarker_form.php @@ -215,11 +215,13 @@ class qtype_ddmarker_edit_form extends qtype_ddtoimage_edit_form_base { $errors["bgimage"] = get_string('formerror_nobgimage', 'qtype_ddmarker'); } + $dropfound = false; for ($i = 0; $i < $data['nodropzone']; $i++) { $choice = $data['drops'][$i]['choice']; $choicepresent = ($choice !== '0'); if ($choicepresent) { + $dropfound = true; // Test coords here. if ($bgimagesize !== null) { $shape = $data['drops'][$i]['shape']; @@ -236,20 +238,32 @@ class qtype_ddmarker_edit_form extends qtype_ddtoimage_edit_form_base { } } else { if (trim($data['drops'][$i]['coords']) !== '') { + $dropfound = true; $errorcode = 'noitemselected'; $errors["drops[{$i}]"] = get_string('formerror_'.$errorcode, 'qtype_ddmarker'); } } - } + if (!$dropfound) { + $errors['drops[0]'] = get_string('formerror_droprequired', 'qtype_ddmarker'); + } + + $markerfound = false; for ($dragindex = 0; $dragindex < $data['noitems']; $dragindex++) { $label = $data['drags'][$dragindex]['label']; + if ($label !== '') { + $markerfound = true; + } if ($label != strip_tags($label, QTYPE_DDMARKER_ALLOWED_TAGS_IN_MARKER)) { $errors["drags[{$dragindex}]"] = get_string('formerror_onlysometagsallowed', 'qtype_ddmarker', s(QTYPE_DDMARKER_ALLOWED_TAGS_IN_MARKER)); } } + if (!$markerfound) { + $errors['drags[0]'] = get_string('formerror_dragrequired', 'qtype_ddmarker'); + } + return $errors; } diff --git a/question/type/ddmarker/lang/en/qtype_ddmarker.php b/question/type/ddmarker/lang/en/qtype_ddmarker.php index e1461580955..679cbfa181c 100644 --- a/question/type/ddmarker/lang/en/qtype_ddmarker.php +++ b/question/type/ddmarker/lang/en/qtype_ddmarker.php @@ -53,6 +53,8 @@ For information the three shapes use coordinates in this way: Selecting a Marker text will add that text to the shape in the preview.'; $string['followingarewrong'] = 'The following markers have been placed in the wrong area : {$a}.'; $string['followingarewrongandhighlighted'] = 'The following markers were incorrectly placed : {$a}. Highlighted marker(s) are now shown with the correct placement(s).
Click on the marker to highlight the allowed area.'; +$string['formerror_dragrequired'] = 'You must add at least one marker to this question.'; +$string['formerror_droprequired'] = 'You must define at least one drop zone for this question.'; $string['formerror_nobgimage'] = 'You need to select an image to use as the background for the drag and drop area.'; $string['formerror_noitemselected'] = 'You have specified a drop zone but not chosen a marker that must be dragged to the zone.'; $string['formerror_nosemicolons'] = 'There are no semicolons in your coordinates string. Your coordinates for a {$a->shape} should be expressed as - {$a->coordsstring}.'; diff --git a/question/type/ddmarker/tests/behat/add.feature b/question/type/ddmarker/tests/behat/add.feature index 5ec52ddfc7c..601f813f277 100644 --- a/question/type/ddmarker/tests/behat/add.feature +++ b/question/type/ddmarker/tests/behat/add.feature @@ -63,3 +63,16 @@ Feature: Test creating a drag and drop markers question And the following fields match these values: | id_showmisplaced | 1 | | id_shuffleanswers | 1 | + + @javascript @_file_upload + Scenario: Question must have at least one marker and one drop zone + When I am on the "Course 1" "core_question > course question bank" page logged in as teacher + And I press "Create a new question ..." + And I set the field "Drag and drop markers" to "1" + And I click on "Add" "button" in the "Choose a question type to add" "dialogue" + And I set the field "Question name" to "Drag and drop markers" + And I set the field "Question text" to "Markers, who need markers?" + And I upload "question/type/ddmarker/tests/fixtures/mkmap.png" file to "Background image" filemanager + And I press "Save changes" + Then I should see "You must add at least one marker to this question." + And I should see "You must define at least one drop zone for this question."