Skip to content

Commit c061592

Browse files
TamaroWalterTamaroWalter
authored
Merge pull request #189 from learnweb/ci/codecheck
Codechecker fixes for 4.4
2 parents 5db19e1 + 7aa4e2a commit c061592

25 files changed

+977
-1579
lines changed

‎backup/moodle2/backup_moodleoverflow_stepslib.php

+11-14
Original file line numberDiff line numberDiff line change
@@ -39,41 +39,35 @@ class backup_moodleoverflow_activity_structure_step extends backup_activity_stru
3939
* @return backup_nested_element
4040
*/
4141
protected function define_structure() {
42-
4342
// To know if we are including userinfo.
4443
$userinfo = $this->get_setting_value('userinfo');
4544

4645
// Define the root element describing the moodleoverflow instance.
4746
$moodleoverflow = new backup_nested_element('moodleoverflow', ['id'], [
48-
'name', 'intro', 'introformat', 'maxbytes', 'maxattachments', 'timecreated', 'timemodified',
49-
'forcesubscribe', 'trackingtype', 'ratingpreference', 'coursewidereputation', 'allowrating',
50-
'allowreputation', 'allownegativereputation', 'grademaxgrade', 'gradescalefactor', 'gradecat',
51-
'anonymous', 'allowmultiplemarks', ]);
47+
'name', 'intro', 'introformat', 'maxbytes', 'maxattachments', 'timecreated', 'timemodified', 'forcesubscribe',
48+
'trackingtype', 'ratingpreference', 'coursewidereputation', 'allowrating', 'allowreputation', 'allownegativereputation',
49+
'grademaxgrade', 'gradescalefactor', 'gradecat', 'anonymous', 'allowmultiplemarks', ]);
5250

5351
// Define each element separated.
5452
$discussions = new backup_nested_element('discussions');
5553
$discussion = new backup_nested_element('discussion', ['id'], [
5654
'name', 'firstpost', 'userid', 'timestart', 'timemodified', 'usermodified', ]);
5755

5856
$posts = new backup_nested_element('posts');
59-
$post = new backup_nested_element('post', ['id'], [
60-
'parent', 'userid', 'created', 'modified',
61-
'message', 'messageformat', 'attachment', 'mailed', 'reviewed', 'timereviewed', ]);
57+
$post = new backup_nested_element('post', ['id'], ['parent', 'userid', 'created', 'modified', 'message',
58+
'messageformat', 'attachment', 'mailed', 'reviewed', 'timereviewed', ]);
6259

6360
$ratings = new backup_nested_element('ratings');
64-
$rating = new backup_nested_element('rating', ['id'], [
65-
'userid', 'rating', 'firstrated', 'lastchanged', ]);
61+
$rating = new backup_nested_element('rating', ['id'], ['userid', 'rating', 'firstrated', 'lastchanged']);
6662

6763
$discussionsubs = new backup_nested_element('discuss_subs');
68-
$discussionsub = new backup_nested_element('discuss_sub', ['id'], [
69-
'userid', 'preference', ]);
64+
$discussionsub = new backup_nested_element('discuss_sub', ['id'], ['userid', 'preference']);
7065

7166
$subscriptions = new backup_nested_element('subscriptions');
7267
$subscription = new backup_nested_element('subscription', ['id'], ['userid']);
7368

7469
$readposts = new backup_nested_element('readposts');
75-
$read = new backup_nested_element('read', ['id'], [
76-
'userid', 'discussionid', 'postid', 'firstread', 'lastread', ]);
70+
$read = new backup_nested_element('read', ['id'], ['userid', 'discussionid', 'postid', 'firstread', 'lastread']);
7771

