Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Update class.discussionmodel.php
GetUnread was not showing new discussions without any comment. Fixed that.
  • Loading branch information
R-J committed Jul 29, 2013
1 parent 557e7b7 commit 1ab2895
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion applications/vanilla/models/class.discussionmodel.php
Expand Up @@ -392,7 +392,8 @@ public function GetUnread($Offset = '0', $Limit = '', $Wheres = '', $AdditionalF
//->Where('w.DateLastViewed', NULL)
//->OrWhere('d.DateLastComment >', 'w.DateLastViewed')
//->EndWhereGroup()
->Where('d.CountComments >', 'COALESCE(w.CountComments, 0)', TRUE, FALSE);
->Where('d.CountComments >', 'COALESCE(w.CountComments, 0)', TRUE, FALSE)
->OrWhere('w.DateLastViewed', NULL);
} else {
$this->SQL
->Select('0', '', 'WatchUserID')
Expand Down

5 comments on commit 1ab2895

@linc
Copy link
Contributor

@linc linc commented on 1ab2895 Apr 28, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey R-J, unfortunately I'm going to have to quash this out of the 2.1 release. It actually opens up a security hole, because it bypasses the permissions check for those discussions. If you can come up with a way of fixing the original issue without doing that I'd be keen for a different fix.

@R-J
Copy link
Contributor Author

@R-J R-J commented on 1ab2895 Apr 28, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Glad you found that! I will look for a way...

@linc
Copy link
Contributor

@linc linc commented on 1ab2895 Apr 28, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was a peregrine find :) #1871

@R-J
Copy link
Contributor Author

@R-J R-J commented on 1ab2895 Apr 28, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it is too late and I have no time for testing, but this seems to work:

     $Perms = self::CategoryPermissions();
     $this->SQL
        ->Select('w.UserID', '', 'WatchUserID')
        ->Select('w.DateLastViewed, w.Dismissed, w.Bookmarked')
        ->Select('w.CountComments', '', 'CountCommentWatch')
        ->Join('UserDiscussion w', 'd.DiscussionID = w.DiscussionID and w.UserID = '.$UserID, 'left')
        //->BeginWhereGroup()
        //->Where('w.DateLastViewed', NULL)
        //->OrWhere('d.DateLastComment >', 'w.DateLastViewed')
        //->EndWhereGroup()
        ->Where('d.CountComments >', 'COALESCE(w.CountComments, 0)', TRUE, FALSE)
        ->OrOp()     
        ->BeginWhereGroup()     
        ->Where('w.DateLastViewed', NULL);
     if($Perms !== TRUE) {
        $this->SQL->WhereIn('d.CategoryID', $Perms);
     }
     $this->SQL
        ->EndWhereGroup();

I'm not sure if this is the best way but using "or" is always risky and so it has to be combined with a permission check.

@R-J
Copy link
Contributor Author

@R-J R-J commented on 1ab2895 Apr 28, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a look on it and tested it with different discussions (with and without comments, in public and private categories).
Made it to a pull request
Hope it is in time for 2.1 ;)

Please sign in to comment.