From 5df815c2ee0c35b023e330dd42a97e19f581ac7b Mon Sep 17 00:00:00 2001 From: David Taylor Date: Mon, 6 Jan 2020 16:15:24 +0000 Subject: [PATCH] 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. --- app/models/topic_tracking_state.rb | 2 +- spec/models/topic_tracking_state_spec.rb | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/app/models/topic_tracking_state.rb b/app/models/topic_tracking_state.rb index 9a5fb66a3b5..aa65917c6ea 100644 --- a/app/models/topic_tracking_state.rb +++ b/app/models/topic_tracking_state.rb @@ -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 diff --git a/spec/models/topic_tracking_state_spec.rb b/spec/models/topic_tracking_state_spec.rb index 5f67317ff10..1691547ff46 100644 --- a/spec/models/topic_tracking_state_spec.rb +++ b/spec/models/topic_tracking_state_spec.rb @@ -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'