diff --git a/mod/lesson/essay.php b/mod/lesson/essay.php index f295735606f..df2ce8c8336 100644 --- a/mod/lesson/essay.php +++ b/mod/lesson/essay.php @@ -406,12 +406,8 @@ switch ($mode) { } $count++; - // Make sure they didn't answer it more than the max number of attmepts - if (count($try) > $lesson->maxattempts) { - $essay = $try[$lesson->maxattempts-1]; - } else { - $essay = end($try); - } + // Make sure they didn't answer it more than the max number of attempts. + $essay = $lesson->get_last_attempt($try); // Start processing the attempt $essayinfo = lesson_page_type_essay::extract_useranswer($essay->useranswer); diff --git a/mod/lesson/locallib.php b/mod/lesson/locallib.php index 5f001e1f224..784678db4aa 100644 --- a/mod/lesson/locallib.php +++ b/mod/lesson/locallib.php @@ -301,9 +301,11 @@ function lesson_grade($lesson, $ntries, $userid = 0) { $attemptset[$useranswer->pageid][] = $useranswer; } - // Drop all attempts that go beyond max attempts for the lesson - foreach ($attemptset as $key => $set) { - $attemptset[$key] = array_slice($set, 0, $lesson->maxattempts); + if (!empty($lesson->maxattempts)) { + // Drop all attempts that go beyond max attempts for the lesson. + foreach ($attemptset as $key => $set) { + $attemptset[$key] = array_slice($set, 0, $lesson->maxattempts); + } } // get only the pages and their answers that the user answered @@ -3659,6 +3661,23 @@ class lesson extends lesson_base { } return $data; } + + /** + * Returns the last "legal" attempt from the list of student attempts. + * + * @param array $attempts The list of student attempts. + * @return stdClass The updated fom data. + */ + public function get_last_attempt(array $attempts): stdClass { + // If there are more tries than the max that is allowed, grab the last "legal" attempt. + if (!empty($this->maxattempts) && (count($attempts) > $this->maxattempts)) { + $lastattempt = $attempts[$this->maxattempts - 1]; + } else { + // Grab the last attempt since there's no limit to the max attempts or the user has made fewer attempts than the max. + $lastattempt = end($attempts); + } + return $lastattempt; + } } @@ -4132,7 +4151,7 @@ abstract class lesson_page extends lesson_base { 'userid' => $USER->id, 'pageid' => $this->properties->id, 'retry' => $nretakes)); // Check if they have reached (or exceeded) the maximum number of attempts allowed. - if ($nattempts >= $this->lesson->maxattempts) { + if (!empty($this->lesson->maxattempts) && $nattempts >= $this->lesson->maxattempts) { $result->maxattemptsreached = true; $result->feedback = get_string('maximumnumberofattemptsreached', 'lesson'); $result->newpageid = $this->lesson->get_next_page($this->properties->nextpageid); @@ -4200,8 +4219,8 @@ abstract class lesson_page extends lesson_base { // "number of attempts remaining" message if $this->lesson->maxattempts > 1 // displaying of message(s) is at the end of page for more ergonomic display if (!$result->correctanswer && ($result->newpageid == 0)) { - // retreive the number of attempts left counter for displaying at bottom of feedback page - if ($nattempts >= $this->lesson->maxattempts) { + // Retrieve the number of attempts left counter for displaying at bottom of feedback page. + if (!empty($this->lesson->maxattempts) && $nattempts >= $this->lesson->maxattempts) { if ($this->lesson->maxattempts > 1) { // don't bother with message if only one attempt $result->maxattemptsreached = true; } diff --git a/mod/lesson/mod_form.php b/mod/lesson/mod_form.php index 21c56464a22..be2cca26a53 100644 --- a/mod/lesson/mod_form.php +++ b/mod/lesson/mod_form.php @@ -278,7 +278,7 @@ class mod_lesson_mod_form extends moodleform_mod { $mform->setDefault('review', $lessonconfig->displayreview); $mform->setAdvanced('review', $lessonconfig->displayreview_adv); - $numbers = array(); + $numbers = array('0' => get_string('unlimited')); for ($i = 10; $i > 0; $i--) { $numbers[$i] = $i; } diff --git a/mod/lesson/pagetypes/essay.php b/mod/lesson/pagetypes/essay.php index cf5d44a4edc..41b3d6fada8 100644 --- a/mod/lesson/pagetypes/essay.php +++ b/mod/lesson/pagetypes/essay.php @@ -269,12 +269,7 @@ class lesson_page_type_essay extends lesson_page { return true; } public function stats(array &$pagestats, $tries) { - if(count($tries) > $this->lesson->maxattempts) { // if there are more tries than the max that is allowed, grab the last "legal" attempt - $temp = $tries[$this->lesson->maxattempts - 1]; - } else { - // else, user attempted the question less than the max, so grab the last one - $temp = end($tries); - } + $temp = $this->lesson->get_last_attempt($tries); $essayinfo = self::extract_useranswer($temp->useranswer); if ($essayinfo->graded) { if (isset($pagestats[$temp->pageid])) { diff --git a/mod/lesson/pagetypes/matching.php b/mod/lesson/pagetypes/matching.php index 69d92bc555f..29ed335c9eb 100644 --- a/mod/lesson/pagetypes/matching.php +++ b/mod/lesson/pagetypes/matching.php @@ -389,12 +389,7 @@ class lesson_page_type_matching extends lesson_page { return true; } public function stats(array &$pagestats, $tries) { - if(count($tries) > $this->lesson->maxattempts) { // if there are more tries than the max that is allowed, grab the last "legal" attempt - $temp = $tries[$this->lesson->maxattempts - 1]; - } else { - // else, user attempted the question less than the max, so grab the last one - $temp = end($tries); - } + $temp = $this->lesson->get_last_attempt($tries); if ($temp->correct) { if (isset($pagestats[$temp->pageid]["correct"])) { $pagestats[$temp->pageid]["correct"]++; diff --git a/mod/lesson/pagetypes/multichoice.php b/mod/lesson/pagetypes/multichoice.php index af7cefa062e..232add56eae 100644 --- a/mod/lesson/pagetypes/multichoice.php +++ b/mod/lesson/pagetypes/multichoice.php @@ -311,12 +311,7 @@ class lesson_page_type_multichoice extends lesson_page { return $table; } public function stats(array &$pagestats, $tries) { - if(count($tries) > $this->lesson->maxattempts) { // if there are more tries than the max that is allowed, grab the last "legal" attempt - $temp = $tries[$this->lesson->maxattempts - 1]; - } else { - // else, user attempted the question less than the max, so grab the last one - $temp = end($tries); - } + $temp = $this->lesson->get_last_attempt($tries); if ($this->properties->qoption) { $userresponse = explode(",", $temp->useranswer); foreach ($userresponse as $response) { diff --git a/mod/lesson/pagetypes/numerical.php b/mod/lesson/pagetypes/numerical.php index ff7bfa24688..9a6f20f7017 100644 --- a/mod/lesson/pagetypes/numerical.php +++ b/mod/lesson/pagetypes/numerical.php @@ -229,12 +229,7 @@ class lesson_page_type_numerical extends lesson_page { return $table; } public function stats(array &$pagestats, $tries) { - if(count($tries) > $this->lesson->maxattempts) { // if there are more tries than the max that is allowed, grab the last "legal" attempt - $temp = $tries[$this->lesson->maxattempts - 1]; - } else { - // else, user attempted the question less than the max, so grab the last one - $temp = end($tries); - } + $temp = $this->lesson->get_last_attempt($tries); if (isset($pagestats[$temp->pageid][$temp->useranswer])) { $pagestats[$temp->pageid][$temp->useranswer]++; } else { diff --git a/mod/lesson/pagetypes/shortanswer.php b/mod/lesson/pagetypes/shortanswer.php index f59d94b1509..bc7ce1fa632 100644 --- a/mod/lesson/pagetypes/shortanswer.php +++ b/mod/lesson/pagetypes/shortanswer.php @@ -292,12 +292,7 @@ class lesson_page_type_shortanswer extends lesson_page { return $table; } public function stats(array &$pagestats, $tries) { - if(count($tries) > $this->lesson->maxattempts) { // if there are more tries than the max that is allowed, grab the last "legal" attempt - $temp = $tries[$this->lesson->maxattempts - 1]; - } else { - // else, user attempted the question less than the max, so grab the last one - $temp = end($tries); - } + $temp = $this->lesson->get_last_attempt($tries); if (isset($pagestats[$temp->pageid][$temp->useranswer])) { $pagestats[$temp->pageid][$temp->useranswer]++; } else { diff --git a/mod/lesson/pagetypes/truefalse.php b/mod/lesson/pagetypes/truefalse.php index 766ad2392bf..3e9fab4e032 100644 --- a/mod/lesson/pagetypes/truefalse.php +++ b/mod/lesson/pagetypes/truefalse.php @@ -225,12 +225,7 @@ class lesson_page_type_truefalse extends lesson_page { } public function stats(array &$pagestats, $tries) { - if(count($tries) > $this->lesson->maxattempts) { // if there are more tries than the max that is allowed, grab the last "legal" attempt - $temp = $tries[$this->lesson->maxattempts - 1]; - } else { - // else, user attempted the question less than the max, so grab the last one - $temp = end($tries); - } + $temp = $this->lesson->get_last_attempt($tries); if ($this->properties->qoption) { $userresponse = explode(",", $temp->useranswer); foreach ($userresponse as $response) { diff --git a/mod/lesson/tests/behat/lesson_navigation.feature b/mod/lesson/tests/behat/lesson_navigation.feature index 61892e3a199..2a88e51d69e 100644 --- a/mod/lesson/tests/behat/lesson_navigation.feature +++ b/mod/lesson/tests/behat/lesson_navigation.feature @@ -127,3 +127,37 @@ Feature: In a lesson activity, students can navigate through a series of pages i And I should not see "Yes, I'd like to try again" And I press "Continue" And I should see "Congratulations - end of lesson reached" + + Scenario: Student should not see remaining attempts notification if maximum number of attempts is set to unlimited + Given I add a "Lesson" to section "1" and I fill the form with: + | Name | Test lesson name | + | Description | Test lesson description | + | id_review | Yes | + | id_maxattempts | 0 | + And I follow "Test lesson name" + And I follow "Add a question page" + And I set the following fields to these values: + | id_qtype | True/false | + And I press "Add a question page" + And I set the following fields to these values: + | Page title | Test question | + | Page contents | Test content | + | id_answer_editor_0 | right | + | id_answer_editor_1 | wrong | + And I press "Save page" + And I log out + And I log in as "student1" + And I am on "Course 1" course homepage + When I follow "Test lesson name" + Then I should see "Test content" + And I set the following fields to these values: + | wrong | 1 | + And I press "Submit" + And I should not see "attempt(s) remaining" + And I press "Yes, I'd like to try again" + And I should see "Test content" + And I set the following fields to these values: + | right | 1 | + And I press "Submit" + And I should not see "Yes, I'd like to try again" + And I should see "Congratulations - end of lesson reached" diff --git a/mod/lesson/tests/locallib_test.php b/mod/lesson/tests/locallib_test.php index 0ea349b11fa..56e66abcd4b 100644 --- a/mod/lesson/tests/locallib_test.php +++ b/mod/lesson/tests/locallib_test.php @@ -251,4 +251,36 @@ class mod_lesson_locallib_testcase extends advanced_testcase { $this->assertEquals(true, $lesson->is_participant($USER->id), 'Admin is enrolled, suspended and can participate'); } + + /** + * Data provider for test_get_last_attempt. + * + * @return array + */ + public function test_get_last_attempt_dataprovider() { + return [ + [0, [(object)['id' => 1], (object)['id' => 2], (object)['id' => 3]], (object)['id' => 3]], + [1, [(object)['id' => 1], (object)['id' => 2], (object)['id' => 3]], (object)['id' => 1]], + [2, [(object)['id' => 1], (object)['id' => 2], (object)['id' => 3]], (object)['id' => 2]], + [3, [(object)['id' => 1], (object)['id' => 2], (object)['id' => 3]], (object)['id' => 3]], + [4, [(object)['id' => 1], (object)['id' => 2], (object)['id' => 3]], (object)['id' => 3]], + ]; + } + + /** + * Test the get_last_attempt() method. + * + * @dataProvider test_get_last_attempt_dataprovider + * @param int $maxattempts Lesson setting. + * @param array $attempts The list of student attempts. + * @param object $expected Expected result. + */ + public function test_get_last_attempt($maxattempts, $attempts, $expected) { + $this->resetAfterTest(); + $this->setAdminUser(); + $course = $this->getDataGenerator()->create_course(); + $lesson = $this->getDataGenerator()->create_module('lesson', ['course' => $course, 'maxattempts' => $maxattempts]); + $lesson = new lesson($lesson); + $this->assertEquals($expected, $lesson->get_last_attempt($attempts)); + } }