Please upgrade here. These earlier versions are no longer being updated and have security issues.
HackerOne users: Testing against this community violates our program's Terms of Service and will result in your bounty being denied.

Wrong total count in discussions/unread

AaronWebsteyAaronWebstey Headband AfficionadoCole Harbour, NS ✭✭✭

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 :)

Comments

  • 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
  • AaronWebsteyAaronWebstey Headband Afficionado Cole Harbour, NS ✭✭✭

    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.

  • AaronWebsteyAaronWebstey Headband Afficionado Cole Harbour, NS ✭✭✭

    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;
       }
    
  • Yes, make a pull request, please!

    Also for your EXIF rotation fix :)

    AaronWebsteyhgtonight
  • AaronWebsteyAaronWebstey Headband Afficionado Cole Harbour, NS ✭✭✭

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

    R_J
  • LincLinc Detroit Admin
    edited January 2015

    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
  • AaronWebsteyAaronWebstey Headband Afficionado Cole Harbour, NS ✭✭✭

    @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 :(

  • LincLinc Detroit Admin

    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".

    AaronWebstey
  • R_JR_J Ex-Fanboy Munich Admin

    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.

    AaronWebstey
  • LincLinc Detroit Admin

    @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.

    AaronWebstey
  • hgtonighthgtonight ∞ · New Moderator

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

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

    Search first

    Check out the Documentation! We are always looking for new content and pull requests.

    Click on insightful, awesome, and funny reactions to thank community volunteers for their valuable posts.

    BleistivtAaronWebstey
  • LincLinc Detroit Admin

    @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.

    AaronWebstey
  • LincLinc Detroit Admin

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

    AaronWebstey
Sign In or Register to comment.