FIX: tag filter dropdown was gone if some tags were restricted to a category.

app/models/tag.rb

@@ -26,12 +26,7 @@ class Tag < ActiveRecord::Base
   def self.top_tags(limit_arg: nil, category: nil)
     limit = limit_arg || SiteSetting.max_tags_in_filter_list
 
-    tags =
-      if category
-        self.category_tags_by_count_query(category, limit: limit)
-      else
-        self.tags_by_count_query(limit: limit)
-      end
+    tags = DiscourseTagging.filter_allowed_tags(tags_by_count_query(limit: limit), nil, category: category)
 
     tags.count.map {|name, _| name}
   end

lib/discourse_tagging.rb

@@ -61,47 +61,46 @@ module DiscourseTagging
       query = query.where('tags.name like ?', "%#{term}%")
     end
 
+    # Filters for category-specific tags:
+    category = opts[:category]
+
+    if category && (category.tags.count > 0 || category.tag_groups.count > 0)
+      if category.tags.count > 0 && category.tag_groups.count > 0
+        tag_group_ids = category.tag_groups.pluck(:id)
+
+        query = query.where(
+          "tags.id IN (SELECT tag_id FROM category_tags WHERE category_id = ?
+            UNION
+            SELECT tag_id FROM tag_group_memberships WHERE tag_group_id IN (?))",
+          category.id, tag_group_ids
+        )
+      elsif category.tags.count > 0
+        query = query.where("tags.id IN (SELECT tag_id FROM category_tags WHERE category_id = ?)", category.id)
+      else # category.tag_groups.count > 0
+        tag_group_ids = category.tag_groups.pluck(:id)
+
+        query = query.where("tags.id IN (SELECT tag_id FROM tag_group_memberships WHERE tag_group_id IN (?))", tag_group_ids)
+      end
+    elsif opts[:for_input] || category
+      # exclude tags that are restricted to other categories
+      if CategoryTag.exists?
+        query = query.where("tags.id NOT IN (SELECT tag_id FROM category_tags)")
+      end
+
+      if CategoryTagGroup.exists?
+        tag_group_ids = CategoryTagGroup.pluck(:tag_group_id).uniq
+        query = query.where("tags.id NOT IN (SELECT tag_id FROM tag_group_memberships WHERE tag_group_id IN (?))", tag_group_ids)
+      end
+    end
+
     if opts[:for_input]
       selected_tag_ids = opts[:selected_tags] ? Tag.where(name: opts[:selected_tags]).pluck(:id) : []
 
-      unless guardian.is_staff?
+      unless guardian.nil? || guardian.is_staff?
         staff_tag_names = SiteSetting.staff_tags.split("|")
         query = query.where('tags.name NOT IN (?)', staff_tag_names) if staff_tag_names.present?
       end
 
-      # Filters for category-specific tags:
-
-      category = opts[:category]
-
-      if category && (category.tags.count > 0 || category.tag_groups.count > 0)
-        if category.tags.count > 0 && category.tag_groups.count > 0
-          tag_group_ids = category.tag_groups.pluck(:id)
-
-          query = query.where(
-            "tags.id IN (SELECT tag_id FROM category_tags WHERE category_id = ?
-              UNION
-              SELECT tag_id FROM tag_group_memberships WHERE tag_group_id IN (?))",
-            category.id, tag_group_ids
-          )
-        elsif category.tags.count > 0
-          query = query.where("tags.id IN (SELECT tag_id FROM category_tags WHERE category_id = ?)", category.id)
-        else # category.tag_groups.count > 0
-          tag_group_ids = category.tag_groups.pluck(:id)
-
-          query = query.where("tags.id IN (SELECT tag_id FROM tag_group_memberships WHERE tag_group_id IN (?))", tag_group_ids)
-        end
-      else
-        # exclude tags that are restricted to other categories
-        if CategoryTag.exists?
-          query = query.where("tags.id NOT IN (SELECT tag_id FROM category_tags)")
-        end
-
-        if CategoryTagGroup.exists?
-          tag_group_ids = CategoryTagGroup.pluck(:tag_group_id).uniq
-          query = query.where("tags.id NOT IN (SELECT tag_id FROM tag_group_memberships WHERE tag_group_id IN (?))", tag_group_ids)
-        end
-      end
-
       # exclude tag groups that have a parent tag which is missing from selected_tags
 
       select_sql = <<-SQL

spec/models/tag_spec.rb

@@ -1,6 +1,20 @@
 require 'rails_helper'
 
 describe Tag do
