DEV: Introduce TopicsFilter#filter_tags method (#20839)

This change sets the ground work for allowing us to filter topics list
by tags in the following ways:

1. Filter for topics that matches all tags in a given set of tags
2. Filter for topics that matches any tags in a given set of tags
3. Exclude topics that matches all tags in a given set of tags
4. Exclude topics that matches any tags in a given set of tags

lib/topic_query.rb

@@ -799,7 +799,7 @@ class TopicQuery
           scope: result,
           guardian: @guardian,
           category_id: options[:category],
-        ).filter(status: options[:status])
+        ).filter_status(status: options[:status])
     end
 
     if (filter = (options[:filter] || options[:f])) && @user

lib/topics_filter.rb

@@ -7,14 +7,32 @@ class TopicsFilter
     @category = category_id.present? ? Category.find_by(id: category_id) : nil
   end
 
-  def filter(status: nil)
-    filter_status(@scope, status) if status
+  def filter_tags(tag_names:, match_all: true, exclude: false)
+    return @scope if !SiteSetting.tagging_enabled?
+    return @scope if tag_names.blank?
+
+    tag_ids =
+      DiscourseTagging
+        .filter_visible(Tag, @guardian)
+        .where_name(tag_names)
+        .pluck(:id, :target_tag_id)
+
+    tag_ids.flatten!
+    tag_ids.uniq!
+    tag_ids.compact!
+
+    return @scope.none if match_all && tag_ids.length != tag_names.length
+    return @scope if tag_ids.empty?
+
+    self.send(
+      "#{exclude ? "exclude" : "include"}_topics_with_#{match_all ? "all" : "any"}_tags",
+      tag_ids,
+    )
+
     @scope
   end
 
-  private
-
-  def filter_status(scope, status)
+  def filter_status(status:)
     case status
     when "open"
       @scope = @scope.where("NOT topics.closed AND NOT topics.archived")
@@ -31,5 +49,47 @@ class TopicsFilter
         @scope = @scope.unscope(where: :deleted_at).where("topics.deleted_at IS NOT NULL")
       end
     end
+
+    @scope
+  end
+
+  private
+
+  def exclude_topics_with_all_tags(tag_ids)
+    where_clause = []
+
+    tag_ids.each_with_index do |tag_id, index|
+      sql_alias = "tt#{index}"
+
+      @scope =
+        @scope.joins(
+          "LEFT JOIN topic_tags #{sql_alias} ON #{sql_alias}.topic_id = topics.id AND #{sql_alias}.tag_id = #{tag_id}",
+        )
+
+      where_clause << "#{sql_alias}.topic_id IS NULL"
+    end
+
+    @scope = @scope.where(where_clause.join(" OR "))
+  end
+
+  def exclude_topics_with_any_tags(tag_ids)
+    @scope =
+      @scope
+        .left_joins(:topic_tags)
+        .where("topic_tags.tag_id IS NULL OR topic_tags.tag_id NOT IN (?)", tag_ids)
+        .distinct(:id)
+  end
+
+  def include_topics_with_all_tags(tag_ids)
+    tag_ids.each_with_index do |tag_id, index|
+      @scope =
+        @scope.joins(
+          "INNER JOIN topic_tags tt#{index} ON tt#{index}.topic_id = topics.id AND tt#{index}.tag_id = #{tag_id}",
+        )
+    end
+  end
+
+  def include_topics_with_any_tags(tag_ids)
+    @scope = @scope.joins(:topic_tags).where("topic_tags.tag_id IN (?)", tag_ids).distinct(:id)
   end
 end

spec/lib/topics_filter_spec.rb

@@ -2,62 +2,177 @@
 
 RSpec.describe TopicsFilter do
   fab!(:admin) { Fabricate(:admin) }
-  fab!(:topic) { Fabricate(:topic) }
-  fab!(:closed_topic) { Fabricate(:topic, closed: true) }
-  fab!(:archived_topic) { Fabricate(:topic, archived: true) }
-  fab!(:deleted_topic_id) { Fabricate(:topic, deleted_at: Time.zone.now).id }
 
