From 6d2e36aa370dc817f507d7a7c2fdbfaaeb1b09d3 Mon Sep 17 00:00:00 2001 From: Jun Pataleta Date: Tue, 16 Feb 2021 10:04:41 +0800 Subject: [PATCH 1/8] MDL-70815 core_completion: Activity custom completion details base class * Base class for defining an activity module's custom completion details --- .../classes/activity_custom_completion.php | 175 ++++++++++++++++++ lib/upgrade.txt | 9 + 2 files changed, 184 insertions(+) create mode 100644 completion/classes/activity_custom_completion.php diff --git a/completion/classes/activity_custom_completion.php b/completion/classes/activity_custom_completion.php new file mode 100644 index 00000000000..7c7806a5f7c --- /dev/null +++ b/completion/classes/activity_custom_completion.php @@ -0,0 +1,175 @@ +. + +declare(strict_types = 1); + +namespace core_completion; + +use cm_info; +use coding_exception; +use moodle_exception; + +/** + * Base class for defining an activity module's custom completion rules. + * + * Class for defining an activity module's custom completion rules and fetching the completion statuses + * of the custom completion rules for a given module instance and a user. + * + * @package core_completion + * @copyright 2021 Jun Pataleta + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +abstract class activity_custom_completion { + + /** @var cm_info The course module information object. */ + protected $cm; + + /** @var int The user's ID. */ + protected $userid; + + /** + * activity_custom_completion constructor. + * + * @param cm_info $cm + * @param int $userid + */ + public function __construct(cm_info $cm, int $userid) { + $this->cm = $cm; + $this->userid = $userid; + } + + /** + * Validates that the custom rule is defined by this plugin and is enabled for this activity instance. + * + * @param string $rule The custom completion rule. + */ + public function validate_rule(string $rule): void { + // Check that this custom completion rule is defined. + if (!$this->is_defined($rule)) { + throw new coding_exception("Undefined custom completion rule '$rule'"); + } + + // Check that this custom rule is included in the course module's custom completion rules. + if (!$this->is_available($rule)) { + throw new moodle_exception("Custom completion rule '$rule' is not used by this activity."); + } + } + + /** + * Whether this module defines this custom rule. + * + * @param string $rule The custom completion rule. + * @return bool + */ + public function is_defined(string $rule): bool { + return in_array($rule, static::get_defined_custom_rules()); + } + + /** + * Checks whether the custom completion rule is being used by the activity module instance. + * + * @param string $rule The custom completion rule. + * @return bool + */ + public function is_available(string $rule): bool { + return in_array($rule, $this->get_available_custom_rules()); + } + + /** + * Fetches the list of custom completion rules that are being used by this activity module instance. + * + * @return array + */ + public function get_available_custom_rules(): array { + $rules = static::get_defined_custom_rules(); + $availablerules = []; + foreach ($rules as $rule) { + $customrule = $this->cm->customdata['customcompletionrules'][$rule] ?? false; + if (!empty($customrule)) { + $availablerules[] = $rule; + } + } + return $availablerules; + } + + /** + * 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). + */ + public function get_overall_completion_state(): int { + 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 this was reached, then all custom rules have been marked complete. + return COMPLETION_COMPLETE; + } + + /** + * Fetches the description for a given custom completion rule. + * + * @param string $rule The custom completion rule. + * @return string + */ + public function get_custom_rule_description(string $rule): string { + $descriptions = $this->get_custom_rule_descriptions(); + if (!isset($descriptions[$rule])) { + // Lang string not found for this custom completion rule. Just return it. + return $rule; + } + return $descriptions[$rule]; + } + + /** + * Fetches the module's custom completion class implementation if it's available. + * + * @param string $modname The activity module name. Usually from cm_info::modname. + * @return string|null + */ + public static function get_cm_completion_class(string $modname): ?string { + $cmcompletionclass = "mod_{$modname}\\completion\\custom_completion"; + if (class_exists($cmcompletionclass) && is_subclass_of($cmcompletionclass, self::class)) { + return $cmcompletionclass; + } + return null; + } + + /** + * Fetches the completion state for a given completion rule. + * + * @param string $rule The completion rule. + * @return int The completion state. + */ + public abstract 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; + + /** + * Returns an associative array of the descriptions of custom completion rules. + * + * @return array + */ + public abstract function get_custom_rule_descriptions(): array; +} diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 53ca66805d3..384419f410e 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -33,6 +33,15 @@ information provided here is intended especially for developers. * New DML driver method `$DB->sql_group_concat` for performing group concatenation of a field within a SQL query * Added new class, AMD modules and WS that allow displaying forms in modal popups or load and submit in AJAX requests. See https://docs.moodle.org/dev/Modal_and_AJAX_forms for more details. +* New base class for defining an activity's custom completion requirements: \core_completion\activity_custom_completion. + Activity module plugins that define custom completion conditions should implement a mod_[modname]\completion\custom_completion + subclass and the following methods: + - get_state(): Provides the completion state for a given custom completion rule. + - get_defined_custom_rules(): Returns an array of the activity module's custom completion rules. + e.g. ['completionsubmit'] + - 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'] === 3.10 === * PHPUnit has been upgraded to 8.5. That comes with a few changes: From e789f013b43bd89d07b844f11fe5fbc68df7ace8 Mon Sep 17 00:00:00 2001 From: Jun Pataleta Date: Sat, 13 Feb 2021 22:45:33 +0800 Subject: [PATCH 2/8] MDL-70815 core_completion: Unit tests for activity_custom_completion Tests cover - get_overall_completion_state() - is_available() - validate_rule() Tests don't cover - methods that rely on static methods such as: - is_defined() - static methods in the class because they can't be mocked - abstract methods that can be tested better by the plugins that extend activity_custom_completion such as: - get_state() - get_defined_custom_rules() - get_custom_rule_descriptions() --- .../tests/activity_custom_completion_test.php | 187 ++++++++++++++++++ 1 file changed, 187 insertions(+) create mode 100644 completion/tests/activity_custom_completion_test.php diff --git a/completion/tests/activity_custom_completion_test.php b/completion/tests/activity_custom_completion_test.php new file mode 100644 index 00000000000..1a237d96424 --- /dev/null +++ b/completion/tests/activity_custom_completion_test.php @@ -0,0 +1,187 @@ +. + +declare(strict_types = 1); + +namespace core_completion; + +use advanced_testcase; +use coding_exception; +use moodle_exception; +use PHPUnit\Framework\MockObject\MockObject; + +/** + * Class for unit testing core_completion/activity_custom_completion. + * + * @package core_completion + * @copyright 2021 Jun Pataleta + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class activity_custom_completion_test extends advanced_testcase { + + /** + * Fetches a mocked activity_custom_completion instance. + * + * @param string[] $methods List of methods to mock. + * @return activity_custom_completion|MockObject + */ + protected function setup_mock(array $methods) { + return $this->getMockBuilder(activity_custom_completion::class) + ->disableOriginalConstructor() + ->onlyMethods($methods) + ->getMockForAbstractClass(); + } + + /** + * Data provider for test_get_overall_completion_state(). + */ + public function overall_completion_state_provider(): array { + global $CFG; + require_once($CFG->libdir . '/completionlib.php'); + return [ + 'First incomplete, second complete' => [ + ['completionsubmit', 'completioncreate'], + [COMPLETION_INCOMPLETE, COMPLETION_COMPLETE], + 1, + COMPLETION_INCOMPLETE + ], + 'First complete, second incomplete' => [ + ['completionsubmit', 'completioncreate'], + [COMPLETION_COMPLETE, COMPLETION_INCOMPLETE], + 2, + COMPLETION_INCOMPLETE + ], + 'All complete' => [ + ['completionsubmit', 'completioncreate'], + [COMPLETION_COMPLETE, COMPLETION_COMPLETE], + 2, + COMPLETION_COMPLETE + ], + 'No rules' => [ + [], + [], + 0, + COMPLETION_COMPLETE + ], + ]; + } + + /** + * Test for \core_completion\activity_custom_completion::get_overall_completion_state(). + * + * @dataProvider overall_completion_state_provider + * @param string[] $rules The custom completion rules. + * @param int[] $rulestates The completion states of these custom completion rules. + * @param int $invokecount Expected invoke count of get_state(). + * @param int $state The expected overall completion state + */ + public function test_get_overall_completion_state(array $rules, array $rulestates, int $invokecount, int $state) { + $stub = $this->setup_mock([ + 'get_available_custom_rules', + 'get_state', + ]); + + // Mock activity_custom_completion's get_available_custom_rules() method. + $stub->expects($this->once()) + ->method('get_available_custom_rules') + ->willReturn($rules); + + // Mock activity_custom_completion's get_state() method. + if ($invokecount > 0) { + $stub->expects($this->exactly($invokecount)) + ->method('get_state') + ->withConsecutive( + [$rules[0]], + [$rules[1]] + ) + ->willReturn($rulestates[0], $rulestates[1]); + } else { + $stub->expects($this->never()) + ->method('get_state'); + } + + $this->assertEquals($state, $stub->get_overall_completion_state()); + } + + /** + * Data provider for test_validate_rule(). + * + * @return array[] + */ + public function validate_rule_provider() { + return [ + 'Not defined' => [ + false, true, coding_exception::class + ], + 'Not available' => [ + true, false, moodle_exception::class + ], + 'Defined and available' => [ + true, true, null + ], + ]; + } + + /** + * Test for validate_rule() + * + * @dataProvider validate_rule_provider + * @param bool $defined is_defined()'s mocked return value. + * @param bool $available is_available()'s mocked return value. + * @param string|null $expectedexception Expected expectation class name. + */ + public function test_validate_rule(bool $defined, bool $available, ?string $expectedexception) { + $stub = $this->setup_mock([ + 'is_defined', + 'is_available' + ]); + + // Mock activity_custom_completion's is_defined() method. + $stub->expects($this->any()) + ->method('is_defined') + ->willReturn($defined); + + // Mock activity_custom_completion's is_available() method. + $stub->expects($this->any()) + ->method('is_available') + ->willReturn($available); + + if ($expectedexception) { + $this->expectException($expectedexception); + } + $stub->validate_rule('customcompletionrule'); + } + + /** + * Test for is_available(). + */ + public function test_is_available() { + $stub = $this->setup_mock([ + 'get_available_custom_rules', + ]); + + // Mock activity_custom_completion's get_available_custom_rules() method. + $stub->expects($this->any()) + ->method('get_available_custom_rules') + ->willReturn(['rule1', 'rule2']); + + // Rule is available. + $this->assertTrue($stub->is_available('rule1')); + + // Rule is not available. + $this->assertFalse($stub->is_available('rule')); + } +} From baa00010377c19030a853ef7442838e59bb57f8a Mon Sep 17 00:00:00 2001 From: Jun Pataleta Date: Thu, 11 Feb 2021 09:39:36 +0800 Subject: [PATCH 3/8] MDL-70815 core_completion: Fix unit tests * Unit tests for completion_info::get_data() and completion_info::internal_get_state are mocked which causes failures with the new implementation. It's more straightforward and realistic to generate real course and modules to test these methods. --- lib/tests/completionlib_test.php | 291 +++++++++++++++++++++---------- 1 file changed, 196 insertions(+), 95 deletions(-) diff --git a/lib/tests/completionlib_test.php b/lib/tests/completionlib_test.php index 08d93a5ff54..ef27f639a77 100644 --- a/lib/tests/completionlib_test.php +++ b/lib/tests/completionlib_test.php @@ -325,42 +325,106 @@ class core_completionlib_testcase extends advanced_testcase { $c->get_data($cm, false, 100); } - public function test_internal_get_state() { - global $DB; - $this->mock_setup(); + /** + * Data provider for test_internal_get_state(). + * + * @return array[] + */ + public function internal_get_state_provider() { + return [ + 'View required, but not viewed yet' => [ + COMPLETION_VIEW_REQUIRED, 1, '', COMPLETION_INCOMPLETE + ], + 'View not required and not viewed yet' => [ + COMPLETION_VIEW_NOT_REQUIRED, 1, '', COMPLETION_INCOMPLETE + ], + 'View not required, grade required but no grade yet, $cm->modname not set' => [ + COMPLETION_VIEW_NOT_REQUIRED, 1, 'modname', COMPLETION_INCOMPLETE + ], + 'View not required, grade required but no grade yet, $cm->course not set' => [ + COMPLETION_VIEW_NOT_REQUIRED, 1, 'course', COMPLETION_INCOMPLETE + ], + 'View not required, grade not required' => [ + COMPLETION_VIEW_NOT_REQUIRED, 0, '', COMPLETION_COMPLETE + ], + ]; + } - $mockbuilder = $this->getMockBuilder('completion_info'); - $mockbuilder->setMethods(array('internal_get_grade_state')); - $mockbuilder->setConstructorArgs(array((object)array('id' => 42))); - $c = $mockbuilder->getMock(); + /** + * Test for completion_info::get_state(). + * + * @dataProvider internal_get_state_provider + * @param int $completionview + * @param int $completionusegrade + * @param string $unsetfield + * @param int $expectedstate + */ + public function test_internal_get_state(int $completionview, int $completionusegrade, string $unsetfield, int $expectedstate) { + $this->setup_data(); - $cm = (object)array('id'=>13, 'course'=>42, 'completiongradeitemnumber'=>null); + /** @var \mod_assign_generator $assigngenerator */ + $assigngenerator = $this->getDataGenerator()->get_plugin_generator('mod_assign'); + $assign = $assigngenerator->create_instance([ + 'course' => $this->course->id, + 'completion' => COMPLETION_ENABLED, + 'completionview' => $completionview, + 'completionusegrade' => $completionusegrade, + ]); + $userid = $this->user->id; + $this->setUser($userid); + + $cm = get_coursemodule_from_instance('assign', $assign->id); + if ($unsetfield) { + unset($cm->$unsetfield); + } // If view is required, but they haven't viewed it yet. - $cm->completionview = COMPLETION_VIEW_REQUIRED; - $current = (object)array('viewed'=>COMPLETION_NOT_VIEWED); - $this->assertEquals(COMPLETION_INCOMPLETE, $c->internal_get_state($cm, 123, $current)); + $current = (object)['viewed' => COMPLETION_NOT_VIEWED]; - // OK set view not required. - $cm->completionview = COMPLETION_VIEW_NOT_REQUIRED; + $completioninfo = new completion_info($this->course); + $this->assertEquals($expectedstate, $completioninfo->internal_get_state($cm, $userid, $current)); + } - // Test not getting module name. - $cm->modname='label'; - $this->assertEquals(COMPLETION_COMPLETE, $c->internal_get_state($cm, 123, $current)); + /** + * Covers the case where internal_get_state() is being called for a user different from the logged in user. + */ + public function test_internal_get_state_with_different_user() { + $this->setup_data(); - // Test getting module name. - $cm->module = 13; - unset($cm->modname); - /** @var $DB PHPUnit_Framework_MockObject_MockObject */ - $DB->expects($this->once()) - ->method('get_field') - ->with('modules', 'name', array('id'=>13)) - ->will($this->returnValue('lable')); - $this->assertEquals(COMPLETION_COMPLETE, $c->internal_get_state($cm, 123, $current)); + /** @var \mod_assign_generator $assigngenerator */ + $assigngenerator = $this->getDataGenerator()->get_plugin_generator('mod_assign'); + $assign = $assigngenerator->create_instance([ + 'course' => $this->course->id, + 'completion' => COMPLETION_ENABLED, + 'completionusegrade' => 1, + ]); - // Note: This function is not fully tested (including kind of the main part) because: - // * the grade_item/grade_grade calls are static and can't be mocked, - // * the plugin_supports call is static and can't be mocked. + $userid = $this->user->id; + + $cm = get_coursemodule_from_instance('assign', $assign->id); + $usercm = cm_info::create($cm, $userid); + + // Create a teacher account. + $teacher = $this->getDataGenerator()->create_user(); + $this->getDataGenerator()->enrol_user($teacher->id, $this->course->id, 'editingteacher'); + // Log in as the teacher. + $this->setUser($teacher); + + // Grade the student for this assignment. + $assign = new assign($usercm->context, $cm, $cm->course); + $data = (object)[ + 'sendstudentnotifications' => false, + 'attemptnumber' => 1, + 'grade' => 90, + ]; + $assign->save_grade($userid, $data); + + // The target user already received a grade, so internal_get_state should be already complete. + $completioninfo = new completion_info($this->course); + $this->assertEquals(COMPLETION_COMPLETE, $completioninfo->internal_get_state($cm, $userid, null)); + + // As the teacher which does not have a grade in this cm, internal_get_state should return incomplete. + $this->assertEquals(COMPLETION_INCOMPLETE, $completioninfo->internal_get_state($cm, $teacher->id, null)); } public function test_set_module_viewed() { @@ -488,77 +552,114 @@ class core_completionlib_testcase extends advanced_testcase { $c->reset_all_state($cm); } - public function test_get_data() { + /** + * Data provider for test_get_data(). + * + * @return array[] + */ + public function get_data_provider() { + return [ + 'No completion record' => [ + false, false, false, COMPLETION_INCOMPLETE + ], + 'Not completed' => [ + false, false, true, COMPLETION_INCOMPLETE + ], + 'Completed' => [ + false, false, true, COMPLETION_COMPLETE + ], + 'Whole course, complete' => [ + true, false, true, COMPLETION_COMPLETE + ], + 'Get data for another user, result should be not cached' => [ + false, true, true, COMPLETION_INCOMPLETE + ], + ]; + } + + /** + * Tests for completion_info::get_data(). + * + * @dataProvider get_data_provider + * @param bool $wholecourse Whole course parameter for get_data(). + * @param bool $sameuser Whether the user calling get_data() is the user itself. + * @param bool $hasrecord Whether to create a course_modules_completion record. + * @param int $completion The completion state expected. + */ + public function test_get_data(bool $wholecourse, bool $sameuser, bool $hasrecord, int $completion) { global $DB; - $this->mock_setup(); + $this->setup_data(); + $user = $this->user; + + /** @var \mod_choice_generator $choicegenerator */ + $choicegenerator = $this->getDataGenerator()->get_plugin_generator('mod_choice'); + $choice = $choicegenerator->create_instance([ + 'course' => $this->course->id, + 'completion' => true, + 'completionview' => true, + ]); + + $cm = get_coursemodule_from_instance('choice', $choice->id); + + // Let's manually create a course completion record instead of going thru the hoops to complete an activity. + if ($hasrecord) { + $cmcompletionrecord = (object)[ + 'coursemoduleid' => $cm->id, + 'userid' => $user->id, + 'completionstate' => $completion, + 'viewed' => 0, + 'overrideby' => null, + 'timemodified' => 0, + ]; + $DB->insert_record('course_modules_completion', $cmcompletionrecord); + } + + // Whether we expect for the returned completion data to be stored in the cache. + $iscached = true; + + if (!$sameuser) { + $iscached = false; + $this->setAdminUser(); + } else { + $this->setUser($user); + } + + // Mock other completion data. + $completioninfo = new completion_info($this->course); + + $result = $completioninfo->get_data($cm, $wholecourse, $user->id); + // Course module ID of the returned completion data must match this activity's course module ID. + $this->assertEquals($cm->id, $result->coursemoduleid); + // User ID of the returned completion data must match the user's ID. + $this->assertEquals($user->id, $result->userid); + // The completion state of the returned completion data must match the expected completion state. + $this->assertEquals($completion, $result->completionstate); + + // If the user has no completion record, then the default record should be returned. + if (!$hasrecord) { + $iscached = false; + $this->assertEquals(0, $result->id); + } + + // Check caching. + $key = "{$user->id}_{$this->course->id}"; $cache = cache::make('core', 'completion'); + if ($iscached) { + // If we expect this to be cached, then fetching the result must match the cached data. + $this->assertEquals($result, (object)$cache->get($key)[$cm->id]); - $c = new completion_info((object)array('id'=>42, 'cacherev'=>1)); - $cm = (object)array('id'=>13, 'course'=>42); - - // 1. Not current user, record exists. - $sillyrecord = (object)array('frog'=>'kermit'); - - /** @var $DB PHPUnit_Framework_MockObject_MockObject */ - $DB->expects($this->at(0)) - ->method('get_record') - ->with('course_modules_completion', array('coursemoduleid'=>13, 'userid'=>123)) - ->will($this->returnValue($sillyrecord)); - $result = $c->get_data($cm, false, 123); - $this->assertEquals($sillyrecord, $result); - $this->assertEquals(false, $cache->get('123_42')); // Not current user is not cached. - - // 2. Not current user, default record, whole course. - $cache->purge(); - $DB->expects($this->at(0)) - ->method('get_records_sql') - ->will($this->returnValue(array())); - $modinfo = new stdClass(); - $modinfo->cms = array((object)array('id'=>13)); - $result=$c->get_data($cm, true, 123, $modinfo); - $this->assertEquals((object)array( - 'id' => '0', 'coursemoduleid' => 13, 'userid' => 123, 'completionstate' => 0, - 'viewed' => 0, 'timemodified' => 0, 'overrideby' => 0), $result); - $this->assertEquals(false, $cache->get('123_42')); // Not current user is not cached. - - // 3. Current user, single record, not from cache. - $DB->expects($this->at(0)) - ->method('get_record') - ->with('course_modules_completion', array('coursemoduleid'=>13, 'userid'=>314159)) - ->will($this->returnValue($sillyrecord)); - $result = $c->get_data($cm); - $this->assertEquals($sillyrecord, $result); - $cachevalue = $cache->get('314159_42'); - $this->assertEquals((array)$sillyrecord, $cachevalue[13]); - - // 4. Current user, 'whole course', but from cache. - $result = $c->get_data($cm, true); - $this->assertEquals($sillyrecord, $result); - - // 5. Current user, 'whole course' and record not in cache. - $cache->purge(); - - // Scenario: Completion data exists for one CMid. - $basicrecord = (object)array('coursemoduleid'=>13); - $DB->expects($this->at(0)) - ->method('get_records_sql') - ->will($this->returnValue(array('1'=>$basicrecord))); - - // There are two CMids in total, the one we had data for and another one. - $modinfo = new stdClass(); - $modinfo->cms = array((object)array('id'=>13), (object)array('id'=>14)); - $result = $c->get_data($cm, true, 0, $modinfo); - - // Check result. - $this->assertEquals($basicrecord, $result); - - // Check the cache contents. - $cachevalue = $cache->get('314159_42'); - $this->assertEquals($basicrecord, (object)$cachevalue[13]); - $this->assertEquals(array('id' => '0', 'coursemoduleid' => 14, - 'userid' => 314159, 'completionstate' => 0, 'viewed' => 0, 'overrideby' => 0, 'timemodified' => 0), - $cachevalue[14]); + // Check cached data for other course modules in the course. + // The sample module created in setup_data() should suffice to confirm this. + if ($wholecourse) { + $this->assertArrayHasKey($this->module1->id, $cache->get($key)); + } else { + $this->assertArrayNotHasKey($this->module1->id, $cache->get($key)); + } + } else { + // Otherwise, this should not be cached. + $this->assertFalse($cache->get($key)); + } } public function test_internal_set_data() { From 31fcbfc2e2b1f721cde13fd828e9f33870cbc3ad Mon Sep 17 00:00:00 2001 From: Jun Pataleta Date: Wed, 10 Feb 2021 10:57:42 +0800 Subject: [PATCH 4/8] MDL-70815 core_completion: completion_info::get_grade_completion() Move the current logic for determining the completion status for the "Student must receive grade" completion rule to a function so it cann be reused. Unit test included. --- lib/completionlib.php | 90 ++++++++++++++++++++------------ lib/tests/completionlib_test.php | 54 +++++++++++++++++++ 2 files changed, 112 insertions(+), 32 deletions(-) diff --git a/lib/completionlib.php b/lib/completionlib.php index 2258259f865..e92d92d3ba0 100644 --- a/lib/completionlib.php +++ b/lib/completionlib.php @@ -643,7 +643,7 @@ class completion_info { * @return mixed */ public function internal_get_state($cm, $userid, $current) { - global $USER, $DB, $CFG; + global $USER, $DB; // Get user ID if (!$userid) { @@ -657,49 +657,37 @@ class completion_info { return COMPLETION_INCOMPLETE; } - // Modname hopefully is provided in $cm but just in case it isn't, let's grab it - if (!isset($cm->modname)) { - $cm->modname = $DB->get_field('modules', 'name', array('id'=>$cm->module)); + if ($cm instanceof stdClass) { + // Modname hopefully is provided in $cm but just in case it isn't, let's grab it. + if (!isset($cm->modname)) { + $cm->modname = $DB->get_field('modules', 'name', array('id' => $cm->module)); + } + // Some functions call this method and pass $cm as an object with ID only. Make sure course is set as well. + if (!isset($cm->course)) { + $cm->course = $this->course_id; + } } + // Make sure we're using a cm_info object. + $cminfo = cm_info::create($cm, $userid); $newstate = COMPLETION_COMPLETE; // Check grade - if (!is_null($cm->completiongradeitemnumber)) { - require_once($CFG->libdir.'/gradelib.php'); - $item = grade_item::fetch(array('courseid'=>$cm->course, 'itemtype'=>'mod', - 'itemmodule'=>$cm->modname, 'iteminstance'=>$cm->instance, - 'itemnumber'=>$cm->completiongradeitemnumber)); - if ($item) { - // Fetch 'grades' (will be one or none) - $grades = grade_grade::fetch_users_grades($item, array($userid), false); - if (empty($grades)) { - // No grade for user - return COMPLETION_INCOMPLETE; - } - if (count($grades) > 1) { - $this->internal_systemerror("Unexpected result: multiple grades for - item '{$item->id}', user '{$userid}'"); - } - $newstate = self::internal_get_grade_state($item, reset($grades)); - if ($newstate == COMPLETION_INCOMPLETE) { - return COMPLETION_INCOMPLETE; - } - - } else { - $this->internal_systemerror("Cannot find grade item for '{$cm->modname}' - cm '{$cm->id}' matching number '{$cm->completiongradeitemnumber}'"); + if (!is_null($cminfo->completiongradeitemnumber)) { + $newstate = $this->get_grade_completion($cminfo, $userid); + if ($newstate == COMPLETION_INCOMPLETE) { + return COMPLETION_INCOMPLETE; } } - if (plugin_supports('mod', $cm->modname, FEATURE_COMPLETION_HAS_RULES)) { - $function = $cm->modname.'_get_completion_state'; + if (plugin_supports('mod', $cminfo->modname, FEATURE_COMPLETION_HAS_RULES)) { + $function = $cminfo->modname . '_get_completion_state'; if (!function_exists($function)) { - $this->internal_systemerror("Module {$cm->modname} claims to support + $this->internal_systemerror("Module {$cminfo->modname} claims to support FEATURE_COMPLETION_HAS_RULES but does not have required {$cm->modname}_get_completion_state function"); } - if (!$function($this->course, $cm, $userid, COMPLETION_AND)) { + if (!$function($this->course, $cminfo, $userid, COMPLETION_AND)) { return COMPLETION_INCOMPLETE; } } @@ -708,6 +696,44 @@ class completion_info { } + /** + * Fetches the completion state for an activity completion's require grade completion requirement. + * + * @param cm_info $cm The course module information. + * @param int $userid The user ID. + * @return int The completion state. + */ + public function get_grade_completion(cm_info $cm, int $userid): int { + global $CFG; + + require_once($CFG->libdir . '/gradelib.php'); + $item = grade_item::fetch([ + 'courseid' => $cm->course, + 'itemtype' => 'mod', + 'itemmodule' => $cm->modname, + 'iteminstance' => $cm->instance, + 'itemnumber' => $cm->completiongradeitemnumber + ]); + if ($item) { + // Fetch 'grades' (will be one or none). + $grades = grade_grade::fetch_users_grades($item, [$userid], false); + if (empty($grades)) { + // No grade for user. + return COMPLETION_INCOMPLETE; + } + if (count($grades) > 1) { + $this->internal_systemerror("Unexpected result: multiple grades for + item '{$item->id}', user '{$userid}'"); + } + return self::internal_get_grade_state($item, reset($grades)); + } else { + $this->internal_systemerror("Cannot find grade item for '{$cm->modname}' + cm '{$cm->id}' matching number '{$cm->completiongradeitemnumber}'"); + } + + return COMPLETION_INCOMPLETE; + } + /** * Marks a module as viewed. * diff --git a/lib/tests/completionlib_test.php b/lib/tests/completionlib_test.php index ef27f639a77..eacc7a3fbf4 100644 --- a/lib/tests/completionlib_test.php +++ b/lib/tests/completionlib_test.php @@ -1143,6 +1143,60 @@ class core_completionlib_testcase extends advanced_testcase { $this->assertTrue(completion_can_view_data($student->id, $this->course->id)); $this->assertFalse(completion_can_view_data($this->user->id, $this->course->id)); } + + /** + * Data provider for test_get_grade_completion(). + * + * @return array[] + */ + public function get_grade_completion_provider() { + return [ + 'Grade not required' => [false, false, null, moodle_exception::class, null], + 'Grade required, but has no grade yet' => [true, false, null, null, COMPLETION_INCOMPLETE], + 'Grade required, grade received' => [true, true, null, null, COMPLETION_COMPLETE], + 'Grade required, passing grade received' => [true, true, 70, null, COMPLETION_COMPLETE_PASS], + 'Grade required, failing grade received' => [true, true, 80, null, COMPLETION_COMPLETE_FAIL], + ]; + } + + /** + * Test for \completion_info::get_grade_completion(). + * + * @dataProvider get_grade_completion_provider + * @param bool $completionusegrade Whether the test activity has grade completion requirement. + * @param bool $hasgrade Whether to set grade for the user in this activity. + * @param int|null $passinggrade Passing grade to set for the test activity. + * @param string|null $expectedexception Expected exception. + * @param int|null $expectedresult The expected completion status. + */ + public function test_get_grade_completion(bool $completionusegrade, bool $hasgrade, ?int $passinggrade, ?string $expectedexception, + ?int $expectedresult) { + $this->setup_data(); + + /** @var \mod_assign_generator $assigngenerator */ + $assigngenerator = $this->getDataGenerator()->get_plugin_generator('mod_assign'); + $assign = $assigngenerator->create_instance([ + 'course' => $this->course->id, + 'completion' => COMPLETION_ENABLED, + 'completionusegrade' => $completionusegrade, + 'gradepass' => $passinggrade, + ]); + + $cm = cm_info::create(get_coursemodule_from_instance('assign', $assign->id)); + if ($completionusegrade && $hasgrade) { + $assigninstance = new assign($cm->context, $cm, $this->course); + $grade = $assigninstance->get_user_grade($this->user->id, true); + $grade->grade = 75; + $assigninstance->update_grade($grade); + } + + $completioninfo = new completion_info($this->course); + if ($expectedexception) { + $this->expectException($expectedexception); + } + $gradecompletion = $completioninfo->get_grade_completion($cm, $this->user->id); + $this->assertEquals($expectedresult, $gradecompletion); + } } class core_completionlib_fake_recordset implements Iterator { From ad5b613c86e82967e58d05898871452da54e1499 Mon Sep 17 00:00:00 2001 From: Jun Pataleta Date: Wed, 10 Feb 2021 11:05:42 +0800 Subject: [PATCH 5/8] MDL-70815 core_completion: Update completion_info * Update completion_info::get_data() to add other completion information from a new method called get_other_cm_completion_data(). This allows the storage of the completion statuses of the following completion rules to completion_info objects: - 'Students must receive a grade' completion rule. - Any custom completion rule defined by an activity. This allows detailed completion information to be fetched for course modules. It also allows custom completion statuses to be cached which will help reduce DB queries when fetching completion statuses. * Update update_state() to fetch overall completion state from the module's activity_custom_completion implementation. Falls back to the *_get_completion_state() callback function. * Update internal_set_data() to include the other cm completion data in the updated cache data for the module instance. --- lib/completionlib.php | 148 ++++++++++++++++++++++++++++++++---------- 1 file changed, 112 insertions(+), 36 deletions(-) diff --git a/lib/completionlib.php b/lib/completionlib.php index e92d92d3ba0..eb77a50ac62 100644 --- a/lib/completionlib.php +++ b/lib/completionlib.php @@ -26,6 +26,8 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ +use core_completion\activity_custom_completion; + defined('MOODLE_INTERNAL') || die(); /** @@ -681,14 +683,24 @@ class completion_info { } if (plugin_supports('mod', $cminfo->modname, FEATURE_COMPLETION_HAS_RULES)) { - $function = $cminfo->modname . '_get_completion_state'; - if (!function_exists($function)) { - $this->internal_systemerror("Module {$cminfo->modname} claims to support + $cmcompletionclass = activity_custom_completion::get_cm_completion_class($cminfo->modname); + if ($cmcompletionclass) { + /** @var activity_custom_completion $cmcompletion */ + $cmcompletion = new $cmcompletionclass($cminfo, $userid); + if ($cmcompletion->get_overall_completion_state() == COMPLETION_INCOMPLETE) { + return COMPLETION_INCOMPLETE; + } + } else { + // Fallback to the get_completion_state callback. + $function = $cminfo->modname . '_get_completion_state'; + if (!function_exists($function)) { + $this->internal_systemerror("Module {$cminfo->modname} claims to support FEATURE_COMPLETION_HAS_RULES but does not have required - {$cm->modname}_get_completion_state function"); - } - if (!$function($this->course, $cminfo, $userid, COMPLETION_AND)) { - return COMPLETION_INCOMPLETE; + {$cminfo->modname}_get_completion_state function"); + } + if (!$function($this->course, $cminfo, $userid, COMPLETION_AND)) { + return COMPLETION_INCOMPLETE; + } } } @@ -953,7 +965,7 @@ class completion_info { * Obtains completion data for a particular activity and user (from the * completion cache if available, or by SQL query) * - * @param stcClass|cm_info $cm Activity; only required field is ->id + * @param stdClass|cm_info $cm Activity; only required field is ->id * @param bool $wholecourse If true (default false) then, when necessary to * fill the cache, retrieves information from the entire course not just for * this one activity @@ -962,10 +974,12 @@ class completion_info { * testing and so that it can be called recursively from within * get_fast_modinfo. (Needs only list of all CMs with IDs.) * Otherwise the method calls get_fast_modinfo itself. - * @return object Completion data (record from course_modules_completion) + * @return object Completion data. Record from course_modules_completion plus other completion statuses such as + * - Completion status for 'must-receive-grade' completion rule. + * - Custom completion statuses defined by the activity module plugin. */ public function get_data($cm, $wholecourse = false, $userid = 0, $modinfo = null) { - global $USER, $CFG, $DB; + global $USER, $DB; $completioncache = cache::make('core', 'completion'); // Get user ID @@ -991,7 +1005,27 @@ class completion_info { } } - // Not there, get via SQL + // Some call completion_info::get_data and pass $cm as an object with ID only. Make sure course is set as well. + if ($cm instanceof stdClass && !isset($cm->course)) { + $cm->course = $this->course_id; + } + // Make sure we're working on a cm_info object. + $cminfo = cm_info::create($cm, $userid); + + // Default data to return when no completion data is found. + $defaultdata = [ + 'id' => 0, + 'coursemoduleid' => $cminfo->id, + 'userid' => $userid, + 'completionstate' => 0, + 'viewed' => 0, + 'overrideby' => null, + 'timemodified' => 0, + ]; + + // If cached completion data is not found, fetch via SQL. + // Fetch completion data for all of the activities in the course ONLY if we're caching the fetched completion data. + // If we're not caching the completion data, then just fetch the completion data for the user in this course module. if ($usecache && $wholecourse) { // Get whole course data for cache $alldatabycmc = $DB->get_records_sql(" @@ -1017,49 +1051,85 @@ class completion_info { if (isset($alldata[$othercm->id])) { $data = $alldata[$othercm->id]; } else { - // Row not present counts as 'not complete' - $data = array(); - $data['id'] = 0; + // Row not present counts as 'not complete'. + $data = $defaultdata; $data['coursemoduleid'] = $othercm->id; - $data['userid'] = $userid; - $data['completionstate'] = 0; - $data['viewed'] = 0; - $data['overrideby'] = null; - $data['timemodified'] = 0; } - $cacheddata[$othercm->id] = $data; + // Make sure we're working on a cm_info object. + $othercminfo = cm_info::create($othercm, $userid); + // Add the other completion data for this user in this module instance. + $data += $this->get_other_cm_completion_data($othercminfo, $userid); + $cacheddata[$othercminfo->id] = $data; } - if (!isset($cacheddata[$cm->id])) { - $this->internal_systemerror("Unexpected error: course-module {$cm->id} could not be found on course {$this->course->id}"); + if (!isset($cacheddata[$cminfo->id])) { + $errormessage = "Unexpected error: course-module {$cminfo->id} could not be found on course {$this->course->id}"; + $this->internal_systemerror($errormessage); } } else { // Get single record - $data = $DB->get_record('course_modules_completion', array('coursemoduleid'=>$cm->id, 'userid'=>$userid)); + $data = $DB->get_record('course_modules_completion', array('coursemoduleid' => $cminfo->id, 'userid' => $userid)); if ($data) { $data = (array)$data; } else { - // Row not present counts as 'not complete' - $data = array(); - $data['id'] = 0; - $data['coursemoduleid'] = $cm->id; - $data['userid'] = $userid; - $data['completionstate'] = 0; - $data['viewed'] = 0; - $data['overrideby'] = null; - $data['timemodified'] = 0; + // Row not present counts as 'not complete'. + $data = $defaultdata; } + // Fill the other completion data for this user in this module instance. + $data += $this->get_other_cm_completion_data($cminfo, $userid); // Put in cache - $cacheddata[$cm->id] = $data; + $cacheddata[$cminfo->id] = $data; } if ($usecache) { $cacheddata['cacherev'] = $this->course->cacherev; $completioncache->set($key, $cacheddata); } - return (object)$cacheddata[$cm->id]; + return (object)$cacheddata[$cminfo->id]; + } + + /** + * Adds the user's custom completion data on the given course module. + * + * @param cm_info $cm The course module information. + * @param int $userid The user ID. + * @return array The additional completion data. + */ + protected function get_other_cm_completion_data(cm_info $cm, int $userid): array { + $data = []; + + // Include in the completion info the grade completion, if necessary. + if (!is_null($cm->completiongradeitemnumber)) { + $data['completiongrade'] = $this->get_grade_completion($cm, $userid); + } + + // Custom activity module completion data. + + // Return early if the plugin does not define custom completion rules. + if (empty($cm->customdata['customcompletionrules'])) { + return []; + } + + // Return early if the activity modules doe not implement the activity_custom_completion class. + $cmcompletionclass = activity_custom_completion::get_cm_completion_class($cm->modname); + if (!$cmcompletionclass) { + return []; + } + + /** @var activity_custom_completion $customcmcompletion */ + $customcmcompletion = new $cmcompletionclass($cm, $userid); + foreach ($cm->customdata['customcompletionrules'] as $rule => $enabled) { + if (!$enabled) { + // Skip inactive completion rules. + continue; + } + // Get this custom completion rule's completion state. + $data['customcompletion'][$rule] = $customcmcompletion->get_state($rule); + } + + return $data; } /** @@ -1089,11 +1159,17 @@ class completion_info { } $transaction->allow_commit(); - $cmcontext = context_module::instance($data->coursemoduleid, MUST_EXIST); - $coursecontext = $cmcontext->get_parent_context(); + $cmcontext = context_module::instance($data->coursemoduleid); $completioncache = cache::make('core', 'completion'); if ($data->userid == $USER->id) { + // Fetch other completion data to cache (e.g. require grade completion status, custom completion rule statues). + $cminfo = cm_info::create($cm, $data->userid); // Make sure we're working on a cm_info object. + $otherdata = $this->get_other_cm_completion_data($cminfo, $data->userid); + foreach ($otherdata as $key => $value) { + $data->$key = $value; + } + // Update module completion in user's cache. if (!($cachedata = $completioncache->get($data->userid . '_' . $cm->course)) || $cachedata['cacherev'] != $this->course->cacherev) { From 4a9aeeca5941aa9dc116061bc315fc449b0e7df2 Mon Sep 17 00:00:00 2001 From: Jun Pataleta Date: Wed, 10 Feb 2021 10:37:06 +0800 Subject: [PATCH 6/8] MDL-70815 mod_choice: Custom completion implementation --- .../classes/completion/custom_completion.php | 72 +++++++++++++++++++ mod/choice/lang/en/choice.php | 1 + 2 files changed, 73 insertions(+) create mode 100644 mod/choice/classes/completion/custom_completion.php diff --git a/mod/choice/classes/completion/custom_completion.php b/mod/choice/classes/completion/custom_completion.php new file mode 100644 index 00000000000..5f04988dc0b --- /dev/null +++ b/mod/choice/classes/completion/custom_completion.php @@ -0,0 +1,72 @@ +. + +declare(strict_types = 1); + +namespace mod_choice\completion; + +use core_completion\activity_custom_completion; + +/** + * Activity custom completion subclass for the choice activity. + * + * Class for defining mod_choice's custom completion rules and fetching the completion statuses + * of the custom completion rules for a given choice instance and a user. + * + * @package mod_choice + * @copyright 2021 Jun Pataleta + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class custom_completion extends activity_custom_completion { + + /** + * Fetches the completion state for a given completion rule. + * + * @param string $rule The completion rule. + * @return int The completion state. + */ + public function get_state(string $rule): int { + global $DB; + + $this->validate_rule($rule); + + // Choice only supports completionsubmit as a custom rule. + $status = $DB->record_exists('choice_answers', ['choiceid' => $this->cm->instance, 'userid' => $this->userid]); + return $status ? COMPLETION_COMPLETE : COMPLETION_INCOMPLETE; + } + + /** + * Fetch the list of custom completion rules that this module defines. + * + * @return array + */ + public static function get_defined_custom_rules(): array { + return [ + 'completionsubmit' + ]; + } + + /** + * Returns an associative array of the descriptions of custom completion rules. + * + * @return array + */ + public function get_custom_rule_descriptions(): array { + return [ + 'completionsubmit' => get_string('completiondetail:submit', 'choice') + ]; + } +} diff --git a/mod/choice/lang/en/choice.php b/mod/choice/lang/en/choice.php index eba9b28090a..033a0a6efc8 100644 --- a/mod/choice/lang/en/choice.php +++ b/mod/choice/lang/en/choice.php @@ -31,6 +31,7 @@ $string['calendarend'] = '{$a} closes'; $string['calendarstart'] = '{$a} opens'; $string['cannotsubmit'] = 'Sorry, there was a problem submitting your choice. Please try again.'; $string['closebeforeopen'] = 'You have specified a close date before the open date.'; +$string['completiondetail:submit'] = 'Make a choice'; $string['completionsubmit'] = 'Show as complete when user makes a choice'; $string['displayhorizontal'] = 'Display horizontally'; $string['displaymode'] = 'Display mode for the options'; From f2599eb8950c8dae288121ac85cc8afafaa198ca Mon Sep 17 00:00:00 2001 From: Jun Pataleta Date: Tue, 16 Feb 2021 02:20:45 +0800 Subject: [PATCH 7/8] MDL-70815 mod_choice: Unit tests for the custom completion class --- mod/choice/tests/custom_completion_test.php | 209 ++++++++++++++++++++ 1 file changed, 209 insertions(+) create mode 100644 mod/choice/tests/custom_completion_test.php diff --git a/mod/choice/tests/custom_completion_test.php b/mod/choice/tests/custom_completion_test.php new file mode 100644 index 00000000000..0de6b5ebf02 --- /dev/null +++ b/mod/choice/tests/custom_completion_test.php @@ -0,0 +1,209 @@ +. + +declare(strict_types = 1); + +namespace mod_choice; + +use advanced_testcase; +use cm_info; +use coding_exception; +use mod_choice\completion\custom_completion; +use moodle_exception; + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; +require_once($CFG->libdir . '/completionlib.php'); + +/** + * Class for unit testing mod_choice/custom_completion. + * + * @package mod_choice + * @copyright 2021 Jun Pataleta + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class custom_completion_test extends advanced_testcase { + + /** + * Data provider for get_state(). + * + * @return array[] + */ + public function get_state_provider(): array { + return [ + 'Undefined rule' => [ + 'somenonexistentrule', COMPLETION_DISABLED, false, null, coding_exception::class + ], + 'Rule not available' => [ + 'completionsubmit', COMPLETION_DISABLED, false, null, moodle_exception::class + ], + 'Rule available, user has not submitted' => [ + 'completionsubmit', COMPLETION_ENABLED, false, COMPLETION_INCOMPLETE, null + ], + 'Rule available, user has submitted' => [ + 'completionsubmit', COMPLETION_ENABLED, true, COMPLETION_COMPLETE, null + ], + ]; + } + + /** + * Test for get_state(). + * + * @dataProvider get_state_provider + * @param string $rule The custom completion rule. + * @param int $available Whether this rule is available. + * @param bool $submitted Whether the user has made a choice. + * @param int|null $status Expected status. + * @param string|null $exception Expected exception. + */ + public function test_get_state(string $rule, int $available, ?bool $submitted, ?int $status, ?string $exception) { + global $DB; + + if (!is_null($exception)) { + $this->expectException($exception); + } + + // Custom completion rule data for cm_info::customdata. + $customdataval = [ + 'customcompletionrules' => [ + $rule => $available + ] + ]; + + // Build a mock cm_info instance. + $mockcminfo = $this->getMockBuilder(cm_info::class) + ->disableOriginalConstructor() + ->onlyMethods(['__get']) + ->getMock(); + + // Mock the return of the magic getter method when fetching the cm_info object's customdata and instance values. + $mockcminfo->expects($this->any()) + ->method('__get') + ->will($this->returnValueMap([ + ['customdata', $customdataval], + ['instance', 1], + ])); + + // Mock the DB calls. + $DB = $this->createMock(get_class($DB)); + $DB->expects($this->atMost(1)) + ->method('record_exists') + ->willReturn($submitted); + + $customcompletion = new custom_completion($mockcminfo, 2); + $this->assertEquals($status, $customcompletion->get_state($rule)); + } + + /** + * Test for get_defined_custom_rules(). + */ + public function test_get_defined_custom_rules() { + $rules = custom_completion::get_defined_custom_rules(); + $this->assertCount(1, $rules); + $this->assertEquals('completionsubmit', reset($rules)); + } + + /** + * Test for get_defined_custom_rule_descriptions(). + */ + public function test_get_custom_rule_descriptions() { + // Get defined custom rules. + $rules = custom_completion::get_defined_custom_rules(); + + // Build a mock cm_info instance. + $mockcminfo = $this->getMockBuilder(cm_info::class) + ->disableOriginalConstructor() + ->onlyMethods(['__get']) + ->getMock(); + + // Instantiate a custom_completion object using the mocked cm_info. + $customcompletion = new custom_completion($mockcminfo, 1); + + // Get custom rule descriptions. + $ruledescriptions = $customcompletion->get_custom_rule_descriptions(); + + // Confirm that defined rules and rule descriptions are consistent with each other. + $this->assertEquals(count($rules), count($ruledescriptions)); + foreach ($rules as $rule) { + $this->assertArrayHasKey($rule, $ruledescriptions); + } + } + + /** + * Test for is_defined(). + */ + public function test_is_defined() { + // Build a mock cm_info instance. + $mockcminfo = $this->getMockBuilder(cm_info::class) + ->disableOriginalConstructor() + ->getMock(); + + $customcompletion = new custom_completion($mockcminfo, 1); + + // Rule is defined. + $this->assertTrue($customcompletion->is_defined('completionsubmit')); + + // Undefined rule. + $this->assertFalse($customcompletion->is_defined('somerandomrule')); + } + + /** + * Data provider for test_get_available_custom_rules(). + * + * @return array[] + */ + public function get_available_custom_rules_provider(): array { + return [ + 'Completion submit available' => [ + COMPLETION_ENABLED, ['completionsubmit'] + ], + 'Completion submit not available' => [ + COMPLETION_DISABLED, [] + ], + ]; + } + + /** + * Test for get_available_custom_rules(). + * + * @dataProvider get_available_custom_rules_provider + * @param int $status + * @param array $expected + */ + public function test_get_available_custom_rules(int $status, array $expected) { + $customdataval = [ + 'customcompletionrules' => [ + 'completionsubmit' => $status + ] + ]; + + // Build a mock cm_info instance. + $mockcminfo = $this->getMockBuilder(cm_info::class) + ->disableOriginalConstructor() + ->onlyMethods(['__get']) + ->getMock(); + + // Mock the return of magic getter for the customdata attribute. + $mockcminfo->expects($this->any()) + ->method('__get') + ->with('customdata') + ->willReturn($customdataval); + + $customcompletion = new custom_completion($mockcminfo, 1); + $this->assertEquals($expected, $customcompletion->get_available_custom_rules()); + } +} From 1292b31ac5ce96cab4643da07ef4f528ecbfbc66 Mon Sep 17 00:00:00 2001 From: Jun Pataleta Date: Tue, 9 Mar 2021 20:14:46 +0800 Subject: [PATCH 8/8] MDL-70815 completion: Test internal_get_state() with custom completion Use the custom completion implementation for mod_choice to test completion_info::get_state() to cover the case where the completion state is being determined from the custom completion condition. --- lib/tests/completionlib_test.php | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/lib/tests/completionlib_test.php b/lib/tests/completionlib_test.php index eacc7a3fbf4..75de3067cac 100644 --- a/lib/tests/completionlib_test.php +++ b/lib/tests/completionlib_test.php @@ -427,6 +427,34 @@ class core_completionlib_testcase extends advanced_testcase { $this->assertEquals(COMPLETION_INCOMPLETE, $completioninfo->internal_get_state($cm, $teacher->id, null)); } + /** + * Test for internal_get_state() for an activity that supports custom completion. + */ + public function test_internal_get_state_with_custom_completion() { + $this->setup_data(); + + $choicerecord = [ + 'course' => $this->course, + 'completion' => COMPLETION_TRACKING_AUTOMATIC, + 'completionsubmit' => COMPLETION_ENABLED, + ]; + $choice = $this->getDataGenerator()->create_module('choice', $choicerecord); + $cminfo = cm_info::create(get_coursemodule_from_instance('choice', $choice->id)); + + $completioninfo = new completion_info($this->course); + + // Fetch completion for the user who hasn't made a choice yet. + $completion = $completioninfo->internal_get_state($cminfo, $this->user->id, COMPLETION_INCOMPLETE); + $this->assertEquals(COMPLETION_INCOMPLETE, $completion); + + // Have the user make a choice. + $choicewithoptions = choice_get_choice($choice->id); + $optionids = array_keys($choicewithoptions->option); + choice_user_submit_response($optionids[0], $choice, $this->user->id, $this->course, $cminfo); + $completion = $completioninfo->internal_get_state($cminfo, $this->user->id, COMPLETION_INCOMPLETE); + $this->assertEquals(COMPLETION_COMPLETE, $completion); + } + public function test_set_module_viewed() { $this->mock_setup();