HackerOne users: Testing against this community violates our program's Terms of Service and will result in your bounty being denied.
Options

Paging inconsistency on Discussions list (with fix)

businessdadbusinessdad Stealth contributor MVP

I noticed recently that paging, on a Client's forum, seemed to be inconsistent. I made an experiment by setting just five Discussions per page, and I got the following result:

  • 9 discussions on first page (4 Announcements and 5 Discussions).
  • 5 discussions on second page.
  • 2 discussions on third page.
  • 1 discussion on fourth page.
  • 5 discussions on fifth page.

And so on. After a short investigation, I figured out what was wrong: the Announcements were skewing the paging, due to the way the are removed by the DiscussionModel.

The code that causes the issue can be found in class.discussionmodel.php, approximately between line 185 and 195. Here is the snippet:

  // Set range and fetch
  $Data = $this->SQL->Get();

  // If not looking at discussions filtered by bookmarks or user, filter announcements out.
  if (!$IncludeAnnouncements) {
     if (!isset($Wheres['w.Bookmarked']) && !isset($Wheres['d.InsertUserID']))
        $this->RemoveAnnouncements($Data);
  }

What the above does (in my opinion, incorrectly) is retrieving the required amount of Discussions (in my case, five) and then removing the Announcements from the result. Whenever there is one (or more) Announcement, the resulting data will be less than the five entries expected.

How to fix it

The fix is, actually, quite simple, and it involves filtering out the Announcements before retrieving the data, using a WHERE clause. The above should, therefore, be changed to:

  // If not looking at discussions filtered by bookmarks or user, filter announcements out.
  if (!$IncludeAnnouncements) {
     if (!isset($Wheres['w.Bookmarked']) && !isset($Wheres['d.InsertUserID'])) {
        $this->SQL->Where('d.Announce', 0); // Filter out Announcements
     }
  }

  // Set range and fetch
  $Data = $this->SQL->Get();

After applying this modification, the pager becomes consistent:

  • 9 discussions on first page (4 Announcements and 5 Discussions).
  • 5 discussions on second page.
  • 5 discussions on third page.
  • 5 discussion on fourth page.
    And so on.

I made some tests, and the fix doesn't seem to have side effects. If anyone is aware of any (existing or potential), please reply to this thread. Thanks.

Comments

  • Options
    phreakphreak Vanilla*APP (White Label) & Vanilla*Skins Shop MVP

    Great you delivered the fix to it. Can you tell which version of Vanilla has this bug?

    • VanillaAPP | iOS & Android App for Vanilla - White label app for Vanilla Forums OS
    • VanillaSkins | Plugins, Themes, Graphics and Custom Development for Vanilla
  • Options
    businessdadbusinessdad Stealth contributor MVP
    edited February 2013

    Whoops, I thought I added it to the post, but it seems I just dreamed of it. Anyway, it's Vanilla 2.0.8.14.

    Would an Admin be so kind to add the version to the original post, please? :)

Sign In or Register to comment.