Skip to content

Commit 95b090a

Browse files
jpahullojpahullo
committed
#372 - capture regrading and recompletion output into logs
1 parent 11b364e commit 95b090a

7 files changed

Lines changed: 230 additions & 13 deletions

‎CHANGES.md‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,16 @@
11
# Release notes
22

3+
## 2025092100
4+
5+
1. 2025-09-21: improvement: #372: add output from last steps of regrading and reaggregation of course completions.
6+
Also, reaggregation of course completion now happens inside the time of the merge process, and not after as before.
7+
8+
9+
## 2025091800
10+
11+
1. 2025-09-18: fix: #371: listuserfields.php CLI scripts supports tables that does not exist on the XML database schema.
12+
13+
314
## 2025090401
415

516
1. 2025-09-04: fix: #367: database settings tab did no show properly

‎classes/local/callbacks/regrading_after_merged_callback.php‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,20 @@ public static function regrade(after_merged_all_tables $hook) {
9090
$activity->modname = $iteminstance->itemmodule;
9191
$activity->cmidnumber = $cm->idnumber;
9292

93+
ob_start();
9394
grade_update_mod_grades($activity, $hook->toid);
95+
$regradeoutput = ob_get_clean();
96+
$hook->add_log(sprintf(
97+
'Regraded grade item with id "%s" from module type "%s" and instance "%s" from course "%s".',
98+
$iteminstance->id,
99+
$iteminstance->itemmodule,
100+
$iteminstance->iteminstance,
101+
$iteminstance->courseid,
102+
));
103+
if (!empty($regradeoutput)) {
104+
// Convert potential HTML returned to HTML entities to prevent formatting errors on merge logs.
105+
$hook->add_log(htmlspecialchars($regradeoutput));
106+
}
94107
}
95108
}
96109
}

‎classes/local/callbacks/update_completion_after_merged_callback.php‎

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
namespace tool_mergeusers\local\callbacks;
2727

28+
use coding_exception;
2829
use dml_exception;
2930
use tool_mergeusers\hook\after_merged_all_tables;
3031

