From 8bfe7b0759a7ca13212b4f9c56ac367e23934738 Mon Sep 17 00:00:00 2001 From: PraiseSatan Date: Fri, 12 Nov 2021 07:27:08 +1100 Subject: [PATCH 1/2] MDL-69496 form: Check if element has attributes Checks if a form element has attributes before trying to get the default value. This fixes an error when trying to get the default value for a frozen group which does not have the attributes array. --- lib/formslib.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/formslib.php b/lib/formslib.php index 6dba9626520..70778e56e9f 100644 --- a/lib/formslib.php +++ b/lib/formslib.php @@ -2250,11 +2250,13 @@ class MoodleQuickForm extends HTML_QuickForm_DHTMLRulesTableless { // iterate over all elements, calling their exportValue() methods foreach (array_keys($this->_elements) as $key) { if ($this->_elements[$key]->isFrozen() && !$this->_elements[$key]->_persistantFreeze) { - $varname = $this->_elements[$key]->_attributes['name']; $value = ''; - // If we have a default value then export it. - if (isset($this->_defaultValues[$varname])) { - $value = $this->prepare_fixed_value($varname, $this->_defaultValues[$varname]); + if (isset($this->_elements[$key]->_attributes['name'])) { + $varname = $this->_elements[$key]->_attributes['name']; + // If we have a default value then export it. + if (isset($this->_defaultValues[$varname])) { + $value = $this->prepare_fixed_value($varname, $this->_defaultValues[$varname]); + } } } else { $value = $this->_elements[$key]->exportValue($this->_submitValues, true); From 4124e8b4bb2f5001a74736e9592e8aef0480834f Mon Sep 17 00:00:00 2001 From: PraiseSatan Date: Fri, 12 Nov 2021 07:29:21 +1100 Subject: [PATCH 2/2] MDL-69496 quiz: Only validate completion if unlocked Quiz completion settings are only validated if they are unlocked. Undefined behaviour in the forms API results in the completion settings 'require passing grade' and 'completion attempts exhausted' not being sent to the process options function if completion settings are locked. This resulted in the completion attempts exhausted setting being disabled whenever the quiz was saved as it did not detect the require passing grade setting. This changes the behaviour so that validation is only performed if the completion settings are unlocked. A behat test is included to ensure the setting does not change. --- mod/quiz/lib.php | 19 ++++--- .../quiz_activity_completion_locked.feature | 56 +++++++++++++++++++ 2 files changed, 67 insertions(+), 8 deletions(-) create mode 100644 mod/quiz/tests/behat/quiz_activity_completion_locked.feature diff --git a/mod/quiz/lib.php b/mod/quiz/lib.php index c0209589272..3b0a042c2c7 100644 --- a/mod/quiz/lib.php +++ b/mod/quiz/lib.php @@ -1128,14 +1128,17 @@ function quiz_process_options($quiz) { $quiz->reviewoverallfeedback &= ~mod_quiz_display_options::DURING; // Ensure that disabled checkboxes in completion settings are set to 0. - if (empty($quiz->completionusegrade)) { - $quiz->completionpass = 0; - } - if (empty($quiz->completionpass)) { - $quiz->completionattemptsexhausted = 0; - } - if (empty($quiz->completionminattemptsenabled)) { - $quiz->completionminattempts = 0; + // But only if the completion settinsg are unlocked. + if (!empty($quiz->completionunlocked)) { + if (empty($quiz->completionusegrade)) { + $quiz->completionpass = 0; + } + if (empty($quiz->completionpass)) { + $quiz->completionattemptsexhausted = 0; + } + if (empty($quiz->completionminattemptsenabled)) { + $quiz->completionminattempts = 0; + } } } diff --git a/mod/quiz/tests/behat/quiz_activity_completion_locked.feature b/mod/quiz/tests/behat/quiz_activity_completion_locked.feature new file mode 100644 index 00000000000..5bdc69069ff --- /dev/null +++ b/mod/quiz/tests/behat/quiz_activity_completion_locked.feature @@ -0,0 +1,56 @@ +@mod @mod_quiz @core_completion +Feature: Ensure saving a quiz does not modify the completion settings. + In order to reliably use completion + As a teacher + I need to be able to update the quiz + without changing the completion settings. + + Background: + Given the following "users" exist: + | username | firstname | lastname | email | + | student1 | Student | 1 | student1@example.com | + | teacher1 | Teacher | 1 | teacher1@example.com | + And the following "courses" exist: + | fullname | shortname | category | enablecompletion | + | Course 1 | C1 | 0 | 1 | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + | student1 | C1 | student | + And the following config values are set as admin: + | grade_item_advanced | hiddenuntil | + And the following "question categories" exist: + | contextlevel | reference | name | + | Course | C1 | Test questions | + And the following "questions" exist: + | questioncategory | qtype | name | questiontext | + | Test questions | truefalse | First question | Answer the first question | + And the following "activity" exists: + | activity | quiz | + | course | C1 | + | idnumber | quiz1 | + | name | Test quiz | + | section | 1 | + | attempts | 2 | + | gradepass | 5.00 | + | completion | 2 | + | completionview | 0 | + | completionusegrade | 1 | + | completionpass | 1 | + | completionattemptsexhausted | 1 | + And quiz "Test quiz" contains the following questions: + | question | page | + | First question | 1 | + And user "student1" has attempted "Test quiz" with responses: + | slot | response | + | 1 | True | + + Scenario: Ensure saving quiz activty does not change completion settings + Given I am on the "Test quiz" "mod_quiz > View" page logged in as "teacher1" + When I navigate to "Edit settings" in current page administration + Then the "completionattemptsexhausted" "field" should be disabled + And the field "completionattemptsexhausted" matches value "1" + And I press "Save and display" + And I navigate to "Edit settings" in current page administration + And the "completionattemptsexhausted" "field" should be disabled + And the field "completionattemptsexhausted" matches value "1"