From c3bd602fb0885548d1391d6ac2596d806f9b79c9 Mon Sep 17 00:00:00 2001 From: Michael Hawkins Date: Wed, 7 Apr 2021 16:50:36 +0800 Subject: [PATCH 1/4] MDL-71189 core_completion: Updates to meet current coding standards --- completion/classes/activity_custom_completion.php | 6 +++--- completion/classes/cm_completion_details.php | 4 ++-- completion/tests/cm_completion_details_test.php | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/completion/classes/activity_custom_completion.php b/completion/classes/activity_custom_completion.php index f401ceb15d5..cb1fc9e4aa0 100644 --- a/completion/classes/activity_custom_completion.php +++ b/completion/classes/activity_custom_completion.php @@ -169,19 +169,19 @@ abstract class activity_custom_completion { * @param string $rule The completion rule. * @return int The completion state. */ - public abstract function get_state(string $rule): int; + abstract public function get_state(string $rule): int; /** * Fetch the list of custom completion rules that this module defines. * * @return array */ - public abstract static function get_defined_custom_rules(): array; + abstract public static function get_defined_custom_rules(): array; /** * Returns an associative array of the descriptions of custom completion rules. * * @return array */ - public abstract function get_custom_rule_descriptions(): array; + abstract public function get_custom_rule_descriptions(): array; } diff --git a/completion/classes/cm_completion_details.php b/completion/classes/cm_completion_details.php index f05c077eef1..5c01153d5a9 100644 --- a/completion/classes/cm_completion_details.php +++ b/completion/classes/cm_completion_details.php @@ -18,7 +18,7 @@ * Contains the class for building the user's activity completion details. * * @package core_completion - * @copyright Jun Pataleta + * @copyright 2021 Jun Pataleta * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ @@ -33,7 +33,7 @@ use completion_info; * Class for building the user's activity completion details. * * @package core_completion - * @copyright Jun Pataleta + * @copyright 2021 Jun Pataleta * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class cm_completion_details { diff --git a/completion/tests/cm_completion_details_test.php b/completion/tests/cm_completion_details_test.php index fa3f49a98a3..cb04a17ff94 100644 --- a/completion/tests/cm_completion_details_test.php +++ b/completion/tests/cm_completion_details_test.php @@ -18,7 +18,7 @@ * Contains unit tests for core_completion/cm_completion_details. * * @package core_completion - * @copyright Jun Pataleta + * @copyright 2021 Jun Pataleta * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ @@ -39,7 +39,7 @@ require_once($CFG->libdir . '/completionlib.php'); * Class for unit testing core_completion/cm_completion_details. * * @package core_completion - * @copyright Jun Pataleta + * @copyright 2021 Jun Pataleta * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class cm_completion_details_test extends advanced_testcase { From 30426fbe5e005fe0174f052c7a444bd174497f8c Mon Sep 17 00:00:00 2001 From: Michael Hawkins Date: Thu, 8 Apr 2021 14:20:50 +0800 Subject: [PATCH 2/4] MDL-71189 core_completion: Implementing custom completion sort ordering This adds the requirement for activities supporting custom completion to specify the order all completion conditions should be displayed for that activity. It also implements the sorting that takes place. --- .../classes/activity_custom_completion.php | 7 + completion/classes/cm_completion_details.php | 31 ++++- .../tests/cm_completion_details_test.php | 120 +++++++++++++++++- lib/upgrade.txt | 3 + 4 files changed, 158 insertions(+), 3 deletions(-) diff --git a/completion/classes/activity_custom_completion.php b/completion/classes/activity_custom_completion.php index cb1fc9e4aa0..4f06cc205ee 100644 --- a/completion/classes/activity_custom_completion.php +++ b/completion/classes/activity_custom_completion.php @@ -184,4 +184,11 @@ abstract class activity_custom_completion { * @return array */ abstract public function get_custom_rule_descriptions(): array; + + /** + * Returns an array of all completion rules, in the order they should be displayed to users. + * + * @return array + */ + abstract public function get_sort_order(): array; } diff --git a/completion/classes/cm_completion_details.php b/completion/classes/cm_completion_details.php index 5c01153d5a9..4c299ed7694 100644 --- a/completion/classes/cm_completion_details.php +++ b/completion/classes/cm_completion_details.php @@ -79,6 +79,7 @@ class cm_completion_details { * Fetches the completion details for a user. * * @return array An array of completion details for a user containing the completion requirement's description and status. + * @throws \coding_exception */ public function get_details(): array { if (!$this->is_automatic()) { @@ -135,6 +136,8 @@ class cm_completion_details { 'description' => $this->cmcompletion->get_custom_rule_description($rule), ]; } + + $details = $this->sort_completion_details($details); } } else { if (function_exists($this->cminfo->modname . '_get_completion_state')) { @@ -149,10 +152,36 @@ class cm_completion_details { } } - return $details; } + /** + * Sort completion details in the order specified by the activity's custom completion implementation. + * + * @param array $details The completion details to be sorted. + * @return array + * @throws \coding_exception + */ + protected function sort_completion_details(array $details): array { + $sortorder = $this->cmcompletion->get_sort_order(); + $sorteddetails = []; + + foreach ($sortorder as $sortedkey) { + if (isset($details[$sortedkey])) { + $sorteddetails[$sortedkey] = $details[$sortedkey]; + } + } + + // Make sure the sorted list includes all of the conditions that were set. + if (count($sorteddetails) < count($details)) { + $exceptiontext = get_class($this->cmcompletion) .'::get_sort_order() is missing one or more completion conditions.' . + ' All custom and standard conditions that apply to this activity must be listed.'; + throw new \coding_exception($exceptiontext); + } + + return $sorteddetails; + } + /** * Fetches the overall completion state of this course module. * diff --git a/completion/tests/cm_completion_details_test.php b/completion/tests/cm_completion_details_test.php index cb04a17ff94..bdbfcf683f9 100644 --- a/completion/tests/cm_completion_details_test.php +++ b/completion/tests/cm_completion_details_test.php @@ -52,10 +52,12 @@ class cm_completion_details_test extends advanced_testcase { * * @param int|null $completion The completion tracking mode for the module. * @param array $completionoptions Completion options (e.g. completionview, completionusegrade, etc.) + * @param object $mockcompletiondata Mock data to be returned by get_data. + * @param string $modname The modname to set in the cm if a specific one is required. * @return cm_completion_details */ protected function setup_data(?int $completion, array $completionoptions = [], - object $mockcompletiondata = null): cm_completion_details { + object $mockcompletiondata = null, $modname = 'somenonexistentmod'): cm_completion_details { if (is_null($completion)) { $completion = COMPLETION_TRACKING_AUTOMATIC; } @@ -88,7 +90,7 @@ class cm_completion_details_test extends advanced_testcase { ->will($this->returnValueMap([ ['completion', $completion], ['instance', 1], - ['modname', 'somenonexistentmod'], + ['modname', $modname], ['completionview', $completionoptions['completionview'] ?? COMPLETION_VIEW_NOT_REQUIRED], ['completiongradeitemnumber', $completionoptions['completionusegrade'] ?? null], ])); @@ -280,4 +282,118 @@ class cm_completion_details_test extends advanced_testcase { $cmcompletion = $this->setup_data($completion, $options, $getdatareturn); $this->assertEquals($expecteddetails, $cmcompletion->get_details()); } + + /** + * Data provider for test_get_details(). + * @return array[] + */ + public function get_details_custom_order_provider() { + return [ + 'Custom and view/grade standard conditions, view first and grade last' => [ + true, + true, + [ + 'completionsubmit' => true, + ], + 'assign', + ['completionview', 'completionsubmit', 'completionusegrade'], + ], + 'Custom and view/grade standard conditions, grade not last' => [ + true, + true, + [ + 'completionminattempts' => 2, + 'completionusegrade' => 50, + 'completionpassorattemptsexhausted' => 1, + ], + 'quiz', + ['completionview', 'completionminattempts', 'completionusegrade', 'completionpassorattemptsexhausted'], + ], + 'Custom and grade standard conditions only, no view condition' => [ + false, + true, + [ + 'completionsubmit' => true, + ], + 'assign', + ['completionsubmit', 'completionusegrade'], + ], + 'Custom and view standard conditions only, no grade condition' => [ + true, + false, + [ + 'completionsubmit' => true + ], + 'assign', + ['completionview', 'completionsubmit'], + ], + 'View and grade conditions only, activity with no custom conditions' => [ + true, + true, + [ + 'completionview' => true, + 'completionusegrade' => true + ], + 'workshop', + ['completionview', 'completionusegrade'], + ], + 'View condition only, activity with no custom conditions' => [ + true, + false, + [ + 'completionview' => true, + ], + 'workshop', + ['completionview'], + ], + ]; + } + + /** + * Test custom sort order is functioning in \core_completion\cm_completion_details::get_details(). + * + * @dataProvider get_details_custom_order_provider + * @param bool $completionview Completion status of the "view" completion condition. + * @param bool $completiongrade Completion status of the "must receive grade" completion condition. + * @param array $customcompletionrules Custom completion requirements, along with their values. + * @param string $modname The name of the module having data fetched. + * @param array $expectedorder The expected order of completion conditions returned about the module. + */ + public function test_get_details_custom_order(bool $completionview, bool $completiongrade, array $customcompletionrules, + string $modname, array $expectedorder) { + + $options['customcompletion'] = []; + $customcompletiondata = []; + + if ($completionview) { + $options['completionview'] = true; + } + + if ($completiongrade) { + $options['completionusegrade'] = true; + } + + // Set up the completion rules for the completion info. + foreach ($customcompletionrules as $customtype => $isenabled) { + $customcompletiondata[$customtype] = COMPLETION_COMPLETE; + } + + $getdatareturn = (object)[ + 'viewed' => $completionview ? COMPLETION_COMPLETE : COMPLETION_INCOMPLETE, + 'completiongrade' => $completiongrade ? COMPLETION_COMPLETE : COMPLETION_INCOMPLETE, + 'customcompletion' => $customcompletiondata, + ]; + + $cmcompletion = $this->setup_data(COMPLETION_TRACKING_AUTOMATIC, $options, $getdatareturn, $modname); + + $this->completioninfo->expects($this->any()) + ->method('get_data') + ->willReturn($getdatareturn); + + $fetcheddetails = $cmcompletion->get_details(); + + // Check the expected number of items are returned, and sorted in the correct order. + $this->assertCount(count($expectedorder), $fetcheddetails); + $this->assertTrue((array_keys($fetcheddetails) === $expectedorder)); + } } diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 0123eb00bde..08599f84b88 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -103,6 +103,9 @@ information provided here is intended especially for developers. - get_custom_rule_descriptions(): Returns an associative array with values containing the user-facing textual description of the custom completion rules (which serve as the keys to these values). e.g. ['completionsubmit' => 'Must submit'] + - get_sort_order(): Returns an array listing the order the activity module's completion rules should be displayed to the user, + including both custom completion and relevant core completion rules + e.g. ['completionview', 'completionsubmit', 'completionusegrade'] * Admin setting admin_setting_configmulticheckbox now supports lazy-loading the options list by supplying a callback function instead of an array of options. * A new core API class \core_user\fields provides ways to get lists of user fields, and SQL related to From 70b16bb87c6b095a63b50b25e1e62610ea8a6bdf Mon Sep 17 00:00:00 2001 From: Michael Hawkins Date: Thu, 8 Apr 2021 14:21:09 +0800 Subject: [PATCH 3/4] MDL-71189 core_completion: Adding completion sort order to activities This implements sort ordering for all completion conditions, in activities which support custom completion conditions. --- .../classes/completion/custom_completion.php | 14 +++++++++++++- .../classes/completion/custom_completion.php | 12 ++++++++++++ .../classes/completion/custom_completion.php | 13 +++++++++++++ .../classes/completion/custom_completion.php | 12 ++++++++++++ .../classes/completion/custom_completion.php | 15 +++++++++++++++ .../classes/completion/custom_completion.php | 13 +++++++++++++ .../classes/completion/custom_completion.php | 10 ++++++++++ .../classes/completion/custom_completion.php | 14 ++++++++++++++ .../classes/completion/custom_completion.php | 14 ++++++++++++++ .../classes/completion/custom_completion.php | 10 ++++++++++ .../classes/completion/custom_completion.php | 16 ++++++++++++++++ .../classes/completion/custom_completion.php | 12 ++++++++++++ mod/url/classes/completion/custom_completion.php | 9 +++++++++ 13 files changed, 163 insertions(+), 1 deletion(-) diff --git a/mod/assign/classes/completion/custom_completion.php b/mod/assign/classes/completion/custom_completion.php index 4753324f9c1..79d2d0b2433 100644 --- a/mod/assign/classes/completion/custom_completion.php +++ b/mod/assign/classes/completion/custom_completion.php @@ -78,5 +78,17 @@ class custom_completion extends activity_custom_completion { 'completionsubmit' => get_string('completiondetail:submit', 'assign') ]; } -} + /** + * Returns an array of all completion rules, in the order they should be displayed to users. + * + * @return array + */ + public function get_sort_order(): array { + return [ + 'completionview', + 'completionsubmit', + 'completionusegrade', + ]; + } +} diff --git a/mod/choice/classes/completion/custom_completion.php b/mod/choice/classes/completion/custom_completion.php index 5f04988dc0b..e925c1e46db 100644 --- a/mod/choice/classes/completion/custom_completion.php +++ b/mod/choice/classes/completion/custom_completion.php @@ -69,4 +69,16 @@ class custom_completion extends activity_custom_completion { 'completionsubmit' => get_string('completiondetail:submit', 'choice') ]; } + + /** + * Returns an array of all completion rules, in the order they should be displayed to users. + * + * @return array + */ + public function get_sort_order(): array { + return [ + 'completionview', + 'completionsubmit', + ]; + } } diff --git a/mod/data/classes/completion/custom_completion.php b/mod/data/classes/completion/custom_completion.php index 4ace7a58e50..0f00b797762 100644 --- a/mod/data/classes/completion/custom_completion.php +++ b/mod/data/classes/completion/custom_completion.php @@ -69,4 +69,17 @@ class custom_completion extends activity_custom_completion { 'completionentries' => get_string('completiondetail:entries', 'data', $entries), ]; } + + /** + * Returns an array of all completion rules, in the order they should be displayed to users. + * + * @return array + */ + public function get_sort_order(): array { + return [ + 'completionview', + 'completionentries', + 'completionusegrade', + ]; + } } diff --git a/mod/feedback/classes/completion/custom_completion.php b/mod/feedback/classes/completion/custom_completion.php index 1a17c713a54..1a8018fbaaf 100644 --- a/mod/feedback/classes/completion/custom_completion.php +++ b/mod/feedback/classes/completion/custom_completion.php @@ -67,4 +67,16 @@ class custom_completion extends activity_custom_completion { 'completionsubmit' => get_string('completiondetail:submit', 'feedback') ]; } + + /** + * Returns an array of all completion rules, in the order they should be displayed to users. + * + * @return array + */ + public function get_sort_order(): array { + return [ + 'completionview', + 'completionsubmit', + ]; + } } diff --git a/mod/forum/classes/completion/custom_completion.php b/mod/forum/classes/completion/custom_completion.php index b168abd44ad..ffe5b98b904 100644 --- a/mod/forum/classes/completion/custom_completion.php +++ b/mod/forum/classes/completion/custom_completion.php @@ -99,4 +99,19 @@ class custom_completion extends activity_custom_completion { 'completionposts' => get_string('completiondetail:posts', 'forum', $completionposts), ]; } + + /** + * Returns an array of all completion rules, in the order they should be displayed to users. + * + * @return array + */ + public function get_sort_order(): array { + return [ + 'completionview', + 'completiondiscussions', + 'completionreplies', + 'completionposts', + 'completionusegrade', + ]; + } } diff --git a/mod/glossary/classes/completion/custom_completion.php b/mod/glossary/classes/completion/custom_completion.php index 60deecbea23..41456447213 100644 --- a/mod/glossary/classes/completion/custom_completion.php +++ b/mod/glossary/classes/completion/custom_completion.php @@ -73,4 +73,17 @@ class custom_completion extends activity_custom_completion { 'completionentries' => get_string('completiondetail:entries', 'glossary', $completionentries), ]; } + + /** + * Returns an array of all completion rules, in the order they should be displayed to users. + * + * @return array + */ + public function get_sort_order(): array { + return [ + 'completionview', + 'completionentries', + 'completionusegrade', + ]; + } } diff --git a/mod/label/classes/completion/custom_completion.php b/mod/label/classes/completion/custom_completion.php index b6c7546b06d..c8ccd58d39b 100644 --- a/mod/label/classes/completion/custom_completion.php +++ b/mod/label/classes/completion/custom_completion.php @@ -70,4 +70,14 @@ class custom_completion extends activity_custom_completion { public function manual_completion_always_shown(): bool { return true; } + + /** + * Returns an array of all completion rules, in the order they should be displayed to users. + * + * @return array + */ + public function get_sort_order(): array { + // This module only supports manual completion. + return []; + } } diff --git a/mod/lesson/classes/completion/custom_completion.php b/mod/lesson/classes/completion/custom_completion.php index 133581261fc..c3ce80d264d 100644 --- a/mod/lesson/classes/completion/custom_completion.php +++ b/mod/lesson/classes/completion/custom_completion.php @@ -91,4 +91,18 @@ class custom_completion extends activity_custom_completion { 'completionendreached' => get_string('completiondetail:reachend', 'lesson'), ]; } + + /** + * Returns an array of all completion rules, in the order they should be displayed to users. + * + * @return array + */ + public function get_sort_order(): array { + return [ + 'completionview', + 'completiontimespent', + 'completionendreached', + 'completionusegrade', + ]; + } } diff --git a/mod/quiz/classes/completion/custom_completion.php b/mod/quiz/classes/completion/custom_completion.php index c647b3b0e84..4c4085920fc 100644 --- a/mod/quiz/classes/completion/custom_completion.php +++ b/mod/quiz/classes/completion/custom_completion.php @@ -159,4 +159,18 @@ class custom_completion extends activity_custom_completion { 'completionminattempts' => get_string('completiondetail:minattempts', 'mod_quiz', $minattempts), ]; } + + /** + * Returns an array of all completion rules, in the order they should be displayed to users. + * + * @return array + */ + public function get_sort_order(): array { + return [ + 'completionview', + 'completionminattempts', + 'completionusegrade', + 'completionpassorattemptsexhausted', + ]; + } } diff --git a/mod/resource/classes/completion/custom_completion.php b/mod/resource/classes/completion/custom_completion.php index 504d13ef99c..b09e75e2c5e 100644 --- a/mod/resource/classes/completion/custom_completion.php +++ b/mod/resource/classes/completion/custom_completion.php @@ -79,4 +79,14 @@ class custom_completion extends activity_custom_completion { return in_array($display, $displaytypes); } + + /** + * Returns an array of all completion rules, in the order they should be displayed to users. + * + * @return array + */ + public function get_sort_order(): array { + // This module only supports manual completion. + return []; + } } diff --git a/mod/scorm/classes/completion/custom_completion.php b/mod/scorm/classes/completion/custom_completion.php index aae539bbe59..6c2f7077059 100644 --- a/mod/scorm/classes/completion/custom_completion.php +++ b/mod/scorm/classes/completion/custom_completion.php @@ -202,4 +202,20 @@ class custom_completion extends activity_custom_completion { 'completionstatusallscos' => get_string('completiondetail:allscos', 'scorm'), ]; } + + /** + * Returns an array of all completion rules, in the order they should be displayed to users. + * + * @return array + */ + public function get_sort_order(): array { + return [ + 'completionview', + 'completionstatusallscos', + 'completionstatusrequired', + 'completionusegrade', + 'completionscorerequired', + ]; + } } + diff --git a/mod/survey/classes/completion/custom_completion.php b/mod/survey/classes/completion/custom_completion.php index 27819934d7b..8b53b19aed3 100644 --- a/mod/survey/classes/completion/custom_completion.php +++ b/mod/survey/classes/completion/custom_completion.php @@ -67,4 +67,16 @@ class custom_completion extends activity_custom_completion { 'completionsubmit' => get_string('completiondetail:submit', 'survey') ]; } + + /** + * Returns an array of all completion rules, in the order they should be displayed to users. + * + * @return array + */ + public function get_sort_order(): array { + return [ + 'completionview', + 'completionsubmit', + ]; + } } diff --git a/mod/url/classes/completion/custom_completion.php b/mod/url/classes/completion/custom_completion.php index 1fa0eded6bb..fe0bbd04da9 100644 --- a/mod/url/classes/completion/custom_completion.php +++ b/mod/url/classes/completion/custom_completion.php @@ -82,4 +82,13 @@ class custom_completion extends activity_custom_completion { return in_array($display, $displaytypes); } + /** + * Returns an array of all completion rules, in the order they should be displayed to users. + * + * @return array + */ + public function get_sort_order(): array { + // This module only supports manual completion. + return []; + } } From de08af9e4a84d4b4083b66090bf67674354aa2b9 Mon Sep 17 00:00:00 2001 From: Michael Hawkins Date: Thu, 29 Apr 2021 13:08:50 +0800 Subject: [PATCH 4/4] MDL-71189 mod_quiz: Update completion rule descriptions method Allow for conditions that are not set, consistent with other modules. They will usually always be set in practice, but best to handle it, which also helps with unit testing. --- mod/quiz/classes/completion/custom_completion.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mod/quiz/classes/completion/custom_completion.php b/mod/quiz/classes/completion/custom_completion.php index 4c4085920fc..f8716043fc7 100644 --- a/mod/quiz/classes/completion/custom_completion.php +++ b/mod/quiz/classes/completion/custom_completion.php @@ -145,9 +145,9 @@ class custom_completion extends activity_custom_completion { * @return array */ public function get_custom_rule_descriptions(): array { - $minattempts = $this->cm->customdata['customcompletionrules']['completionminattempts']; + $minattempts = $this->cm->customdata['customcompletionrules']['completionminattempts'] ?? 0; - $completionpassorattempts = $this->cm->customdata['customcompletionrules']['completionpassorattemptsexhausted']; + $completionpassorattempts = $this->cm->customdata['customcompletionrules']['completionpassorattemptsexhausted'] ?? []; if (!empty($completionpassorattempts['completionattemptsexhausted'])) { $passorallattemptslabel = get_string('completiondetail:passorexhaust', 'mod_quiz'); } else {