FIX: staff-only tags visible on /tags page when restricted to a category

FIX: staff-only tags visible on /tags page when restricted to a category

If a tag group is set to only be visible to staff, and is restricted to a category that is visible by everyone, the tags in the group were being shown on the /tags page. They weren’t visible anywhere else. This commit fixes it so they don’t show on the /tags page.

diff --git a/app/models/tag_group.rb b/app/models/tag_group.rb
index 91018c5..124a487 100644
--- a/app/models/tag_group.rb
+++ b/app/models/tag_group.rb
@@ -68,14 +68,19 @@ class TagGroup < ActiveRecord::Base
     if guardian.is_staff?
       TagGroup
     else
+      # (
+      #   tag group is restricted to a category you can see
+      #   OR
+      #   tag group is not restricted to any categories
+      # )
+      # AND tag group can be seen by everyone
       filter_sql = <<~SQL
         (
           id IN (SELECT tag_group_id FROM category_tag_groups WHERE category_id IN (?))
-        ) OR (
+          OR
           id NOT IN (SELECT tag_group_id FROM category_tag_groups)
-          AND
-          id IN (SELECT tag_group_id FROM tag_group_permissions WHERE group_id = ?)
         )
+        AND id IN (SELECT tag_group_id FROM tag_group_permissions WHERE group_id = ?)
       SQL
 
       TagGroup.where(filter_sql, guardian.allowed_category_ids, Group::AUTO_GROUPS[:everyone])
diff --git a/spec/models/tag_group_spec.rb b/spec/models/tag_group_spec.rb
index b2fcd20..a5f1e57 100644
--- a/spec/models/tag_group_spec.rb
+++ b/spec/models/tag_group_spec.rb
@@ -52,24 +52,37 @@ describe TagGroup do
       staff_only_tag_group.save!
     end
 
-    it "returns correct groups based on category & tag group permissions" do
-      expect(TagGroup.visible(Guardian.new(admin)).pluck(:name)).to match_array(TagGroup.pluck(:name))
-      expect(TagGroup.visible(Guardian.new(moderator)).pluck(:name)).to match_array(TagGroup.pluck(:name))
-
-      expect(TagGroup.visible(Guardian.new(user2)).pluck(:name)).to match_array([
-        public_tag_group.name, unrestricted_tag_group.name, private_tag_group.name,
-        everyone_tag_group.name, visible_tag_group.name,
-      ])
-
-      expect(TagGroup.visible(Guardian.new(user1)).pluck(:name)).to match_array([
-        public_tag_group.name, unrestricted_tag_group.name, everyone_tag_group.name,
-        visible_tag_group.name,
-      ])
-
-      expect(TagGroup.visible(Guardian.new(nil)).pluck(:name)).to match_array([
-        public_tag_group.name, unrestricted_tag_group.name, everyone_tag_group.name,
-        visible_tag_group.name,
-      ])
+    shared_examples "correct visible tag groups" do
+      it "returns correct groups based on category & tag group permissions" do
+        expect(TagGroup.visible(Guardian.new(admin)).pluck(:name)).to match_array(TagGroup.pluck(:name))
+        expect(TagGroup.visible(Guardian.new(moderator)).pluck(:name)).to match_array(TagGroup.pluck(:name))
+
+        expect(TagGroup.visible(Guardian.new(user2)).pluck(:name)).to match_array([
+          public_tag_group.name, unrestricted_tag_group.name, private_tag_group.name,
+          everyone_tag_group.name, visible_tag_group.name,
+        ])
+
+        expect(TagGroup.visible(Guardian.new(user1)).pluck(:name)).to match_array([
+          public_tag_group.name, unrestricted_tag_group.name, everyone_tag_group.name,
+          visible_tag_group.name,
+        ])
+
+        expect(TagGroup.visible(Guardian.new(nil)).pluck(:name)).to match_array([
+          public_tag_group.name, unrestricted_tag_group.name, everyone_tag_group.name,
+          visible_tag_group.name,
+        ])
+      end
+    end
+
+    include_examples "correct visible tag groups"
+
+    context "staff-only tag group restricted to a public category" do
+      before do
+        public_category.allowed_tag_groups = [public_tag_group.name, staff_only_tag_group.name]
+        private_category.allowed_tag_groups = [private_tag_group.name, staff_only_tag_group.name]
+      end
+
+      include_examples "correct visible tag groups"
     end
   end
 end

GitHub sha: f8f7091e

1 Like

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