diff --git a/lib/discourse_tagging.rb b/lib/discourse_tagging.rb index e3832fd0e12..8e64bab2a56 100644 --- a/lib/discourse_tagging.rb +++ b/lib/discourse_tagging.rb @@ -13,29 +13,6 @@ module DiscourseTagging ON tgm.tag_group_id = tg.id 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) if guardian.can_tag?(topic) tag_names = DiscourseTagging.tags_for_saving(tag_names_arg, guardian) || [] @@ -116,7 +93,14 @@ module DiscourseTagging # add missing mandatory parent tags 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] << v.parent_tag_id h diff --git a/spec/lib/discourse_tagging_spec.rb b/spec/lib/discourse_tagging_spec.rb index 548489c31b7..ddcd88916d7 100644 --- a/spec/lib/discourse_tagging_spec.rb +++ b/spec/lib/discourse_tagging_spec.rb @@ -497,19 +497,9 @@ RSpec.describe DiscourseTagging do end it "adds all parent tags that are missing" do - parent_tag_group = Fabricate(:tag_group) 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.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]) expect(valid).to eq(true) expect(topic.reload.tags.map(&:name)).to contain_exactly( @@ -529,49 +519,6 @@ RSpec.describe DiscourseTagging do expect(valid).to eq(true) expect(topic.reload.tags.map(&:name)).to contain_exactly(*[parent_tag, common].map(&:name)) 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 context "when enforcing required tags from a tag group" do