diff --git a/blocks/completionstatus/tests/behat/block_completionstatus_activity_completion.feature b/blocks/completionstatus/tests/behat/block_completionstatus_activity_completion.feature index 397afe0d0ac..018472c8afb 100644 --- a/blocks/completionstatus/tests/behat/block_completionstatus_activity_completion.feature +++ b/blocks/completionstatus/tests/behat/block_completionstatus_activity_completion.feature @@ -61,10 +61,7 @@ Feature: Enable Block Completion in a course using activity completion And I am on "Course 1" course homepage And I follow "Test page name" And I am on "Course 1" course homepage - Then I should see "Status: Pending" in the "Course completion status" "block" - And I should see "0 of 1" in the "Activity completion" "table_row" - And I trigger cron - And I am on "Course 1" course homepage + Then I should see "Status: Complete" in the "Course completion status" "block" And I should see "1 of 1" in the "Activity completion" "table_row" And I follow "More details" And I should see "Yes" in the "Activity completion" "table_row" diff --git a/completion/classes/api.php b/completion/classes/api.php index f55aa90ca06..4912f514cfe 100644 --- a/completion/classes/api.php +++ b/completion/classes/api.php @@ -123,4 +123,65 @@ class api { return true; } + + /** + * Mark users who completed course based on activity criteria. + * @param array $userdata If set only marks specified user in given course else checks all courses/users. + * @return int Completion record id if $userdata is set, 0 else. + * @since Moodle 4.0 + */ + public static function mark_course_completions_activity_criteria($userdata = null): int { + global $DB; + + // Get all users who meet this criteria + $sql = "SELECT DISTINCT c.id AS course, + cr.id AS criteriaid, + ra.userid AS userid, + mc.timemodified AS timecompleted + FROM {course_completion_criteria} cr + INNER JOIN {course} c ON cr.course = c.id + INNER JOIN {context} con ON con.instanceid = c.id + INNER JOIN {role_assignments} ra ON ra.contextid = con.id + INNER JOIN {course_modules_completion} mc ON mc.coursemoduleid = cr.moduleinstance AND mc.userid = ra.userid + LEFT JOIN {course_completion_crit_compl} cc ON cc.criteriaid = cr.id AND cc.userid = ra.userid + WHERE cr.criteriatype = :criteriatype + AND con.contextlevel = :contextlevel + AND c.enablecompletion = 1 + AND cc.id IS NULL + AND ( + mc.completionstate = :completionstate + OR mc.completionstate = :completionstatepass + OR mc.completionstate = :completionstatefail + )"; + + $params = [ + 'criteriatype' => COMPLETION_CRITERIA_TYPE_ACTIVITY, + 'contextlevel' => CONTEXT_COURSE, + 'completionstate' => COMPLETION_COMPLETE, + 'completionstatepass' => COMPLETION_COMPLETE_PASS, + 'completionstatefail' => COMPLETION_COMPLETE_FAIL + ]; + + if ($userdata) { + $params['courseid'] = $userdata['courseid']; + $params['userid'] = $userdata['userid']; + $sql .= " AND c.id = :courseid AND ra.userid = :userid"; + // Mark as complete. + $record = $DB->get_record_sql($sql, $params); + if ($record) { + $completion = new \completion_criteria_completion((array) $record, DATA_OBJECT_FETCH_BY_KEY); + $result = $completion->mark_complete($record->timecompleted); + return $result; + } + } else { + // Loop through completions, and mark as complete. + $rs = $DB->get_recordset_sql($sql, $params); + foreach ($rs as $record) { + $completion = new \completion_criteria_completion((array) $record, DATA_OBJECT_FETCH_BY_KEY); + $completion->mark_complete($record->timecompleted); + } + $rs->close(); + } + return 0; + } } diff --git a/completion/completion_completion.php b/completion/completion_completion.php index 4ddc81ab281..d1dc22b7670 100644 --- a/completion/completion_completion.php +++ b/completion/completion_completion.php @@ -101,6 +101,7 @@ class completion_completion extends data_object { * If the user is already marked as started, no change will occur * * @param integer $timeenrolled Time enrolled (optional) + * @return int|null id of completion record on successful update. */ public function mark_enrolled($timeenrolled = null) { @@ -122,6 +123,7 @@ class completion_completion extends data_object { * If the user is already marked as inprogress, the time will not be changed * * @param integer $timestarted Time started (optional) + * @return int|null id of completion record on successful update. */ public function mark_inprogress($timestarted = null) { @@ -149,14 +151,14 @@ class completion_completion extends data_object { * in the course are complete. * * @param integer $timecomplete Time completed (optional) - * @return void + * @return int|null id of completion record on successful update. */ public function mark_complete($timecomplete = null) { global $USER; // Never change a completion time. if ($this->timecompleted) { - return; + return null; } // Use current time if nothing supplied. @@ -166,7 +168,6 @@ class completion_completion extends data_object { // Set time complete. $this->timecompleted = $timecomplete; - // Save record. if ($result = $this->_save()) { $data = $this->get_record_data(); @@ -211,17 +212,16 @@ class completion_completion extends data_object { * * This method creates a course_completions record if none exists * @access private - * @return bool + * @return int|null id of completion record on successful update. */ private function _save() { if ($this->timeenrolled === null) { $this->timeenrolled = 0; } - $result = false; // Save record - if ($this->id) { - $result = $this->update(); + if (isset($this->id)) { + $success = $this->update(); } else { // Make sure reaggregate field is not null if (!$this->reaggregate) { @@ -233,17 +233,18 @@ class completion_completion extends data_object { $this->timestarted = 0; } - $result = $this->insert(); + $success = $this->insert(); } - if ($result) { + if ($success) { // Update the cached record. $cache = cache::make('core', 'coursecompletion'); $data = $this->get_record_data(); $key = $data->userid . '_' . $data->course; $cache->set($key, ['value' => $data]); + return $this->id; } - return $result; + return null; } } diff --git a/completion/completion_criteria_completion.php b/completion/completion_criteria_completion.php index bf4f7a770eb..c84d1c7546f 100644 --- a/completion/completion_criteria_completion.php +++ b/completion/completion_criteria_completion.php @@ -102,6 +102,7 @@ class completion_criteria_completion extends data_object { * Mark this criteria complete for the associated user * * This method creates a course_completion_crit_compl record + * @return int id of completion record. */ public function mark_complete() { // Create record @@ -120,7 +121,8 @@ class completion_criteria_completion extends data_object { 'userid' => $this->userid ); $ccompletion = new completion_completion($cc); - $ccompletion->mark_inprogress($this->timecompleted); + $result = $ccompletion->mark_inprogress($this->timecompleted); + return $result; } /** diff --git a/completion/criteria/completion_criteria_activity.php b/completion/criteria/completion_criteria_activity.php index 3e43ef1e5c6..118db635b31 100644 --- a/completion/criteria/completion_criteria_activity.php +++ b/completion/criteria/completion_criteria_activity.php @@ -203,53 +203,7 @@ class completion_criteria_activity extends completion_criteria { * Find users who have completed this criteria and mark them accordingly */ public function cron() { - global $DB; - - // Get all users who meet this criteria - $sql = ' - SELECT DISTINCT - c.id AS course, - cr.id AS criteriaid, - ra.userid AS userid, - mc.timemodified AS timecompleted - FROM - {course_completion_criteria} cr - INNER JOIN - {course} c - ON cr.course = c.id - INNER JOIN - {context} con - ON con.instanceid = c.id - INNER JOIN - {role_assignments} ra - ON ra.contextid = con.id - INNER JOIN - {course_modules_completion} mc - ON mc.coursemoduleid = cr.moduleinstance - AND mc.userid = ra.userid - LEFT JOIN - {course_completion_crit_compl} cc - ON cc.criteriaid = cr.id - AND cc.userid = ra.userid - WHERE - cr.criteriatype = '.COMPLETION_CRITERIA_TYPE_ACTIVITY.' - AND con.contextlevel = '.CONTEXT_COURSE.' - AND c.enablecompletion = 1 - AND cc.id IS NULL - AND ( - mc.completionstate = '.COMPLETION_COMPLETE.' - OR mc.completionstate = '.COMPLETION_COMPLETE_PASS.' - OR mc.completionstate = '.COMPLETION_COMPLETE_FAIL.' - ) - '; - - // Loop through completions, and mark as complete - $rs = $DB->get_recordset_sql($sql); - foreach ($rs as $record) { - $completion = new completion_criteria_completion((array) $record, DATA_OBJECT_FETCH_BY_KEY); - $completion->mark_complete($record->timecompleted); - } - $rs->close(); + \core_completion\api::mark_course_completions_activity_criteria(); } /** diff --git a/completion/tests/api_test.php b/completion/tests/api_test.php index 984393e70b0..91d150a72a2 100644 --- a/completion/tests/api_test.php +++ b/completion/tests/api_test.php @@ -222,4 +222,78 @@ class core_completion_api_testcase extends advanced_testcase { // Check that there is now no event in the database. $this->assertEquals(0, $DB->count_records('event')); } + + /** + * Test for mark_course_completions_activity_criteria(). + */ + public function test_mark_course_completions_activity_criteria() { + global $DB, $CFG; + require_once($CFG->dirroot.'/completion/criteria/completion_criteria_activity.php'); + $this->resetAfterTest(true); + + $course = $this->getDataGenerator()->create_course(array('enablecompletion' => 1)); + $student1 = $this->getDataGenerator()->create_user(); + $student2 = $this->getDataGenerator()->create_user(); + + $teacher = $this->getDataGenerator()->create_user(); + $studentrole = $DB->get_record('role', array('shortname' => 'student')); + $teacherrole = $DB->get_record('role', array('shortname' => 'editingteacher')); + + $this->getDataGenerator()->enrol_user($teacher->id, $course->id, $teacherrole->id); + $this->getDataGenerator()->enrol_user($student1->id, $course->id, $studentrole->id); + $this->getDataGenerator()->enrol_user($student2->id, $course->id, $studentrole->id); + + $data = $this->getDataGenerator()->create_module('data', array('course' => $course->id), + array('completion' => 1)); + $cmdata = get_coursemodule_from_id('data', $data->cmid); + $cm = get_coursemodule_from_instance('data', $data->id); + $c = new completion_info($course); + + // Add activity completion criteria. + $criteriadata = new stdClass(); + $criteriadata->id = $course->id; + $criteriadata->criteria_activity = array(); + // Some activities. + $criteriadata->criteria_activity[$cmdata->id] = 1; + $criterion = new completion_criteria_activity(); + $criterion->update_config($criteriadata); + + $this->setUser($teacher); + + // Mark activity complete for both users. + $completion = new stdClass(); + $completion->coursemoduleid = $cm->id; + $completion->completionstate = COMPLETION_COMPLETE; + $completion->timemodified = time(); + $completion->viewed = COMPLETION_NOT_VIEWED; + $completion->overrideby = null; + + $completion->id = 0; + $completion->userid = $student1->id; + $c->internal_set_data($cm, $completion, true); + + $completion->id = 0; + $completion->userid = $student2->id; + $c->internal_set_data($cm, $completion, true); + + // Run instant course completions for student1. Only student1 will be marked as completed a course. + $userdata = ['userid' => $student1->id, 'courseid' => $course->id]; + $actual = $DB->get_records('course_completions'); + $this->assertEmpty($actual); + + $coursecompletionid = \core_completion\api::mark_course_completions_activity_criteria($userdata); + + $actual = $DB->get_records('course_completions'); + $this->assertEquals(reset($actual)->id, $coursecompletionid); + $this->assertEquals(1, count($actual)); + $this->assertEquals($student1->id, reset($actual)->userid); + + // Run course completions cron. Both students will be marked as completed a course. + $coursecompletionid = \core_completion\api::mark_course_completions_activity_criteria(); + $this->assertEquals(0, $coursecompletionid); + $actual = $DB->get_records('course_completions'); + $this->assertEquals(2, count($actual)); + $this->assertEquals($student1->id, reset($actual)->userid); + $this->assertEquals($student2->id, end($actual)->userid); + } } diff --git a/completion/tests/behat/activity_completion_criteria.feature b/completion/tests/behat/activity_completion_criteria.feature new file mode 100644 index 00000000000..f3d8df13df2 --- /dev/null +++ b/completion/tests/behat/activity_completion_criteria.feature @@ -0,0 +1,187 @@ +@core @core_completion +Feature: Allow to mark course as completed without cron for activity completion criteria + In order for students to see instant course completion updates + I need to be able update completion state without cron + + Background: + Given the following "courses" exist: + | fullname | shortname | category | + | Completion course | CC1 | 0 | + And the following "users" exist: + | username | firstname | lastname | email | + | student1 | Student | First | student1@example.com | + | student2 | Student | Second | student2@example.com | + | teacher1 | Teacher | First | teacher1@example.com | + And the following "course enrolments" exist: + | user | course | role | + | student1 | CC1 | student | + | student2 | CC1 | student | + | teacher1 | CC1 | editingteacher | + And the following "activity" exists: + | activity | assign | + | course | CC1 | + | name | Test assignment name | + | idnumber | assign1 | + | description | Test assignment description | + And I log in as "admin" + And I am on "Completion course" course homepage + And I navigate to "Edit settings" in current page administration + And I expand all fieldsets + And I set the field "Enable completion tracking" to "Yes" + And I click on "Save and display" "button" + And I follow "Test assignment name" + And I navigate to "Edit settings" in current page administration + And I follow "Expand all" + And I set the field "Completion tracking" to "Show activity as complete when conditions are met" + And I set the field "completionusegrade" to "1" + And I press "Save and return to course" + And I navigate to "Course completion" in current page administration + And I expand all fieldsets + And I set the field "Assignment - Test assignment name" to "1" + And I press "Save changes" + And I turn editing mode on + And I add the "Course completion status" block + And I log out + + @javascript + Scenario: Update course completion when student marks activity as complete + Given I log in as "teacher1" + And I am on "Completion course" course homepage + And I follow "Test assignment name" + And I navigate to "Edit settings" in current page administration + And I follow "Expand all" + And I set the field "Completion tracking" to "Students can manually mark the activity as completed" + And I press "Save and return to course" + And I log out + When I log in as "student1" + And I am on "Completion course" course homepage + And I should see "Status: Not yet started" + And I press "Mark as done" + And I wait until "Done" "button" exists + And "Mark as done" "button" should not exist + And I reload the page + Then I should see "Status: Complete" + + @javascript + Scenario: Update course completion when teacher grades a single assignment + Given I log in as "teacher1" + And I am on "Completion course" course homepage + And I follow "Test assignment name" + And I navigate to "View all submissions" in current page administration + And I click on "Grade" "link" in the "student1@example.com" "table_row" + And I set the field "Grade out of 100" to "40" + And I click on "Save changes" "button" + And I am on "Completion course" course homepage + And I log out + And I log in as "student1" + When I am on "Completion course" course homepage + Then I should see "Status: Complete" + + @javascript + Scenario: Update course completion with multiple activity criteria + Given I log in as "admin" + And the following "activity" exists: + | activity | assign | + | course | CC1 | + | name | Test assignment name2 | + | idnumber | assign2 | + | description | Test assignment description | + And I am on "Completion course" course homepage + And I follow "Test assignment name2" + And I navigate to "Edit settings" in current page administration + And I follow "Expand all" + And I set the field "Completion tracking" to "Show activity as complete when conditions are met" + And I set the field "completionusegrade" to "1" + And I press "Save and return to course" + And I navigate to "Course completion" in current page administration + And I expand all fieldsets + And I set the field "Assignment - Test assignment name" to "1" + And I set the field "Assignment - Test assignment name2" to "1" + And I press "Save changes" + And I am on "Completion course" course homepage + And I follow "Test assignment name" + And I navigate to "View all submissions" in current page administration + And I click on "Grade" "link" in the "student1@example.com" "table_row" + And I set the field "Grade out of 100" to "40" + And I click on "Save changes" "button" + And I am on "Completion course" course homepage + And I log out + And I log in as "student1" + And I am on "Completion course" course homepage + And I should see "Status: In progress" + And I log out + When I log in as "teacher1" + And I am on "Completion course" course homepage + And I follow "Test assignment name2" + And I navigate to "View all submissions" in current page administration + And I click on "Grade" "link" in the "student1@example.com" "table_row" + And I set the field "Grade out of 100" to "40" + And I click on "Save changes" "button" + And I am on "Completion course" course homepage + And I log out + And I log in as "student1" + And I am on "Completion course" course homepage + Then I should see "Status: Complete" + + @javascript + Scenario: Course completion should not be updated when teacher grades assignment on course grader report page + Given I log in as "teacher1" + And I am on "Completion course" course homepage + And I navigate to "View > Grader report" in the course gradebook + And I press "Turn editing on" + And I give the grade "57" to the user "Student First" for the grade item "Test assignment name" + And I press "Save changes" + And I log out + And I log in as "student1" + When I am on "Completion course" course homepage + Then I should see "Status: Pending" + And I run the scheduled task "core\task\completion_regular_task" + And I wait "1" seconds + And I run the scheduled task "core\task\completion_regular_task" + And I reload the page + And I should see "Status: Complete" + + @javascript + Scenario: Course completion should not be updated when teacher grades assignment on activity grader report page + Given I log in as "teacher1" + And I am on "Completion course" course homepage + And I navigate to "View > Grader report" in the course gradebook + And I follow "Single view" + And I select "Student First" from the "Select user..." singleselect + And I set the field "Override for Test assignment name" to "1" + When I set the following fields to these values: + | Grade for Test assignment name | 10.00 | + | Feedback for Test assignment name | test data | + And I press "Save" + And I press "Continue" + And I log out + And I log in as "student1" + And I am on "Completion course" course homepage + And I should see "Status: Pending" + And I run the scheduled task "core\task\completion_regular_task" + And I wait "1" seconds + And I run the scheduled task "core\task\completion_regular_task" + And I reload the page + Then I should see "Status: Complete" + + @javascript @_file_upload + Scenario: Course completion should not be updated when teacher imports grades with csv file + Given I log in as "teacher1" + And I am on "Completion course" course homepage + And I navigate to "Import" in the course gradebook + And I upload "lib/tests/fixtures/upload_grades.csv" file to "File" filemanager + And I press "Upload grades" + And I set the field "Map to" to "Email address" + And I set the field "Test assignment name" to "Assignment: Test assignment name" + And I press "Upload grades" + And I press "Continue" + And I should see "10.00" in the "Student First" "table_row" + And I log out + And I log in as "student1" + And I am on "Completion course" course homepage + And I should see "Status: Pending" + When I run the scheduled task "core\task\completion_regular_task" + And I wait "1" seconds + And I run the scheduled task "core\task\completion_regular_task" + And I reload the page + Then I should see "Status: Complete" diff --git a/completion/tests/privacy_test.php b/completion/tests/privacy_test.php index 7bab239986d..f9423448b6f 100644 --- a/completion/tests/privacy_test.php +++ b/completion/tests/privacy_test.php @@ -98,7 +98,7 @@ class core_completion_privacy_test extends \core_privacy\tests\provider_testcase $this->create_course_completion(); $this->complete_course($user); $coursecompletion = \core_completion\privacy\provider::get_course_completion_info($user, $this->course); - $this->assertEquals('In progress', $coursecompletion['status']); + $this->assertEquals('Complete', $coursecompletion['status']); $this->assertCount(2, $coursecompletion['criteria']); } diff --git a/completion/upgrade.txt b/completion/upgrade.txt index a976cd01a7a..5cdbea218b3 100644 --- a/completion/upgrade.txt +++ b/completion/upgrade.txt @@ -1,6 +1,10 @@ This files describes API changes in /completion/* - completion, 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. + === 3.11 === * New Behat steps for activity completion in the behat_completion class: - activity_completion_condition_displayed_as() diff --git a/course/tests/privacy_test.php b/course/tests/privacy_test.php index 512a8d852f7..63dbfd5603b 100644 --- a/course/tests/privacy_test.php +++ b/course/tests/privacy_test.php @@ -148,7 +148,7 @@ class core_course_privacy_testcase extends \core_privacy\tests\provider_testcase $writer = \core_privacy\local\request\writer::with_context($this->coursecontext); \core_course\privacy\provider::export_user_data($approvedlist); $completiondata = $writer->get_data([get_string('privacy:completionpath', 'course')]); - $this->assertEquals('In progress', $completiondata->status); + $this->assertEquals('Complete', $completiondata->status); $this->assertCount(2, $completiondata->criteria); // User has a favourite course. @@ -272,7 +272,7 @@ class core_course_privacy_testcase extends \core_privacy\tests\provider_testcase $records = $DB->get_records('course_modules_completion'); $this->assertCount(2, $records); $records = $DB->get_records('course_completion_crit_compl'); - $this->assertCount(2, $records); + $this->assertCount(4, $records); // Delete data for all users in a context different than the course context (system context). \core_course\privacy\provider::delete_data_for_all_users_in_context($systemcontext); diff --git a/grade/import/lib.php b/grade/import/lib.php index b5d7b37d7e8..37186bd9ce7 100644 --- a/grade/import/lib.php +++ b/grade/import/lib.php @@ -73,7 +73,8 @@ function grade_import_commit($courseid, $importcode, $importfeedback=true, $verb // insert each individual grade to this new grade item foreach ($grades as $grade) { - if (!$gradeitem->update_final_grade($grade->userid, $grade->finalgrade, 'import', $grade->feedback, FORMAT_MOODLE)) { + if (!$gradeitem->update_final_grade($grade->userid, $grade->finalgrade, 'import', + $grade->feedback, FORMAT_MOODLE, null, null, true)) { $failed = true; break 2; } @@ -119,7 +120,8 @@ function grade_import_commit($courseid, $importcode, $importfeedback=true, $verb // False means do not change. See grade_itme::update_final_grade(). $grade->finalgrade = false; } - if (!$gradeitem->update_final_grade($grade->userid, $grade->finalgrade, 'import', $grade->feedback)) { + if (!$gradeitem->update_final_grade($grade->userid, $grade->finalgrade, 'import', + $grade->feedback, FORMAT_MOODLE, null, null, true)) { $errordata = new stdClass(); $errordata->itemname = $gradeitem->itemname; $errordata->userid = $grade->userid; diff --git a/grade/report/grader/lib.php b/grade/report/grader/lib.php index bcbec6a0020..fa5b2d80a4b 100644 --- a/grade/report/grader/lib.php +++ b/grade/report/grader/lib.php @@ -334,7 +334,8 @@ class grade_report_grader extends grade_report { } } - $gradeitem->update_final_grade($userid, $finalgrade, 'gradebook', $feedback, FORMAT_MOODLE); + $gradeitem->update_final_grade($userid, $finalgrade, 'gradebook', $feedback, + FORMAT_MOODLE, null, null, true); // We can update feedback without reloading the grade item as it doesn't affect grade calculations if ($datatype === 'feedback') { diff --git a/grade/report/singleview/classes/local/ui/finalgrade.php b/grade/report/singleview/classes/local/ui/finalgrade.php index 5921fc24616..a3e19e1416e 100644 --- a/grade/report/singleview/classes/local/ui/finalgrade.php +++ b/grade/report/singleview/classes/local/ui/finalgrade.php @@ -174,7 +174,8 @@ class finalgrade extends grade_attribute_format implements unique_value, be_disa } // Only update grades if there are no errors. - $gradeitem->update_final_grade($userid, $finalgrade, 'singleview', $feedback, FORMAT_MOODLE); + $gradeitem->update_final_grade($userid, $finalgrade, 'singleview', $feedback, FORMAT_MOODLE, + null, null, true); return ''; } } diff --git a/lib/classes/grades_external.php b/lib/classes/grades_external.php index 31e3cead682..ec9535e0b74 100644 --- a/lib/classes/grades_external.php +++ b/lib/classes/grades_external.php @@ -555,7 +555,7 @@ class core_grades_external extends external_api { } return grade_update($params['source'], $params['courseid'], $itemtype, - $itemmodule, $iteminstance, $itemnumber, $gradestructure, $params['itemdetails']); + $itemmodule, $iteminstance, $itemnumber, $gradestructure, $params['itemdetails'], true); } /** diff --git a/lib/classes/task/completion_regular_task.php b/lib/classes/task/completion_regular_task.php index 40d45728e15..618dd57b9ef 100644 --- a/lib/classes/task/completion_regular_task.php +++ b/lib/classes/task/completion_regular_task.php @@ -64,135 +64,7 @@ class completion_regular_task extends scheduled_task { } } - if (debugging()) { - mtrace('Aggregating completions'); - } - - // Save time started. - $timestarted = time(); - - // Grab all criteria and their associated criteria completions. - $sql = 'SELECT DISTINCT c.id AS course, cr.id AS criteriaid, crc.userid AS userid, - cr.criteriatype AS criteriatype, cc.timecompleted AS timecompleted - FROM {course_completion_criteria} cr - INNER JOIN {course} c ON cr.course = c.id - INNER JOIN {course_completions} crc ON crc.course = c.id - LEFT JOIN {course_completion_crit_compl} cc ON cc.criteriaid = cr.id AND crc.userid = cc.userid - WHERE c.enablecompletion = 1 - AND crc.timecompleted IS NULL - AND crc.reaggregate > 0 - AND crc.reaggregate < :timestarted - ORDER BY course, userid'; - $rs = $DB->get_recordset_sql($sql, ['timestarted' => $timestarted]); - - // Check if result is empty. - if (!$rs->valid()) { - $rs->close(); - return; - } - - $currentuser = null; - $currentcourse = null; - $completions = []; - while (1) { - // Grab records for current user/course. - foreach ($rs as $record) { - // If we are still grabbing the same users completions. - if ($record->userid === $currentuser && $record->course === $currentcourse) { - $completions[$record->criteriaid] = $record; - } else { - break; - } - } - - // Aggregate. - if (!empty($completions)) { - if (debugging()) { - mtrace('Aggregating completions for user ' . $currentuser . ' in course ' . $currentcourse); - } - - // Get course info object. - $info = new \completion_info((object)['id' => $currentcourse]); - - // Setup aggregation. - $overall = $info->get_aggregation_method(); - $activity = $info->get_aggregation_method(COMPLETION_CRITERIA_TYPE_ACTIVITY); - $prerequisite = $info->get_aggregation_method(COMPLETION_CRITERIA_TYPE_COURSE); - $role = $info->get_aggregation_method(COMPLETION_CRITERIA_TYPE_ROLE); - - $overallstatus = null; - $activitystatus = null; - $prerequisitestatus = null; - $rolestatus = null; - - // Get latest timecompleted. - $timecompleted = null; - - // Check each of the criteria. - foreach ($completions as $params) { - $timecompleted = max($timecompleted, $params->timecompleted); - $completion = new \completion_criteria_completion((array)$params, false); - - // Handle aggregation special cases. - if ($params->criteriatype == COMPLETION_CRITERIA_TYPE_ACTIVITY) { - completion_cron_aggregate($activity, $completion->is_complete(), $activitystatus); - } else if ($params->criteriatype == COMPLETION_CRITERIA_TYPE_COURSE) { - completion_cron_aggregate($prerequisite, $completion->is_complete(), $prerequisitestatus); - } else if ($params->criteriatype == COMPLETION_CRITERIA_TYPE_ROLE) { - completion_cron_aggregate($role, $completion->is_complete(), $rolestatus); - } else { - completion_cron_aggregate($overall, $completion->is_complete(), $overallstatus); - } - } - - // Include role criteria aggregation in overall aggregation. - if ($rolestatus !== null) { - completion_cron_aggregate($overall, $rolestatus, $overallstatus); - } - - // Include activity criteria aggregation in overall aggregation. - if ($activitystatus !== null) { - completion_cron_aggregate($overall, $activitystatus, $overallstatus); - } - - // Include prerequisite criteria aggregation in overall aggregation. - if ($prerequisitestatus !== null) { - completion_cron_aggregate($overall, $prerequisitestatus, $overallstatus); - } - - // If aggregation status is true, mark course complete for user. - if ($overallstatus) { - if (debugging()) { - mtrace('Marking complete'); - } - - $ccompletion = new \completion_completion([ - 'course' => $params->course, - 'userid' => $params->userid - ]); - $ccompletion->mark_complete($timecompleted); - } - } - - // If this is the end of the recordset, break the loop. - if (!$rs->valid()) { - $rs->close(); - break; - } - - // New/next user, update user details, reset completions. - $currentuser = $record->userid; - $currentcourse = $record->course; - $completions = []; - $completions[$record->criteriaid] = $record; - } - - // Mark all users as aggregated. - $sql = "UPDATE {course_completions} - SET reaggregate = 0 - WHERE reaggregate < :timestarted - AND reaggregate > 0"; - $DB->execute($sql, ['timestarted' => $timestarted]); + aggregate_completions(0, true); } } diff --git a/lib/completionlib.php b/lib/completionlib.php index 1a13f06c796..8c1766c9517 100644 --- a/lib/completionlib.php +++ b/lib/completionlib.php @@ -585,10 +585,12 @@ class completion_info { * must be used; these directly set the specified state. * @param int $userid User ID to be updated. Default 0 = current user * @param bool $override Whether manually overriding the existing completion state. + * @param bool $isbulkupdate If bulk grade update is happening. * @return void * @throws moodle_exception if trying to override without permission. */ - public function update_state($cm, $possibleresult=COMPLETION_UNKNOWN, $userid=0, $override = false) { + public function update_state($cm, $possibleresult=COMPLETION_UNKNOWN, $userid=0, + $override = false, $isbulkupdate = false) { global $USER; // Do nothing if completion is not enabled for that activity @@ -662,7 +664,7 @@ class completion_info { $current->completionstate = $newstate; $current->timemodified = time(); $current->overrideby = $override ? $USER->id : null; - $this->internal_set_data($cm, $current); + $this->internal_set_data($cm, $current, $isbulkupdate); } } @@ -1177,9 +1179,11 @@ class completion_info { * * @param stdClass|cm_info $cm Activity * @param stdClass $data Data about completion for that user + * @param bool $isbulkupdate If bulk grade update is happening. */ - public function internal_set_data($cm, $data) { - global $USER, $DB; + public function internal_set_data($cm, $data, $isbulkupdate = false) { + global $USER, $DB, $CFG; + require_once($CFG->dirroot.'/completion/criteria/completion_criteria_activity.php'); $transaction = $DB->start_delegated_transaction(); if (!$data->id) { @@ -1222,6 +1226,17 @@ class completion_info { $completioncache->delete($data->userid . '_' . $cm->course); } + // For single user actions the code must reevaluate some completion state instantly, see MDL-32103. + if ($isbulkupdate) { + return; + } else { + $userdata = ['userid' => $data->userid, 'courseid' => $this->course_id]; + $coursecompletionid = \core_completion\api::mark_course_completions_activity_criteria($userdata); + if ($coursecompletionid) { + aggregate_completions($coursecompletionid); + } + } + // Trigger an event for course module completion changed. $event = \core\event\course_module_completion_updated::create(array( 'objectid' => $data->id, @@ -1421,8 +1436,9 @@ class completion_info { * @param grade_item $item Grade item * @param stdClass $grade * @param bool $deleted + * @param bool $isbulkupdate If bulk grade update is happening. */ - public function inform_grade_changed($cm, $item, $grade, $deleted) { + public function inform_grade_changed($cm, $item, $grade, $deleted, $isbulkupdate = false) { // Bail out now if completion is not enabled for course-module, it is enabled // but is set to manual, grade is not used to compute completion, or this // is a different numbered grade @@ -1442,7 +1458,7 @@ class completion_info { } // OK, let's update state based on this - $this->update_state($cm, $possibleresult, $grade->userid); + $this->update_state($cm, $possibleresult, $grade->userid, false, $isbulkupdate); } /** @@ -1541,3 +1557,157 @@ function completion_cron_aggregate($method, $data, &$state) { } } } + +/** + * Aggregate courses completions. This function is called when activity completion status is updated + * for single user. Also when regular completion task runs it aggregates completions for all courses and users. + * + * @param int $coursecompletionid Course completion ID to update (if 0 - update for all courses and users) + * @param bool $mtraceprogress To output debug info + * @since Moodle 4.0 + */ +function aggregate_completions(int $coursecompletionid, bool $mtraceprogress = false) { + global $DB; + + if (!$coursecompletionid && $mtraceprogress) { + mtrace('Aggregating completions'); + } + // Save time started. + $timestarted = time(); + + // Grab all criteria and their associated criteria completions. + $sql = "SELECT DISTINCT c.id AS courseid, cr.id AS criteriaid, cco.userid, cr.criteriatype, ccocr.timecompleted + FROM {course_completion_criteria} cr + INNER JOIN {course} c ON cr.course = c.id + INNER JOIN {course_completions} cco ON cco.course = c.id + LEFT JOIN {course_completion_crit_compl} ccocr + ON ccocr.criteriaid = cr.id AND cco.userid = ccocr.userid + WHERE c.enablecompletion = 1 + AND cco.timecompleted IS NULL + AND cco.reaggregate > 0"; + + if ($coursecompletionid) { + $sql .= " AND cco.id = ?"; + $param = $coursecompletionid; + } else { + $sql .= " AND cco.reaggregate < ? ORDER BY courseid, cco.userid"; + $param = $timestarted; + } + $rs = $DB->get_recordset_sql($sql, [$param]); + + // Check if result is empty. + if (!$rs->valid()) { + $rs->close(); + return; + } + + $currentuser = null; + $currentcourse = null; + $completions = []; + while (1) { + // Grab records for current user/course. + foreach ($rs as $record) { + // If we are still grabbing the same users completions. + if ($record->userid === $currentuser && $record->courseid === $currentcourse) { + $completions[$record->criteriaid] = $record; + } else { + break; + } + } + + // Aggregate. + if (!empty($completions)) { + if (!$coursecompletionid && $mtraceprogress) { + mtrace('Aggregating completions for user ' . $currentuser . ' in course ' . $currentcourse); + } + + // Get course info object. + $info = new \completion_info((object)['id' => $currentcourse]); + + // Setup aggregation. + $overall = $info->get_aggregation_method(); + $activity = $info->get_aggregation_method(COMPLETION_CRITERIA_TYPE_ACTIVITY); + $prerequisite = $info->get_aggregation_method(COMPLETION_CRITERIA_TYPE_COURSE); + $role = $info->get_aggregation_method(COMPLETION_CRITERIA_TYPE_ROLE); + + $overallstatus = null; + $activitystatus = null; + $prerequisitestatus = null; + $rolestatus = null; + + // Get latest timecompleted. + $timecompleted = null; + + // Check each of the criteria. + foreach ($completions as $params) { + $timecompleted = max($timecompleted, $params->timecompleted); + $completion = new \completion_criteria_completion((array)$params, false); + + // Handle aggregation special cases. + if ($params->criteriatype == COMPLETION_CRITERIA_TYPE_ACTIVITY) { + completion_cron_aggregate($activity, $completion->is_complete(), $activitystatus); + } else if ($params->criteriatype == COMPLETION_CRITERIA_TYPE_COURSE) { + completion_cron_aggregate($prerequisite, $completion->is_complete(), $prerequisitestatus); + } else if ($params->criteriatype == COMPLETION_CRITERIA_TYPE_ROLE) { + completion_cron_aggregate($role, $completion->is_complete(), $rolestatus); + } else { + completion_cron_aggregate($overall, $completion->is_complete(), $overallstatus); + } + } + + // Include role criteria aggregation in overall aggregation. + if ($rolestatus !== null) { + completion_cron_aggregate($overall, $rolestatus, $overallstatus); + } + + // Include activity criteria aggregation in overall aggregation. + if ($activitystatus !== null) { + completion_cron_aggregate($overall, $activitystatus, $overallstatus); + } + + // Include prerequisite criteria aggregation in overall aggregation. + if ($prerequisitestatus !== null) { + completion_cron_aggregate($overall, $prerequisitestatus, $overallstatus); + } + + // If aggregation status is true, mark course complete for user. + if ($overallstatus) { + if (!$coursecompletionid && $mtraceprogress) { + mtrace('Marking complete'); + } + + $ccompletion = new \completion_completion([ + 'course' => $params->courseid, + 'userid' => $params->userid + ]); + $ccompletion->mark_complete($timecompleted); + } + } + + // If this is the end of the recordset, break the loop. + if (!$rs->valid()) { + $rs->close(); + break; + } + + // New/next user, update user details, reset completions. + $currentuser = $record->userid; + $currentcourse = $record->courseid; + $completions = []; + $completions[$record->criteriaid] = $record; + } + + // Mark all users as aggregated. + if ($coursecompletionid) { + $select = "reaggregate > 0 AND id = ?"; + $param = $coursecompletionid; + } else { + $select = "reaggregate > 0 AND reaggregate < ?"; + $param = $timestarted; + if (PHPUNIT_TEST) { + // MDL-33320: for instant completions we need aggregate to work in a single run. + $DB->set_field('course_completions', 'reaggregate', $timestarted - 2); + } + } + $DB->set_field_select('course_completions', 'reaggregate', 0, $select, [$param]); +} diff --git a/lib/grade/grade_category.php b/lib/grade/grade_category.php index bbcf41b7db3..b057fce0b5d 100644 --- a/lib/grade/grade_category.php +++ b/lib/grade/grade_category.php @@ -227,9 +227,10 @@ class grade_category extends grade_object { * In addition to update() as defined in grade_object, call force_regrading of parent categories, if applicable. * * @param string $source from where was the object updated (mod/forum, manual, etc.) + * @param bool $isbulkupdate If bulk grade update is happening. * @return bool success */ - public function update($source=null) { + public function update($source = null, $isbulkupdate = false) { // load the grade item or create a new one $this->load_grade_item(); @@ -352,9 +353,10 @@ class grade_category extends grade_object { * This method also creates an associated grade_item if this wasn't done during construction. * * @param string $source from where was the object inserted (mod/forum, manual, etc.) + * @param bool $isbulkupdate If bulk grade update is happening. * @return int PK ID if successful, false otherwise */ - public function insert($source=null) { + public function insert($source = null, $isbulkupdate = false) { if (empty($this->courseid)) { print_error('cannotinsertgrade'); diff --git a/lib/grade/grade_grade.php b/lib/grade/grade_grade.php index 27062aba373..54c5cf1c71a 100644 --- a/lib/grade/grade_grade.php +++ b/lib/grade/grade_grade.php @@ -442,12 +442,12 @@ class grade_grade extends grade_object { public function set_overridden($state, $refresh = true) { if (empty($this->overridden) and $state) { $this->overridden = time(); - $this->update(); + $this->update(null, true); return true; } else if (!empty($this->overridden) and !$state) { $this->overridden = 0; - $this->update(); + $this->update(null, true); if ($refresh) { //refresh when unlocking @@ -1025,12 +1025,13 @@ class grade_grade extends grade_object { * Insert the grade_grade instance into the database. * * @param string $source From where was the object inserted (mod/forum, manual, etc.) + * @param bool $isbulkupdate If bulk grade update is happening. * @return int The new grade_grade ID if successful, false otherwise */ - public function insert($source=null) { + public function insert($source = null, $isbulkupdate = false) { // TODO: dategraded hack - do not update times, they are used for submission and grading (MDL-31379) //$this->timecreated = $this->timemodified = time(); - return parent::insert($source); + return parent::insert($source, $isbulkupdate); } /** @@ -1038,14 +1039,15 @@ class grade_grade extends grade_object { * the reason is we need to compare the db value with computed number to skip updates if possible. * * @param string $source from where was the object inserted (mod/forum, manual, etc.) + * @param bool $isbulkupdate If bulk grade update is happening. * @return bool success */ - public function update($source=null) { + public function update($source=null, $isbulkupdate = false) { $this->rawgrade = grade_floatval($this->rawgrade); $this->finalgrade = grade_floatval($this->finalgrade); $this->rawgrademin = grade_floatval($this->rawgrademin); $this->rawgrademax = grade_floatval($this->rawgrademax); - return parent::update($source); + return parent::update($source, $isbulkupdate); } @@ -1138,8 +1140,9 @@ class grade_grade extends grade_object { * has changed, and clear up a possible score cache. * * @param bool $deleted True if grade was actually deleted + * @param bool $isbulkupdate If bulk grade update is happening. */ - protected function notify_changed($deleted) { + protected function notify_changed($deleted, $isbulkupdate = false) { global $CFG; // Condition code may cache the grades for conditional availability of @@ -1200,7 +1203,7 @@ class grade_grade extends grade_object { } // Pass information on to completion system - $completion->inform_grade_changed($cm, $this->grade_item, $this, $deleted); + $completion->inform_grade_changed($cm, $this->grade_item, $this, $deleted, $isbulkupdate); } /** diff --git a/lib/grade/grade_item.php b/lib/grade/grade_item.php index 961a4608a31..e0a7c39925b 100644 --- a/lib/grade/grade_item.php +++ b/lib/grade/grade_item.php @@ -282,9 +282,10 @@ class grade_item extends grade_object { * the reason is we need to compare the db value with computed number to skip regrading if possible. * * @param string $source from where was the object inserted (mod/forum, manual, etc.) + * @param bool $isbulkupdate If bulk grade update is happening. * @return bool success */ - public function update($source=null) { + public function update($source = null, $isbulkupdate = false) { // reset caches $this->dependson_cache = null; @@ -309,7 +310,7 @@ class grade_item extends grade_object { $this->aggregationcoef = grade_floatval($this->aggregationcoef); $this->aggregationcoef2 = grade_floatval($this->aggregationcoef2); - $result = parent::update($source); + $result = parent::update($source, $isbulkupdate); if ($result) { $event = \core\event\grade_item_updated::create_from_grade_item($this); @@ -499,9 +500,10 @@ class grade_item extends grade_object { * In addition to perform parent::insert(), calls force_regrading() method too. * * @param string $source From where was the object inserted (mod/forum, manual, etc.) + * @param string $isbulkupdate If bulk grade update is happening. * @return int PK ID if successful, false otherwise */ - public function insert($source=null) { + public function insert($source = null, $isbulkupdate = false) { global $CFG, $DB; if (empty($this->courseid)) { @@ -540,7 +542,7 @@ class grade_item extends grade_object { $this->timecreated = $this->timemodified = time(); - if (parent::insert($source)) { + if (parent::insert($source, $isbulkupdate)) { // force regrading of items if needed $this->force_regrading(); @@ -1790,12 +1792,11 @@ class grade_item extends grade_object { * @param int $feedbackformat A format like FORMAT_PLAIN or FORMAT_HTML * @param int $usermodified The ID of the user making the modification * @param int $timemodified Optional parameter to set the time modified, if not present current time. + * @param bool $isbulkupdate If bulk grade update is happening. * @return bool success */ - public function update_final_grade($userid, $finalgrade = false, - $source = null, $feedback = false, - $feedbackformat = FORMAT_MOODLE, - $usermodified = null, $timemodified = null) { + public function update_final_grade($userid, $finalgrade = false, $source = null, $feedback = false, + $feedbackformat = FORMAT_MOODLE, $usermodified = null, $timemodified = null, $isbulkupdate = false) { global $USER, $CFG; $result = true; @@ -1863,7 +1864,7 @@ class grade_item extends grade_object { if (empty($grade->id)) { $grade->timecreated = null; // Hack alert - date submitted - no submission yet. $grade->timemodified = $timemodified ?? time(); // Hack alert - date graded. - $result = (bool)$grade->insert($source); + $result = (bool)$grade->insert($source, $isbulkupdate); // If the grade insert was successful and the final grade was not null then trigger a user_graded event. if ($result && !is_null($grade->finalgrade)) { @@ -1887,7 +1888,7 @@ class grade_item extends grade_object { } $grade->timemodified = $timemodified ?? time(); // Hack alert - date graded. - $result = $grade->update($source); + $result = $grade->update($source, $isbulkupdate); // If the grade update was successful and the actual grade has changed then trigger a user_graded event. if ($result && grade_floats_different($grade->finalgrade, $oldgrade->finalgrade)) { @@ -1949,11 +1950,12 @@ class grade_item extends grade_object { * 'filearea' => 'mod_xyz_feedback', * 'itemid' => 2 * ]; + * @param bool $isbulkupdate If bulk grade update is happening. * @return bool success */ public function update_raw_grade($userid, $rawgrade = false, $source = null, $feedback = false, $feedbackformat = FORMAT_MOODLE, $usermodified = null, $dategraded = null, $datesubmitted=null, - $grade = null, array $feedbackfiles = []) { + $grade = null, array $feedbackfiles = [], $isbulkupdate = false) { global $USER; $result = true; @@ -2053,7 +2055,7 @@ class grade_item extends grade_object { $gradechanged = false; if (empty($grade->id)) { - $result = (bool)$grade->insert($source); + $result = (bool)$grade->insert($source, $isbulkupdate); // If the grade insert was successful and the final grade was not null then trigger a user_graded event. if ($result && !is_null($grade->finalgrade)) { @@ -2080,7 +2082,7 @@ class grade_item extends grade_object { // No changes. return $result; } - $result = $grade->update($source); + $result = $grade->update($source, $isbulkupdate); // If the grade update was successful and the actual grade has changed then trigger a user_graded event. if ($result && grade_floats_different($grade->finalgrade, $oldgrade->finalgrade)) { diff --git a/lib/grade/grade_object.php b/lib/grade/grade_object.php index 6e7373dd902..66a795452bd 100644 --- a/lib/grade/grade_object.php +++ b/lib/grade/grade_object.php @@ -238,9 +238,10 @@ abstract class grade_object { * Updates this object in the Database, based on its object variables. ID must be set. * * @param string $source from where was the object updated (mod/forum, manual, etc.) + * @param bool $isbulkupdate If bulk grade update is happening. * @return bool success */ - public function update($source=null) { + public function update($source = null, $isbulkupdate = false) { global $USER, $CFG, $DB; if (empty($this->id)) { @@ -263,7 +264,7 @@ abstract class grade_object { $historyid = $DB->insert_record($this->table.'_history', $data); } - $this->notify_changed(false); + $this->notify_changed(false, $isbulkupdate); $this->update_feedback_files($historyid); @@ -334,9 +335,10 @@ abstract class grade_object { * in object properties. * * @param string $source From where was the object inserted (mod/forum, manual, etc.) + * @param string $isbulkupdate If bulk grade update is happening. * @return int The new grade object ID if successful, false otherwise */ - public function insert($source=null) { + public function insert($source = null, $isbulkupdate = false) { global $USER, $CFG, $DB; if (!empty($this->id)) { @@ -364,7 +366,7 @@ abstract class grade_object { $historyid = $DB->insert_record($this->table.'_history', $data); } - $this->notify_changed(false); + $this->notify_changed(false, $isbulkupdate); $this->add_feedback_files($historyid); diff --git a/lib/grade/grade_outcome.php b/lib/grade/grade_outcome.php index 064cdd160cb..fc5e074713e 100644 --- a/lib/grade/grade_outcome.php +++ b/lib/grade/grade_outcome.php @@ -123,9 +123,10 @@ class grade_outcome extends grade_object { * in object properties. * * @param string $source from where was the object inserted (mod/forum, manual, etc.) + * @param bool $isbulkupdate If bulk grade update is happening. * @return int PK ID if successful, false otherwise */ - public function insert($source=null) { + public function insert($source = null, $isbulkupdate = false) { global $DB; $this->timecreated = $this->timemodified = time(); @@ -145,9 +146,10 @@ class grade_outcome extends grade_object { * In addition to update() it also updates grade_outcomes_courses if needed * * @param string $source from where was the object inserted + * @param bool $isbulkupdate If bulk grade update is happening. * @return bool success */ - public function update($source=null) { + public function update($source = null, $isbulkupdate = false) { $this->timemodified = time(); if ($result = parent::update($source)) { diff --git a/lib/grade/grade_scale.php b/lib/grade/grade_scale.php index 6a94d78898f..c4546a20987 100644 --- a/lib/grade/grade_scale.php +++ b/lib/grade/grade_scale.php @@ -114,9 +114,10 @@ class grade_scale extends grade_object { * in object properties. * * @param string $source from where was the object inserted (mod/forum, manual, etc.) + * @param bool $isbulkupdate If bulk grade update is happening. * @return int PK ID if successful, false otherwise */ - public function insert($source=null) { + public function insert($source = null, $isbulkupdate = false) { $this->timecreated = time(); $this->timemodified = time(); @@ -145,9 +146,10 @@ class grade_scale extends grade_object { * In addition to update() it also updates grade_outcomes_courses if needed * * @param string $source from where was the object inserted + * @param bool $isbulkupdate If bulk grade update is happening. * @return bool success */ - public function update($source=null) { + public function update($source = null, $isbulkupdate = false) { $this->timemodified = time(); $result = parent::update($source); diff --git a/lib/gradelib.php b/lib/gradelib.php index 05aee9f9501..9ac9097021f 100644 --- a/lib/gradelib.php +++ b/lib/gradelib.php @@ -58,9 +58,11 @@ require_once($CFG->libdir . '/grade/grade_outcome.php'); * @param int $itemnumber Most probably 0. Modules can use other numbers when having more than one grade for each user * @param mixed $grades Grade (object, array) or several grades (arrays of arrays or objects), NULL if updating grade_item definition only * @param mixed $itemdetails Object or array describing the grading item, NULL if no change + * @param bool $isbulkupdate If bulk grade update is happening. * @return int Returns GRADE_UPDATE_OK, GRADE_UPDATE_FAILED, GRADE_UPDATE_MULTIPLE or GRADE_UPDATE_ITEM_LOCKED */ -function grade_update($source, $courseid, $itemtype, $itemmodule, $iteminstance, $itemnumber, $grades=NULL, $itemdetails=NULL) { +function grade_update($source, $courseid, $itemtype, $itemmodule, $iteminstance, $itemnumber, $grades = null, + $itemdetails = null, $isbulkupdate = false) { global $USER, $CFG, $DB; // only following grade_item properties can be changed in this function @@ -127,7 +129,7 @@ function grade_update($source, $courseid, $itemtype, $itemmodule, $iteminstance, } } $grade_item = new grade_item($params); - $grade_item->insert(); + $grade_item->insert(null, $isbulkupdate); } else { if ($grade_item->is_locked()) { @@ -157,7 +159,7 @@ function grade_update($source, $courseid, $itemtype, $itemmodule, $iteminstance, } } if ($update) { - $grade_item->update(); + $grade_item->update(null, $isbulkupdate); } } } @@ -289,7 +291,7 @@ function grade_update($source, $courseid, $itemtype, $itemmodule, $iteminstance, // update or insert the grade if (!$grade_item->update_raw_grade($userid, $rawgrade, $source, $feedback, $feedbackformat, $usermodified, - $dategraded, $datesubmitted, $grade_grade, $feedbackfiles)) { + $dategraded, $datesubmitted, $grade_grade, $feedbackfiles, $isbulkupdate)) { $failed = true; } } diff --git a/lib/tests/completionlib_test.php b/lib/tests/completionlib_test.php index a629f7b037e..4712b5b5d64 100644 --- a/lib/tests/completionlib_test.php +++ b/lib/tests/completionlib_test.php @@ -826,6 +826,38 @@ class core_completionlib_testcase extends advanced_testcase { $d3->overrideby = null; $DB->insert_record('course_modules_completion', $d3); $c->internal_set_data($cm, $data); + + // 4) Test instant course completions. + $dataactivity = $this->getDataGenerator()->create_module('data', array('course' => $this->course->id), + array('completion' => 1)); + $cm = get_coursemodule_from_instance('data', $dataactivity->id); + $c = new completion_info($this->course); + $cmdata = get_coursemodule_from_id('data', $dataactivity->cmid); + + // Add activity completion criteria. + $criteriadata = new stdClass(); + $criteriadata->id = $this->course->id; + $criteriadata->criteria_activity = array(); + // Some activities. + $criteriadata->criteria_activity[$cmdata->id] = 1; + $class = 'completion_criteria_activity'; + $criterion = new $class(); + $criterion->update_config($criteriadata); + + $actual = $DB->get_records('course_completions'); + $this->assertEmpty($actual); + + $data->coursemoduleid = $cm->id; + $c->internal_set_data($cm, $data); + $actual = $DB->get_records('course_completions'); + $this->assertEquals(1, count($actual)); + $this->assertEquals($this->user->id, reset($actual)->userid); + + $data->userid = $newuser2->id; + $c->internal_set_data($cm, $data, true); + $actual = $DB->get_records('course_completions'); + $this->assertEquals(1, count($actual)); + $this->assertEquals($this->user->id, reset($actual)->userid); } public function test_get_progress_all_few() { @@ -1350,6 +1382,302 @@ class core_completionlib_testcase extends advanced_testcase { // The implicitly created grade_item does not have grade to pass defined so it is not distinguished. $this->assertEquals(COMPLETION_COMPLETE, $completioninfo->get_grade_completion($cm, $this->user->id)); } + + /** + * Test for aggregate_completions(). + */ + public function test_aggregate_completions() { + global $DB; + $this->resetAfterTest(true); + $time = time(); + + $course = $this->getDataGenerator()->create_course(array('enablecompletion' => 1)); + + for ($i = 0; $i < 4; $i++) { + $students[] = $this->getDataGenerator()->create_user(); + } + + $teacher = $this->getDataGenerator()->create_user(); + $studentrole = $DB->get_record('role', array('shortname' => 'student')); + $teacherrole = $DB->get_record('role', array('shortname' => 'editingteacher')); + + $this->getDataGenerator()->enrol_user($teacher->id, $course->id, $teacherrole->id); + foreach ($students as $student) { + $this->getDataGenerator()->enrol_user($student->id, $course->id, $studentrole->id); + } + + $data = $this->getDataGenerator()->create_module('data', array('course' => $course->id), + array('completion' => 1)); + $cmdata = get_coursemodule_from_id('data', $data->cmid); + + // Add activity completion criteria. + $criteriadata = new stdClass(); + $criteriadata->id = $course->id; + $criteriadata->criteria_activity = array(); + // Some activities. + $criteriadata->criteria_activity[$cmdata->id] = 1; + $class = 'completion_criteria_activity'; + $criterion = new $class(); + $criterion->update_config($criteriadata); + + $this->setUser($teacher); + + // Mark activity complete for both students. + $cm = get_coursemodule_from_instance('data', $data->id); + $completioncriteria = $DB->get_record('course_completion_criteria', []); + foreach ($students as $student) { + $cmcompletionrecords[] = (object)[ + 'coursemoduleid' => $cm->id, + 'userid' => $student->id, + 'completionstate' => 1, + 'viewed' => 0, + 'overrideby' => null, + 'timemodified' => 0, + ]; + + $usercompletions[] = (object)[ + 'criteriaid' => $completioncriteria->id, + 'userid' => $student->id, + 'timecompleted' => $time, + ]; + + $cc = array( + 'course' => $course->id, + 'userid' => $student->id + ); + $ccompletion = new completion_completion($cc); + $completion[] = $ccompletion->mark_inprogress($time); + } + $DB->insert_records('course_modules_completion', $cmcompletionrecords); + $DB->insert_records('course_completion_crit_compl', $usercompletions); + + // MDL-33320: for instant completions we need aggregate to work in a single run. + $DB->set_field('course_completions', 'reaggregate', $time - 2); + + foreach ($students as $student) { + $result = $DB->get_record('course_completions', ['userid' => $student->id, 'reaggregate' => 0]); + $this->assertFalse($result); + } + + aggregate_completions($completion[0]); + + $result1 = $DB->get_record('course_completions', ['userid' => $students[0]->id, 'reaggregate' => 0]); + $result2 = $DB->get_record('course_completions', ['userid' => $students[1]->id, 'reaggregate' => 0]); + $result3 = $DB->get_record('course_completions', ['userid' => $students[2]->id, 'reaggregate' => 0]); + + $this->assertIsObject($result1); + $this->assertFalse($result2); + $this->assertFalse($result3); + + aggregate_completions(0); + + foreach ($students as $student) { + $result = $DB->get_record('course_completions', ['userid' => $student->id, 'reaggregate' => 0]); + $this->assertIsObject($result); + } + } + + /** + * Test for completion_completion::_save(). + */ + public function test_save() { + global $DB; + $this->resetAfterTest(true); + + $course = $this->getDataGenerator()->create_course(array('enablecompletion' => 1)); + + $student = $this->getDataGenerator()->create_user(); + $teacher = $this->getDataGenerator()->create_user(); + $studentrole = $DB->get_record('role', array('shortname' => 'student')); + $teacherrole = $DB->get_record('role', array('shortname' => 'editingteacher')); + + $this->getDataGenerator()->enrol_user($teacher->id, $course->id, $teacherrole->id); + $this->getDataGenerator()->enrol_user($student->id, $course->id, $studentrole->id); + + $this->setUser($teacher); + + $cc = array( + 'course' => $course->id, + 'userid' => $student->id + ); + $ccompletion = new completion_completion($cc); + + $completions = $DB->get_records('course_completions'); + $this->assertEmpty($completions); + + // We're testing a private method, so we need to setup reflector magic. + $method = new ReflectionMethod($ccompletion, '_save'); + $method->setAccessible(true); // Allow accessing of private method. + $completionid = $method->invoke($ccompletion); + $completions = $DB->get_records('course_completions'); + $this->assertEquals(count($completions), 1); + $this->assertEquals(reset($completions)->id, $completionid); + + $ccompletion->id = 0; + $method = new ReflectionMethod($ccompletion, '_save'); + $method->setAccessible(true); // Allow accessing of private method. + $completionid = $method->invoke($ccompletion); + $this->assertDebuggingCalled('Can not update data object, no id!'); + $this->assertNull($completionid); + } + + /** + * Test for completion_completion::mark_enrolled(). + */ + public function test_mark_enrolled() { + global $DB; + $this->resetAfterTest(true); + + $course = $this->getDataGenerator()->create_course(array('enablecompletion' => 1)); + + $student = $this->getDataGenerator()->create_user(); + $teacher = $this->getDataGenerator()->create_user(); + $studentrole = $DB->get_record('role', array('shortname' => 'student')); + $teacherrole = $DB->get_record('role', array('shortname' => 'editingteacher')); + + $this->getDataGenerator()->enrol_user($teacher->id, $course->id, $teacherrole->id); + $this->getDataGenerator()->enrol_user($student->id, $course->id, $studentrole->id); + + $this->setUser($teacher); + + $cc = array( + 'course' => $course->id, + 'userid' => $student->id + ); + $ccompletion = new completion_completion($cc); + + $completions = $DB->get_records('course_completions'); + $this->assertEmpty($completions); + + $completionid = $ccompletion->mark_enrolled(); + $completions = $DB->get_records('course_completions'); + $this->assertEquals(count($completions), 1); + $this->assertEquals(reset($completions)->id, $completionid); + + $ccompletion->id = 0; + $completionid = $ccompletion->mark_enrolled(); + $this->assertDebuggingCalled('Can not update data object, no id!'); + $this->assertNull($completionid); + $completions = $DB->get_records('course_completions'); + $this->assertEquals(1, count($completions)); + } + + /** + * Test for completion_completion::mark_inprogress(). + */ + public function test_mark_inprogress() { + global $DB; + $this->resetAfterTest(true); + + $course = $this->getDataGenerator()->create_course(array('enablecompletion' => 1)); + + $student = $this->getDataGenerator()->create_user(); + $teacher = $this->getDataGenerator()->create_user(); + $studentrole = $DB->get_record('role', array('shortname' => 'student')); + $teacherrole = $DB->get_record('role', array('shortname' => 'editingteacher')); + + $this->getDataGenerator()->enrol_user($teacher->id, $course->id, $teacherrole->id); + $this->getDataGenerator()->enrol_user($student->id, $course->id, $studentrole->id); + + $this->setUser($teacher); + + $cc = array( + 'course' => $course->id, + 'userid' => $student->id + ); + $ccompletion = new completion_completion($cc); + + $completions = $DB->get_records('course_completions'); + $this->assertEmpty($completions); + + $completionid = $ccompletion->mark_inprogress(); + $completions = $DB->get_records('course_completions'); + $this->assertEquals(1, count($completions)); + $this->assertEquals(reset($completions)->id, $completionid); + + $ccompletion->id = 0; + $completionid = $ccompletion->mark_inprogress(); + $this->assertDebuggingCalled('Can not update data object, no id!'); + $this->assertNull($completionid); + $completions = $DB->get_records('course_completions'); + $this->assertEquals(1, count($completions)); + } + + /** + * Test for completion_completion::mark_complete(). + */ + public function test_mark_complete() { + global $DB; + $this->resetAfterTest(true); + + $course = $this->getDataGenerator()->create_course(array('enablecompletion' => 1)); + + $student = $this->getDataGenerator()->create_user(); + $teacher = $this->getDataGenerator()->create_user(); + $studentrole = $DB->get_record('role', array('shortname' => 'student')); + $teacherrole = $DB->get_record('role', array('shortname' => 'editingteacher')); + + $this->getDataGenerator()->enrol_user($teacher->id, $course->id, $teacherrole->id); + $this->getDataGenerator()->enrol_user($student->id, $course->id, $studentrole->id); + + $this->setUser($teacher); + + $cc = array( + 'course' => $course->id, + 'userid' => $student->id + ); + $ccompletion = new completion_completion($cc); + + $completions = $DB->get_records('course_completions'); + $this->assertEmpty($completions); + + $completionid = $ccompletion->mark_complete(); + $completions = $DB->get_records('course_completions'); + $this->assertEquals(1, count($completions)); + $this->assertEquals(reset($completions)->id, $completionid); + + $ccompletion->id = 0; + $completionid = $ccompletion->mark_complete(); + $this->assertNull($completionid); + $completions = $DB->get_records('course_completions'); + $this->assertEquals(1, count($completions)); + } + + /** + * Test for completion_criteria_completion::mark_complete(). + */ + public function test_criteria_mark_complete() { + global $DB; + $this->resetAfterTest(true); + + $course = $this->getDataGenerator()->create_course(array('enablecompletion' => 1)); + + $student = $this->getDataGenerator()->create_user(); + $teacher = $this->getDataGenerator()->create_user(); + $studentrole = $DB->get_record('role', array('shortname' => 'student')); + $teacherrole = $DB->get_record('role', array('shortname' => 'editingteacher')); + + $this->getDataGenerator()->enrol_user($teacher->id, $course->id, $teacherrole->id); + $this->getDataGenerator()->enrol_user($student->id, $course->id, $studentrole->id); + + $this->setUser($teacher); + + $record = [ + 'course' => $course->id, + 'criteriaid' => 1, + 'userid' => $student->id, + 'timecompleted' => time() + ]; + $completion = new completion_criteria_completion($record, DATA_OBJECT_FETCH_BY_KEY); + + $completions = $DB->get_records('course_completions'); + $this->assertEmpty($completions); + + $completionid = $completion->mark_complete($record['timecompleted']); + $completions = $DB->get_records('course_completions'); + $this->assertEquals(1, count($completions)); + $this->assertEquals(reset($completions)->id, $completionid); + } } class core_completionlib_fake_recordset implements Iterator { diff --git a/lib/tests/fixtures/upload_grades.csv b/lib/tests/fixtures/upload_grades.csv new file mode 100644 index 00000000000..1321d3dacec --- /dev/null +++ b/lib/tests/fixtures/upload_grades.csv @@ -0,0 +1,2 @@ +Email address, Test assignment name +student1@example.com, 10 diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 6b62f484f4d..fe23f87e111 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -26,6 +26,36 @@ information provided here is intended especially for developers. should no longer be used. * The completion_info function print_help_icon() which has been deprecated since Moodle 2.0 should no longer be used. * @babel/polyfill has been removed in favour of corejs@3 +* A new parameter $isbulkupdate has been added to the following functions: + - grade_category::update() + - grade_category::insert() + - grade_grade::update() + - grade_grade::insert() + - grade_grade::notify_changed() + - grade_item::insert() + - grade_item::update() + - grade_item::update_final_grade() + - grade_item::update_raw_grade() + - grade_object::update() + - grade_object::insert() + - grade_outcome::update() + - grade_outcome::insert() + - grade_scale::update() + - grade_scale::insert() + - grade_update() + - completion_info::inform_grade_changed() + - completion_info::update_state() + - completion_info::internal_set_data() + All functions except completion_info::internal_set_data() are only passing this parameter from very beginning of + workflow (like grade report page where bulk grade update is possible) so this parameter is used in + completion_info::internal_set_data() to decide if we need to mark completions instantly without waiting for cron. +* Following methods now return an int instead of bool: + - completion_completion::_save() + - completion_completion::mark_enrolled() + - completion_completion::mark_inprogress() + - completion_completion::mark_complete() + which is needed to store id of completion record on successful update which is later beeing used by + completion_info::internal_set_data() to reaggregate completions that have been marked for instant course completion. === 3.11 === * PHPUnit has been upgraded to 9.5 (see MDL-71036 for details).