Skip to content

Commit 80fbb0e

Browse files
committed
solved pull request issues, cleaning of code checker warnings
1 parent 83de9e8 commit 80fbb0e

File tree

7 files changed

+52
-45
lines changed

7 files changed

+52
-45
lines changed

classes/task/send_daily_mail.php

+7-7
Original file line numberDiff line numberDiff line change
@@ -52,39 +52,39 @@ public function execute() {
5252
}
5353
// Go through each user that has unread posts.
5454
foreach ($users as $user) {
55-
$userdata = $DB->get_records('moodleoverflow_mail_info', array('userid' => $user->userid), 'courseid, forumid'); // order by courseid
55+
// Sorts the records with "Order by courseid".
56+
$userdata = $DB->get_records('moodleoverflow_mail_info', array('userid' => $user->userid), 'courseid, forumid');
5657
$mail = array();
57-
// fill the $mail array.
58+
// Fill the $mail array.
5859
foreach ($userdata as $row) {
5960
$currentcourse = $DB->get_record('course', array('id' => $row->courseid), 'fullname, id');
6061
$currentforum = $DB->get_record('moodleoverflow', array('id' => $row->forumid), 'name, id');
6162
$coursemoduleid = get_coursemodule_from_instance('moodleoverflow', $row->forumid);
6263
$discussion = $DB->get_record('moodleoverflow_discussions', array('id' => $row->forumdiscussionid), 'name, id');
6364
$unreadposts = $row->numberofposts;
6465

65-
// build url to the course, forum, and discussion.
66+
// Build url to the course, forum, and discussion.
6667
$linktocourse = new \moodle_url('/course/view.php', array('id' => $currentcourse->id));
6768
$linktoforum = new \moodle_url('/mod/moodleoverflow/view.php', array('id' => $coursemoduleid->id));
6869
$linktodiscussion = new \moodle_url('/mod/moodleoverflow/discussion.php', array('d' => $discussion->id));
6970

70-
// now change the url to a clickable html link.
71+
// Now change the url to a clickable html link.
7172
$linktocourse = \html_writer::link($linktocourse->out(), $currentcourse->fullname);
7273
$linktoforum = \html_writer::link($linktoforum->out(), $currentforum->name);
7374
$linktodiscussion = \html_writer::link($linktodiscussion->out(), $discussion->name);
7475

75-
// build a single line string with the digest information and add it to the mailarray.
76+
// Build a single line string with the digest information and add it to the mailarray.
7677
$string = get_string('digestunreadpost', 'mod_moodleoverflow', array('linktocourse' => $linktocourse,
7778
'linktoforum' => $linktoforum,
7879
'linktodiscussion' => $linktodiscussion,
7980
'unreadposts' => $unreadposts));
8081
array_push($mail, $string);
8182
}
82-
// build the final message and send it to user. Then remove the sent records.
83+
// Build the final message and send it to user. Then remove the sent records.
8384
$message = implode('<br>', $mail);
8485
$userto = $DB->get_record('user', array('id' => $user->userid));
8586
$from = \core_user::get_noreply_user();
8687
$subject = get_string('tasksenddailymail', 'mod_moodleoverflow');
87-
mtrace($message);
8888
email_to_user($userto, $from, $subject, $message);
8989
$DB->delete_records('moodleoverflow_mail_info', array('userid' => $user->userid));
9090
}

db/upgrade.php

