Wrong total count in discussions/unread

AaronWebstey

I've noticed that on my 'unread discussions' page, the paginator is confused. It seems to think that there are 5 pages of unread discussions when there is actually only one single discussion.

I've been searching for answers in this forum, and I can't seem to find anything - although I feel like I stumbled across something about this by accident while back. Couldn't find anything on github, either. Does anyone know if this is a common issue and/or if there's a known fix?

If you're interested, I've added an 'Unread Discussions' link to the discussions filter module on my site, and I was trying to figure out a way to make it show a count of all of the user's unread discussions. I.e., I'd like to see a little grey box with a number in it (0 or otherwise) beside 'Unread Discussions' here:

Once I have the pagination thing figured out, I'll see if that helps me figure out the unread count - and I'll probably either ask a separate question, or post a tutorial

Bleistivt

I noticed that too. There might be something wrong with DiscussionModel::GetUnreadCount()

As for the PageModule, the only thing you really need to look at to understand how a pager ist built is the Configure() function:

https://github.com/vanilla/vanilla/blob/master/applications/dashboard/modules/class.pagermodule.php#L139

AaronWebstey

Thanks @Bleistivt‌ ! Yep, the pager looks pretty straightforward and there are loads of good examples there. I guess I was wondering about GetUnreadCount() mostly, as I'd like to use that to populate an 'Unread Discussions' count in the panel.

I'll have to take a look at the code for that tomorrow and see if I can figure out what's broken. As my Recent Discussions page also has 5 pages, I'm guessing that either a) the value of GetUnreadCount is not getting passed to the pager function (and the total number of all discussions is), or b) as you mentioned, GetUnreadCount() is broken.

AaronWebstey

Update: I fixed GetUnreadCount() - at least it seems to work properly now, I may have missed some style/practices points.

It looks like it was copy/pasted from GetCount() and the author forgot to address the part that gets counts from the category cache - which I just commented out. I presumed (and think I confirmed in the code) that unread counts are not stored in the category cache.

I'm not sure about calling this function on every page (as I had planned to do, per above comments) as it looks like it might be a significant query happening on every page request; but I don't know enough about how the cache works to know for sure.

Here is my updated function. At the very least, I can confirm that the discussions/unread page on my forum works properly now. I marked my changes with WEBSTEY comments.

@Linc‌ , @hgtonight‌ , @peregrine‌ , and others have provided me with github instructions/primers in other discussions, if it is deemed that I should try to submit this (or a cleaned-up version) to the master. I'm working on a few other things right now too, but I will commit to taking whatever the best next steps are, if any.

/**
    * Count how many discussions match the given criteria.
    * 
    * @since 2.0.0
    * @access public
    * 
     * @param array $Wheres SQL conditions.
     * @param bool $ForceNoAnnouncements Not used.
     * @return int Number of discussions.
     */
   public function GetUnreadCount($Wheres = '', $ForceNoAnnouncements = FALSE) {
      if (is_array($Wheres) && count($Wheres) == 0)
         $Wheres = '';

      // Check permission and limit to categories as necessary
      if ($this->Watching)
         $Perms = CategoryModel::CategoryWatch();
      else
         $Perms = self::CategoryPermissions();

/*      // WEBSTEY commtned this stuff out
      if (!$Wheres || (count($Wheres) == 1 && isset($Wheres['d.CategoryID']))) {
         // Grab the counts from the faster category cache.
         if (isset($Wheres['d.CategoryID'])) {
            $CategoryIDs = (array)$Wheres['d.CategoryID'];
            if ($Perms === FALSE)
               $CategoryIDs = array();
            elseif (is_array($Perms))
               $CategoryIDs = array_intersect($CategoryIDs, $Perms);

            if (count($CategoryIDs) == 0) {
               return 0;
            } else {
               $Perms = $CategoryIDs;
            }
         }

         $Categories = CategoryModel::Categories();
         $Count = 0;

         foreach ($Categories as $Cat) {
            if (is_array($Perms) && !in_array($Cat['CategoryID'], $Perms))
               continue;
            $Count += (int)$Cat['CountDiscussions'];
         }
         return $Count;
      }
*/    
      if ($Perms !== TRUE) {
         $this->SQL->WhereIn('c.CategoryID', $Perms);
      }

      $this->EventArguments['Wheres'] = &$Wheres;
        $this->FireEvent('BeforeGetUnreadCount'); // @see 'BeforeGet' for consistency in count vs. results

      $this->SQL
         ->Select('d.DiscussionID', 'count', 'CountDiscussions')
         ->From('Discussion d')
         ->Join('Category c', 'd.CategoryID = c.CategoryID')
         ->Join('UserDiscussion w', 'd.DiscussionID = w.DiscussionID and w.UserID = '.Gdn::Session()->UserID, 'left')
         //->BeginWhereGroup()
         //->Where('w.DateLastViewed', NULL)
         //->OrWhere('d.DateLastComment >', 'w.DateLastViewed')
         //->EndWhereGroup()
         ->Where('d.CountComments >', 'COALESCE(w.CountComments, 0)', TRUE, FALSE); // WEBSTEY Added a semicolon
//       ->Where($Wheres); // WEBSTEY commented this out
      if(!empty($Wheres)) $this->SQL->Where($Wheres); // WEBSTEY added this

      $Result = $this->SQL
         ->Get()
         ->FirstRow()
         ->CountDiscussions;

      return $Result;
   }

