FIX: Revert recursively tag lookup with missing ancestor tags (#18439)
This reverts commit 049f8569d8
.
To be revisited with a more comprehensive solution covering parent
selection when multiple parents exist.
This commit is contained in:
parent
58cc35fc78
commit
0c38757250
|
@ -13,29 +13,6 @@ module DiscourseTagging
|
||||||
ON tgm.tag_group_id = tg.id
|
ON tgm.tag_group_id = tg.id
|
||||||
SQL
|
SQL
|
||||||
|
|
||||||
ANCESTOR_TAG_IDS_SQL ||= <<~SQL
|
|
||||||
WITH RECURSIVE ancestors AS (
|
|
||||||
SELECT
|
|
||||||
tgm.tag_id,
|
|
||||||
tg.parent_tag_id
|
|
||||||
FROM
|
|
||||||
tag_group_memberships tgm
|
|
||||||
INNER JOIN tag_groups tg ON tg.id = tgm.tag_group_id
|
|
||||||
WHERE
|
|
||||||
tg.parent_tag_id IS NOT NULL
|
|
||||||
AND tgm.tag_id IN (:tag_ids)
|
|
||||||
UNION
|
|
||||||
SELECT
|
|
||||||
tgm.tag_id,
|
|
||||||
tg.parent_tag_id
|
|
||||||
FROM
|
|
||||||
tag_group_memberships tgm
|
|
||||||
INNER JOIN tag_groups tg ON tg.id = tgm.tag_group_id
|
|
||||||
INNER JOIN ancestors ON tgm.tag_id = ancestors.parent_tag_id
|
|
||||||
)
|
|
||||||
SELECT * FROM ancestors
|
|
||||||
SQL
|
|
||||||
|
|
||||||
def self.tag_topic_by_names(topic, guardian, tag_names_arg, append: false)
|
def self.tag_topic_by_names(topic, guardian, tag_names_arg, append: false)
|
||||||
if guardian.can_tag?(topic)
|
if guardian.can_tag?(topic)
|
||||||
tag_names = DiscourseTagging.tags_for_saving(tag_names_arg, guardian) || []
|
tag_names = DiscourseTagging.tags_for_saving(tag_names_arg, guardian) || []
|
||||||
|
@ -116,7 +93,14 @@ module DiscourseTagging
|
||||||
# add missing mandatory parent tags
|
# add missing mandatory parent tags
|
||||||
tag_ids = tags.map(&:id)
|
tag_ids = tags.map(&:id)
|
||||||
|
|
||||||
parent_tags_map = DB.query(ANCESTOR_TAG_IDS_SQL, tag_ids: tag_ids).inject({}) do |h, v|
|
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 (?)
|
||||||
|
", tag_ids).inject({}) do |h, v|
|
||||||
h[v.tag_id] ||= []
|
h[v.tag_id] ||= []
|
||||||
h[v.tag_id] << v.parent_tag_id
|
h[v.tag_id] << v.parent_tag_id
|
||||||
h
|
h
|
||||||
|
|
|
@ -497,19 +497,9 @@ RSpec.describe DiscourseTagging do
|
||||||
end
|
end
|
||||||
|
|
||||||
it "adds all parent tags that are missing" do
|
it "adds all parent tags that are missing" do
|
||||||
parent_tag_group = Fabricate(:tag_group)
|
|
||||||
parent_tag = Fabricate(:tag, name: 'parent')
|
parent_tag = Fabricate(:tag, name: 'parent')
|
||||||
parent_tag_group.tags = [parent_tag]
|
|
||||||
|
|
||||||
tag_group2 = Fabricate(:tag_group, parent_tag_id: parent_tag.id)
|
tag_group2 = Fabricate(:tag_group, parent_tag_id: parent_tag.id)
|
||||||
tag_group2.tags = [tag2]
|
tag_group2.tags = [tag2]
|
||||||
|
|
||||||
tag_group1 = Fabricate(:tag_group)
|
|
||||||
tag_group1.tags = [tag1]
|
|
||||||
|
|
||||||
tag_group3 = Fabricate(:tag_group, parent_tag_id: tag1.id)
|
|
||||||
tag_group3.tags = [tag3]
|
|
||||||
|
|
||||||
valid = DiscourseTagging.tag_topic_by_names(topic, Guardian.new(user), [tag3.name, tag2.name])
|
valid = DiscourseTagging.tag_topic_by_names(topic, Guardian.new(user), [tag3.name, tag2.name])
|
||||||
expect(valid).to eq(true)
|
expect(valid).to eq(true)
|
||||||
expect(topic.reload.tags.map(&:name)).to contain_exactly(
|
expect(topic.reload.tags.map(&:name)).to contain_exactly(
|
||||||
|
@ -529,49 +519,6 @@ RSpec.describe DiscourseTagging do
|
||||||
expect(valid).to eq(true)
|
expect(valid).to eq(true)
|
||||||
expect(topic.reload.tags.map(&:name)).to contain_exactly(*[parent_tag, common].map(&:name))
|
expect(topic.reload.tags.map(&:name)).to contain_exactly(*[parent_tag, common].map(&:name))
|
||||||
end
|
end
|
||||||
|
|
||||||
context "when tag group has grandparents" do
|
|
||||||
let(:tag_group1) { Fabricate(:tag_group) }
|
|
||||||
let(:foo) { Fabricate(:tag, name: 'foo') }
|
|
||||||
let(:tag_group2) { Fabricate(:tag_group, parent_tag_id: foo.id) }
|
|
||||||
let(:bar) { Fabricate(:tag, name: 'bar') }
|
|
||||||
let(:tag_group3) { Fabricate(:tag_group, parent_tag_id: bar.id) }
|
|
||||||
let(:baz) { Fabricate(:tag, name: 'baz') }
|
|
||||||
|
|
||||||
before do
|
|
||||||
tag_group1.tags = [foo]
|
|
||||||
tag_group2.tags = [bar]
|
|
||||||
tag_group3.tags = [baz]
|
|
||||||
end
|
|
||||||
|
|
||||||
it "recursively adds all ancestors" do
|
|
||||||
valid = DiscourseTagging.tag_topic_by_names(topic, Guardian.new(user), [baz.name])
|
|
||||||
expect(valid).to eq(true)
|
|
||||||
expect(topic.reload.tags.map(&:name)).to contain_exactly(*[foo, bar, baz].map(&:name))
|
|
||||||
end
|
|
||||||
|
|
||||||
it "adds only one parent when multiple parents exist" do
|
|
||||||
alt_parent_group = Fabricate(:tag_group)
|
|
||||||
alt = Fabricate(:tag, name: 'alt')
|
|
||||||
alt_parent_group.tags = [alt]
|
|
||||||
|
|
||||||
alt_baz_group = Fabricate(:tag_group, parent_tag_id: alt.id)
|
|
||||||
alt_baz_group.tags = [baz]
|
|
||||||
|
|
||||||
valid = DiscourseTagging.tag_topic_by_names(topic, Guardian.new(user), [baz.name])
|
|
||||||
expect(valid).to eq(true)
|
|
||||||
expect(topic.reload.tags.map(&:name)).to contain_exactly(*[foo, bar, baz].map(&:name))
|
|
||||||
end
|
|
||||||
|
|
||||||
it "adds missing tags even with cycles" do
|
|
||||||
tag_group4 = Fabricate(:tag_group, parent_tag_id: baz.id)
|
|
||||||
tag_group4.tags = [foo]
|
|
||||||
|
|
||||||
valid = DiscourseTagging.tag_topic_by_names(topic, Guardian.new(user), [baz.name])
|
|
||||||
expect(valid).to eq(true)
|
|
||||||
expect(topic.reload.tags.map(&:name)).to contain_exactly(*[foo, bar, baz].map(&:name))
|
|
||||||
end
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
|
|
||||||
context "when enforcing required tags from a tag group" do
|
context "when enforcing required tags from a tag group" do
|
||||||
|
|
Loading…
Reference in New Issue