FIX: muted tags removed topics with no tags from counts

FIX: muted tags removed topics with no tags from counts

We previously did not account for completely untagged topics when looking at muted tags, this caused new/unread counts to be off if

  1. You had muted tags
  2. You had an unread/new topic
  3. This topic had no tags
diff --git a/app/models/topic_tracking_state.rb b/app/models/topic_tracking_state.rb
index eb92667..867961e 100644
--- a/app/models/topic_tracking_state.rb
+++ b/app/models/topic_tracking_state.rb
@@ -315,18 +315,26 @@ class TopicTrackingState
         "(topics.visible #{append}) AND"
       end
 
-    tags_filter =
-      if opts[:muted_tag_ids].present? && SiteSetting.remove_muted_tags_from_latest == 'always'
-        <<~SQL
-          NOT ((select array_agg(tag_id) from topic_tags where topic_tags.topic_id = topics.id) && ARRAY[#{opts[:muted_tag_ids].join(',')}]) AND
+    tags_filter = ""
+
+    if (muted_tag_ids = opts[:muted_tag_ids]).present? && ['always', 'only_muted'].include?(SiteSetting.remove_muted_tags_from_latest)
+      existing_tags_sql = "(select array_agg(tag_id) from topic_tags where topic_tags.topic_id = topics.id)"
+      muted_tags_array_sql = "ARRAY[#{opts[:muted_tag_ids].join(',')}]"
+
+      if SiteSetting.remove_muted_tags_from_latest == 'always'
+        tags_filter = <<~SQL
+          NOT (
+            COALESCE(#{existing_tags_sql}, ARRAY[]::int[]) && #{muted_tags_array_sql}
+          ) AND
         SQL
-      elsif opts[:muted_tag_ids].present? && SiteSetting.remove_muted_tags_from_latest == 'only_muted'
-        <<~SQL
-          NOT ((select array_agg(tag_id) from topic_tags where topic_tags.topic_id = topics.id) <@ ARRAY[#{opts[:muted_tag_ids].join(',')}]) AND
+      else # only muted
+        tags_filter = <<~SQL
+          NOT (
+            COALESCE(#{existing_tags_sql}, ARRAY[-999]) <@ #{muted_tags_array_sql}
+          ) AND
         SQL
-      else
-        ""
       end
+    end
 
     sql = +<<~SQL
     SELECT #{select}
diff --git a/spec/models/topic_tracking_state_spec.rb b/spec/models/topic_tracking_state_spec.rb
index 3ac5282..ae0f852 100644
--- a/spec/models/topic_tracking_state_spec.rb
+++ b/spec/models/topic_tracking_state_spec.rb
@@ -446,6 +446,11 @@ describe TopicTrackingState do
 
       report = TopicTrackingState.report(user)
       expect(report.length).to eq(0)
+
+      TopicTag.where(topic_id: topic.id).delete_all
+
+      report = TopicTrackingState.report(user)
+      expect(report.length).to eq(1)
     end
 
     it "remove_muted_tags_from_latest is set to only_muted" do
@@ -475,6 +480,11 @@ describe TopicTrackingState do
 
       report = TopicTrackingState.report(user)
       expect(report.length).to eq(0)
+
+      TopicTag.where(topic_id: topic.id).delete_all
+
+      report = TopicTrackingState.report(user)
+      expect(report.length).to eq(1)
     end
 
     it "remove_muted_tags_from_latest is set to never" do

GitHub sha: 2acec437

1 Like

This commit has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/notification-counts-are-inconsistent-and-wrong/151092/20