+2-1
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,8 @@ function xmldb_moodleoverflow_upgrade($oldversion) {
262262
$table->add_key('userid', XMLDB_KEY_FOREIGN, array('userid'), 'user', array('id'));
263263
$table->add_key('courseid', XMLDB_KEY_FOREIGN, array('courseid'), 'course', array('id'));
264264
$table->add_key('forumid', XMLDB_KEY_FOREIGN, array('forumid'), 'moodleoverflow', array('id'));
265-
$table->add_key('forumdiscussionid', XMLDB_KEY_FOREIGN, array('forumdiscussionid'), 'moodleoverflow_discussions', array('id'));
265+
$table->add_key('forumdiscussionid', XMLDB_KEY_FOREIGN,
266+
array('forumdiscussionid'), 'moodleoverflow_discussions', array('id'));
266267

267268
// Conditionally launch create table for moodleoverflow_mail_info.
268269
if (!$dbman->table_exists($table)) {

lib.php

+6-6
Original file line numberDiff line numberDiff line change
@@ -743,7 +743,6 @@ function moodleoverflow_send_mails() {
743743

744744
// Tracing information.
745745
mtrace('Processing user ' . $userto->id);
746-
mtrace('Mail setting of user: ' . $userto->maildigest);
747746
// Initiate the user caches to save memory.
748747
$userto = clone($userto);
749748
$userto->ciewfullnames = array();
@@ -779,11 +778,12 @@ function moodleoverflow_send_mails() {
779778
$dataobject->courseid = $course->id;
780779
$dataobject->forumid = $moodleoverflow->id;
781780
$dataobject->forumdiscussionid = $discussion->id;
782-
$record = $DB->get_record('moodleoverflow_mail_info', array('userid' => $dataobject->userid,
783-
'courseid' => $dataobject->courseid,
784-
'forumid' => $dataobject->forumid,
785-
'forumdiscussionid' => $dataobject->forumdiscussionid),
786-
'numberofposts, id');
781+
$record = $DB->get_record('moodleoverflow_mail_info',
782+
array('userid' => $dataobject->userid,
783+
'courseid' => $dataobject->courseid,
784+
'forumid' => $dataobject->forumid,
785+
'forumdiscussionid' => $dataobject->forumdiscussionid),
786+
'numberofposts, id');
787787
if (is_object($record)) {
788788
$dataset = $record;
789789
$dataobject->numberofposts = $dataset->numberofposts + 1;

locallib.php

+9-7
Original file line numberDiff line numberDiff line change
@@ -178,15 +178,16 @@ function moodleoverflow_print_latest_discussions($moodleoverflow, $cm, $page = -
178178
$markallread = null;
179179
}
180180

181-
// Check wether the user can move a topic
181+
// Check wether the user can move a topic.
182182
$canmovetopic = false;
183183
if ((!is_guest($context, $USER) && isloggedin()) && has_capability('mod/moodleoverflow:movetopic', $context)) {
184184
$canmovetopic = true;
185185
}
186186

187187
// Check whether the user can subscribe to the discussion.
188188
$cansubtodiscussion = false;
189-
if ((!is_guest($context, $USER) && isloggedin()) && has_capability('mod/moodleoverflow:viewdiscussion', $context)) {
189+
if ((!is_guest($context, $USER) && isloggedin()) && has_capability('mod/moodleoverflow:viewdiscussion', $context)
190+
&& \mod_moodleoverflow\subscriptions::is_subscribable($moodleoverflow, $context)) {
190191
$cansubtodiscussion = true;
191192
}
192193

@@ -352,7 +353,7 @@ function moodleoverflow_print_latest_discussions($moodleoverflow, $cm, $page = -
352353
], 'p' . $reviewinfo->first))->out(false);
353354
}
354355

355-
// build linktopopup to move a topic
356+
// Build linktopopup to move a topic.
356357
$linktopopup = $CFG->wwwroot . '/mod/moodleoverflow/view.php?id=' . $cm->id . '&movetopopup=' . $discussion->discussion;
357358
$preparedarray[$i]['linktopopup'] = $linktopopup;
358359

@@ -380,7 +381,7 @@ function moodleoverflow_print_latest_discussions($moodleoverflow, $cm, $page = -
380381
$mustachedata->markallread = $markallread;
381382
$mustachedata->cansubtodiscussion = $cansubtodiscussion;
382383
$mustachedata->canmovetopic = $canmovetopic;
383-
384+
$mustachedata->cannormoveorsub = ((!$canmovetopic) && (!$cansubtodiscussion));
384385
// Print the template.
385386
echo $renderer->render_discussion_list($mustachedata);
386387

@@ -403,14 +404,15 @@ function moodleoverflow_print_forum_list($course, $cm, $movetopopup) {
403404
$amountforums = count($forums);
404405

405406
if ($amountforums > 1) {
406-
// write the moodleoverflow-names in an array.
407+
// Write the moodleoverflow-names in an array.
407408
$i = 0;
408409
foreach ($forums as $forum) {
409410
if ($forum->id == $currentforum->moodleoverflow) {
410411
continue;
411412
} else {
412413
$forumarray[$i]['name'] = $forum->name;
413-
$movetoforum = $CFG->wwwroot . '/mod/moodleoverflow/view.php?id=' . $cm->id . '&movetopopup=' . $movetopopup . '&movetoforum=' . $forum->id;
414+
$movetoforum = $CFG->wwwroot . '/mod/moodleoverflow/view.php?id=' . $cm->id . '&movetopopup='
415+
. $movetopopup . '&movetoforum=' . $forum->id;
414416
$forumarray[$i]['movetoforum'] = $movetoforum;
415417
}
416418
$i++;
@@ -420,7 +422,7 @@ function moodleoverflow_print_forum_list($course, $cm, $movetopopup) {
420422
$amountforums = false;
421423
}
422424

423-
// build popup
425+
// Build popup.
424426
$renderer = $PAGE->get_renderer('mod_moodleoverflow');
425427
$mustachedata = new stdClass();
426428
$mustachedata->hasforums = $amountforums;

templates/discussion_list.mustache

+10-9
Original file line numberDiff line numberDiff line change
@@ -203,15 +203,16 @@
203203
<a href="{{ lastpostlink }}">{{ lastpostdate }}</a>
204204
</td>
205205

206-
207-
<td class="discussionsubscription">
208-
{{#cansubtodiscussion}}
209-
{{{discussionsubicon}}} <br>
210-
{{/cansubtodiscussion}}
211-
{{#canmovetopic}}
212-
<a href='{{ linktopopup }}'>{{#pix}} i/arrow-right, core, {{#str}}movetopicicon, moodleoverflow{{/str}} {{/pix}}</a>
213-
{{/canmovetopic}}
214-
</td>
206+
{{^cannormoveorsub}}
207+
<td class="discussionsubscription">
208+
{{#cansubtodiscussion}}
209+
{{{discussionsubicon}}} <br/>
210+
{{/cansubtodiscussion}}
211+
{{#canmovetopic}}
212+
<a href='{{ linktopopup }}'>{{#pix}} i/arrow-right, core, {{#str}}movetopicicon, moodleoverflow{{/str}} {{/pix}}</a>
213+
{{/canmovetopic}}
214+
</td>
215+
{{/cannormoveorsub}}
215216

216217

217218

tests/dailymail_test.php

+17-14
Original file line numberDiff line numberDiff line change
@@ -130,42 +130,45 @@ private function helper_run_send_mails() {
130130
public function test_mail_delivery() {
131131
global $DB;
132132

133-
// Create user with maildigest = on
133+
// Create user with maildigest = on.
134134
$this->helper_create_user_and_discussion('1');
135135

136136
// Send a mail and test if the mail was sent.
137-
138-
$this->helper_run_send_mails(); // content2
139-
$this->helper_run_send_daily_mail(); // content
137+
$this->helper_run_send_mails();
138+
$this->helper_run_send_daily_mail();
140139
$messages = $this->sink->count();
141140

142141
$this->assertEquals(1, $messages);
143142
}
144143

145144

146145
/**
147-
* Test if the content of the mail matches the supposed content
146+
* Test if the content of the mail matches the supposed content.
148147
*/
149148
public function test_content_of_mail_delivery() {
150149
global $DB;
151150

152-
// Creat user with maildigest = on.
151+
// Create user with maildigest = on.
153152
$this->helper_create_user_and_discussion('1');
154153

155-
// send the mails and count the messages.
154+
// Send the mails and count the messages.
156155
$this->helper_run_send_mails();
157156
$content = $this->helper_run_send_daily_mail();
158157
$content = str_replace(["\n\r", "\n", "\r"], '', $content);
159158
$messages = $this->sink->count();
160159

161160
// Build the text that the mail should have.
162-
// Text structure: $string['digestunreadpost'] = 'Course: {$a->linktocourse}-> {$a->linktoforum}, Topic: {$a->linktodiscussion} has {$a->unreadposts} unread posts.';.
163-
$linktocourse = '<a href="https://www.example.com/moodle/course/view.php?id=' . $this->course->id . '">' . $this->course->fullname . '</a>';
164-
$linktoforum = '<a href="https://www.example.com/moodle/mod/moodleoverflow/view.php?id=' . $this->coursemodule->id . '">' . $this->moodleoverflow->name . '</a>';
165-
$linktodiscussion = '<a href="https://www.example.com/moodle/mod/moodleoverflow/discussion.php?d=' . $this->discussion[0]->id . '">' . $this->discussion[0]->name . '</a>';
166-
167-
// assemble text
168-
$text = 'Course: ' . $linktocourse . ' -> ' . $linktoforum . ', Topic: ' . $linktodiscussion . ' has ' . $messages . ' unread posts.';
161+
// Text structure at get_string('digestunreadpost', moodleoverflow).
162+
$linktocourse = '<a href="https://www.example.com/moodle/course/view.php?id='
163+
. $this->course->id . '">' . $this->course->fullname . '</a>';
164+
$linktoforum = '<a href="https://www.example.com/moodle/mod/moodleoverflow/view.php?id='
165+
. $this->coursemodule->id . '">' . $this->moodleoverflow->name . '</a>';
166+
$linktodiscussion = '<a href="https://www.example.com/moodle/mod/moodleoverflow/discussion.php?d='
167+
. $this->discussion[0]->id . '">' . $this->discussion[0]->name . '</a>';
168+
169+
// Assemble text.
170+
$text = 'Course: ' . $linktocourse . ' -> ' . $linktoforum . ', Topic: '
171+
. $linktodiscussion . ' has ' . $messages . ' unread posts.';
169172

170173
$this->assertEquals($text, $content);
171174
}

view.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@
120120
}
121121

122122
if ($linktoforum && $movetopopup && has_capability('mod/moodleoverflow:movetopic', $context)) {
123-
// Take the $movetopopup-id and the $linktoforum-id and move the discussion to the forum
123+
// Take the $movetopopup-id and the $linktoforum-id and move the discussion to the forum.
124124
$topic = $DB->get_record('moodleoverflow_discussions', array('id' => $movetopopup));
125125
$topic->moodleoverflow = $linktoforum;
126126
$DB->update_record('moodleoverflow_discussions', $topic);

0 commit comments

Comments
 (0)