mirror_php/moodle
1
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-03-23 09:00:30 +01:00
Code Issues Projects Releases Wiki Activity

MDL-50109 forum: Respect chronological order of blog posts in prev/next

Browse Source 
The function forum_get_discussion_neighbours() is used to generate links
to neighbour discussions. I had to change the signature of the function
to have access to the forum's type without the need to re-read it from
the database in additional query.

The SQL query for blogs is a variant of the existing query, just using
the first post's creation time instead of the discussion's timemodified
time.
This commit is contained in:
David Mudrák 2015-08-14 01:21:57 +02:00
parent 8d032f59f4
commit 9e381efd1f
3 changed files with 104 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
mod/forum/discuss.php
View File

@ -266,7 +266,7 @@ if (!$canreply and $forum->type !== 'news') {
}
// Output the links to neighbour discussions.
$neighbours = forum_get_discussion_neighbours($cm, $discussion);
$neighbours = forum_get_discussion_neighbours($cm, $discussion, $forum);
$neighbourlinks = $renderer->neighbouring_discussion_navigation($neighbours['prev'], $neighbours['next']);
echo $neighbourlinks;

67
mod/forum/lib.php
View File

@ -2704,20 +2704,24 @@ function forum_get_discussions($cm, $forumsort="d.timemodified DESC", $fullpost=
* other mean to sort the records, e.g. we cannot use IDs as a greater ID can have a lower
* timemodified.
*
* For blog-style forums, the calculation is based on the original creation time of the
* blog post.
*
* Please note that this does not check whether or not the discussion passed is accessible
* by the user, it simply uses it as a reference to find the neighbours. On the other hand,
* the returned neighbours are checked and are accessible to the current user.
*
* @param object $cm The CM record.
* @param object $discussion The discussion record.
* @param object $forum The forum instance record.
* @return array That always contains the keys 'prev' and 'next'. When there is a result
* they contain the record with minimal information such as 'id' and 'name'.
* When the neighbour is not found the value is false.
*/
function forum_get_discussion_neighbours($cm, $discussion) {
function forum_get_discussion_neighbours($cm, $discussion, $forum) {
global $CFG, $DB, $USER;
if ($cm->instance != $discussion->forum) {
if ($cm->instance != $discussion->forum or $discussion->forum != $forum->id or $forum->id != $cm->instance) {
throw new coding_exception('Discussion is not part of the same forum.');
}
@ -2762,25 +2766,54 @@ function forum_get_discussion_neighbours($cm, $discussion) {
}
}
$params['forumid'] = $cm->instance;
$params['discid'] = $discussion->id;
$params['disctimemodified'] = $discussion->timemodified;
if ($forum->type === 'blog') {
$params['forumid'] = $cm->instance;
$params['discid1'] = $discussion->id;
$params['discid2'] = $discussion->id;
$sql = "SELECT d.id, d.name, d.timemodified, d.groupid, d.timestart, d.timeend
FROM {forum_discussions} d
WHERE d.forum = :forumid
AND d.id <> :discid
$timelimit
$groupselect";
$sql = "SELECT d.id, d.name, d.timemodified, d.groupid, d.timestart, d.timeend
FROM {forum_discussions} d
JOIN {forum_posts} p ON d.firstpost = p.id
WHERE d.forum = :forumid
AND d.id <> :discid1
$timelimit
$groupselect";
$prevsql = $sql . " AND d.timemodified < :disctimemodified
ORDER BY d.timemodified DESC";
$sub = "SELECT pp.created
FROM {forum_discussions} dd
JOIN {forum_posts} pp ON dd.firstpost = pp.id
WHERE dd.id = :discid2";
$nextsql = $sql . " AND d.timemodified > :disctimemodified
ORDER BY d.timemodified ASC";
$prevsql = $sql . " AND p.created < ($sub)
ORDER BY p.created DESC";
$neighbours['prev'] = $DB->get_record_sql($prevsql, $params, IGNORE_MULTIPLE);
$neighbours['next'] = $DB->get_record_sql($nextsql, $params, IGNORE_MULTIPLE);
$nextsql = $sql . " AND p.created > ($sub)
ORDER BY p.created ASC";
$neighbours['prev'] = $DB->get_record_sql($prevsql, $params, IGNORE_MULTIPLE);
$neighbours['next'] = $DB->get_record_sql($nextsql, $params, IGNORE_MULTIPLE);
} else {
$params['forumid'] = $cm->instance;
$params['discid'] = $discussion->id;
$params['disctimemodified'] = $discussion->timemodified;
$sql = "SELECT d.id, d.name, d.timemodified, d.groupid, d.timestart, d.timeend
FROM {forum_discussions} d
WHERE d.forum = :forumid
AND d.id <> :discid
$timelimit
$groupselect";
$prevsql = $sql . " AND d.timemodified < :disctimemodified
ORDER BY d.timemodified DESC";
$nextsql = $sql . " AND d.timemodified > :disctimemodified
ORDER BY d.timemodified ASC";
$neighbours['prev'] = $DB->get_record_sql($prevsql, $params, IGNORE_MULTIPLE);
$neighbours['next'] = $DB->get_record_sql($nextsql, $params, IGNORE_MULTIPLE);
}
return $neighbours;
}

106
mod/forum/tests/lib_test.php
View File

@ -821,23 +821,23 @@ class mod_forum_lib_testcase extends advanced_testcase {
$disc5 = $forumgen->create_discussion($record);
// Getting the neighbours.
$neighbours = forum_get_discussion_neighbours($cm, $disc1);
$neighbours = forum_get_discussion_neighbours($cm, $disc1, $forum);
$this->assertEmpty($neighbours['prev']);
$this->assertEquals($disc2->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm, $disc2);
$neighbours = forum_get_discussion_neighbours($cm, $disc2, $forum);
$this->assertEquals($disc1->id, $neighbours['prev']->id);
$this->assertEquals($disc3->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm, $disc3);
$neighbours = forum_get_discussion_neighbours($cm, $disc3, $forum);
$this->assertEquals($disc2->id, $neighbours['prev']->id);
$this->assertEquals($disc4->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm, $disc4);
$neighbours = forum_get_discussion_neighbours($cm, $disc4, $forum);
$this->assertEquals($disc3->id, $neighbours['prev']->id);
$this->assertEquals($disc5->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm, $disc5);
$neighbours = forum_get_discussion_neighbours($cm, $disc5, $forum);
$this->assertEquals($disc4->id, $neighbours['prev']->id);
$this->assertEmpty($neighbours['next']);
@ -847,28 +847,28 @@ class mod_forum_lib_testcase extends advanced_testcase {
$disc1->timemodified = time();
$DB->update_record('forum_discussions', $disc1);
$neighbours = forum_get_discussion_neighbours($cm, $disc5);
$neighbours = forum_get_discussion_neighbours($cm, $disc5, $forum);
$this->assertEquals($disc4->id, $neighbours['prev']->id);
$this->assertEquals($disc1->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm, $disc2);
$neighbours = forum_get_discussion_neighbours($cm, $disc2, $forum);
$this->assertEmpty($neighbours['prev']);
$this->assertEquals($disc3->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm, $disc1);
$neighbours = forum_get_discussion_neighbours($cm, $disc1, $forum);
$this->assertEquals($disc5->id, $neighbours['prev']->id);
$this->assertEmpty($neighbours['next']);
// After some discussions were created.
sleep(1);
$disc6 = $forumgen->create_discussion($record);
$neighbours = forum_get_discussion_neighbours($cm, $disc6);
$neighbours = forum_get_discussion_neighbours($cm, $disc6, $forum);
$this->assertEquals($disc1->id, $neighbours['prev']->id);
$this->assertEmpty($neighbours['next']);
sleep(1);
$disc7 = $forumgen->create_discussion($record);
$neighbours = forum_get_discussion_neighbours($cm, $disc7);
$neighbours = forum_get_discussion_neighbours($cm, $disc7, $forum);
$this->assertEquals($disc6->id, $neighbours['prev']->id);
$this->assertEmpty($neighbours['next']);
@ -905,59 +905,59 @@ class mod_forum_lib_testcase extends advanced_testcase {
// Admin user ignores the timed settings of discussions.
$this->setAdminUser();
$neighbours = forum_get_discussion_neighbours($cm, $disc8);
$neighbours = forum_get_discussion_neighbours($cm, $disc8, $forum);
$this->assertEquals($disc7->id, $neighbours['prev']->id);
$this->assertEquals($disc9->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm, $disc9);
$neighbours = forum_get_discussion_neighbours($cm, $disc9, $forum);
$this->assertEquals($disc8->id, $neighbours['prev']->id);
$this->assertEquals($disc10->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm, $disc10);
$neighbours = forum_get_discussion_neighbours($cm, $disc10, $forum);
$this->assertEquals($disc9->id, $neighbours['prev']->id);
$this->assertEquals($disc11->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm, $disc11);
$neighbours = forum_get_discussion_neighbours($cm, $disc11, $forum);
$this->assertEquals($disc10->id, $neighbours['prev']->id);
$this->assertEquals($disc12->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm, $disc12);
$neighbours = forum_get_discussion_neighbours($cm, $disc12, $forum);
$this->assertEquals($disc11->id, $neighbours['prev']->id);
$this->assertEmpty($neighbours['next']);
// Normal user can see their own timed discussions.
$this->setUser($user);
$neighbours = forum_get_discussion_neighbours($cm, $disc8);
$neighbours = forum_get_discussion_neighbours($cm, $disc8, $forum);
$this->assertEquals($disc7->id, $neighbours['prev']->id);
$this->assertEquals($disc9->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm, $disc9);
$neighbours = forum_get_discussion_neighbours($cm, $disc9, $forum);
$this->assertEquals($disc8->id, $neighbours['prev']->id);
$this->assertEquals($disc10->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm, $disc10);
$neighbours = forum_get_discussion_neighbours($cm, $disc10, $forum);
$this->assertEquals($disc9->id, $neighbours['prev']->id);
$this->assertEquals($disc11->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm, $disc11);
$neighbours = forum_get_discussion_neighbours($cm, $disc11, $forum);
$this->assertEquals($disc10->id, $neighbours['prev']->id);
$this->assertEquals($disc12->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm, $disc12);
$neighbours = forum_get_discussion_neighbours($cm, $disc12, $forum);
$this->assertEquals($disc11->id, $neighbours['prev']->id);
$this->assertEmpty($neighbours['next']);
// Normal user does not ignore timed settings.
$this->setUser($user2);
$neighbours = forum_get_discussion_neighbours($cm, $disc8);
$neighbours = forum_get_discussion_neighbours($cm, $disc8, $forum);
$this->assertEquals($disc7->id, $neighbours['prev']->id);
$this->assertEquals($disc10->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm, $disc10);
$neighbours = forum_get_discussion_neighbours($cm, $disc10, $forum);
$this->assertEquals($disc8->id, $neighbours['prev']->id);
$this->assertEquals($disc12->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm, $disc12);
$neighbours = forum_get_discussion_neighbours($cm, $disc12, $forum);
$this->assertEquals($disc10->id, $neighbours['prev']->id);
$this->assertEmpty($neighbours['next']);
@ -973,11 +973,11 @@ class mod_forum_lib_testcase extends advanced_testcase {
$disc2 = $DB->get_record('forum_discussions', array('id' => $disc2->id));
$disc3 = $DB->get_record('forum_discussions', array('id' => $disc3->id));
$neighbours = forum_get_discussion_neighbours($cm, $disc2);
$neighbours = forum_get_discussion_neighbours($cm, $disc2, $forum);
$this->assertEquals($disc12->id, $neighbours['prev']->id);
$this->assertEmpty($neighbours['next']);
$neighbours = forum_get_discussion_neighbours($cm, $disc3);
$neighbours = forum_get_discussion_neighbours($cm, $disc3, $forum);
$this->assertEquals($disc12->id, $neighbours['prev']->id);
$this->assertEmpty($neighbours['next']);
}
@ -1050,38 +1050,38 @@ class mod_forum_lib_testcase extends advanced_testcase {
// Admin user can see all groups.
$this->setAdminUser();
$neighbours = forum_get_discussion_neighbours($cm1, $disc11);
$neighbours = forum_get_discussion_neighbours($cm1, $disc11, $forum1);
$this->assertEmpty($neighbours['prev']);
$this->assertEquals($disc12->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm2, $disc21);
$neighbours = forum_get_discussion_neighbours($cm2, $disc21, $forum2);
$this->assertEmpty($neighbours['prev']);
$this->assertEquals($disc22->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm1, $disc12);
$neighbours = forum_get_discussion_neighbours($cm1, $disc12, $forum1);
$this->assertEquals($disc11->id, $neighbours['prev']->id);
$this->assertEquals($disc13->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm2, $disc22);
$neighbours = forum_get_discussion_neighbours($cm2, $disc22, $forum2);
$this->assertEquals($disc21->id, $neighbours['prev']->id);
$this->assertEquals($disc23->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm1, $disc13);
$neighbours = forum_get_discussion_neighbours($cm1, $disc13, $forum1);
$this->assertEquals($disc12->id, $neighbours['prev']->id);
$this->assertEquals($disc14->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm2, $disc23);
$neighbours = forum_get_discussion_neighbours($cm2, $disc23, $forum2);
$this->assertEquals($disc22->id, $neighbours['prev']->id);
$this->assertEquals($disc24->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm1, $disc14);
$neighbours = forum_get_discussion_neighbours($cm1, $disc14, $forum1);
$this->assertEquals($disc13->id, $neighbours['prev']->id);
$this->assertEquals($disc15->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm2, $disc24);
$neighbours = forum_get_discussion_neighbours($cm2, $disc24, $forum2);
$this->assertEquals($disc23->id, $neighbours['prev']->id);
$this->assertEquals($disc25->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm1, $disc15);
$neighbours = forum_get_discussion_neighbours($cm1, $disc15, $forum1);
$this->assertEquals($disc14->id, $neighbours['prev']->id);
$this->assertEmpty($neighbours['next']);
$neighbours = forum_get_discussion_neighbours($cm2, $disc25);
$neighbours = forum_get_discussion_neighbours($cm2, $disc25, $forum2);
$this->assertEquals($disc24->id, $neighbours['prev']->id);
$this->assertEmpty($neighbours['next']);
@ -1090,24 +1090,24 @@ class mod_forum_lib_testcase extends advanced_testcase {
$this->assertEquals($group1->id, groups_get_activity_group($cm1, true));
$this->assertEquals($group1->id, groups_get_activity_group($cm2, true));
$neighbours = forum_get_discussion_neighbours($cm1, $disc11);
$neighbours = forum_get_discussion_neighbours($cm1, $disc11, $forum1);
$this->assertEmpty($neighbours['prev']);
$this->assertEquals($disc13->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm2, $disc21);
$neighbours = forum_get_discussion_neighbours($cm2, $disc21, $forum2);
$this->assertEmpty($neighbours['prev']);
$this->assertEquals($disc23->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm1, $disc13);
$neighbours = forum_get_discussion_neighbours($cm1, $disc13, $forum1);
$this->assertEquals($disc11->id, $neighbours['prev']->id);
$this->assertEquals($disc15->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm2, $disc23);
$neighbours = forum_get_discussion_neighbours($cm2, $disc23, $forum2);
$this->assertEquals($disc21->id, $neighbours['prev']->id);
$this->assertEquals($disc25->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm1, $disc15);
$neighbours = forum_get_discussion_neighbours($cm1, $disc15, $forum1);
$this->assertEquals($disc13->id, $neighbours['prev']->id);
$this->assertEmpty($neighbours['next']);
$neighbours = forum_get_discussion_neighbours($cm2, $disc25);
$neighbours = forum_get_discussion_neighbours($cm2, $disc25, $forum2);
$this->assertEquals($disc23->id, $neighbours['prev']->id);
$this->assertEmpty($neighbours['next']);
@ -1117,10 +1117,10 @@ class mod_forum_lib_testcase extends advanced_testcase {
$this->assertEquals(0, groups_get_activity_group($cm1, true));
// They can see anything in visible groups.
$neighbours = forum_get_discussion_neighbours($cm1, $disc12);
$neighbours = forum_get_discussion_neighbours($cm1, $disc12, $forum1);
$this->assertEquals($disc11->id, $neighbours['prev']->id);
$this->assertEquals($disc13->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm1, $disc13);
$neighbours = forum_get_discussion_neighbours($cm1, $disc13, $forum1);
$this->assertEquals($disc12->id, $neighbours['prev']->id);
$this->assertEquals($disc14->id, $neighbours['next']->id);
@ -1129,15 +1129,15 @@ class mod_forum_lib_testcase extends advanced_testcase {
$_POST['group'] = 0;
$this->assertEquals(0, groups_get_activity_group($cm2, true));
$neighbours = forum_get_discussion_neighbours($cm2, $disc23);
$neighbours = forum_get_discussion_neighbours($cm2, $disc23, $forum2);
$this->assertEmpty($neighbours['prev']);
$this->assertEmpty($neighbours['next']);
$neighbours = forum_get_discussion_neighbours($cm2, $disc22);
$neighbours = forum_get_discussion_neighbours($cm2, $disc22, $forum2);
$this->assertEmpty($neighbours['prev']);
$this->assertEquals($disc23->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm2, $disc24);
$neighbours = forum_get_discussion_neighbours($cm2, $disc24, $forum2);
$this->assertEquals($disc23->id, $neighbours['prev']->id);
$this->assertEmpty($neighbours['next']);
@ -1148,30 +1148,30 @@ class mod_forum_lib_testcase extends advanced_testcase {
$this->assertEquals($group1->id, groups_get_activity_group($cm2, true));
// They can see non-grouped or same group.
$neighbours = forum_get_discussion_neighbours($cm1, $disc11);
$neighbours = forum_get_discussion_neighbours($cm1, $disc11, $forum1);
$this->assertEmpty($neighbours['prev']);
$this->assertEquals($disc13->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm2, $disc21);
$neighbours = forum_get_discussion_neighbours($cm2, $disc21, $forum2);
$this->assertEmpty($neighbours['prev']);
$this->assertEquals($disc23->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm1, $disc13);
$neighbours = forum_get_discussion_neighbours($cm1, $disc13, $forum1);
$this->assertEquals($disc11->id, $neighbours['prev']->id);
$this->assertEquals($disc15->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm2, $disc23);
$neighbours = forum_get_discussion_neighbours($cm2, $disc23, $forum2);
$this->assertEquals($disc21->id, $neighbours['prev']->id);
$this->assertEquals($disc25->id, $neighbours['next']->id);
$neighbours = forum_get_discussion_neighbours($cm1, $disc15);
$neighbours = forum_get_discussion_neighbours($cm1, $disc15, $forum1);
$this->assertEquals($disc13->id, $neighbours['prev']->id);
$this->assertEmpty($neighbours['next']);
$neighbours = forum_get_discussion_neighbours($cm2, $disc25);
$neighbours = forum_get_discussion_neighbours($cm2, $disc25, $forum2);
$this->assertEquals($disc23->id, $neighbours['prev']->id);
$this->assertEmpty($neighbours['next']);
// Querying the neighbours of a discussion passing the wrong CM.
$this->setExpectedException('coding_exception');
forum_get_discussion_neighbours($cm2, $disc11);
forum_get_discussion_neighbours($cm2, $disc11, $forum2);
}
public function test_count_discussion_replies_basic() {