Skip to content

Commit f7affcb

Browse files
ralferlebachralferlebach
authored
Merge pull request #506 from Wunderbyte-GmbH/ralferlebach-fix-495-496-497-498-500-502
Ralferlebach fix 495 496 497 498 500 502
2 parents df278af + 0297e85 commit f7affcb

28 files changed

Lines changed: 2273 additions & 79 deletions

‎.gitattributes‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# Enforce LF line endings for all text files.
2+
* text=auto eol=lf
3+
4+
# Binary files — no conversion.
5+
*.png binary
6+
*.jpg binary
7+
*.gif binary
8+
*.ico binary
9+
*.woff binary
10+
*.woff2 binary
11+
*.ttf binary
12+
*.eot binary
13+
*.min.js binary
14+
*.min.css binary
15+
16+
# Exclude developer artefacts from release archives.
17+
.github/ export-ignore
18+
docs/ export-ignore
19+
tools/ export-ignore
20+
makefile export-ignore
21+
CHANGELOG.md export-ignore
22+
.gitattributes export-ignore
23+
.gitignore export-ignore
24+
.phpcsignore export-ignore
25+
.phpcs.xml export-ignore

‎.gitignore‎

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,53 @@
1-
/vue3/node_modules/
2-
/vue3/coverage
1+
# OS artefacts
32
.DS_Store
3+
Thumbs.db
4+
desktop.ini
5+
*Zone.Identifier
46
._*
7+
8+
# IDE
9+
.idea/
10+
.vscode/
11+
*.iml
12+
*.iws
13+
14+
# Node
15+
node_modules/
16+
npm-debug.log*
17+
yarn-error.log*
18+
package-lock.json
19+
yarn.lock
20+
21+
# vue
22+
/vue3/node_modules/
23+
/vue3/coverage
24+
25+
# PHP / Composer
26+
vendor/
27+
composer.lock
28+
29+
# AMD build artefacts (keep src, ignore build output)
30+
amd/build/
31+
32+
# moodle-plugin-ci working directories
33+
ci/
34+
moodle/
35+
36+
# PHPUnit / coverage
37+
.phpunit.result.cache
38+
.phpunit.cache/
39+
coverage/
40+
*.clover
41+
42+
# Behat screenshots
43+
behatfaildumps/
44+
45+
# Temporary / backup files
46+
*.bak
47+
*.tmp
48+
*.swp
49+
*~
50+
51+
# Local developer overrides
52+
.env
53+
.env.local

‎classes/completion.php‎

Lines changed: 67 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,10 @@
2626
declare(strict_types=1);
2727

2828
namespace local_adele;
29+
30+
use core\event\course_completed;
2931
use local_adele\helper\user_path_relation;
30-
use context_system;
31-
use local_adele\event\user_path_updated;
32+
use local_adele\learning_path_update;
3233