7872
$grades = new backup_nested_element('grades');
7973
$grade = new backup_nested_element('grade', ['id'], ['userid', 'grade']);
@@ -100,6 +94,9 @@ protected function define_structure() {
10094
$moodleoverflow->add_child($readposts);
10195
$readposts->add_child($read);
10296

97+
$moodleoverflow->add_child($grades);
98+
$grades->add_child($grade);
99+
103100
$moodleoverflow->add_child($tracking);
104101
$tracking->add_child($track);
105102

‎classes/manager/mail_manager.php

+57-79
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ class mail_manager {
6565
* @return bool
6666
*/
6767
public static function moodleoverflow_send_mails(): bool {
68-
global $DB, $CFG, $PAGE;
68+
global $CFG, $DB, $PAGE;
6969

7070
// Get the course object of the top level site.
7171
$site = get_site();
@@ -86,10 +86,8 @@ public static function moodleoverflow_send_mails(): bool {
8686
$moodleoverflows = [];
8787
$courses = [];
8888
$coursemodules = [];
89-
$subscribedusers = [];
9089

91-
// Posts older than x days will not be mailed.
92-
// This will avoid problems with the cron not ran for a long time.
90+
// Posts older than x days will not be mailed. This will avoid problems with the cron not ran for a long time.
9391
$timenow = time();
9492
$endtime = $timenow - get_config('moodleoverflow', 'maxeditingtime');
9593
$starttime = $endtime - (get_config('moodleoverflow', 'maxmailingtime') * 60 * 60);
@@ -102,7 +100,6 @@ public static function moodleoverflow_send_mails(): bool {
102100
mtrace('Errors occurred while trying to mark some posts as being mailed.');
103101
return false;
104102
}
105-
106103
// Loop through all posts to be mailed.
107104
foreach ($posts as $postid => $post) {
108105
self::check_post($post, $mailcount, $users, $discussions, $errorcount, $posts, $postid,
@@ -126,18 +123,17 @@ public static function moodleoverflow_send_mails(): bool {
126123
$userto->markposts = [];
127124

128125
// Cache the capabilities of the user.
129-
cron_setup_user($userto);
126+
$CFG->branch >= 402 ? \core\cron::setup_user($userto) : cron_setup_user($userto);
130127

131128
// Reset the caches.
132-
foreach ($coursemodules as $moodleoverflowid => $unused) {
129+
foreach ($coursemodules as $moodleoverflowid) {
133130
$coursemodules[$moodleoverflowid]->cache = new stdClass();
134131
$coursemodules[$moodleoverflowid]->cache->caps = [];
135132
unset($coursemodules[$moodleoverflowid]->uservisible);
136133
}
137134

138135
// Loop through all posts of this users.
139136
foreach ($posts as $post) {
140-
141137
self::send_post($userto, $post, $coursemodules, $errorcount,
142138
$discussions, $moodleoverflows, $courses, $mailcount, $users, $site, $textout, $htmlout);
143139
}
@@ -148,18 +144,13 @@ public static function moodleoverflow_send_mails(): bool {
148144
}
149145

150146
// Check for all posts whether errors occurred.
151-
if ($posts) {
152-
153-
// Loop through all posts.
154-
foreach ($posts as $post) {
155-
156-
// Tracing information.
157-
mtrace($mailcount[$post->id] . " users were sent post $post->id");
147+
foreach ($posts as $post) {
148+
// Tracing information.
149+
mtrace($mailcount[$post->id] . " users were sent post $post->id");
158150

159-
// Mark the posts with errors in the database.
160-
if ($errorcount[$post->id]) {
161-
$DB->set_field('moodleoverflow_posts', 'mailed', self::MOODLEOVERFLOW_MAILED_ERROR, ['id' => $post->id]);
162-
}
151+
// Mark the posts with errors in the database.
152+
if ($errorcount[$post->id]) {
153+
$DB->set_field('moodleoverflow_posts', 'mailed', self::MOODLEOVERFLOW_MAILED_ERROR, ['id' => $post->id]);
163154
}
164155
}
165156

@@ -232,7 +223,6 @@ public static function moodleoverflow_mark_old_posts_as_mailed($endtime) {
232223
* @param stdClass $user
233224
*/
234225
public static function moodleoverflow_minimise_user_record(stdClass $user) {
235-
236226
// Remove all information for the mail generation that are not needed.
237227
unset($user->institution);
238228
unset($user->department);
@@ -268,61 +258,31 @@ public static function moodleoverflow_minimise_user_record(stdClass $user) {
268258
private static function check_post($post, array &$mailcount, array &$users, array &$discussions, array &$errorcount,
269259
array &$posts, int $postid, array &$moodleoverflows, array &$courses,
270260
array &$coursemodules) {
271-
global $DB;
272261
// Check the cache if the discussion exists.
273262
$discussionid = $post->discussion;
274-
if (!isset($discussions[$discussionid])) {
275-
// Retrieve the discussion from the database.
276-
$discussion = $DB->get_record('moodleoverflow_discussions', ['id' => $post->discussion]);
277-
278-
// If there is a record, update the cache. Else ignore the post.
279-
if ($discussion) {
280-
$discussions[$discussionid] = $discussion;
281-
subscriptions::fill_subscription_cache($discussion->moodleoverflow);
282-
subscriptions::fill_discussion_subscription_cache($discussion->moodleoverflow);
283-
} else {
284-
mtrace('Could not find discussion ' . $discussionid);
285-
unset($posts[$postid]);
286-
return;
287-
}
263+
if (!self::cache_record('moodleoverflow_discussions', $discussionid, $discussions,
264+
'Could not find discussion ', $posts, $postid, true)) {
265+
return;
288266
}
289267

290268
// Retrieve the connected moodleoverflow instance from the database.
291269
$moodleoverflowid = $discussions[$discussionid]->moodleoverflow;
292-
if (!isset($moodleoverflows[$moodleoverflowid])) {
293-
294-
// Retrieve the record from the database and update the cache.
295-
$moodleoverflow = $DB->get_record('moodleoverflow', ['id' => $moodleoverflowid]);
296-
if ($moodleoverflow) {
297-
$moodleoverflows[$moodleoverflowid] = $moodleoverflow;
298-
} else {
299-
mtrace('Could not find moodleoverflow ' . $moodleoverflowid);
300-
unset($posts[$postid]);
301-
return;
302-
}
270+
if (!self::cache_record('moodleoverflow', $moodleoverflowid, $moodleoverflows,
271+
'Could not find moodleoverflow ', $posts, $postid, false)) {
272+
return;
303273
}
304274

305275
// Retrieve the connected courses from the database.
306276
$courseid = $moodleoverflows[$moodleoverflowid]->course;
307-
if (!isset($courses[$courseid])) {
308-
309-
// Retrieve the record from the database and update the cache.
310-
$course = $DB->get_record('course', ['id' => $courseid]);
311-
if ($course) {
312-
$courses[$courseid] = $course;
313-
} else {
314-
mtrace('Could not find course ' . $courseid);
315-
unset($posts[$postid]);
316-
return;
317-
}
277+
if (!self::cache_record('course', $courseid, $courses,
278+
'Could not find course ', $posts, $postid, false)) {
279+
return;
318280
}
319281

320282
// Retrieve the connected course modules from the database.
321283
if (!isset($coursemodules[$moodleoverflowid])) {
322-
323284
// Retrieve the coursemodule and update the cache.
324-
$cm = get_coursemodule_from_instance('moodleoverflow', $moodleoverflowid, $courseid);
325-
if ($cm) {
285+
if ($cm = get_coursemodule_from_instance('moodleoverflow', $moodleoverflowid, $courseid)) {
326286
$coursemodules[$moodleoverflowid] = $cm;
327287
} else {
328288
mtrace('Could not find course module for moodleoverflow ' . $moodleoverflowid);
@@ -333,14 +293,12 @@ private static function check_post($post, array &$mailcount, array &$users, arra
333293

334294
// Cache subscribed users of each moodleoverflow.
335295
if (!isset($subscribedusers[$moodleoverflowid])) {
336-
337296
// Retrieve the context module.
338297
$modulecontext = context_module::instance($coursemodules[$moodleoverflowid]->id);
339298

340299
// Retrieve all subscribed users.
341300
$mid = $moodleoverflows[$moodleoverflowid];
342-
$subusers = subscriptions::get_subscribed_users($mid, $modulecontext, 'u.*', true);
343-
if ($subusers) {
301+
if ($subusers = subscriptions::get_subscribed_users($mid, $modulecontext, 'u.*', true)) {
344302
// Loop through all subscribed users.
345303
foreach ($subusers as $postuser) {
346304
// Save the user into the cache.
@@ -361,6 +319,39 @@ private static function check_post($post, array &$mailcount, array &$users, arra
361319
$errorcount[$postid] = 0;
362320
}
363321

322+
/**
323+
* Helper function for check_post(). Caches the a record exists in the database and caches the record if needed.
324+
* @param string $table
325+
* @param int $id
326+
* @param array $cache
327+
* @param string $errorMessage
328+
* @param array $posts
329+
* @param int $postid
330+
* @param bool $fillsubscache If the subscription cache is being filled (only when checking discussion cache)
331+
* @return bool
332+
* @throws \dml_exception
333+
*/
334+
private static function cache_record($table, $id, &$cache, $errormessage, &$posts, $postid, $fillsubscache) {
335+
global $DB;
336+
// Check if cache if an record exists already in the cache.
337+
if (!isset($cache[$id])) {
338+
// If there is a record in the database, update the cache. Else ignore the post.
339+
if ($record = $DB->get_record($table, ['id' => $id])) {
340+
$cache[$id] = $record;
341+
if ($fillsubscache) {
342+
subscriptions::fill_subscription_cache($record->moodleoverflow);
343+
subscriptions::fill_discussion_subscription_cache($record->moodleoverflow);
344+
}
345+
} else {
346+
mtrace($errormessage . $id);
347+
unset($posts[$postid]);
348+
return false;
349+
}
350+
}
351+
return true;
352+
}
353+
354+
364355
/**
365356
* Send the Mail with information of the post depending on theinformation available.
366357
* E.g. anonymous post do not include names, users who want resumes do not get single mails.
@@ -427,11 +418,8 @@ private static function send_post($userto, $post, array &$coursemodules, array &
427418
}
428419

429420
// Check whether the user is subscribed to the discussion.
430-
$iscm = $coursemodules[$moodleoverflow->id];
431421
$uid = $userto->id;
432-
$did = $post->discussion;
433-
$issubscribed = subscriptions::is_subscribed($uid, $moodleoverflow, $modulecontext, $did);
434-
if (!$issubscribed) {
422+
if (!subscriptions::is_subscribed($uid, $moodleoverflow, $modulecontext, $post->discussion)) {
435423
return;
436424
}
437425

@@ -464,7 +452,7 @@ private static function send_post($userto, $post, array &$coursemodules, array &
464452
}
465453

466454
// Setup roles and languages.
467-
cron_setup_user($userto, $course);
455+
$CFG->branch >= 402 ? \core\cron::setup_user($userto, $course) : cron_setup_user($userto, $course);
468456

469457
// Cache the users capability to view full names.
470458
if (!isset($userto->viewfullnames[$moodleoverflow->id])) {
@@ -522,16 +510,7 @@ private static function send_post($userto, $post, array &$coursemodules, array &
522510
}
523511

524512
// Format the data.
525-
$data = new moodleoverflow_email(
526-
$course,
527-
$cm,
528-
$moodleoverflow,
529-
$discussion,
530-
$post,
531-
$userfrom,
532-
$userto,
533-
$canreply
534-
);
513+
$data = new moodleoverflow_email($course, $cm, $moodleoverflow, $discussion, $post, $userfrom, $userto, $canreply);
535514

536515
// Retrieve the unsubscribe-link.
537516
$userfrom->customheaders[] = sprintf('List-Unsubscribe: <%s>', $data->get_unsubscribediscussionlink());
@@ -557,7 +536,6 @@ private static function send_post($userto, $post, array &$coursemodules, array &
557536

558537
// Check whether the post is a reply.
559538
if ($post->parent) {
560-
561539
// Add a reply header.
562540
$parentid = generate_email_messageid(hash('sha256', $post->parent . 'to' . $userto->id));
563541
$userfrom->customheaders[] = "In-Reply-To: $parentid";

0 commit comments

Comments
 (0)