+  def make_some_tags(count: 3, tag_a_topic: false)
+    @tags = []
+    if tag_a_topic
+      count.times { |i| @tags << Fabricate(:tag, topics: [Fabricate(:topic)]) }
+    else
+      count.times { |i| @tags << Fabricate(:tag) }
+    end
+  end
+
+  before do
+    SiteSetting.tagging_enabled = true
+    SiteSetting.min_trust_level_to_tag_topics = 0
+  end
+
   describe '#tags_by_count_query' do
     it "returns empty hash if nothing is tagged" do
       expect(described_class.tags_by_count_query.count).to eq({})
@@ -9,9 +23,8 @@ describe Tag do
     context "with some tagged topics" do
       before do
         @topics = []
-        @tags = []
         3.times { @topics << Fabricate(:topic) }
-        2.times { @tags << Fabricate(:tag) }
+        make_some_tags(count: 2)
         @topics[0].tags << @tags[0]
         @topics[0].tags << @tags[1]
         @topics[1].tags << @tags[0]
@@ -29,4 +42,40 @@ describe Tag do
       end
     end
   end
+
+  describe '#top_tags' do
+    it "returns nothing if nothing has been tagged" do
+      make_some_tags(tag_a_topic: false)
+      expect(described_class.top_tags.sort).to be_empty
+    end
+
+    it "can return all tags" do
+      make_some_tags(tag_a_topic: true)
+      expect(described_class.top_tags.sort).to eq(@tags.map(&:name).sort)
+    end
+
+    context "with category-specific tags" do
+      before do
+        make_some_tags(count: 3)
+        @category1 = Fabricate(:category, tags: [@tags[0]]) # only one tag allowed in this category
+        @category2 = Fabricate(:category)
+        @topics = []
+        @topics << Fabricate(:topic, category: @category1, tags: [@tags[0]])
+        @topics << Fabricate(:topic, category: @category2, tags: [@tags[1], @tags[2]])
+        @topics << Fabricate(:topic, tags: [@tags[2]]) # uncategorized
+      end
+
+      it "for category with restricted tags, lists those tags" do
+        expect(described_class.top_tags(category: @category1)).to eq([@tags[0].name])
+      end
+
+      it "for category without tags, lists allowed tags" do
+        expect(described_class.top_tags(category: @category2).sort).to eq([@tags[1].name, @tags[2].name].sort)
+      end
+
+      it "for no category arg, lists all tags" do
+        expect(described_class.top_tags.sort).to eq([@tags[0].name, @tags[1].name, @tags[2].name].sort)
+      end
+    end
+  end
 end

spec/models/topic_list_spec.rb

@@ -38,25 +38,34 @@ describe TopicList do
   end
 
   describe '#tags' do
-    let(:tag) { Fabricate(:tag, topics: [topic]) }
-    let(:other_tag) { Fabricate(:tag, topics: [topic]) }
-
     it 'should return the right tags' do
+      tag = Fabricate(:tag, topics: [topic])
+      other_tag = Fabricate(:tag, topics: [topic], name: "use-anywhere")
       output = [tag.name, other_tag.name]
       expect(topic_list.tags.sort).to eq(output.sort)
     end
 
-    describe 'when topic list is filtered by category' do
-      let(:category) { Fabricate(:category) }
-      let(:topic) { Fabricate(:topic, category: category) }
-      let(:tag) { Fabricate(:tag, topics: [topic], categories: [category]) }
+    describe 'when there are tags restricted to a category' do
+      let!(:category) { Fabricate(:category) }
+      let!(:topic) { Fabricate(:topic, category: category) }
+      let!(:other_topic) { Fabricate(:topic) } # uncategorized
+      let!(:tag) { Fabricate(:tag, topics: [topic], categories: [category], name: "category-tag") }
+      let!(:other_tag) { Fabricate(:tag, topics: [topic], name: "use-anywhere") }
       let(:topic_list) { TopicList.new('latest', topic.user, [topic], { category: category.id, category_id: category.id }) }
 
       it 'should only return tags allowed in the category' do
-        other_tag
-        output = [tag.name]
+        expect(topic_list.tags).to eq([tag.name])
+      end
 
-        expect(topic_list.tags).to eq(output)
+      it "with no category, should return all tags" do
+        expect(TopicList.new('latest', other_topic.user, [other_topic]).tags.sort).to eq([tag.name, other_tag.name].sort)
+      end
+
+      it "with another category with no tags, should return exclude tags restricted to other categories" do
+        other_category = Fabricate(:category)
+        topic3 = Fabricate(:topic, category: other_category)
+        list = TopicList.new('latest', topic3.user, [topic3], { category: other_category.id, category_id: other_category.id })
+        expect(list.tags).to eq([other_tag.name])
       end
     end
   end