FIX: Topics with muted tag didn't show up when filtering by category and tag

FIX: Topics with muted tag didn’t show up when filtering by category and tag

It also removes the redundant filter parameter. Previously URLs looked like this:

http://example.com/tags/c/some-category/muted-tag/l/latest.json?filter=tags/c/some-category/muted-tag/l/latest

But it looks like the filter parameter was only used to find out if topics with a muted tag should be removed or not. But the same thing can be accomplished by using the first tag ID. The following URL looks a lot cleaner.

http://example.com/tags/c/some-category/muted-tag/l/latest.json
diff --git a/app/assets/javascripts/discourse/routes/tags-show.js.es6 b/app/assets/javascripts/discourse/routes/tags-show.js.es6
index 49a4231..3a10287 100644
--- a/app/assets/javascripts/discourse/routes/tags-show.js.es6
+++ b/app/assets/javascripts/discourse/routes/tags-show.js.es6
@@ -73,8 +73,9 @@ export default Discourse.Route.extend({
     const params = filterQueryParams(transition.to.queryParams, {});
     const categorySlug = this.categorySlug;
     const parentCategorySlug = this.parentCategorySlug;
-    const filter = this.navMode;
+    const topicFilter = this.navMode;
     const tagId = tag ? tag.id.toLowerCase() : "none";
+    let filter;
 
     if (categorySlug) {
       const category = Discourse.Category.findBySlug(
@@ -82,31 +83,25 @@ export default Discourse.Route.extend({
         parentCategorySlug
       );
       if (parentCategorySlug) {
-        params.filter = `tags/c/${parentCategorySlug}/${categorySlug}/${tagId}/l/${filter}`;
+        filter = `tags/c/${parentCategorySlug}/${categorySlug}/${tagId}/l/${topicFilter}`;
       } else {
-        params.filter = `tags/c/${categorySlug}/${tagId}/l/${filter}`;
+        filter = `tags/c/${categorySlug}/${tagId}/l/${topicFilter}`;
       }
       if (category) {
         category.setupGroupsAndPermissions();
         this.set("category", category);
       }
     } else if (this.additionalTags) {
-      params.filter = `tags/intersection/${tagId}/${this.get(
-        "additionalTags"
-      ).join("/")}`;
+      filter = `tags/intersection/${tagId}/${this.additionalTags.join("/")}`;
       this.set("category", null);
     } else {
-      params.filter = `tags/${tagId}/l/${filter}`;
+      filter = `tags/${tagId}/l/${topicFilter}`;
       this.set("category", null);
     }
 
-    return findTopicList(
-      this.store,
-      this.topicTrackingState,
-      params.filter,
-      params,
-      { cached: true }
-    ).then(list => {
+    return findTopicList(this.store, this.topicTrackingState, filter, params, {
+      cached: true
+    }).then(list => {
       if (list.topic_list.tags && list.topic_list.tags.length === 1) {
         // Update name of tag (case might be different)
         tag.setProperties({
diff --git a/lib/topic_query.rb b/lib/topic_query.rb
index 51d60f5..06a89f4 100644
--- a/lib/topic_query.rb
+++ b/lib/topic_query.rb
@@ -863,6 +863,7 @@ class TopicQuery
 
     list
   end
+
   def remove_muted_categories(list, user, opts = nil)
     category_id = get_category_id(opts[:exclude]) if opts
 
@@ -885,6 +886,7 @@ class TopicQuery
 
     list
   end
+
   def remove_muted_tags(list, user, opts = nil)
     if user.nil? || !SiteSetting.tagging_enabled || SiteSetting.remove_muted_tags_from_latest == 'never'
       return list
@@ -895,16 +897,9 @@ class TopicQuery
       return list
     end
 
-    showing_tag = if opts[:filter]
-      f = opts[:filter].split('/')
-      f[0] == 'tags' ? f[1] : nil
-    else
-      nil
-    end
-
     # if viewing the topic list for a muted tag, show all the topics
-    if showing_tag.present? && TagUser.lookup(user, :muted).joins(:tag).where('tags.name = ?', showing_tag).exists?
-      return list
+    if !opts[:no_tags] && opts[:tags].present?
+      return list if TagUser.lookup(user, :muted).joins(:tag).where('tags.name = ?', opts[:tags].first).exists?
     end
 
     if SiteSetting.remove_muted_tags_from_latest == 'always'
diff --git a/spec/requests/tags_controller_spec.rb b/spec/requests/tags_controller_spec.rb
index ea96873..26bf959 100644
--- a/spec/requests/tags_controller_spec.rb
+++ b/spec/requests/tags_controller_spec.rb
@@ -207,6 +207,11 @@ describe TagsController do
     end
 
     context 'tagging enabled' do
+      def parse_topic_ids
+        JSON.parse(response.body)["topic_list"]["topics"]
+          .map { |topic| topic["id"] }
+      end
+
       it "can filter by tag" do
         get "/tags/#{tag.name}/l/latest.json"
         expect(response.status).to eq(200)
@@ -221,9 +226,7 @@ describe TagsController do
 
         expect(response.status).to eq(200)
 
-        topic_ids = JSON.parse(response.body)["topic_list"]["topics"]
-          .map { |topic| topic["id"] }
-
+        topic_ids = parse_topic_ids
         expect(topic_ids).to include(all_tag_topic.id)
         expect(topic_ids).to include(multi_tag_topic.id)
         expect(topic_ids).to_not include(single_tag_topic.id)
@@ -238,9 +241,7 @@ describe TagsController do
 
         expect(response.status).to eq(200)
 
-        topic_ids = JSON.parse(response.body)["topic_list"]["topics"]
-          .map { |topic| topic["id"] }
-
+        topic_ids = parse_topic_ids
         expect(topic_ids).to include(all_tag_topic.id)
         expect(topic_ids).to_not include(multi_tag_topic.id)
         expect(topic_ids).to_not include(single_tag_topic.id)
@@ -255,9 +256,7 @@ describe TagsController do
 
         expect(response.status).to eq(200)
 
-        topic_ids = JSON.parse(response.body)["topic_list"]["topics"]
-          .map { |topic| topic["id"] }
-
+        topic_ids = parse_topic_ids
         expect(topic_ids).to_not include(single_tag_topic.id)
       end
 
@@ -288,17 +287,53 @@ describe TagsController do
 
         expect(response.status).to eq(200)
 
-        topic_ids = JSON.parse(response.body)["topic_list"]["topics"]
-          .map { |topic| topic["id"] }
-
+        topic_ids = parse_topic_ids
         expect(topic_ids).to include(t.id)
       end
 
-      it "can filter by bookmarked" do
-        sign_in(Fabricate(:user))
-        get "/tags/#{tag.name}/l/bookmarks.json"
+      context "when logged in" do
+        fab!(:user) { Fabricate(:user) }
 
-        expect(response.status).to eq(200)
+        before do
+          sign_in(user)
+        end
+
+        it "can filter by bookmarked" do
+          get "/tags/#{tag.name}/l/bookmarks.json"
+
+          expect(response.status).to eq(200)
+        end
+
+        context "muted tags" do
+          before do
+            TagUser.create!(
+              user_id: user.id,
+              tag_id: tag.id,
+              notification_level: CategoryUser.notification_levels[:muted]
+            )
+          end
+
+          it "includes topics when filtered by muted tag" do
+            single_tag_topic
+
+            get "/tags/#{tag.name}/l/latest.json"
+            expect(response.status).to eq(200)
+
+            topic_ids = parse_topic_ids
+            expect(topic_ids).to include(single_tag_topic.id)
+          end
+
+          it "includes topics when filtered by category and muted tag" do
+            category = Fabricate(:category)
+            single_tag_topic.update!(category: category)
+
+            get "/tags/c/#{category.slug}/#{tag.name}/l/latest.json"
+            expect(response.status).to eq(200)
+
+            topic_ids = parse_topic_ids
+            expect(topic_ids).to include(single_tag_topic.id)
+          end
+        end
       end
     end
   end

GitHub sha: 63131562

1 Like