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

MDL-72827 completion: Revert completion flow logic

Browse Source 
- Reverts the custom flow logic introduced in the 52206
- Expand unit tests to cover different grade based completion settings
This commit is contained in:
Peter Dias
2021-10-28 11:38:10 +08:00
parent 9145d80b0b
commit cb2afc668d
4 changed files with 102 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files
completion/upgrade.txt
-7
View File
@@ -4,15 +4,8 @@ information provided here is intended especially for developers.
=== 4.0 ===
* New method mark_course_completions_activity_criteria() has been added to mark course completions instantly. It is
based on cron for completion_criteria_activity.php which is refactored to use it as well.
* Modified completion criteria to allow plugins to override core completion logic.
* Core now passes an additional parameter to '_get_completion_state'. This is an array representation of the completion results that have already been
tested. Currently contains - viewed, usegrade, passgrade. Any plugin that are dependent on these criteria can now check this array instead of retesting it.
* Introduced a new plugin function - '_get_completion_aggregation_state', that would indicate the aggregation type/relationship between the plugin and core
completion criteria. This callback should either return a COMPLETION_STANDARD_FLOW / COMPLETION_CUSTOM_MODULE_FLOW. The former for default existing core
behaviour while the latter enforces the override logic from the plugin. Defaults to COMPLETION_STANDARD_FLOW if not defined. This is useful when plugins
need to override the core completion criteria in cases where it may be dependent on them. In these cases, the 'source of truth' would be the response
from the plugin's 'get_completion_state' function. e.g. Quiz's completion defines a criteria of 'requires passing grade OR all attempts AND min attempts
reached.' In these cases, even if a passing grade has not been achieved, the activity should be marked as completed if the no.of attempts have been reached.
* The method \completion_criteria_completion::mark_complete() now has the optional $timecompleted parameter to specify when the
criteria was completed.
lib/completionlib.php
+7 -23
View File
@@ -132,16 +132,6 @@ define('COMPLETION_OR', false);
*/
define('COMPLETION_AND', true);
/**
* When a module implements this, completion state is dependent to the
* module's _get_completion_state callback and activity_custom_completion class.
*/
define('COMPLETION_CUSTOM_MODULE_FLOW', true);
/**
* Standard flow indicates ALL conditions need to be met for completion to be marked as done.
*/
define('COMPLETION_STANDARD_FLOW', false);
/**
* Course completion criteria aggregation method.
*/
@@ -733,28 +723,22 @@ class completion_info {
$response = $function($this->course, $cm, $userid, COMPLETION_AND, $completionstate);
}
// Get the relationship between the core_completion and plugin_completion criteria.
$aggregationtype = COMPLETION_STANDARD_FLOW;
if ($aggregationfn = component_callback_exists("mod_$cminfo->modname", 'get_completion_aggregation_state')) {
$aggregationtype = $aggregationfn();
}
// If the module aggregates using COMPLETION_STANDARD_FLOW, it requires ALL conditions to be met.
// If the aggregation type is COMPLETION_CUSTOM_MODULE_FLOW, completion can be overridden by the plugin.
if (!$response && $aggregationtype == COMPLETION_STANDARD_FLOW) {
if (!$response) {
return COMPLETION_INCOMPLETE;
} else if ($aggregationtype == COMPLETION_CUSTOM_MODULE_FLOW) {
return ($response ? COMPLETION_COMPLETE : COMPLETION_INCOMPLETE);
}
}
if ($completionstate) {
// We have allowed the plugins to do it's thing and run their own checks.
// We have now reached a state where we need to AND all the calculated results.
// Preference for COMPLETION_COMPLETE_PASS over COMPLETION_COMPLETE for proper indication in reports.
$newstate = array_reduce($completionstate, function($carry, $value) {
if ($carry == COMPLETION_INCOMPLETE) {
return $carry;
if (in_array(COMPLETION_INCOMPLETE, [$carry, $value])) {
return COMPLETION_INCOMPLETE;
} else if (in_array(COMPLETION_COMPLETE_FAIL, [$carry, $value])) {
return COMPLETION_COMPLETE_FAIL;
} else {
return $value;
return in_array(COMPLETION_COMPLETE_PASS, [$carry, $value]) ? COMPLETION_COMPLETE_PASS : $value;
}
}, COMPLETION_COMPLETE);
lib/tests/completionlib_test.php
+95
View File
@@ -375,6 +375,101 @@ class core_completionlib_testcase extends advanced_testcase {
$this->assertEquals($expectedstate, $completioninfo->internal_get_state($cm, $userid, $current));
}
/**
* Provider for the test_internal_get_state_with_grade_criteria.
*
* @return array
*/
public function test_internal_get_state_with_grade_criteria_provider() {
return [
"Passing grade enabled and achieve. State should be COMPLETION_COMPLETE_PASS" => [
[
'completionusegrade' => 1,
'completionpassgrade' => 1,
'gradepass' => 50,
],
50,
COMPLETION_COMPLETE_PASS
],
"Passing grade enabled and not achieve. State should be COMPLETION_COMPLETE_FAIL" => [
[
'completionusegrade' => 1,
'completionpassgrade' => 1,
'gradepass' => 50,
],
40,
COMPLETION_COMPLETE_FAIL
],
"Passing grade not enabled with passing grade set." => [
[
'completionusegrade' => 1,
'gradepass' => 50,
],
50,
COMPLETION_COMPLETE_PASS
],
"Passing grade not enabled with passing grade not set." => [
[
'completionusegrade' => 1,
],
90,
COMPLETION_COMPLETE
],
"Passing grade not enabled with passing grade not set. No submission made." => [
[
'completionusegrade' => 1,
],
null,
COMPLETION_INCOMPLETE
],
];
}
/**
* Tests that the right completion state is being set based on the grade criteria.
*
* @dataProvider test_internal_get_state_with_grade_criteria_provider
* @param array $completioncriteria The completion criteria to use
* @param int|null $studentgrade Grade to assign to student
* @param int $expectedstate Expected completion state
*/
public function test_internal_get_state_with_grade_criteria(array $completioncriteria, ?int $studentgrade, int $expectedstate) {
$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,
] + $completioncriteria);
$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);
if ($studentgrade) {
$data = (object)[
'sendstudentnotifications' => false,
'attemptnumber' => 1,
'grade' => $studentgrade,
];
$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($expectedstate, $completioninfo->internal_get_state($cm, $userid, null));
}
/**
* Covers the case where internal_get_state() is being called for a user different from the logged in user.
*/
mod/quiz/lib.php
-9
View File
@@ -1908,15 +1908,6 @@ function quiz_get_navigation_options() {
);
}
/**
* Get the aggregation state for the module.
*
* @return bool
*/
function quiz_get_completion_aggregation_state() {
return COMPLETION_CUSTOM_MODULE_FLOW;
}
/**
* Check if the module has any update that affects the current user since a given time.
*