From a9745eb95af649395ee1ff526d65cf53a2407642 Mon Sep 17 00:00:00 2001 From: Josh Marshall Date: Thu, 29 Oct 2015 15:51:13 +1030 Subject: [PATCH 1/2] MDL-52930 forum: Correct get_unmailed_posts logic --- mod/forum/lib.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/mod/forum/lib.php b/mod/forum/lib.php index 8b81cc866ba..efa1897f9f0 100644 --- a/mod/forum/lib.php +++ b/mod/forum/lib.php @@ -2224,18 +2224,21 @@ function forum_get_unmailed_posts($starttime, $endtime, $now=null) { if (empty($now)) { $now = time(); } + $selectsql = "AND (p.created >= :ptimestart OR d.timestart >= :pptimestart)"; + $params['pptimestart'] = $starttime; $timedsql = "AND (d.timestart < :dtimestart AND (d.timeend = 0 OR d.timeend > :dtimeend))"; $params['dtimestart'] = $now; $params['dtimeend'] = $now; } else { $timedsql = ""; + $selectsql = "AND p.created >= :ptimestart"; } return $DB->get_records_sql("SELECT p.*, d.course, d.forum FROM {forum_posts} p JOIN {forum_discussions} d ON d.id = p.discussion WHERE p.mailed = :mailed - AND p.created >= :ptimestart + $selectsql AND (p.created < :ptimeend OR p.mailnow = :mailnow) $timedsql ORDER BY p.modified ASC", $params); From e66281a8b03c82d19afd4842a23c7842ce986546 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Fri, 5 Feb 2016 14:32:47 +0800 Subject: [PATCH 2/2] MDL-52930 mod_forum: Add get_unmailed_posts tests --- mod/forum/tests/generator/lib.php | 22 ++- mod/forum/tests/lib_test.php | 213 ++++++++++++++++++++++++++++++ 2 files changed, 230 insertions(+), 5 deletions(-) diff --git a/mod/forum/tests/generator/lib.php b/mod/forum/tests/generator/lib.php index 3492860d42e..6af4c27bbd6 100644 --- a/mod/forum/tests/generator/lib.php +++ b/mod/forum/tests/generator/lib.php @@ -189,18 +189,30 @@ class mod_forum_generator extends testing_module_generator { $timemodified = $record['timemodified']; } + if (isset($record['mailed'])) { + $mailed = $record['mailed']; + } + $record = (object) $record; // Add the discussion. $record->id = forum_add_discussion($record, null, null, $record->userid); - if (isset($timemodified)) { - // Enforce the time modified. + if (isset($timemodified) || isset($mailed)) { $post = $DB->get_record('forum_posts', array('discussion' => $record->id)); - $record->timemodified = $timemodified; - $post->modified = $post->created = $timemodified; - $DB->update_record('forum_discussions', $record); + if (isset($mailed)) { + $post->mailed = $mailed; + } + + if (isset($timemodified)) { + // Enforce the time modified. + $record->timemodified = $timemodified; + $post->modified = $post->created = $timemodified; + + $DB->update_record('forum_discussions', $record); + } + $DB->update_record('forum_posts', $post); } diff --git a/mod/forum/tests/lib_test.php b/mod/forum/tests/lib_test.php index 0fc90e8a9d7..10fc9418512 100644 --- a/mod/forum/tests/lib_test.php +++ b/mod/forum/tests/lib_test.php @@ -1835,4 +1835,217 @@ class mod_forum_lib_testcase extends advanced_testcase { ), ); } + + /** + * @dataProvider forum_get_unmailed_posts_provider + */ + public function test_forum_get_unmailed_posts($discussiondata, $enabletimedposts, $expectedcount, $expectedreplycount) { + global $CFG, $DB; + + $this->resetAfterTest(); + + // Configure timed posts. + $CFG->forum_enabletimedposts = $enabletimedposts; + + $course = $this->getDataGenerator()->create_course(); + $forum = $this->getDataGenerator()->create_module('forum', ['course' => $course->id]); + $user = $this->getDataGenerator()->create_user(); + $forumgen = $this->getDataGenerator()->get_plugin_generator('mod_forum'); + + $record = new stdClass(); + $record->course = $course->id; + $record->userid = $user->id; + $record->forum = $forum->id; + if (isset($discussiondata['timecreated'])) { + $record->timemodified = time() + $discussiondata['timecreated']; + } + if (isset($discussiondata['timestart'])) { + $record->timestart = time() + $discussiondata['timestart']; + } + if (isset($discussiondata['timeend'])) { + $record->timeend = time() + $discussiondata['timeend']; + } + if (isset($discussiondata['mailed'])) { + $record->mailed = $discussiondata['mailed']; + } + + $discussion = $forumgen->create_discussion($record); + + // Fetch the unmailed posts. + $timenow = time(); + $endtime = $timenow - $CFG->maxeditingtime; + $starttime = $endtime - 2 * DAYSECS; + + $unmailed = forum_get_unmailed_posts($starttime, $endtime, $timenow); + $this->assertCount($expectedcount, $unmailed); + + // Add a reply just outside the maxeditingtime. + $replyto = $DB->get_record('forum_posts', array('discussion' => $discussion->id)); + $reply = new stdClass(); + $reply->userid = $user->id; + $reply->discussion = $discussion->id; + $reply->parent = $replyto->id; + $reply->created = max($replyto->created, $endtime - 1); + $r = $forumgen->create_post($reply); + + $unmailed = forum_get_unmailed_posts($starttime, $endtime, $timenow); + $this->assertCount($expectedreplycount, $unmailed); + } + + public function forum_get_unmailed_posts_provider() { + return [ + 'Untimed discussion; Single post; maxeditingtime not expired' => [ + 'discussion' => [ + ], + 'timedposts' => false, + 'postcount' => 0, + 'replycount' => 0, + ], + 'Untimed discussion; Single post; maxeditingtime expired' => [ + 'discussion' => [ + 'timecreated' => - DAYSECS, + ], + 'timedposts' => false, + 'postcount' => 1, + 'replycount' => 2, + ], + 'Timed discussion; Single post; Posted 1 week ago; timestart maxeditingtime not expired' => [ + 'discussion' => [ + 'timecreated' => - WEEKSECS, + 'timestart' => 0, + ], + 'timedposts' => true, + 'postcount' => 0, + 'replycount' => 0, + ], + 'Timed discussion; Single post; Posted 1 week ago; timestart maxeditingtime expired' => [ + 'discussion' => [ + 'timecreated' => - WEEKSECS, + 'timestart' => - DAYSECS, + ], + 'timedposts' => true, + 'postcount' => 1, + 'replycount' => 2, + ], + 'Timed discussion; Single post; Posted 1 week ago; timestart maxeditingtime expired; timeend not reached' => [ + 'discussion' => [ + 'timecreated' => - WEEKSECS, + 'timestart' => - DAYSECS, + 'timeend' => + DAYSECS + ], + 'timedposts' => true, + 'postcount' => 1, + 'replycount' => 2, + ], + 'Timed discussion; Single post; Posted 1 week ago; timestart maxeditingtime expired; timeend passed' => [ + 'discussion' => [ + 'timecreated' => - WEEKSECS, + 'timestart' => - DAYSECS, + 'timeend' => - HOURSECS, + ], + 'timedposts' => true, + 'postcount' => 0, + 'replycount' => 0, + ], + 'Timed discussion; Single post; Posted 1 week ago; timeend not reached' => [ + 'discussion' => [ + 'timecreated' => - WEEKSECS, + 'timeend' => + DAYSECS + ], + 'timedposts' => true, + 'postcount' => 0, + 'replycount' => 1, + ], + 'Timed discussion; Single post; Posted 1 week ago; timeend passed' => [ + 'discussion' => [ + 'timecreated' => - WEEKSECS, + 'timeend' => - DAYSECS, + ], + 'timedposts' => true, + 'postcount' => 0, + 'replycount' => 0, + ], + + 'Previously mailed; Untimed discussion; Single post; maxeditingtime not expired' => [ + 'discussion' => [ + 'mailed' => 1, + ], + 'timedposts' => false, + 'postcount' => 0, + 'replycount' => 0, + ], + + 'Previously mailed; Untimed discussion; Single post; maxeditingtime expired' => [ + 'discussion' => [ + 'timecreated' => - DAYSECS, + 'mailed' => 1, + ], + 'timedposts' => false, + 'postcount' => 0, + 'replycount' => 1, + ], + 'Previously mailed; Timed discussion; Single post; Posted 1 week ago; timestart maxeditingtime not expired' => [ + 'discussion' => [ + 'timecreated' => - WEEKSECS, + 'timestart' => 0, + 'mailed' => 1, + ], + 'timedposts' => true, + 'postcount' => 0, + 'replycount' => 0, + ], + 'Previously mailed; Timed discussion; Single post; Posted 1 week ago; timestart maxeditingtime expired' => [ + 'discussion' => [ + 'timecreated' => - WEEKSECS, + 'timestart' => - DAYSECS, + 'mailed' => 1, + ], + 'timedposts' => true, + 'postcount' => 0, + 'replycount' => 1, + ], + 'Previously mailed; Timed discussion; Single post; Posted 1 week ago; timestart maxeditingtime expired; timeend not reached' => [ + 'discussion' => [ + 'timecreated' => - WEEKSECS, + 'timestart' => - DAYSECS, + 'timeend' => + DAYSECS, + 'mailed' => 1, + ], + 'timedposts' => true, + 'postcount' => 0, + 'replycount' => 1, + ], + 'Previously mailed; Timed discussion; Single post; Posted 1 week ago; timestart maxeditingtime expired; timeend passed' => [ + 'discussion' => [ + 'timecreated' => - WEEKSECS, + 'timestart' => - DAYSECS, + 'timeend' => - HOURSECS, + 'mailed' => 1, + ], + 'timedposts' => true, + 'postcount' => 0, + 'replycount' => 0, + ], + 'Previously mailed; Timed discussion; Single post; Posted 1 week ago; timeend not reached' => [ + 'discussion' => [ + 'timecreated' => - WEEKSECS, + 'timeend' => + DAYSECS, + 'mailed' => 1, + ], + 'timedposts' => true, + 'postcount' => 0, + 'replycount' => 1, + ], + 'Previously mailed; Timed discussion; Single post; Posted 1 week ago; timeend passed' => [ + 'discussion' => [ + 'timecreated' => - WEEKSECS, + 'timeend' => - DAYSECS, + 'mailed' => 1, + ], + 'timedposts' => true, + 'postcount' => 0, + 'replycount' => 0, + ], + ]; + } }