Bleistivt

Yes, make a pull request, please!

Also for your EXIF rotation fix

AaronWebstey

Ha - I almost forgot about that! I'll figure out the process and do both ASAP

Linc

My flyby 2 cents is that the entire Unread feature subset doesn't scale and we never use it on cloud, nor do I have any suggestions of how to fix it - it needs to be cached, but the cache would get burned nearly as fast as it was made. Add to that that I don't really think the feature is terribly useful, and I kinda wish it was just refactored out of core into an addon.

I guess I'd take a pull request to fix what's already broken (minus the authorship comments) but I'm still not happy with anything about this featureset.

AaronWebstey

@Linc‌ thanks, you answered my question about scalability, even though I'm not sure I properly asked it. I kind of suspected that it was a resource hog - in fact, just including the bug fix that I made must certainly slow down any installation (how much, I have no idea). It was hitting the categories cache to get an incorrect number, while after the fix it will do a DB query to get the correct number.

As a user, though, I actually do find it extremely useful to view a list of unread discussions. It's currently my go-to page on our forum. I had even considered adding an 'unread' icon next to the 'notifications' icon (in the sprites next to the username), with a counter and an ajax pop-in containing the latest unread titles when you click on it. However, given your comments, I don't think that'll happen.

As for caching, I agree (for what that's worth) that there definitely doesn't seem to be any way that could work (with my limited knowledge of the system, at least). If there is any way to make it scale reasonably, I'm guessing it would have to involve some kind of inserts to new or existing DB table(s) whenever something gets posted, whenever someone views a discussion (or clicks 'mark as read', or probably some cases I haven't thought of ...). Sounds like a mess to me

Linc

Personally, I find a discussions page with clear unread markers more useful, at least on non-huge forums. I don't necessarily care if I didn't read a discussion 3 pages back; I probably skipped it on purpose and don't want it surfaced again. Liberal use of "Mark All Viewed" is easier.

We've added a notification for "participated" discussions to core that I think is a more helpful way of tracking things you want to know about that are now "unread".

R_J

How often do you open discussions you have already read? Good usability is to present only the links needed. In most cases you open unread discussions, having already read discussions at hand is a waste of space.

Linc

@R_J said:
Good usability is to present only the links needed. In most cases you open unread discussions, having already read discussions at hand is a waste of space.

To me, good usability is giving the full context of what's happening, not shrinking it down to CliffsNotes.

How often do you open discussions you have already read?

Actually, quite a bit, to see new reactions.

hgtonight

@Linc said:
Actually, quite a bit, to see new reactions.

Since I am a narcissist, I use /profile/reactions constantly.

Linc

@hgtonight said:
Since I am a narcissist, I use /profile/reactions constantly.

I really want to expand on the feedback of reactions when we refactor it into core. It's so powerful it changed my forum habits.

Linc

I've filed removing Unread as an issue: https://github.com/vanilla/vanilla/issues/2417