mirror of
https://github.com/discourse/discourse.git
synced 2024-11-27 03:10:46 -06:00
FIX: tag cannot be used if it belongs to two tag groups with parent tag
If two tag groups exist with a mandatory parent tag, and one tag is added to both tag groups, then the tag couldn't be used on any topics.
This commit is contained in:
parent
61f6a6e836
commit
5ef49692e0
@ -65,16 +65,27 @@ module DiscourseTagging
|
||||
end
|
||||
|
||||
# add missing mandatory parent tags
|
||||
parent_tags = TagGroup.includes(:parent_tag).where("tag_groups.id IN (
|
||||
SELECT tg.id
|
||||
tag_ids = tags.map(&:id)
|
||||
|
||||
parent_tags_map = DB.query("
|
||||
SELECT tgm.tag_id, tg.parent_tag_id
|
||||
FROM tag_groups tg
|
||||
INNER JOIN tag_group_memberships tgm
|
||||
ON tgm.tag_group_id = tg.id
|
||||
WHERE tg.parent_tag_id IS NOT NULL
|
||||
AND tgm.tag_id IN (?))", tags.map(&:id)).map(&:parent_tag)
|
||||
AND tgm.tag_id IN (?)
|
||||
", tag_ids).inject({}) do |h, v|
|
||||
h[v.tag_id] ||= []
|
||||
h[v.tag_id] << v.parent_tag_id
|
||||
h
|
||||
end
|
||||
|
||||
parent_tags.reject { |t| tag_names.include?(t.name) }.each do |tag|
|
||||
tags << tag
|
||||
missing_parent_tag_ids = parent_tags_map.map do |_, parent_tag_ids|
|
||||
(tag_ids & parent_tag_ids).size == 0 ? parent_tag_ids.first : nil
|
||||
end.compact.uniq
|
||||
|
||||
unless missing_parent_tag_ids.empty?
|
||||
tags = tags + Tag.where(id: missing_parent_tag_ids).all
|
||||
end
|
||||
|
||||
# validate minimum required tags for a category
|
||||
@ -187,8 +198,18 @@ module DiscourseTagging
|
||||
exclude_group_ids = one_per_topic_group_ids(selected_tag_ids)
|
||||
|
||||
if exclude_group_ids.empty?
|
||||
sql = "tags.id NOT IN (#{TAG_GROUP_TAG_IDS_SQL} WHERE tg.parent_tag_id NOT IN (?))"
|
||||
query = query.where(sql, selected_tag_ids)
|
||||
# tags that don't belong to groups that require a parent tag,
|
||||
# and tags that belong to groups with parent tag selected
|
||||
query = query.where(<<~SQL, selected_tag_ids, selected_tag_ids)
|
||||
tags.id NOT IN (
|
||||
#{TAG_GROUP_TAG_IDS_SQL}
|
||||
WHERE tg.parent_tag_id NOT IN (?)
|
||||
)
|
||||
OR tags.id IN (
|
||||
#{TAG_GROUP_TAG_IDS_SQL}
|
||||
WHERE tg.parent_tag_id IN (?)
|
||||
)
|
||||
SQL
|
||||
else
|
||||
# It's possible that the selected tags violate some one-tag-per-group restrictions,
|
||||
# so filter them out by picking one from each group.
|
||||
|
@ -194,9 +194,9 @@ describe DiscourseTagging do
|
||||
context 'tag group with parent tag' do
|
||||
let(:topic) { Fabricate(:topic, user: user) }
|
||||
let(:post) { Fabricate(:post, user: user, topic: topic, post_number: 1) }
|
||||
let(:tag_group) { Fabricate(:tag_group, parent_tag_id: tag1.id) }
|
||||
|
||||
before do
|
||||
tag_group = Fabricate(:tag_group, parent_tag_id: tag1.id)
|
||||
tag_group.tags = [tag3]
|
||||
end
|
||||
|
||||
@ -222,6 +222,19 @@ describe DiscourseTagging do
|
||||
*[tag1, tag2, tag3, parent_tag].map(&:name)
|
||||
)
|
||||
end
|
||||
|
||||
it "adds only the necessary parent tags" do
|
||||
common = Fabricate(:tag, name: 'common')
|
||||
tag_group.tags = [tag3, common]
|
||||
|
||||
parent_tag = Fabricate(:tag, name: 'parent')
|
||||
tag_group2 = Fabricate(:tag_group, parent_tag_id: parent_tag.id)
|
||||
tag_group2.tags = [tag2, common]
|
||||
|
||||
valid = DiscourseTagging.tag_topic_by_names(topic, Guardian.new(user), [parent_tag.name, common.name])
|
||||
expect(valid).to eq(true)
|
||||
expect(topic.reload.tags.map(&:name)).to contain_exactly(*[parent_tag, common].map(&:name))
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -212,6 +212,23 @@ describe "category tag restrictions" do
|
||||
expect(filter_allowed_tags(for_topic: true, selected_tags: [tag1.name, tag3.name])).to contain_exactly(tag1, tag2, tag3, tag4)
|
||||
end
|
||||
|
||||
it "filter_allowed_tags returns tags common to more than one tag group with parent tag" do
|
||||
common = Fabricate(:tag, name: 'common')
|
||||
tag_group = Fabricate(:tag_group, parent_tag_id: tag1.id)
|
||||
tag_group.tags = [tag2, common]
|
||||
tag_group = Fabricate(:tag_group, parent_tag_id: tag3.id)
|
||||
|
||||
tag_group.tags = [tag4]
|
||||
expect(filter_allowed_tags(for_input: true)).to contain_exactly(tag1, tag3)
|
||||
expect(filter_allowed_tags(for_input: true, selected_tags: [tag1.name])).to contain_exactly(tag2, tag3, common)
|
||||
expect(filter_allowed_tags(for_input: true, selected_tags: [tag3.name])).to contain_exactly(tag4, tag1)
|
||||
|
||||
tag_group.tags = [tag4, common]
|
||||
expect(filter_allowed_tags(for_input: true)).to contain_exactly(tag1, tag3)
|
||||
expect(filter_allowed_tags(for_input: true, selected_tags: [tag1.name])).to contain_exactly(tag2, tag3, common)
|
||||
expect(filter_allowed_tags(for_input: true, selected_tags: [tag3.name])).to contain_exactly(tag4, tag1, common)
|
||||
end
|
||||
|
||||
context "and category restrictions" do
|
||||
fab!(:car_category) { Fabricate(:category) }
|
||||
fab!(:other_category) { Fabricate(:category) }
|
||||
|
Loading…
Reference in New Issue
Block a user