FIX: New/unread count after dismissing new topics in a regular category (#8659)

6e1fe22 introduced the possiblity for category_users to have a NULL notification_level, so that we can store `last_seen_at` dates without locking the notification level. At the time, this did not affect the topic-tracking-state query. However, the query changes in f434de2 introduced a slight change in behavior.

Previously, a subquery would look for a category_user with notification_level=mute. f434de2 refactored this to remove the subquery, and inverted some of the logic to suit.

The new query checked for `notification_level <> :muted`. If `notification_level` is NULL, this comparison will return NULL. In this scenario, notification_level=NULL means that we should fall back to the default tracking level (regular), and so we want the expression to resolve as true, not false. There was already a check for the existence of the category_users row, but it did not check for the existence of a NOT NULL notification_level.

This commit amends the expression so that the notification_level will only be compared if it is non-null.
This commit is contained in:
David Taylor 2020-01-06 16:15:24 +00:00 committed by GitHub
parent ff151cb580
commit 5df815c2ee
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 14 additions and 1 deletions

View File

@ -306,7 +306,7 @@ class TopicTrackingState
#{tags_filter}
topics.deleted_at IS NULL AND
#{category_filter}
(category_users.id IS NULL OR
(category_users.notification_level IS NULL OR
last_read_post_number IS NOT NULL OR
category_users.notification_level <> #{CategoryUser.notification_levels[:muted]})
SQL

View File

@ -353,6 +353,19 @@ describe TopicTrackingState do
expect(report.length).to eq(1)
end
it "correctly handles category_users with null notification level" do
user = Fabricate(:user)
post
report = TopicTrackingState.report(user)
expect(report.length).to eq(1)
CategoryUser.create!(user_id: user.id, category_id: post.topic.category_id)
report = TopicTrackingState.report(user)
expect(report.length).to eq(1)
end
context 'muted tags' do
it "remove_muted_tags_from_latest is set to always" do
SiteSetting.remove_muted_tags_from_latest = 'always'