From 1ed929a22b1dccc26cb0dfaefc21ff799237e6a4 Mon Sep 17 00:00:00 2001 From: Mahmoud Kassaei Date: Thu, 25 Nov 2021 14:12:53 +0000 Subject: [PATCH] MDL-73155 qtype_essay: Errors when Allow attachments is reset to 'No' --- question/type/essay/edit_essay_form.php | 5 +- question/type/essay/questiontype.php | 7 +- question/type/essay/tests/edit_form_test.php | 124 +++++++++++++++++++ 3 files changed, 132 insertions(+), 4 deletions(-) create mode 100644 question/type/essay/tests/edit_form_test.php diff --git a/question/type/essay/edit_essay_form.php b/question/type/essay/edit_essay_form.php index 464b3ae7222..dc035ef1adc 100644 --- a/question/type/essay/edit_essay_form.php +++ b/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'); } diff --git a/question/type/essay/questiontype.php b/question/type/essay/questiontype.php index 9f89adee55d..dba6f0a3f60 100644 --- a/question/type/essay/questiontype.php +++ b/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 { diff --git a/question/type/essay/tests/edit_form_test.php b/question/type/essay/tests/edit_form_test.php new file mode 100644 index 00000000000..f59792fa3ff --- /dev/null +++ b/question/type/essay/tests/edit_form_test.php @@ -0,0 +1,124 @@ +. + +/** + * 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], + ]; + } +}