MDL-81749 core_completion: Let custom rules to return failed state

.upgradenotes/MDL-81749-2024053015081306.yml

@@ -0,0 +1,8 @@
+issueNumber: MDL-81749
+notes:
+  core_completion:
+    - message: >-
+        get_overall_completion_state() function could also return
+        COMPLETION_COMPLETE_FAIL and not only COMPLETION_COMPLETE and
+        COMPLETION_INCOMPLETE
+      type: changed

completion/classes/activity_custom_completion.php

@@ -114,16 +114,25 @@ abstract class activity_custom_completion {
     /**
      * Fetches the overall completion status of this activity instance for a user based on its available custom completion rules.
      *
-     * @return int The completion state (e.g. COMPLETION_COMPLETE, COMPLETION_INCOMPLETE).
+     * @return int The completion state (e.g. COMPLETION_COMPLETE, COMPLETION_INCOMPLETE, COMPLETION_COMPLETE_FAIL).
      */
     public function get_overall_completion_state(): int {
+        $iscompletefail = false;
         foreach ($this->get_available_custom_rules() as $rule) {
             $state = $this->get_state($rule);
             // Return early if one of the custom completion rules is not yet complete.
             if ($state == COMPLETION_INCOMPLETE) {
                 return $state;
             }
+            if (!$iscompletefail) {
+                $iscompletefail = ($state === COMPLETION_COMPLETE_FAIL || $state === COMPLETION_COMPLETE_FAIL_HIDDEN);
+            }
         }
+
+        if ($iscompletefail) {
+            return COMPLETION_COMPLETE_FAIL;
+        }
+
         // If this was reached, then all custom rules have been marked complete.
         return COMPLETION_COMPLETE;
     }

completion/classes/cm_completion_details.php

@@ -207,7 +207,10 @@ class cm_completion_details {
             $completionstates = [COMPLETION_COMPLETE];
         } else if ($this->is_automatic()) {
             // Successfull completion states depend on the completion settings.
-            if (isset($this->completiondata->passgrade)) {
+            if (property_exists($this->completiondata, 'customcompletion') && !empty($this->completiondata->customcompletion)) {
+                // If the module has any failed custom completion rule the state could be COMPLETION_COMPLETE_FAIL.
+                $completionstates = [COMPLETION_COMPLETE, COMPLETION_COMPLETE_PASS];
+            } else if (isset($this->completiondata->passgrade)) {
                 // Passing grade is required. Don't mark it as complete when state is COMPLETION_COMPLETE_FAIL.
                 $completionstates = [COMPLETION_COMPLETE, COMPLETION_COMPLETE_PASS];
             } else {

completion/tests/activity_custom_completion_test.php

@@ -56,25 +56,37 @@ class activity_custom_completion_test extends advanced_testcase {
                 ['completionsubmit', 'completioncreate'],
                 [COMPLETION_INCOMPLETE, COMPLETION_COMPLETE],
                 1,
-                COMPLETION_INCOMPLETE
+                COMPLETION_INCOMPLETE,
             ],
             'First complete, second incomplete' => [
                 ['completionsubmit', 'completioncreate'],
                 [COMPLETION_COMPLETE, COMPLETION_INCOMPLETE],
                 2,
-                COMPLETION_INCOMPLETE
+                COMPLETION_INCOMPLETE,
+            ],
+            'First complete, second failed' => [
+                ['completionsubmit', 'completioncreate'],
+                [COMPLETION_COMPLETE, COMPLETION_COMPLETE_FAIL],
+                2,
+                COMPLETION_COMPLETE_FAIL,
+            ],
+            'First complete, second incomplete, third failed' => [
+                ['completionsubmit', 'completioncreate'],
+                [COMPLETION_COMPLETE, COMPLETION_INCOMPLETE, COMPLETION_COMPLETE_FAIL],
+                2,
+                COMPLETION_INCOMPLETE,
             ],
             'All complete' => [
                 ['completionsubmit', 'completioncreate'],
                 [COMPLETION_COMPLETE, COMPLETION_COMPLETE],
                 2,
-                COMPLETION_COMPLETE
+                COMPLETION_COMPLETE,
             ],
             'No rules' => [
                 [],
                 [],
                 0,
-                COMPLETION_COMPLETE
+                COMPLETION_COMPLETE,
             ],
         ];
     }

course/format/tests/behat/courseindex_completion.feature

@@ -128,8 +128,8 @@ Feature: Course index completion icons
       | questioncategory | qtype     | name           | questiontext              |
       | Test questions   | truefalse | First question | Answer the first question |
     And the following "activities" exist:
-      | activity | name           | course | idnumber | attempts | gradepass | completion | completionusegrade | completionpassgrade | completionattemptsexhausted |
+      | activity | name           | course | idnumber | attempts | gradepass | completion | completionusegrade | completionpassgrade |
-      | quiz     | Test quiz name | C1     | quiz1    | 1        | 5.00      | 2          | 1                  | 0                   | 1                           |
+      | quiz     | Test quiz name | C1     | quiz1    | 1        | 5.00      | 2          | 1                  | 0                   |
     And quiz "Test quiz name" contains the following questions:
       | question       | page |
       | First question | 1    |
@@ -138,3 +138,47 @@ Feature: Course index completion icons
       | 1    | False    |
     When I am on the "C1" "Course" page logged in as "student1"
     And "Done" "icon" should exist in the "courseindex-content" "region"
+
+  @javascript
+  Scenario: Activities with custom completion rules could fail
+    Given the following "activity" exists:
+      | activity                 | scorm                                                         |
+      | course                   | C1                                                            |
+      | name                     | Music history                                              |
+      | packagefilepath          | mod/scorm/tests/packages/RuntimeMinimumCalls_SCORM12-mini.zip |
+      | maxattempt               | 1                                                             |
+      | latattemptlock           | 1                                                             |
+      # Add requirements
+      | completion               | 2                                                             |
+      | completionscorerequired  | 90                                                            |
+    Given I am on the "Music history" "scorm activity" page logged in as student1
+    # We need a little taller window because Firefox is, apparently, unable to auto-scroll within
+    # an iframe, so we need to ensure that the "Save changes" button is visible in the viewport.
+    And I change window size to "large"
+    And I press "Enter"
+    And I switch to the main frame
+    And I click on "Par?" "list_item"
+    And I switch to "scorm_object" iframe
+    And I wait until the page is ready
+    And I switch to the main frame
+    And I click on "Keeping Score" "list_item"
+    And I switch to "scorm_object" iframe
+    And I wait until the page is ready
+    And I switch to the main frame
+    And I click on "Other Scoring Systems" "list_item"
+    And I switch to "scorm_object" iframe
+    And I wait until the page is ready
+    And I switch to the main frame
+    And I click on "The Rules of Golf" "list_item"
+    And I switch to "scorm_object" iframe
+    And I wait until the page is ready
+    And I switch to the main frame
+    And I click on "Playing Golf Quiz" "list_item"
+    And I switch to "scorm_object" iframe
+    And I wait until the page is ready
+    And I click on "[id='question_com.scorm.golfsamples.interactions.playing_1_1']" "css_element"
+    And I press "Submit Answers"
+    And I wait until "Score: 20" "text" exists
+    And I switch to the main frame
+    And I click on "Exit activity" "link"
+    And "Failed" "icon" should exist in the "courseindex-content" "region"

lib/completionlib.php

@@ -696,16 +696,15 @@ class completion_info {
         $completionstate = $this->get_core_completion_state($cminfo, $userid);
 
         if (plugin_supports('mod', $cminfo->modname, FEATURE_COMPLETION_HAS_RULES)) {
-            $response = true;
             $cmcompletionclass = activity_custom_completion::get_cm_completion_class($cminfo->modname);
             if ($cmcompletionclass) {
                 /** @var activity_custom_completion $cmcompletion */
                 $cmcompletion = new $cmcompletionclass($cminfo, $userid, $completionstate);
-                $response = $cmcompletion->get_overall_completion_state() != COMPLETION_INCOMPLETE;
+                $customstate = $cmcompletion->get_overall_completion_state();
-            }
+                if ($customstate == COMPLETION_INCOMPLETE) {
-
+                    return $customstate;
-            if (!$response) {
+                }
-                return COMPLETION_INCOMPLETE;
+                $completionstate[] = $customstate;
             }
         }