@@ -43,22 +44,51 @@ class update_completion_after_merged_callback {
4344
* This makes Moodle core updating course completions for the user to keep.
4445
* Moodle internals updates the course's completion status in short.
4546
*
47+
* This version of the course completion is inspired by the completionlib_test.php::test_aggregate_completions().
48+
*
4649
* @param after_merged_all_tables $hook
4750
* @return void
4851
* @throws dml_exception
52+
* @throws coding_exception
4953
*/
5054
public static function update_completion(after_merged_all_tables $hook): void {
51-
global $DB;
55+
global $CFG, $DB;
56+
require_once($CFG->libdir . '/completionlib.php');
5257

53-
$DB->execute(
54-
'UPDATE {course_completions}
55-
SET reaggregate = :now
56-
WHERE userid = :toid
57-
AND (timecompleted IS NULL OR timecompleted = 0)',
58-
[
59-
'now' => time(),
60-
'toid' => $hook->toid,
61-
],
58+
$now = time() - 2; // MDL-33320: for instant completions we need aggregate to work in a single run.
59+
$params = [
60+
'toid' => $hook->toid,
61+
'notime' => 0,
62+
];
63+
$courseids = $DB->get_fieldset_sql(
64+
'SELECT course
65+
FROM {course_completions}
66+
WHERE userid = :toid
67+
AND (timecompleted IS NULL OR timecompleted = :notime)',
68+
$params,
6269
);
70+
$ncourses = count($courseids);
71+
if ($ncourses <= 0) {
72+
$hook->add_log('Course completion reaggregation asked for no courses.');
73+
return;
74+
}
75+
// Look for courses to reaggregate course completion.
76+
$cc = [
77+
'userid' => $hook->toid,
78+
];
79+
foreach ($courseids as $courseid) {
80+
$cc['course'] = $courseid;
81+
$ccompletion = new \completion_completion($cc);
82+
$completion = $ccompletion->mark_inprogress($now);
83+
// Just aggregate this course completion for this user.
84+
aggregate_completions($completion);
85+
}
86+
// With this version of the updating process, the course completion reaggregation is included into the merge process.
87+
$hook->add_log(sprintf(
88+
'Course completion reaggregated for user %d and these %d courses: %s.',
89+
$hook->toid,
90+
$ncourses,
91+
implode(',', $courseids),
92+
));
6393
}
6494
}

‎tests/assign_test.php‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ public function setUp(): void {
4545
* has no.
4646
* @group tool_mergeusers
4747
* @group tool_mergeusers_assign
48+
* @group tool_mergeusers_reaggregate
49+
* @group tool_mergeusers_regrade
4850
*/
4951
public function test_merge_non_conflicting_assign_grades(): void {
5052
global $DB;
@@ -68,7 +70,27 @@ public function test_merge_non_conflicting_assign_grades(): void {
6870
// Merge student 1 into student 0.
6971
$mut = new user_merger();
7072
// This merge already invokes the callback for regrading.
71-
$mut->merge($student1->id, $student2->id);
73+
[$success, $logs, $logid] = $mut->merge($student1->id, $student2->id);
74+
75+
// Check that logs contain regrading log lines.
76+
$this->assertTrue($success);
77+
$found = '';
78+
foreach ($logs as $logline) {
79+
$found = strstr($logline, 'Regraded grade item with id');
80+
if (!empty($found)) {
81+
break;
82+
}
83+
}
84+
$this->assertNotEmpty($found);
85+
// Check that there is no reaggregation of course completion.
86+
$found = '';
87+
foreach ($logs as $logline) {
88+
$found = strstr($logline, 'Course completion reaggregation asked for no courses.');
89+
if (!empty($found)) {
90+
break;
91+
}
92+
}
93+
$this->assertNotEmpty($found);
7294

7395
// Student 0 should now have a grade of 75.00.
7496
$this->assertEquals(true, $assign->testable_is_graded($student1->id));

‎tests/enrolments_test.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public function setUp(): void {
4242
* @group tool_mergeusers
4343
* @group tool_mergeusers_enrolments
4444
*/
45-
public function test_mergeenrolments(): void {
45+
public function test_merge_enrolments(): void {
4646
global $DB;
4747

4848
// Setup two users to merge.

‎tests/last_merge_test.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
use advanced_testcase;
2020
use tool_mergeusers\local\last_merge;
2121
use tool_mergeusers\local\user_merger;
22-
use tool_mergeusers_renderer;
2322

2423
/**
2524
* Testing last_merge API.

‎tests/reaggregate_course_completion_test.php‎

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
<?php
2+
// This file is part of Moodle - http://moodle.org/
3+
//
4+
// Moodle is free software: you can redistribute it and/or modify
5+
// it under the terms of the GNU General Public License as published by
6+
// the Free Software Foundation, either version 3 of the License, or
7+
// (at your option) any later version.
8+
//
9+
// Moodle is distributed in the hope that it will be useful,
10+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
// GNU General Public License for more details.
13+
//
14+
// You should have received a copy of the GNU General Public License
15+
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
16+
17+
namespace tool_mergeusers;
18+
19+
use advanced_testcase;
20+
use coding_exception;
21+
use completion_completion;
22+
use dml_exception;
23+
use stdClass;
24+
use tool_mergeusers\local\user_merger;
25+
26+
defined('MOODLE_INTERNAL') || die();
27+
28+
global $CFG;
29+
require_once($CFG->libdir.'/completionlib.php');
30+
31+
/**
32+
* Testing reaggregation of courses completion.
33+
*
34+
* This test covers the case of positive reaggregation of courses completion.
35+
* Check assign_test.php for the case when no reaggregation exists.
36+
* Check them out with --group=tool_mergeusers_reaggregate.
37+
*
38+
* @package tool_mergeusers
39+
* @author Jordi Pujol Ahulló <jordi.pujol@urv.cat>
40+
* @copyright 2025 Universitat Rovira i Virgili (https://www.urv.cat)
41+
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
42+
*/
43+
final class reaggregate_course_completion_test extends advanced_testcase {
44+
/**
45+
* Tests reaggregate field is set for courses completion.
46+
*
47+
* @group tool_mergeusers
48+
* @group tool_mergeusers_reaggregate
49+
* @throws dml_exception
50+
* @throws coding_exception
51+
*/
52+
public function test_reaggregate_field_is_updated(): void {
53+
// Inspired by lib/tests/completionlib_test.php::test_aggregate_completions().
54+
global $DB, $CFG;
55+
require_once($CFG->dirroot.'/completion/criteria/completion_criteria_activity.php');
56+
$this->resetAfterTest(true);
57+
$time = time();
58+
59+
$course = $this->getDataGenerator()->create_course(['enablecompletion' => 1]);
60+
61+
$fromstudent = $this->getDataGenerator()->create_user();
62+
$tostudent = $this->getDataGenerator()->create_user();
63+
$teacher = $this->getDataGenerator()->create_user();
64+
$studentrole = $DB->get_record('role', ['shortname' => 'student']);
65+
$teacherrole = $DB->get_record('role', ['shortname' => 'editingteacher']);
66+
$this->getDataGenerator()->enrol_user($teacher->id, $course->id, $teacherrole->id);
67+
$this->getDataGenerator()->enrol_user($fromstudent->id, $course->id, $studentrole->id);
68+
$this->getDataGenerator()->enrol_user($tostudent->id, $course->id, $studentrole->id);
69+
70+
$data = $this->getDataGenerator()->create_module('data', ['course' => $course->id], ['completion' => 1]);
71+
$cmdata = get_coursemodule_from_id('data', $data->cmid);
72+
73+
// Add activity completion criteria.
74+
$criteriadata = new stdClass();
75+
$criteriadata->id = $course->id;
76+
$criteriadata->criteria_activity = [];
77+
// Some activities.
78+
$criteriadata->criteria_activity[$cmdata->id] = 1;
79+
$class = 'completion_criteria_activity';
80+
$criterion = new $class();
81+
$criterion->update_config($criteriadata);
82+
83+
$this->setUser($teacher);
84+
85+
// Mark activity incomplete for one of the students.
86+
$cm = get_coursemodule_from_instance('data', $data->id);
87+
$completioncriteria = $DB->get_record('course_completion_criteria', []);
88+
$cmcompletionrecord = (object)[
89+
'coursemoduleid' => $cm->id,
90+
'userid' => $fromstudent->id,
91+
'completionstate' => 1,
92+
'viewed' => 0,
93+
'overrideby' => null,
94+
'timemodified' => 0,
95+
];
96+
97+
$usercompletion = (object)[
98+
'criteriaid' => $completioncriteria->id,
99+
'userid' => $fromstudent->id,
100+
'timecompleted' => 0,
101+
];
102+
103+
$cc = array(
104+
'course' => $course->id,
105+
'userid' => $fromstudent->id
106+
);
107+
$ccompletion = new completion_completion($cc);
108+
$completion = $ccompletion->mark_inprogress($time);
109+
110+
$DB->insert_records('course_modules_completion', [$cmcompletionrecord]);
111+
$DB->insert_records('course_completion_crit_compl', [$usercompletion]);
112+
113+
// MDL-33320: for instant completions we need aggregate to work in a single run.
114+
$DB->set_field('course_completions', 'reaggregate', $time - 2);
115+
116+
$result = $DB->get_record('course_completions', ['userid' => $fromstudent->id]);
117+
$this->assertIsObject($result);
118+
$result = $DB->get_record('course_completions', ['userid' => $tostudent->id]);
119+
$this->assertFalse($result);
120+
aggregate_completions(0);
121+
122+
123+
$mut = new user_merger();
124+
// This merge already invokes the callback for reaggregate course completion.
125+
[$success, $logs, $logid] = $mut->merge($tostudent->id, $fromstudent->id);
126+
$this->assertTrue($success);
127+
// Check that there is reaggregation of course completion.
128+
$found = '';
129+
foreach ($logs as $logline) {
130+
$found = strstr($logline, 'Course completion reaggregated for user');
131+
if (!empty($found)) {
132+
break;
133+
}
134+
}
135+
$this->assertNotEmpty($found);
136+
137+
$this->assertFalse($DB->get_record('course_completions', ['userid' => $fromstudent->id]));
138+
$record = $DB->get_record('course_completions', ['userid' => $tostudent->id]);
139+
$this->assertIsObject($record);
140+
$this->assertEquals(0, $record->reaggregate);
141+
}
142+
}

0 commit comments

Comments
 (0)