MDL-53966 lesson: Allow maximum number of attempts to be unlimited

diff --git a/mod/lesson/essay.php b/mod/lesson/essay.php
index f295735..df2ce8c 100644 (file)
--- 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 5f001e1..784678d 100644 (file)
--- 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 21c5646..be2cca2 100644 (file)
--- 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 cf5d44a..41b3d6f 100644 (file)
--- 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 69d92bc..29ed335 100644 (file)
--- 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 af7cefa..232add5 100644 (file)
--- 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 ff7bfa2..9a6f20f 100644 (file)
--- 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 f59d94b..bc7ce1f 100644 (file)
--- 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 766ad23..3e9fab4 100644 (file)
--- 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 61892e3..2a88e51 100644 (file)
--- 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 0ea349b..56e66ab 100644 (file)
--- 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));
+    }
 }