MDL-73155 qtype_essay: Errors when Allow attachments is reset to 'No'

question/type/essay/edit_essay_form.php

@@ -45,7 +45,6 @@ class qtype_essay_edit_form extends question_edit_form {
                 get_string('responseformat', 'qtype_essay'), $qtype->response_formats());
         $mform->setDefault('responseformat', $this->get_default_value('responseformat', 'editor'));
 
-
         $mform->addElement('select', 'responserequired',
                 get_string('responserequired', 'qtype_essay'), $qtype->response_required_options());
         $mform->setDefault('responserequired', $this->get_default_value('responserequired', 1));
@@ -80,7 +79,7 @@ class qtype_essay_edit_form extends question_edit_form {
 
         $mform->addElement('select', 'attachments',
                 get_string('allowattachments', 'qtype_essay'), $qtype->attachment_options());
-         $mform->setDefault('attachments', $this->get_default_value('attachments', 0));
+        $mform->setDefault('attachments', $this->get_default_value('attachments', 0));
 
         $mform->addElement('select', 'attachmentsrequired',
                 get_string('attachmentsrequired', 'qtype_essay'), $qtype->attachments_required_options());
@@ -165,7 +164,7 @@ class qtype_essay_edit_form extends question_edit_form {
 
         // Don't allow the teacher to require more attachments than they allow; as this would
         // create a condition that it's impossible for the student to meet.
-        if ($fromform['attachments'] != -1 && $fromform['attachments'] < $fromform['attachmentsrequired'] ) {
+        if ($fromform['attachments'] > 0 && $fromform['attachments'] < $fromform['attachmentsrequired'] ) {
             $errors['attachmentsrequired']  = get_string('mustrequirefewer', 'qtype_essay');
         }
 

question/type/essay/questiontype.php

@@ -78,7 +78,12 @@ class qtype_essay extends question_type {
         $options->minwordlimit = isset($formdata->minwordenabled) ? $formdata->minwordlimit : null;
         $options->maxwordlimit = isset($formdata->maxwordenabled) ? $formdata->maxwordlimit : null;
         $options->attachments = $formdata->attachments;
-        $options->attachmentsrequired = $formdata->attachmentsrequired;
+        if ((int)$formdata->attachments === 0 && $formdata->attachmentsrequired > 0) {
+            // Adjust the value for the field 'attachmentsrequired' when the field 'attachments' is set to 'No'.
+            $options->attachmentsrequired = 0;
+        } else {
+            $options->attachmentsrequired = $formdata->attachmentsrequired;
+        }
         if (!isset($formdata->filetypeslist)) {
             $options->filetypeslist = null;
         } else {

question/type/essay/tests/edit_form_test.php

@@ -0,0 +1,124 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Unit tests for the essay edit form.
+ *
+ * @package   qtype_essay
+ * @copyright 2021 The Open University
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+defined('MOODLE_INTERNAL') || die();
+global $CFG;
+
+require_once($CFG->dirroot . '/question/engine/tests/helpers.php');
+require_once($CFG->dirroot . '/question/type/edit_question_form.php');
+require_once($CFG->dirroot . '/question/type/essay/edit_essay_form.php');
+
+/**
+ * Unit tests for the essay edit form.
+ *
+ * @copyright  2021 The Open University
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class qtype_essay_edit_form_test extends advanced_testcase {
+    /**
+     * Helper method.
+     *
+     * @param string $classname the question form class to instantiate.
+     *
+     * @return array with two elements:
+     *      question_edit_form great a question form instance that can be tested.
+     *      stdClass the question category.
+     */
+    protected function get_form($classname) {
+        global $USER;
+        $this->setAdminUser();
+        $this->resetAfterTest();
+
+        $syscontext = context_system::instance();
+        $category = question_make_default_categories(array($syscontext));
+        $fakequestion = new stdClass();
+        $fakequestion->qtype = 'essay';
+        $fakequestion->contextid = $syscontext->id;
+        $fakequestion->createdby = $USER->id;
+        $fakequestion->category = $category->id;
+        $fakequestion->questiontext = 'please writer an assay about ...';
+        $fakequestion->responseformat = 'editorfilepicker';
+        $fakequestion->responserequired = 1;
+        $fakequestion->responsefieldlines = 10;
+        $fakequestion->attachments = -1;
+        $fakequestion->attachmentsrequired = 3;
+        $fakequestion->filetypeslist = '';
+
+        $form = new $classname(new moodle_url('/'), $fakequestion, $category,
+                new question_edit_contexts($syscontext));
+
+        return [$form, $category];
+    }
+
+    /**
+     * Test the form for correct validation of attachments options.
+     *
+     * @dataProvider user_preference_provider
+     * @param int $allowed
+     * @param int $required
+     * @param array $expected
+     */
+    public function test_attachments_validation(int $allowed, int $required, array $expected): void {
+        list($form, $category) = $this->get_form('qtype_essay_edit_form');
+        $submitteddata = [
+                'category' => $category->id,
+                'questiontext' => ['text' => 'please writer an assay about ...',
+                        'format' => FORMAT_HTML],
+                'responseformat' => 'editorfilepicker',
+                'responserequired' => '1',
+                'attachments' => $allowed,
+                'attachmentsrequired' => $required,
+        ];
+        $errors = $form->validation($submitteddata, []);
+        $this->assertArrayNotHasKey('attachments', $errors);
+        $this->assertEquals($expected, $errors);
+    }
+
+    /**
+     * Return an array of all possible allowed and required attachments,
+     * and the expected results from the form validation method.
+     *
+     * @return array, an array of all possible options.
+     */
+    public function user_preference_provider(): array {
+        $valid = [];
+        $invalid = ['attachmentsrequired' => get_string('mustrequirefewer', 'qtype_essay')];
+        return [
+                'Attachments allowed=0, required=0, valid' => [0, 0, $valid],
+                'Attachments allowed=0, required=1, invalid, so required is set to 0 when saving' => [0, 1, $valid],
+                'Attachments allowed=0, required=2, invalid, so required is set to 0 when saving' => [0, 2, $valid],
+                'Attachments allowed=0, required=3, invalid, so required is set to 0 when saving' => [0, 3, $valid],
+
+                'Attachments allowed=1, required=0, valid' => [1, 0, $valid],
+                'Attachments allowed=1, required=1, valid' => [1, 1, $valid],
+                'Attachments allowed=1, required=2, invalid' => [1, 2, $invalid],
+
+                'Attachments allowed=2, required=3, invalid' => [2, 3, $invalid],
+
+                'Attachments allowed=3, required=4, invalid' => [3, 4, $invalid],
+
+                'Attachments allowed=-1, required=4, valid' => [-1, 4, $valid],
+        ];
+    }
+}