diff --git a/app/models/tag.rb b/app/models/tag.rb
index fabae29a1e8..5e3d6809205 100644
--- a/app/models/tag.rb
+++ b/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
diff --git a/lib/discourse_tagging.rb b/lib/discourse_tagging.rb
index 855b27f178f..044935f5bef 100644
--- a/lib/discourse_tagging.rb
+++ b/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
diff --git a/spec/models/tag_spec.rb b/spec/models/tag_spec.rb
index 22cda4a4b99..1a42a27cea0 100644
--- a/spec/models/tag_spec.rb
+++ b/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
diff --git a/spec/models/topic_list_spec.rb b/spec/models/topic_list_spec.rb
index baedb5c3561..38a13515d97 100644
--- a/spec/models/topic_list_spec.rb
+++ b/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