-  describe "#filter" do
-    it "should return all topics when input is blank" do
-      expect(TopicsFilter.new(guardian: Guardian.new).filter.pluck(:id)).to contain_exactly(
-        topic.id,
-        closed_topic.id,
-        archived_topic.id,
+  describe "#filter_status" do
+    fab!(:topic) { Fabricate(:topic) }
+    fab!(:closed_topic) { Fabricate(:topic, closed: true) }
+    fab!(:archived_topic) { Fabricate(:topic, archived: true) }
+    fab!(:deleted_topic_id) { Fabricate(:topic, deleted_at: Time.zone.now).id }
+
+    it "should only return topics that have not been closed or archived when status is `open`" do
+      expect(
+        TopicsFilter.new(guardian: Guardian.new).filter_status(status: "open").pluck(:id),
+      ).to contain_exactly(topic.id)
+    end
+
+    it "should only return topics that have been deleted when status is `deleted` and user can see deleted topics" do
+      expect(
+        TopicsFilter.new(guardian: Guardian.new(admin)).filter_status(status: "deleted").pluck(:id),
+      ).to contain_exactly(deleted_topic_id)
+    end
+
+    it "should status filter when status is `deleted` and user cannot see deleted topics" do
+      expect(
+        TopicsFilter.new(guardian: Guardian.new).filter_status(status: "deleted").pluck(:id),
+      ).to contain_exactly(topic.id, closed_topic.id, archived_topic.id)
+    end
+
+    it "should only return topics that have been archived when status is `archived`" do
+      expect(
+        TopicsFilter.new(guardian: Guardian.new).filter_status(status: "archived").pluck(:id),
+      ).to contain_exactly(archived_topic.id)
+    end
+
+    it "should only return topics that are visible when status is `listed`" do
+      Topic.update_all(visible: false)
+      topic.update!(visible: true)
+
+      expect(
+        TopicsFilter.new(guardian: Guardian.new).filter_status(status: "listed").pluck(:id),
+      ).to contain_exactly(topic.id)
+    end
+
+    it "should only return topics that are not visible when status is `unlisted`" do
+      Topic.update_all(visible: true)
+      topic.update!(visible: false)
+
+      expect(
+        TopicsFilter.new(guardian: Guardian.new).filter_status(status: "unlisted").pluck(:id),
+      ).to contain_exactly(topic.id)
+    end
+  end
+
+  describe "#filter_tags" do
+    fab!(:tag) { Fabricate(:tag) }
+    fab!(:tag2) { Fabricate(:tag) }
+
+    fab!(:group_only_tag) { Fabricate(:tag) }
+    fab!(:group) { Fabricate(:group) }
+
+    let!(:staff_tag_group) do
+      Fabricate(
+        :tag_group,
+        permissions: {
+          group.name => TagGroupPermission.permission_types[:full],
+        },
+        tag_names: [group_only_tag.name],
       )
     end
 
-    context "when filtering by topic's status" do
-      it "should only return topics that have not been closed or archived when status is `open`" do
-        expect(
-          TopicsFilter.new(guardian: Guardian.new).filter(status: "open").pluck(:id),
-        ).to contain_exactly(topic.id)
-      end
+    fab!(:topic_without_tag) { Fabricate(:topic) }
+    fab!(:topic_with_tag) { Fabricate(:topic, tags: [tag]) }
+    fab!(:topic_with_tag_and_tag2) { Fabricate(:topic, tags: [tag, tag2]) }
+    fab!(:topic_with_tag2) { Fabricate(:topic, tags: [tag2]) }
+    fab!(:topic_with_group_only_tag) { Fabricate(:topic, tags: [group_only_tag]) }
 
-      it "should only return topics that have been deleted when status is `deleted` and user can see deleted topics" do
-        expect(
-          TopicsFilter.new(guardian: Guardian.new(admin)).filter(status: "deleted").pluck(:id),
-        ).to contain_exactly(deleted_topic_id)
-      end
+    it "should not filter any topics by tags when tagging is disabled" do
+      SiteSetting.tagging_enabled = false
 
-      it "should status filter when status is `deleted` and user cannot see deleted topics" do
-        expect(
-          TopicsFilter.new(guardian: Guardian.new).filter(status: "deleted").pluck(:id),
-        ).to contain_exactly(topic.id, closed_topic.id, archived_topic.id)
-      end
+      expect(
+        TopicsFilter
+          .new(guardian: Guardian.new)
+          .filter_tags(tag_names: [tag.name, tag2.name], match_all: true, exclude: false)
+          .pluck(:id),
+      ).to contain_exactly(
+        topic_without_tag.id,
+        topic_with_tag.id,
+        topic_with_tag_and_tag2.id,
+        topic_with_tag2.id,
+        topic_with_group_only_tag.id,
+      )
+    end
 
-      it "should only return topics that have been archived when status is `archived`" do
-        expect(
-          TopicsFilter.new(guardian: Guardian.new).filter(status: "archived").pluck(:id),
-        ).to contain_exactly(archived_topic.id)
-      end
+    it "should only return topics that are tagged with all of the specified tags when `match_all` is `true`" do
+      expect(
+        TopicsFilter
+          .new(guardian: Guardian.new)
+          .filter_tags(tag_names: [tag.name, tag2.name], match_all: true, exclude: false)
+          .pluck(:id),
+      ).to contain_exactly(topic_with_tag_and_tag2.id)
+    end
 
-      it "should only return topics that are visible when status is `listed`" do
-        Topic.update_all(visible: false)
-        topic.update!(visible: true)
+    it "should only return topics that are tagged with any of the specified tags when `match_all` is `false`" do
+      expect(
+        TopicsFilter
+          .new(guardian: Guardian.new)
+          .filter_tags(tag_names: [tag2.name], match_all: false, exclude: false)
+          .pluck(:id),
+      ).to contain_exactly(topic_with_tag_and_tag2.id, topic_with_tag2.id)
+    end
 
-        expect(
-          TopicsFilter.new(guardian: Guardian.new).filter(status: "listed").pluck(:id),
-        ).to contain_exactly(topic.id)
-      end
+    it "should not return any topics when `match_all` is `true` and one of specified tags is invalid" do
+      expect(
+        TopicsFilter
+          .new(guardian: Guardian.new)
+          .filter_tags(tag_names: ["invalid", tag.name, tag2.name], match_all: true, exclude: false)
+          .pluck(:id),
+      ).to eq([])
+    end
 
-      it "should only return topics that are not visible when status is `unlisted`" do
-        Topic.update_all(visible: true)
-        topic.update!(visible: false)
+    it "should still filter topics by specificed tags when `match_all` is `false` even if one of the tags is invalid" do
+      expect(
+        TopicsFilter
+          .new(guardian: Guardian.new)
+          .filter_tags(
+            tag_names: ["invalid", tag.name, tag2.name],
+            match_all: false,
+            exclude: false,
+          )
+          .pluck(:id),
+      ).to contain_exactly(topic_with_tag_and_tag2.id, topic_with_tag.id, topic_with_tag2.id)
+    end
 
-        expect(
-          TopicsFilter.new(guardian: Guardian.new).filter(status: "unlisted").pluck(:id),
-        ).to contain_exactly(topic.id)
-      end
+    it "should not return any topics when user tries to filter topics by tags that are hidden" do
+      expect(
+        TopicsFilter
+          .new(guardian: Guardian.new)
+          .filter_tags(tag_names: [group_only_tag.name], match_all: true, exclude: false)
+          .pluck(:id),
+      ).to eq([])
+    end
+
+    it "should allow user with permission to filter topics by tags that are hidden" do
+      group.add(admin)
+
+      expect(
+        TopicsFilter
+          .new(guardian: Guardian.new(admin))
+          .filter_tags(tag_names: [group_only_tag.name])
+          .pluck(:id),
+      ).to contain_exactly(topic_with_group_only_tag.id)
+    end
+
+    it "should only return topics that are not tagged with all of the specified tags when `match_all` is `true` and `exclude` is `true`" do
+      expect(
+        TopicsFilter
+          .new(guardian: Guardian.new)
+          .filter_tags(tag_names: [tag.name, tag2.name], match_all: true, exclude: true)
+          .pluck(:id),
+      ).to contain_exactly(
+        topic_without_tag.id,
+        topic_with_tag.id,
+        topic_with_tag2.id,
+        topic_with_group_only_tag.id,
+      )
+    end
+
+    it "should only return topics that are not tagged with any of the specified tags when `match_all` is `false` and `exclude` is `true`" do
+      expect(
+        TopicsFilter
+          .new(guardian: Guardian.new)
+          .filter_tags(tag_names: [tag.name, tag2.name], match_all: false, exclude: true)
+          .pluck(:id),
+      ).to contain_exactly(topic_without_tag.id, topic_with_group_only_tag.id)
     end
   end
 end