From f185fdca52e405a1a8722e70b41704b8eb54ef3d Mon Sep 17 00:00:00 2001 From: Amaia Anabitarte Date: Thu, 30 May 2024 17:09:32 +0200 Subject: [PATCH] MDL-81749 core_completion: Let custom rules to return failed state --- .upgradenotes/MDL-81749-2024053015081306.yml | 8 ++++ .../classes/activity_custom_completion.php | 11 ++++- completion/classes/cm_completion_details.php | 5 +- .../tests/activity_custom_completion_test.php | 20 ++++++-- .../behat/courseindex_completion.feature | 48 ++++++++++++++++++- lib/completionlib.php | 11 ++--- 6 files changed, 89 insertions(+), 14 deletions(-) create mode 100644 .upgradenotes/MDL-81749-2024053015081306.yml diff --git a/.upgradenotes/MDL-81749-2024053015081306.yml b/.upgradenotes/MDL-81749-2024053015081306.yml new file mode 100644 index 00000000000..08c7b3fbd4d --- /dev/null +++ b/.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 diff --git a/completion/classes/activity_custom_completion.php b/completion/classes/activity_custom_completion.php index b7f95a42136..50723e612c5 100644 --- a/completion/classes/activity_custom_completion.php +++ b/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; } diff --git a/completion/classes/cm_completion_details.php b/completion/classes/cm_completion_details.php index 72aed1ff239..874b4ad0597 100644 --- a/completion/classes/cm_completion_details.php +++ b/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 { diff --git a/completion/tests/activity_custom_completion_test.php b/completion/tests/activity_custom_completion_test.php index 1a237d96424..54cb1cc91bf 100644 --- a/completion/tests/activity_custom_completion_test.php +++ b/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, ], ]; } diff --git a/course/format/tests/behat/courseindex_completion.feature b/course/format/tests/behat/courseindex_completion.feature index 4807f410dcb..71a4cead3b2 100644 --- a/course/format/tests/behat/courseindex_completion.feature +++ b/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 | - | quiz | Test quiz name | C1 | quiz1 | 1 | 5.00 | 2 | 1 | 0 | 1 | + | activity | name | course | idnumber | attempts | gradepass | completion | completionusegrade | completionpassgrade | + | 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" diff --git a/lib/completionlib.php b/lib/completionlib.php index ab7f7deac70..95ed97a4f12 100644 --- a/lib/completionlib.php +++ b/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; - } - - if (!$response) { - return COMPLETION_INCOMPLETE; + $customstate = $cmcompletion->get_overall_completion_state(); + if ($customstate == COMPLETION_INCOMPLETE) { + return $customstate; + } + $completionstate[] = $customstate; } }