3334
/**
3435
* External Service for local adele.
@@ -40,35 +41,77 @@
4041
*/
4142
class completion {
4243
/**
43-
* Observer for course completed
44+
* Observer for the core course_completed event.
45+
*
46+
* When a Moodle course is completed, every active learning path of the
47+
* affected student that references this course in one of its nodes has to be
48+
* re-evaluated so that the corresponding node-completion criterion can turn
49+
* the node into "completed".
4450
*
45-
* @param object $event
51+
* Important: on \core\event\course_completed the affected student is carried
52+
* in $event->relateduserid. $event->userid is the ACTOR that caused the
53+
* completion (a grading teacher, an administrator or - most commonly - the
54+
* cron/system process aggregating the completion criteria) and must NOT be
55+
* used to look up the learning path owner.
56+
*
57+
* @param course_completed $event The Moodle course_completed event.
58+
* @return void
4659
*/
47-
public static function completed($event) {
48-
// The course_completed event carries the actor in userid and the affected student
49-
// in relateduserid. Route by the student, otherwise a teacher- or cron-triggered
50-
// completion recomputes the wrong user's paths (or none) and the node never
51-
// completes (#495).
60+
public static function completed(course_completed $event): void {
61+
// The student whose course was completed is represented by relateduser, not userid.
62+
$userid = (int) $event->relateduserid;
63+
$courseid = (int) $event->courseid;
64+
65+
// Nothing sensible to do without both a student and a course.
66+
if ($userid <= 0 || $courseid <= 0) {
67+
return;
68+
}
69+
5270
$userpathrelation = new user_path_relation();
53-
$learningpaths = $userpathrelation->get_learning_paths($event->relateduserid);
54-
if (!$learningpaths) {
71+
$learningpaths = $userpathrelation->get_learning_paths($userid);
72+
73+
if (empty($learningpaths)) {
5574
return;
5675
}
76+
5777
foreach ($learningpaths as $learningpath) {
58-
$learningpath->json = json_decode($learningpath->json, true);
59-
foreach ($learningpath->json['tree']['nodes'] as $node) {
60-
if (is_array($node['data']['course_node_id']) && in_array($event->courseid, $node['data']['course_node_id'])) {
61-
// Recompute the path once per event, even if the course maps to several nodes.
62-
$eventsingle = user_path_updated::create([
63-
'objectid' => $learningpath->id,
64-
'context' => context_system::instance(),
65-
'other' => [
66-
'userpath' => $learningpath,
67-
],
68-
]);
69-
$eventsingle->trigger();
70-
break;
78+
$pathjson = json_decode($learningpath->json, true);
79+
80+
// A single malformed or incomplete snapshot must not abort the
81+
// processing of the remaining (valid) learning paths.
82+
if (
83+
!is_array($pathjson) ||
84+
empty($pathjson['tree']['nodes']) ||
85+
!is_array($pathjson['tree']['nodes'])
86+
) {
87+
continue;
88+
}
89+
90+
foreach ($pathjson['tree']['nodes'] as $node) {
91+
$courseids = $node['data']['course_node_id'] ?? [];
92+
93+
if (!is_array($courseids)) {
94+
$courseids = [$courseids];
7195
}
96+
97+
// The stored course_node_id values may be strings or integers
98+
// depending on the JSON origin - normalise both sides for a
99+
// type-safe match.
100+
$courseids = array_map('intval', $courseids);
101+
102+
if (!in_array($courseid, $courseids, true)) {
103+
continue;
104+
}
105+
106+
// Hand the decoded snapshot to the central update service, which
107+
// re-evaluates every node of the path against the current state.
108+
$learningpath->json = $pathjson;
109+
learning_path_update::trigger_user_path_update($learningpath);
110+
111+
// The recompute already covers all nodes of this path, so a
112+
// single trigger per learning path is sufficient even if the
113+
// course is referenced by more than one node.
114+
break;
72115
}
73116
}
74117
}

‎classes/course_completion/conditions/course_completed.php‎

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,16 +163,27 @@ public function get_completion_status($node, $userid) {
163163
if ($course->enablecompletion) {
164164
// Get the course completion instance.
165165
$completion = new completion_info($course);
166-
$progress = progress::get_course_progress_percentage($course, $userid) ?? 0;
167-
if ($progress !== null) {
166+
$progress = progress::get_course_progress_percentage($course, $userid);
167+
// Ticket #502: "in progress" starts at the first course access
168+
// (erster Kursaufruf), not at mere enrolment. A learner who is
169+
// only enrolled but has never opened the course is NOT started.
170+
// The timed processing-duration restriction is unaffected; it
171+
// still keys off the node's first unlock.
172+
if ($this->has_accessed_course((int) $course->id, (int) $userid)) {
168173
$isinbetween = true;
169174
}
175+
$progress ??= 0;
170176
// Check if the user has completed the course.
171177
$coursecompleted = $completion->is_course_complete($userid);
172178
if ($coursecompleted) {
173179
$progress = 100;
174180
$completed = true;
175181
$finished++;
182+
// A completed course cannot have stayed untouched, so it also
183+
// counts as started. This keeps partially completed
184+
// multi-course nodes (some done, not enough yet) in the
185+
// inbetween state.
186+
$isinbetween = true;
176187
}
177188
$progresses[] = $progress;
178189
// Ticket #464 H4: this list is rendered via v-html in the node feedback, so the
@@ -248,6 +259,23 @@ public function get_completion_status($node, $userid) {
248259
return $coursecompletion;
249260
}
250261

262+
/**
263+
* Whether the user has ever accessed the given course (first course view).
264+
*
265+
* Ticket #502: this is the "started" (Bearbeitungsbeginn) criterion for the
266+
* progress display. Moodle records a row in user_lastaccess the first time a
267+
* user accesses a course, so the presence of that row means the course has
268+
* been opened at least once.
269+
*
270+
* @param int $courseid
271+
* @param int $userid
272+
* @return bool
273+
*/
274+
private function has_accessed_course(int $courseid, int $userid) {
275+
global $DB;
276+
return $DB->record_exists('user_lastaccess', ['courseid' => $courseid, 'userid' => $userid]);
277+
}
278+
251279
/**
252280
* Get the average of the furthest nodes.
253281
* @param array $progresses

‎classes/enrollment.php‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ public static function subscribe_user_to_learning_path($learningpath, $params, $
7070
}
7171
$userpath = self::buildsqlqueryuserpath($learningpath->id, $params->relateduserid, $courseid);
7272
if (!$userpath) {
73-
$id = $DB->insert_record('local_adele_path_user', [
73+
$newrecord = [
7474
'user_id' => $params->relateduserid,
7575
'course_id' => $courseid,
7676
'learning_path_id' => $learningpath->id,
@@ -86,8 +86,22 @@ public static function subscribe_user_to_learning_path($learningpath, $params, $
8686
'tree' => $learningpath->json['tree'] ?? ['nodes' => [], 'edges' => []],
8787
'modules' => $learningpath->json['modules'] ?? null,
8888
]),
89-
]);
90-
$userpath = $DB->get_record('local_adele_path_user', ['id' => $id]);
89+
];
90+
try {
91+
$id = $DB->insert_record('local_adele_path_user', $newrecord);
92+
$userpath = $DB->get_record('local_adele_path_user', ['id' => $id]);
93+
} catch (\dml_exception $e) {
94+
// Ticket #501: a concurrent request won the race and inserted
95+
// the row first; the unique index (user_id, course_id,
96+
// learning_path_id) rejected this insert. Reuse the row that
97+
// now exists instead of failing or creating a duplicate. If no
98+
// such row is found the exception was not a duplicate conflict
99+
// and must propagate.
100+
$userpath = self::buildsqlqueryuserpath($learningpath->id, $params->relateduserid, $courseid);
101+
if (!$userpath) {
102+
throw $e;
103+
}
104+
}
91105
}
92106
$userpath->json = json_decode($userpath->json, true);
93107
$eventsingle = user_path_updated::create([

0 commit comments

Comments
 (0)