FIX: Recursively tag topics with missing ancestor tags (#18344)

* FIX: Recursively tag topics with missing ancestor tags

Given only a child tag, walk up the ancestry chain, get all of it's
ancestors for use in tagging a topic

* FIX: Ensure only one parent tag is returned for topic tagging

Current implementation selects and return first parent tag if child tag
has multiple parents.

This change updates recursive parent tag implementation to only return
parent tags via only one ancestry line.

* DEV: Add test case for tag cycles

Given we aren't performing a strict graph traversal to get a tag's
parent, cycles do not have any effect on the tags returned for topic
tagging.

lib/discourse_tagging.rb

@@ -13,6 +13,29 @@ 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) || []
@@ -93,14 +116,7 @@ module DiscourseTagging
         # add missing mandatory parent tags
         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 (?)
-        ", tag_ids).inject({}) do |h, v|
+        parent_tags_map = DB.query(ANCESTOR_TAG_IDS_SQL, tag_ids: tag_ids).inject({}) do |h, v|
           h[v.tag_id] ||= []
           h[v.tag_id] << v.parent_tag_id
           h

spec/lib/discourse_tagging_spec.rb

@@ -497,9 +497,19 @@ 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(
@@ -519,6 +529,49 @@ 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