Skip to content

Commit 17514af

Browse files
TamaroWalterTamaroWalter
committed
phpmd cleaning
1 parent 7420b34 commit 17514af

File tree

12 files changed

+95
-158
lines changed

12 files changed

+95
-158
lines changed

‎classes/manager/mail_manager.php

Lines changed: 56 additions & 76 deletions
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, $PAGE;
68+
global $CFG, $DB, $PAGE;
6969

7070
// Get the course object of the top level site.
7171
$site = get_site();
@@ -87,8 +87,7 @@ public static function moodleoverflow_send_mails(): bool {
8787
$courses = [];
8888
$coursemodules = [];
8989

90-
// Posts older than x days will not be mailed.
91-
// 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.
9291
$timenow = time();
9392
$endtime = $timenow - get_config('moodleoverflow', 'maxeditingtime');
9493
$starttime = $endtime - (get_config('moodleoverflow', 'maxmailingtime') * 60 * 60);
@@ -101,7 +100,6 @@ public static function moodleoverflow_send_mails(): bool {
101100
mtrace('Errors occurred while trying to mark some posts as being mailed.');
102101
return false;
103102
}
104-
105103
// Loop through all posts to be mailed.
106104
foreach ($posts as $postid => $post) {
107105
self::check_post($post, $mailcount, $users, $discussions, $errorcount, $posts, $postid,
@@ -125,7 +123,7 @@ public static function moodleoverflow_send_mails(): bool {
125123
$userto->markposts = [];
126124

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

130128
// Reset the caches.
131129
foreach ($coursemodules as $moodleoverflowid) {
@@ -136,7 +134,6 @@ public static function moodleoverflow_send_mails(): bool {
136134

137135
// Loop through all posts of this users.
138136
foreach ($posts as $post) {
139-
140137
self::send_post($userto, $post, $coursemodules, $errorcount,
141138
$discussions, $moodleoverflows, $courses, $mailcount, $users, $site, $textout, $htmlout);
142139
}
@@ -147,18 +144,13 @@ public static function moodleoverflow_send_mails(): bool {
147144
}
148145

149146
// Check for all posts whether errors occurred.
150-
if ($posts) {
151-
152-
// Loop through all posts.
153-
foreach ($posts as $post) {
154-
155-
// Tracing information.
156-
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");
157150

158-
// Mark the posts with errors in the database.
159-
if ($errorcount[$post->id]) {
160-
$DB->set_field('moodleoverflow_posts', 'mailed', self::MOODLEOVERFLOW_MAILED_ERROR, ['id' => $post->id]);
161-
}
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]);
162154
}
163155
}
164156

@@ -231,7 +223,6 @@ public static function moodleoverflow_mark_old_posts_as_mailed($endtime) {
231223
* @param stdClass $user
232224
*/
233225
public static function moodleoverflow_minimise_user_record(stdClass $user) {
234-
235226
// Remove all information for the mail generation that are not needed.
236227
unset($user->institution);
237228
unset($user->department);
@@ -267,61 +258,31 @@ public static function moodleoverflow_minimise_user_record(stdClass $user) {
267258
private static function check_post($post, array &$mailcount, array &$users, array &$discussions, array &$errorcount,
268259
array &$posts, int $postid, array &$moodleoverflows, array &$courses,
269260
array &$coursemodules) {
270-
global $DB;
271261
// Check the cache if the discussion exists.
272262
$discussionid = $post->discussion;
273-
if (!isset($discussions[$discussionid])) {
274-
// Retrieve the discussion from the database.
275-
$discussion = $DB->get_record('moodleoverflow_discussions', ['id' => $post->discussion]);
276-
277-
// If there is a record, update the cache. Else ignore the post.
278-
if ($discussion) {
279-
$discussions[$discussionid] = $discussion;
280-
subscriptions::fill_subscription_cache($discussion->moodleoverflow);
281-
subscriptions::fill_discussion_subscription_cache($discussion->moodleoverflow);
282-
} else {
283-
mtrace('Could not find discussion ' . $discussionid);
284-
unset($posts[$postid]);
285-
return;
286-
}
263+
if (!self::cache_record('moodleoverflow_discussions', $discussionid, $discussions,
264+
'Could not find discussion ', $posts, $postid, true)) {
265+
return;
287266
}
288267

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

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

319282
// Retrieve the connected course modules from the database.
320283
if (!isset($coursemodules[$moodleoverflowid])) {
321-
322284
// Retrieve the coursemodule and update the cache.
323-
$cm = get_coursemodule_from_instance('moodleoverflow', $moodleoverflowid, $courseid);
324-
if ($cm) {
285+
if ($cm = get_coursemodule_from_instance('moodleoverflow', $moodleoverflowid, $courseid)) {
325286
$coursemodules[$moodleoverflowid] = $cm;
326287
} else {
327288
mtrace('Could not find course module for moodleoverflow ' . $moodleoverflowid);
@@ -332,14 +293,12 @@ private static function check_post($post, array &$mailcount, array &$users, arra
332293

333294
// Cache subscribed users of each moodleoverflow.
334295
if (!isset($subscribedusers[$moodleoverflowid])) {
335-
336296
// Retrieve the context module.
337297
$modulecontext = context_module::instance($coursemodules[$moodleoverflowid]->id);
338298

339299
// Retrieve all subscribed users.
340300
$mid = $moodleoverflows[$moodleoverflowid];
341-
$subusers = subscriptions::get_subscribed_users($mid, $modulecontext, 'u.*', true);
342-
if ($subusers) {
301+
if ($subusers = subscriptions::get_subscribed_users($mid, $modulecontext, 'u.*', true)) {
343302
// Loop through all subscribed users.
344303
foreach ($subusers as $postuser) {
345304
// Save the user into the cache.
@@ -360,6 +319,39 @@ private static function check_post($post, array &$mailcount, array &$users, arra
360319
$errorcount[$postid] = 0;
361320
}
362321

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+
363355
/**
364356
* Send the Mail with information of the post depending on theinformation available.
365357
* E.g. anonymous post do not include names, users who want resumes do not get single mails.
@@ -427,9 +419,7 @@ private static function send_post($userto, $post, array &$coursemodules, array &
427419

428420
// Check whether the user is subscribed to the discussion.
429421
$uid = $userto->id;
430-
$did = $post->discussion;
431-
$issubscribed = subscriptions::is_subscribed($uid, $moodleoverflow, $modulecontext, $did);
432-
if (!$issubscribed) {
422+
if (!subscriptions::is_subscribed($uid, $moodleoverflow, $modulecontext, $post->discussion)) {
433423
return;
434424
}
435425

@@ -462,7 +452,7 @@ private static function send_post($userto, $post, array &$coursemodules, array &
462452
}
463453

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

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

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

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

556537
// Check whether the post is a reply.
557538
if ($post->parent) {
558-
559539
// Add a reply header.
560540
$parentid = generate_email_messageid(hash('sha256', $post->parent . 'to' . $userto->id));
561541
$userfrom->customheaders[] = "In-Reply-To: $parentid";

‎classes/ratings.php

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -281,9 +281,7 @@ public static function moodleoverflow_get_rating($postid) {
281281
global $DB;
282282

283283
// Retrieve the full post.
284-
if (!$post = $DB->get_record('moodleoverflow_posts', ['id' => $postid])) {
285-
throw new moodle_exception('postnotexist', 'moodleoverflow');
286-
}
284+
$post = moodleoverflow_get_record_or_exception('moodleoverflow_posts', ['id' => $postid], 'postnotexist');
287285

288286
// Get the rating for this single post.
289287
return self::moodleoverflow_get_ratings_by_discussion($post->discussion, $postid);

‎classes/readtracking.php

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -90,13 +90,8 @@ public static function moodleoverflow_is_tracked($moodleoverflow, $user = null)
9090
$user = $USER;
9191
}
9292

93-
// Guests cannot track a moodleoverflow.
94-
if (isguestuser($USER) || empty($USER->id)) {
95-
return false;
96-
}
97-
98-
// Check if the moodleoverflow can be generally tracked.
99-
if (!self::moodleoverflow_can_track_moodleoverflows($moodleoverflow)) {
93+
// Guests cannot track a moodleoverflow. The moodleoverflow should be generally trackable.
94+
if (isguestuser($USER) || empty($USER->id) || !self::moodleoverflow_can_track_moodleoverflows($moodleoverflow)) {
10095
return false;
10196
}
10297

@@ -135,11 +130,9 @@ public static function moodleoverflow_mark_moodleoverflow_read($cm, $userid = nu
135130

136131
// Iterate through all of this discussions.
137132
foreach ($discussions as $discussionid) {
138-
139133
// Mark the discussion as read.
140-
if (!self::moodleoverflow_mark_discussion_read($discussionid, context_module::instance($cm->id), $userid)) {
141-
throw new moodle_exception('markreadfailed', 'moodleoverflow');
142-
}
134+
$markedcheck = self::moodleoverflow_mark_discussion_read($discussionid, context_module::instance($cm->id), $userid);
135+
moodleoverflow_throw_exception_with_check($markedcheck !== true, 'markreadfailed');
143136
}
144137

145138
return true;
@@ -172,11 +165,8 @@ public static function moodleoverflow_mark_discussion_read($discussionid, $modco
172165
}
173166

174167
// Mark the post as read.
175-
if (!self::moodleoverflow_mark_post_read($userid, $post)) {
176-
throw new moodle_exception('markreadfailed', 'moodleoverflow');
177-
178-
return false;
179-
}
168+
$postreadcheck = self::moodleoverflow_mark_post_read($userid, $post);
169+
moodleoverflow_throw_exception_with_check(!$postreadcheck, 'markreadfailed');
180170
}
181171

182172
// The discussion has been marked as read.

‎classes/task/send_mails.php

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -89,13 +89,10 @@ public function send_review_notifications() {
8989
}
9090

9191
$course = null;
92-
9392
$moodleoverflow = null;
9493
$usersto = null;
9594
$cm = null;
96-
9795
$discussion = null;
98-
9996
$success = [];
10097

10198
foreach ($postinfos as $postinfo) {
@@ -135,16 +132,8 @@ public function send_review_notifications() {
135132
cron_setup_user($userto, $course);
136133
}
137134

138-
$maildata = new moodleoverflow_email(
139-
$course,
140-
$cm,
141-
$moodleoverflow,
142-
$discussion,
143-
$post,
144-
$userfrom,
145-
$userto,
146-
false
147-
);
135+
$maildata = new moodleoverflow_email($course, $cm, $moodleoverflow, $discussion,
136+
$post, $userfrom, $userto, false);
148137

149138
$textcontext = $maildata->export_for_template($renderertext, true);
150139
$htmlcontext = $maildata->export_for_template($rendererhtml, false);

‎discussion.php

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
*/
2424
defined('MOODLE_INTERNAL') || die();
2525

26-
global $CFG, $PAGE, $USER, $SESSION, $OUTPUT;
26+
global $CFG, $DB, $PAGE, $USER, $SESSION, $OUTPUT;
2727

2828
// Include config and locallib.
2929
require_once('../../config.php');
@@ -42,19 +42,14 @@
4242
$PAGE->add_body_class('limitedwidth');
4343

4444
// Check if the discussion is valid.
45-
if (!$discussion = $DB->get_record('moodleoverflow_discussions', ['id' => $d])) {
46-
throw new moodle_exception('invaliddiscussionid', 'moodleoverflow');
47-
}
45+
$discussion = moodleoverflow_get_record_or_exception('moodleoverflow_discussions', ['id' => $d], 'invaliddiscussionid');
4846

4947
// Check if the related moodleoverflow instance is valid.
50-
if (!$moodleoverflow = $DB->get_record('moodleoverflow', ['id' => $discussion->moodleoverflow])) {
51-
throw new moodle_exception('invalidmoodleoverflowid', 'moodleoverflow');
52-
}
48+
$moodleoverflow = moodleoverflow_get_record_or_exception('moodleoverflow', ['id' => $discussion->moodleoverflow],
49+
'invalidmoodleoverflowid');
5350

5451
// Check if the related moodleoverflow instance is valid.
55-
if (!$course = $DB->get_record('course', ['id' => $discussion->course])) {
56-
throw new moodle_exception('invalidcourseid');
57-
}
52+
$course = moodleoverflow_get_record_or_exception('course', ['id' => $discussion->course], 'invalidcourseid', '*', true);
5853

5954
// Save the allowmultiplemarks setting.
6055
$marksetting = $DB->get_record('moodleoverflow', ['id' => $moodleoverflow->id], 'allowmultiplemarks');

‎externallib.php

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -88,19 +88,15 @@ public static function record_vote($postid, $ratingid) {
8888
$post = $DB->get_record('moodleoverflow_posts', ['id' => $params['postid']], '*', MUST_EXIST);
8989

9090
// Check if the discussion is valid.
91-
if (!$discussion = $DB->get_record('moodleoverflow_discussions', ['id' => $post->discussion])) {
92-
throw new moodle_exception('invaliddiscussionid', 'moodleoverflow');
93-
}
91+
$discussion = moodleoverflow_get_record_or_exception('moodleoverflow_discussions', ['id' => $post->discussion],
92+
'invaliddiscussionid');
9493

9594
// Check if the related moodleoverflow instance is valid.
96-
if (!$moodleoverflow = $DB->get_record('moodleoverflow', ['id' => $discussion->moodleoverflow])) {
97-
throw new moodle_exception('invalidmoodleoverflowid', 'moodleoverflow');
98-
}
95+
$moodleoverflow = moodleoverflow_get_record_or_exception('moodleoverflow', ['id' => $discussion->moodleoverflow],
96+
'invalidmoodleoverflowid');
9997

10098
// Check if the related moodleoverflow instance is valid.
101-
if (!$course = $DB->get_record('course', ['id' => $discussion->course])) {
102-
throw new moodle_exception('invalidcourseid');
103-
}
99+
$course = moodleoverflow_get_record_or_exception('course', ['id' => $discussion->course], 'invalidcourseid', '*', true);
104100

105101
// Get the related coursemodule and its context.
106102
if (!$cm = get_coursemodule_from_instance('moodleoverflow', $moodleoverflow->id, $course->id)) {

0 commit comments

Comments
 (0)