L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch 'MDL-72246-master-2' of https://github.com/andrewnicols/moodle

Browse Source
This commit is contained in:
Jun Pataleta
2022-02-08 11:36:59 +08:00
parent 95c6d3c8d9 70cc41eec8
commit 939cb7a90f
3 changed files with 263 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files
completion/tests/coverage.php
+47
View File
@@ -0,0 +1,47 @@
<?php
// This file is part of Moodle - http://moodle.org/
//
// Moodle is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// Moodle is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
defined('MOODLE_INTERNAL') || die();
/**
* Coverage information for the core_completion.
*
* @package core
* @category phpunit
* @copyright 2022 Andrew Nicols <[email protected]>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
/**
* Coverage information for the core subsystem.
*
* @copyright 2018 Andrew Nicols <[email protected]>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
return new class extends phpunit_coverage_info {
/** @var array The list of folders relative to the plugin root to include in coverage generation. */
protected $includelistfolders = [
'criteria',
];
/** @var array The list of files relative to the plugin root to include in coverage generation. */
protected $includelistfiles = [
'completion_aggregation.php',
'completion_completion.php',
'completion_criteria_completion.php',
'data_object.php'
];
};
lib/completionlib.php
+45 -22
View File
@@ -1024,6 +1024,20 @@ class completion_info {
$userid = $USER->id;
}
// 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);
// Create an anonymous function to remove the 'other_cm_completion_data_fetched' key.
$returnfilteredvalue = function(array $value): stdClass {
return (object) array_filter($value, function(string $key): bool {
return $key !== 'other_cm_completion_data_fetched';
}, ARRAY_FILTER_USE_KEY);
};
// See if requested data is present in cache (use cache for current user only).
$usecache = $userid == $USER->id;
$cacheddata = array();
@@ -1036,19 +1050,22 @@ class completion_info {
if ($cacheddata['cacherev'] != $this->course->cacherev) {
// Course structure has been changed since the last caching, forget the cache.
$cacheddata = array();
} else if (isset($cacheddata[$cm->id])) {
return (object)$cacheddata[$cm->id];
} else if (isset($cacheddata[$cminfo->id])) {
$data = (array) $cacheddata[$cminfo->id];
if (empty($data['other_cm_completion_data_fetched'])) {
$data += $this->get_other_cm_completion_data($cminfo, $userid);
$data['other_cm_completion_data_fetched'] = true;
// Put in cache.
$cacheddata[$cminfo->id] = $data;
$completioncache->set($key, $cacheddata);
}
return $returnfilteredvalue($cacheddata[$cminfo->id]);
}
}
}
// 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,
@@ -1086,12 +1103,9 @@ class completion_info {
$cacheddata[$data->cmid] = $defaultdata;
$cacheddata[$data->cmid]['coursemoduleid'] = $data->cmid;
} else {
$cacheddata[$data->cmid] = (array) $data;
unset($data->cmid);
$cacheddata[$data->coursemoduleid] = (array) $data;
}
// Add the other completion data for this user in this module instance.
$othercminfo = $cminfos[$data->cmid];
$cacheddata[$othercminfo->id] += $this->get_other_cm_completion_data($othercminfo, $userid);
}
if (!isset($cacheddata[$cminfo->id])) {
@@ -1099,6 +1113,7 @@ class completion_info {
$this->internal_systemerror($errormessage);
}
$data = $cacheddata[$cminfo->id];
} else {
// Get single record
$data = $DB->get_record('course_modules_completion', array('coursemoduleid' => $cminfo->id, 'userid' => $userid));
@@ -1108,18 +1123,24 @@ class completion_info {
// 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
// Put in cache.
$cacheddata[$cminfo->id] = $data;
}
// Fill the other completion data for this user in this module instance.
$data += $this->get_other_cm_completion_data($cminfo, $userid);
$data['other_cm_completion_data_fetched'] = true;
// Put in cache
$cacheddata[$cminfo->id] = $data;
if ($usecache) {
$cacheddata['cacherev'] = $this->course->cacherev;
$completioncache->set($key, $cacheddata);
}
return (object)$cacheddata[$cminfo->id];
return $returnfilteredvalue($cacheddata[$cminfo->id]);
}
/**
@@ -1240,6 +1261,7 @@ class completion_info {
$cmcontext = context_module::instance($data->coursemoduleid);
$completioncache = cache::make('core', 'completion');
$cachekey = "{$data->userid}_{$cm->course}";
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.
@@ -1249,18 +1271,19 @@ class completion_info {
}
// Update module completion in user's cache.
if (!($cachedata = $completioncache->get($data->userid . '_' . $cm->course))
if (!($cachedata = $completioncache->get($cachekey))
|| $cachedata['cacherev'] != $this->course->cacherev) {
$cachedata = array('cacherev' => $this->course->cacherev);
}
$cachedata[$cm->id] = $data;
$completioncache->set($data->userid . '_' . $cm->course, $cachedata);
$cachedata[$cm->id] = (array) $data;
$cachedata[$cm->id]['other_cm_completion_data_fetched'] = true;
$completioncache->set($cachekey, $cachedata);
// reset modinfo for user (no need to call rebuild_course_cache())
get_fast_modinfo($cm->course, 0, true);
} else {
// Remove another user's completion cache for this course.
$completioncache->delete($data->userid . '_' . $cm->course);
$completioncache->delete($cachekey);
}
// For single user actions the code must reevaluate some completion state instantly, see MDL-32103.
lib/tests/completionlib_test.php
+171 -25
View File
@@ -29,7 +29,17 @@ defined('MOODLE_INTERNAL') || die();
global $CFG;
require_once($CFG->libdir.'/completionlib.php');
class core_completionlib_testcase extends advanced_testcase {
/**
* Completion tests.
*
* @package core_completion
* @category phpunit
* @copyright 2008 Sam Marshall
* @copyright 2013 Frédéric Massart
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
* @coversDefaultClass \completion_info
*/
class completionlib_test extends advanced_testcase {
protected $course;
protected $user;
protected $module1;
@@ -79,6 +89,10 @@ class core_completionlib_testcase extends advanced_testcase {
public static function assertEquals($expected, $actual, string $message = '', float $delta = 0, int $maxDepth = 10,
bool $canonicalize = false, bool $ignoreCase = false): void {
// Nasty cheating hack: prevent random failures on timemodified field.
if (is_array($actual) && (is_object($expected) || is_array($expected))) {
$actual = (object) $actual;
$expected = (object) $expected;
}
if (is_object($expected) and is_object($actual)) {
if (property_exists($expected, 'timemodified') and property_exists($actual, 'timemodified')) {
if ($expected->timemodified + 1 == $actual->timemodified) {
@@ -90,6 +104,10 @@ class core_completionlib_testcase extends advanced_testcase {
parent::assertEquals($expected, $actual, $message, $delta, $maxDepth, $canonicalize, $ignoreCase);
}
/**
* @covers ::is_enabled_for_site
* @covers ::is_enabled
*/
public function test_is_enabled() {
global $CFG;
$this->mock_setup();
@@ -125,6 +143,9 @@ class core_completionlib_testcase extends advanced_testcase {
$this->assertEquals(COMPLETION_TRACKING_AUTOMATIC, $c->is_enabled($cm));
}
/**
* @covers ::update_state
*/
public function test_update_state() {
$this->mock_setup();
@@ -348,6 +369,7 @@ class core_completionlib_testcase extends advanced_testcase {
* @param int $completionusegrade
* @param string $unsetfield
* @param int $expectedstate
* @covers ::internal_get_state
*/
public function test_internal_get_state(int $completionview, int $completionusegrade, string $unsetfield, int $expectedstate) {
$this->setup_data();
@@ -379,6 +401,7 @@ class core_completionlib_testcase extends advanced_testcase {
* Provider for the test_internal_get_state_with_grade_criteria.
*
* @return array
* @covers ::internal_get_state
*/
public function test_internal_get_state_with_grade_criteria_provider() {
return [
@@ -432,6 +455,7 @@ class core_completionlib_testcase extends advanced_testcase {
* @param array $completioncriteria The completion criteria to use
* @param int|null $studentgrade Grade to assign to student
* @param int $expectedstate Expected completion state
* @covers ::internal_get_state
*/
public function test_internal_get_state_with_grade_criteria(array $completioncriteria, ?int $studentgrade, int $expectedstate) {
$this->setup_data();
@@ -472,6 +496,8 @@ class core_completionlib_testcase extends advanced_testcase {
/**
* Covers the case where internal_get_state() is being called for a user different from the logged in user.
*
* @covers ::internal_get_state
*/
public function test_internal_get_state_with_different_user() {
$this->setup_data();
@@ -514,6 +540,8 @@ class core_completionlib_testcase extends advanced_testcase {
/**
* Test for internal_get_state() for an activity that supports custom completion.
*
* @covers ::internal_get_state
*/
public function test_internal_get_state_with_custom_completion() {
$this->setup_data();
@@ -540,6 +568,9 @@ class core_completionlib_testcase extends advanced_testcase {
$this->assertEquals(COMPLETION_COMPLETE, $completion);
}
/**
* @covers ::set_module_viewed
*/
public function test_set_module_viewed() {
$this->mock_setup();
@@ -595,6 +626,9 @@ class core_completionlib_testcase extends advanced_testcase {
$c->set_module_viewed($cm, 1337);
}
/**
* @covers ::count_user_data
*/
public function test_count_user_data() {
global $DB;
$this->mock_setup();
@@ -611,6 +645,9 @@ class core_completionlib_testcase extends advanced_testcase {
$this->assertEquals(666, $c->count_user_data($cm));
}
/**
* @covers ::delete_all_state
*/
public function test_delete_all_state() {
global $DB;
$this->mock_setup();
@@ -628,6 +665,9 @@ class core_completionlib_testcase extends advanced_testcase {
$c->delete_all_state($cm);
}
/**
* @covers ::reset_all_state
*/
public function test_reset_all_state() {
global $DB;
$this->mock_setup();
@@ -703,6 +743,7 @@ class core_completionlib_testcase extends advanced_testcase {
* @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.
* @covers ::get_data
*/
public function test_get_data(bool $wholecourse, bool $sameuser, bool $hasrecord, int $completion) {
global $DB;
@@ -760,26 +801,6 @@ class core_completionlib_testcase extends advanced_testcase {
$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]);
// Check cached data for other course modules in the course.
// The sample module created in setup_data() should suffice to confirm this.
$othercm = get_coursemodule_from_instance('forum', $this->module1->id);
if ($wholecourse) {
$this->assertArrayHasKey($othercm->id, $cache->get($key));
} else {
$this->assertArrayNotHasKey($othercm->id, $cache->get($key));
}
} else {
// Otherwise, this should not be cached.
$this->assertFalse($cache->get($key));
}
// Check that we are including relevant completion data for the module.
if (!$wholecourse) {
$this->assertTrue(property_exists($result, 'viewed'));
@@ -787,8 +808,74 @@ class core_completionlib_testcase extends advanced_testcase {
}
}
/**
* @covers ::get_data
*/
public function test_get_data_successive_calls(): void {
global $DB;
$this->setup_data();
$this->setUser($this->user);
$choicegenerator = $this->getDataGenerator()->get_plugin_generator('mod_choice');
$choice = $choicegenerator->create_instance([
'course' => $this->course->id,
'completion' => COMPLETION_TRACKING_AUTOMATIC,
'completionview' => true,
'completionsubmit' => true,
]);
$cm = get_coursemodule_from_instance('choice', $choice->id);
// Let's manually create a course completion record instead of going through the hoops to complete an activity.
$cmcompletionrecord = (object) [
'coursemoduleid' => $cm->id,
'userid' => $this->user->id,
'completionstate' => COMPLETION_NOT_VIEWED,
'viewed' => 0,
'overrideby' => null,
'timemodified' => 0,
];
$DB->insert_record('course_modules_completion', $cmcompletionrecord);
// Mock other completion data.
$completioninfo = new completion_info($this->course);
$modinfo = get_fast_modinfo($this->course);
$results = [];
foreach ($modinfo->cms as $testcm) {
$result = $completioninfo->get_data($testcm, true);
$this->assertTrue(property_exists($result, 'id'));
$this->assertTrue(property_exists($result, 'coursemoduleid'));
$this->assertTrue(property_exists($result, 'userid'));
$this->assertTrue(property_exists($result, 'completionstate'));
$this->assertTrue(property_exists($result, 'viewed'));
$this->assertTrue(property_exists($result, 'overrideby'));
$this->assertTrue(property_exists($result, 'timemodified'));
$this->assertFalse(property_exists($result, 'other_cm_completion_data_fetched'));
$this->assertEquals($testcm->id, $result->coursemoduleid);
$this->assertEquals($this->user->id, $result->userid);
$this->assertEquals(0, $result->viewed);
$results[$testcm->id] = $result;
}
$result = $completioninfo->get_data($cm);
$this->assertTrue(property_exists($result, 'customcompletion'));
// The data should match when fetching modules individually.
(cache::make('core', 'completion'))->purge();
foreach ($modinfo->cms as $testcm) {
$result = $completioninfo->get_data($testcm, false);
$this->assertEquals($result, $results[$testcm->id]);
}
}
/**
* Tests for completion_info::get_other_cm_completion_data().
*
* @covers ::get_other_cm_completion_data
*/
public function test_get_other_cm_completion_data() {
global $DB;
@@ -862,6 +949,9 @@ class core_completionlib_testcase extends advanced_testcase {
$this->assertEmpty($choice2completiondata);
}
/**
* @covers ::internal_set_data
*/
public function test_internal_set_data() {
global $DB;
$this->setup_data();
@@ -887,8 +977,15 @@ class core_completionlib_testcase extends advanced_testcase {
$this->assertEquals($d1, $data->id);
$cache = cache::make('core', 'completion');
// Cache was not set for another user.
$this->assertEquals(array('cacherev' => $this->course->cacherev, $cm->id => $data),
$cache->get($data->userid . '_' . $cm->course));
$cachevalue = $cache->get("{$data->userid}_{$cm->course}");
$this->assertEquals([
'cacherev' => $this->course->cacherev,
$cm->id => array_merge(
(array) $data,
['other_cm_completion_data_fetched' => true]
),
],
$cachevalue);
// 2) Test with existing data and for different user.
$forum2 = $this->getDataGenerator()->create_module('forum', array('course' => $this->course->id), $completionauto);
@@ -906,7 +1003,11 @@ class core_completionlib_testcase extends advanced_testcase {
$c->internal_set_data($cm2, $d2);
// Cache for current user returns the data.
$cachevalue = $cache->get($data->userid . '_' . $cm->course);
$this->assertEquals($data, $cachevalue[$cm->id]);
$this->assertEquals(array_merge(
(array) $data,
['other_cm_completion_data_fetched' => true]
), $cachevalue[$cm->id]);
// Cache for another user is not filled.
$this->assertEquals(false, $cache->get($d2->userid . '_' . $cm2->course));
@@ -958,6 +1059,9 @@ class core_completionlib_testcase extends advanced_testcase {
$this->assertEquals($this->user->id, reset($actual)->userid);
}
/**
* @covers ::get_progress_all
*/
public function test_get_progress_all_few() {
global $DB;
$this->mock_setup();
@@ -992,6 +1096,9 @@ class core_completionlib_testcase extends advanced_testcase {
), $c->get_progress_all(false));
}
/**
* @covers ::get_progress_all
*/
public function test_get_progress_all_lots() {
global $DB;
$this->mock_setup();
@@ -1047,6 +1154,9 @@ class core_completionlib_testcase extends advanced_testcase {
$this->assertCount(count($tracked), $result);
}
/**
* @covers ::inform_grade_changed
*/
public function test_inform_grade_changed() {
$this->mock_setup();
@@ -1115,6 +1225,9 @@ class core_completionlib_testcase extends advanced_testcase {
$c->inform_grade_changed($cm, $item, $grade, true);
}
/**
* @covers ::internal_get_grade_state
*/
public function test_internal_get_grade_state() {
$this->mock_setup();
@@ -1157,6 +1270,9 @@ class core_completionlib_testcase extends advanced_testcase {
completion_info::internal_get_grade_state($item, $grade));
}
/**
* @test ::get_activities
*/
public function test_get_activities() {
global $CFG;
$this->resetAfterTest();
@@ -1198,6 +1314,9 @@ class core_completionlib_testcase extends advanced_testcase {
$this->assertFalse(isset($activities[$data2->cmid]));
}
/**
* @test ::has_activities
*/
public function test_has_activities() {
global $CFG;
$this->resetAfterTest();
@@ -1223,7 +1342,8 @@ class core_completionlib_testcase extends advanced_testcase {
/**
* Test that data is cleaned up when we delete courses that are set as completion criteria for other courses
*
* @return void
* @covers ::delete_course_completion_data
* @covers ::delete_all_completion_data
*/
public function test_course_delete_prerequisite() {
global $DB;
@@ -1260,6 +1380,8 @@ class core_completionlib_testcase extends advanced_testcase {
/**
* Test course module completion update event.
*
* @covers \core\event\course_module_completion_updated
*/
public function test_course_module_completion_updated_event() {
global $USER, $CFG;
@@ -1297,6 +1419,8 @@ class core_completionlib_testcase extends advanced_testcase {
/**
* Test course completed event.
*
* @covers \core\event\course_completed
*/
public function test_course_completed_event() {
global $USER;
@@ -1326,6 +1450,8 @@ class core_completionlib_testcase extends advanced_testcase {
/**
* Test course completed message.
*
* @covers \core\event\course_completed
*/
public function test_course_completed_message() {
$this->setup_data();
@@ -1352,6 +1478,8 @@ class core_completionlib_testcase extends advanced_testcase {
/**
* Test course completed event.
*
* @covers \core\event\course_completion_updated
*/
public function test_course_completion_updated_event() {
$this->setup_data();
@@ -1378,6 +1506,9 @@ class core_completionlib_testcase extends advanced_testcase {
$this->assertEventLegacyLogData($expectedlegacylog, $event);
}
/**
* @covers \completion_can_view_data
*/
public function test_completion_can_view_data() {
$this->setup_data();
@@ -1413,6 +1544,7 @@ class core_completionlib_testcase extends advanced_testcase {
* @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.
* @covers ::get_grade_completion
*/
public function test_get_grade_completion(bool $completionusegrade, bool $hasgrade, ?int $passinggrade,
?string $expectedexception, ?int $expectedresult) {
@@ -1445,6 +1577,8 @@ class core_completionlib_testcase extends advanced_testcase {
/**
* Test the return value for cases when the activity module does not have associated grade_item.
*
* @covers ::get_grade_completion
*/
public function test_get_grade_completion_without_grade_item() {
global $DB;
@@ -1483,6 +1617,8 @@ class core_completionlib_testcase extends advanced_testcase {
/**
* Test for aggregate_completions().
*
* @covers \aggregate_completions
*/
public function test_aggregate_completions() {
global $DB;
@@ -1577,6 +1713,8 @@ class core_completionlib_testcase extends advanced_testcase {
/**
* Test for completion_completion::_save().
*
* @covers \completion_completion::_save
*/
public function test_save() {
global $DB;
@@ -1621,6 +1759,8 @@ class core_completionlib_testcase extends advanced_testcase {
/**
* Test for completion_completion::mark_enrolled().
*
* @covers \completion_completion::mark_enrolled
*/
public function test_mark_enrolled() {
global $DB;
@@ -1662,6 +1802,8 @@ class core_completionlib_testcase extends advanced_testcase {
/**
* Test for completion_completion::mark_inprogress().
*
* @covers \completion_completion::mark_inprogress
*/
public function test_mark_inprogress() {
global $DB;
@@ -1703,6 +1845,8 @@ class core_completionlib_testcase extends advanced_testcase {
/**
* Test for completion_completion::mark_complete().
*
* @covers \completion_completion::mark_complete
*/
public function test_mark_complete() {
global $DB;
@@ -1743,6 +1887,8 @@ class core_completionlib_testcase extends advanced_testcase {
/**
* Test for completion_criteria_completion::mark_complete().
*
* @covers \completion_criteria_completion::mark_complete
*/
public function test_criteria_mark_complete() {
global $DB;