DEV: Update experimental /filter route with tags support (#20874)

The following are the changes being introduced in this commit:

1. Instead of mapping the query language to various query params on the
client side, we've decided that the benefits of having a more robust
query language far outweighs the benefits of having a more human readable query params in the URL.
As such, the `/filter` route will just accept a single `q` query param
and the query string will be parsed on the server side.

1. On the `/filter` route, the tags filtering query language is now
   supported in the input per the example provided below:

   ```
   tags:bug+feature tagged both bug and feature
   tags:bug,feature tagged either bug or feature
   -tags:bug+feature excluding topics tagged bug and feature
   -tags:bug,feature excluding topics tagged bug or feature
   ```

   The `tags` filter can also be specified multiple
times in the query string like so `tags:bug tags:feature` which will
filter topics that contain both the `bug` tag and `feature` tag. More
complex query like `tags:bug+feature -tags:experimental` will also work.

app/assets/javascripts/discourse/app/controllers/discovery-filter.js

@@ -3,31 +3,12 @@ import { action } from "@ember/object";
 import { tracked } from "@glimmer/tracking";
 
 export default class extends Controller {
-  @tracked status = "";
+  @tracked q = "";
 
-  queryParams = ["status"];
+  queryParams = ["q"];
-
-  get queryString() {
-    let paramStrings = [];
-
-    this.queryParams.forEach((key) => {
-      if (this[key]) {
-        paramStrings.push(`${key}:${this[key]}`);
-      }
-    });
-
-    return paramStrings.join(" ");
-  }
 
   @action
   updateTopicsListQueryParams(queryString) {
-    for (const match of queryString.matchAll(/(\w+):([^:\s]+)/g)) {
+    this.q = queryString;
-      const key = match[1];
-      const value = match[2];
-
-      if (this.queryParams.includes(key)) {
-        this.set(key, value);
-      }
-    }
   }
 }

app/assets/javascripts/discourse/app/controllers/navigation/filter.js

@@ -7,6 +7,6 @@ export default class extends Controller {
 
   constructor() {
     super(...arguments);
-    this.queryString = this.discoveryFilter.queryString;
+    this.queryString = this.discoveryFilter.q;
   }
 }

app/assets/javascripts/discourse/app/routes/discovery-filter.js

@@ -1,18 +1,17 @@
 import I18n from "I18n";
 
 import DiscourseRoute from "discourse/routes/discourse";
-import { isEmpty } from "@ember/utils";
 import { action } from "@ember/object";
 
 export default class extends DiscourseRoute {
   queryParams = {
-    status: { replace: true, refreshModel: true },
+    q: { replace: true, refreshModel: true },
   };
 
   model(data) {
     return this.store.findFiltered("topicList", {
       filter: "filter",
-      params: this.#filterQueryParams(data),
+      params: { q: data.q },
     });
   }
 
@@ -38,16 +37,4 @@ export default class extends DiscourseRoute {
   // Figure out a way to remove this.
   @action
   changeSort() {}
-
-  #filterQueryParams(data) {
-    const params = {};
-
-    Object.keys(this.queryParams).forEach((key) => {
-      if (!isEmpty(data[key])) {
-        params[key] = data[key];
-      }
-    });
-
-    return params;
-  }
 }

app/controllers/list_controller.rb

@@ -83,6 +83,7 @@ class ListController < ApplicationController
 
       list.more_topics_url = construct_url_with(:next, list_opts)
       list.prev_topics_url = construct_url_with(:prev, list_opts)
+
       if Discourse.anonymous_filters.include?(filter)
         @description = SiteSetting.site_description
         @rss = filter
@@ -91,12 +92,14 @@ class ListController < ApplicationController
         # Note the first is the default and we don't add a title
         if (filter.to_s != current_homepage) && use_crawler_layout?
           filter_title = I18n.t("js.filters.#{filter.to_s}.title", count: 0)
+
           if list_opts[:category] && @category
             @title =
               I18n.t("js.filters.with_category", filter: filter_title, category: @category.name)
           else
             @title = I18n.t("js.filters.with_topics", filter: filter_title)
           end
+
           @title << " - #{SiteSetting.title}"
         elsif @category.blank? && (filter.to_s == current_homepage) &&
               SiteSetting.short_site_description.present?
@@ -119,7 +122,23 @@ class ListController < ApplicationController
 
   def filter
     raise Discourse::NotFound if !SiteSetting.experimental_topics_filter
-    latest
+
+    topic_query_opts = { no_definitions: !SiteSetting.show_category_definitions_in_topic_lists }
+
+    %i[page q].each do |key|
+      if params.key?(key.to_s)
+        value = params[key]
+        raise Discourse::InvalidParameters.new(key) if !TopicQuery.validate?(key, value)
+        topic_query_opts[key] = value
+      end
+    end
+
+    user = list_target_user
+    list = TopicQuery.new(user, topic_query_opts).list_filter
+    list.more_topics_url = construct_url_with(:next, topic_query_opts)
+    list.prev_topics_url = construct_url_with(:prev, topic_query_opts)
+
+    respond_with_list(list)
   end
 
   def category_default

lib/topic_query.rb

@@ -269,7 +269,13 @@ class TopicQuery
   end
 
   def list_filter
-    list_latest
+    create_list(
+      :filter,
+      {},
+      TopicsFilter.new(guardian: @guardian, scope: latest_results).filter_from_query_string(
+        @options[:q],
+      ),
+    )
   end
 
   def list_read
@@ -796,11 +802,10 @@ class TopicQuery
 
     if status = options[:status]
       result =
-        TopicsFilter.new(
+        TopicsFilter.new(scope: result, guardian: @guardian).filter_status(
-          scope: result,
+          status: options[:status],
-          guardian: @guardian,
           category_id: options[:category],
-        ).filter_status(status: options[:status])
+        )
     end
 
     if (filter = (options[:filter] || options[:f])) && @user

lib/topics_filter.rb

@@ -1,12 +1,63 @@
 # frozen_string_literal: true
 
 class TopicsFilter
-  def initialize(guardian:, scope: Topic, category_id: nil)
+  def initialize(guardian:, scope: Topic)
     @guardian = guardian
     @scope = scope
-    @category = category_id.present? ? Category.find_by(id: category_id) : nil
   end
 
+  def filter_from_query_string(query_string)
+    return @scope if query_string.blank?
+
+    query_string.scan(/(?<exclude>-)?(?<key>\w+):(?<value>[^:\s]+)/) do |exclude, key, value|
+      case key
+      when "status"
+        @scope = filter_status(status: value)
+      when "tags"
+        value.scan(
+          /^(?<tags>([a-zA-Z0-9\-]+)(?<delimiter>[,+])?([a-zA-Z0-9\-]+)?(\k<delimiter>[a-zA-Z0-9\-]+)*)$/,
+        ) do |value, delimiter|
+          match_all =
+            if delimiter == ","
+              false
+            else
+              true
+            end
+
+          @scope =
+            filter_tags(tag_names: value.split(delimiter), exclude: exclude, match_all: match_all)
+        end
+      end
+    end
+
+    @scope
+  end
+
+  def filter_status(status:, category_id: nil)
+    case status
+    when "open"
+      @scope = @scope.where("NOT topics.closed AND NOT topics.archived")
+    when "closed"
+      @scope = @scope.where("topics.closed")
+    when "archived"
+      @scope = @scope.where("topics.archived")
+    when "listed"
+      @scope = @scope.where("topics.visible")
+    when "unlisted"
+      @scope = @scope.where("NOT topics.visible")
+    when "deleted"
+      category = category_id.present? ? Category.find_by(id: category_id) : nil
+
+      if @guardian.can_see_deleted_topics?(category)
+        @scope = @scope.unscope(where: :deleted_at).where("topics.deleted_at IS NOT NULL")
+      end
+    end
+
+    @scope
+  end
+
+  private
+
   def filter_tags(tag_names:, match_all: true, exclude: false)
     return @scope if !SiteSetting.tagging_enabled?
     return @scope if tag_names.blank?
@@ -32,34 +83,16 @@ class TopicsFilter
     @scope
   end
 
-  def filter_status(status:)
+  def topic_tags_alias
-    case status
+    @topic_tags_alias ||= 0
-    when "open"
+    "tt#{@topic_tags_alias += 1}"
-      @scope = @scope.where("NOT topics.closed AND NOT topics.archived")
-    when "closed"
-      @scope = @scope.where("topics.closed")
-    when "archived"
-      @scope = @scope.where("topics.archived")
-    when "listed"
-      @scope = @scope.where("topics.visible")
-    when "unlisted"
-      @scope = @scope.where("NOT topics.visible")
-    when "deleted"
-      if @guardian.can_see_deleted_topics?(@category)
-        @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|
+    tag_ids.each do |tag_id|
-      sql_alias = "tt#{index}"
+      sql_alias = "tt#{topic_tags_alias}"
 
       @scope =
         @scope.joins(
@@ -81,10 +114,12 @@ class TopicsFilter
   end
 
   def include_topics_with_all_tags(tag_ids)
-    tag_ids.each_with_index do |tag_id, index|
+    tag_ids.each do |tag_id|
+      sql_alias = "tt#{topic_tags_alias}"
+
       @scope =
         @scope.joins(
-          "INNER JOIN topic_tags tt#{index} ON tt#{index}.topic_id = topics.id AND tt#{index}.tag_id = #{tag_id}",
+          "INNER JOIN topic_tags #{sql_alias} ON #{sql_alias}.topic_id = topics.id AND #{sql_alias}.tag_id = #{tag_id}",
         )
     end
   end

spec/lib/topics_filter_spec.rb

@@ -3,60 +3,98 @@
 RSpec.describe TopicsFilter do
   fab!(:admin) { Fabricate(:admin) }
 
-  describe "#filter_status" do
+  describe "#filter_from_query_string" do
+    describe "when filtering with multiple filters" do
+      fab!(:tag) { Fabricate(:tag, name: "tag1") }
+      fab!(:tag2) { Fabricate(:tag, name: "tag2") }
+      fab!(:topic_with_tag) { Fabricate(:topic, tags: [tag]) }
+      fab!(:closed_topic_with_tag) { Fabricate(:topic, tags: [tag], closed: true) }
+      fab!(:topic_with_tag2) { Fabricate(:topic, tags: [tag2]) }
+      fab!(:closed_topic_with_tag2) { Fabricate(:topic, tags: [tag2], closed: true) }
+
+      it "should return the right topics when query string is `status:closed tags:tag1,tag2`" do
+        expect(
+          TopicsFilter
+            .new(guardian: Guardian.new)
+            .filter_from_query_string("status:closed tags:tag1,tag2")
+            .pluck(:id),
+        ).to contain_exactly(closed_topic_with_tag.id, closed_topic_with_tag2.id)
+      end
+    end
+
+    describe "when filtering by 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
+      it "should only return topics that have not been closed or archived when query string is `status:open`" do
         expect(
-        TopicsFilter.new(guardian: Guardian.new).filter_status(status: "open").pluck(:id),
+          TopicsFilter
+            .new(guardian: Guardian.new)
+            .filter_from_query_string("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
+      it "should only return topics that have been deleted when query string is `status:deleted` and user can see deleted topics" do
         expect(
-        TopicsFilter.new(guardian: Guardian.new(admin)).filter_status(status: "deleted").pluck(:id),
+          TopicsFilter
+            .new(guardian: Guardian.new(admin))
+            .filter_from_query_string("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
+      it "should ignore status filter when query string is `status:deleted` and user cannot see deleted topics" do
         expect(
-        TopicsFilter.new(guardian: Guardian.new).filter_status(status: "deleted").pluck(:id),
+          TopicsFilter
+            .new(guardian: Guardian.new)
+            .filter_from_query_string("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
+      it "should only return topics that have been archived when query string is `status:archived`" do
         expect(
-        TopicsFilter.new(guardian: Guardian.new).filter_status(status: "archived").pluck(:id),
+          TopicsFilter
+            .new(guardian: Guardian.new)
+            .filter_from_query_string("status:archived")
+            .pluck(:id),
         ).to contain_exactly(archived_topic.id)
       end
 
-    it "should only return topics that are visible when status is `listed`" do
+      it "should only return topics that are visible when query string is `status:listed`" do
         Topic.update_all(visible: false)
         topic.update!(visible: true)
 
         expect(
-        TopicsFilter.new(guardian: Guardian.new).filter_status(status: "listed").pluck(:id),
+          TopicsFilter
+            .new(guardian: Guardian.new)
+            .filter_from_query_string("status:listed")
+            .pluck(:id),
         ).to contain_exactly(topic.id)
       end
 
-    it "should only return topics that are not visible when status is `unlisted`" do
+      it "should only return topics that are not visible when query string is `status:unlisted`" do
         Topic.update_all(visible: true)
         topic.update!(visible: false)
 
         expect(
-        TopicsFilter.new(guardian: Guardian.new).filter_status(status: "unlisted").pluck(:id),
+          TopicsFilter
+            .new(guardian: Guardian.new)
+            .filter_from_query_string("status:unlisted")
+            .pluck(:id),
         ).to contain_exactly(topic.id)
       end
     end
 
-  describe "#filter_tags" do
+    describe "when filtering by tags" do
-    fab!(:tag) { Fabricate(:tag) }
+      fab!(:tag) { Fabricate(:tag, name: "tag1") }
-    fab!(:tag2) { Fabricate(:tag) }
+      fab!(:tag2) { Fabricate(:tag, name: "tag2") }
+      fab!(:tag3) { Fabricate(:tag, name: "tag3") }
 
-    fab!(:group_only_tag) { Fabricate(:tag) }
+      fab!(:group_only_tag) { Fabricate(:tag, name: "group-only-tag") }
       fab!(:group) { Fabricate(:group) }
 
       let!(:staff_tag_group) do
@@ -81,7 +119,7 @@ RSpec.describe TopicsFilter do
         expect(
           TopicsFilter
             .new(guardian: Guardian.new)
-          .filter_tags(tag_names: [tag.name, tag2.name], match_all: true, exclude: false)
+            .filter_from_query_string("tags:#{tag.name}+#{tag2.name}")
             .pluck(:id),
         ).to contain_exactly(
           topic_without_tag.id,
@@ -92,78 +130,98 @@ RSpec.describe TopicsFilter do
         )
       end
 
-    it "should only return topics that are tagged with all of the specified tags when `match_all` is `true`" do
+      it "should only return topics that are tagged with all of the specified tags when query string is `tags:tag1+tag2`" do
         expect(
           TopicsFilter
             .new(guardian: Guardian.new)
-          .filter_tags(tag_names: [tag.name, tag2.name], match_all: true, exclude: false)
+            .filter_from_query_string("tags:#{tag.name}+#{tag2.name}")
             .pluck(:id),
         ).to contain_exactly(topic_with_tag_and_tag2.id)
       end
 
-    it "should only return topics that are tagged with any of the specified tags when `match_all` is `false`" do
+      it "should only return topics that are tagged with tag1 and tag2 when query string is `tags:tag1 tags:tag2`" do
+        topic_with_tag_and_tag2_and_tag3 = Fabricate(:topic, tags: [tag, tag2, tag3])
+
         expect(
           TopicsFilter
             .new(guardian: Guardian.new)
-          .filter_tags(tag_names: [tag2.name], match_all: false, exclude: false)
+            .filter_from_query_string("tags:#{tag.name} tags:#{tag2.name}")
             .pluck(:id),
-      ).to contain_exactly(topic_with_tag_and_tag2.id, topic_with_tag2.id)
+        ).to contain_exactly(topic_with_tag_and_tag2.id, topic_with_tag_and_tag2_and_tag3.id)
       end
 
-    it "should not return any topics when `match_all` is `true` and one of specified tags is invalid" do
+      it "should only return topics that are tagged with tag1 and tag2 but not tag3 when query string is `tags:tag1 tags:tag2 -tags:tag3`" do
+        topic_with_tag_and_tag2_and_tag3 = Fabricate(:topic, tags: [tag, tag2, tag3])
+
         expect(
           TopicsFilter
             .new(guardian: Guardian.new)
-          .filter_tags(tag_names: ["invalid", tag.name, tag2.name], match_all: true, exclude: false)
+            .filter_from_query_string("tags:#{tag.name} tags:#{tag2.name} -tags:tag3")
+            .pluck(:id),
+        ).to contain_exactly(topic_with_tag_and_tag2.id)
+      end
+
+      it "should only return topics that are tagged with any of the specified tags when query string is `tags:tag1,tag2`" do
+        expect(
+          TopicsFilter
+            .new(guardian: Guardian.new)
+            .filter_from_query_string("tags:#{tag.name},#{tag2.name}")
+            .pluck(:id),
+        ).to contain_exactly(topic_with_tag.id, topic_with_tag_and_tag2.id, topic_with_tag2.id)
+      end
+
+      it "should not return any topics when query string is `tags:tag1+tag2+invalid`" do
+        expect(
+          TopicsFilter
+            .new(guardian: Guardian.new)
+            .filter_from_query_string("tags:tag1+tag2+invalid")
             .pluck(:id),
         ).to eq([])
       end
 
-    it "should still filter topics by specificed tags when `match_all` is `false` even if one of the tags is invalid" do
+      it "should still filter topics by specificed tags when query string is `tags:tag1,tag2,invalid`" do
         expect(
           TopicsFilter
             .new(guardian: Guardian.new)
-          .filter_tags(
+            .filter_from_query_string("tags:tag1,tag2,invalid")
-            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
 
-    it "should not return any topics when user tries to filter topics by tags that are hidden" do
+      it "should not return any topics when query string is `tags:group-only-tag` because specified tag is hidden to user" do
         expect(
           TopicsFilter
             .new(guardian: Guardian.new)
-          .filter_tags(tag_names: [group_only_tag.name], match_all: true, exclude: false)
+            .filter_from_query_string("tags:group-only-tag")
             .pluck(:id),
         ).to eq([])
       end
 
-    it "should allow user with permission to filter topics by tags that are hidden" do
+      it "should return the right topics when query string is `tags:group-only-tag` and user has access to specified tag" do
         group.add(admin)
 
         expect(
           TopicsFilter
             .new(guardian: Guardian.new(admin))
-          .filter_tags(tag_names: [group_only_tag.name])
+            .filter_from_query_string("tags:group-only-tag")
             .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
+      it "should only return topics that are not tagged with specified tag when query string is `-tags:tag1`" do
         expect(
           TopicsFilter
             .new(guardian: Guardian.new)
-          .filter_tags(tag_names: [tag.name], match_all: true, exclude: true)
+            .filter_from_query_string("-tags:tag1")
             .pluck(:id),
         ).to contain_exactly(topic_without_tag.id, topic_with_tag2.id, topic_with_group_only_tag.id)
+      end
 
+      it "should only return topics that are not tagged with all of the specified tags when query string is `-tags:tag1+tag2`" do
         expect(
           TopicsFilter
             .new(guardian: Guardian.new)
-          .filter_tags(tag_names: [tag.name, tag2.name], match_all: true, exclude: true)
+            .filter_from_query_string("-tags:tag1+tag2")
             .pluck(:id),
         ).to contain_exactly(
           topic_without_tag.id,
@@ -173,20 +231,14 @@ RSpec.describe TopicsFilter do
         )
       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
+      it "should only return topics that are not tagged with any of the specified tags when query string is `-tags:tag1,tag2`" do
         expect(
           TopicsFilter
             .new(guardian: Guardian.new)
-          .filter_tags(tag_names: [tag.name], match_all: false, exclude: true)
+            .filter_from_query_string("-tags:tag1,tag2")
-          .pluck(:id),
-      ).to contain_exactly(topic_without_tag.id, topic_with_group_only_tag.id, topic_with_tag2.id)
-
-      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
+end

spec/requests/list_controller_spec.rb

@@ -1086,9 +1086,14 @@ RSpec.describe ListController do
   end
 
   describe "#filter" do
-    it "should respond with 403 response code for an anonymous user" do
+    fab!(:category) { Fabricate(:category) }
-      SiteSetting.experimental_topics_filter = true
+    fab!(:tag) { Fabricate(:tag, name: "tag1") }
+    fab!(:topic_with_tag) { Fabricate(:topic, tags: [tag]) }
+    fab!(:topic2_with_tag) { Fabricate(:topic, tags: [tag]) }
 
+    before { SiteSetting.experimental_topics_filter = true }
+
+    it "should respond with 403 response code for an anonymous user" do
       get "/filter.json"
 
       expect(response.status).to eq(403)
@@ -1096,11 +1101,83 @@ RSpec.describe ListController do
 
     it "should respond with 404 response code when `experimental_topics_filter` site setting has not been enabled" do
       SiteSetting.experimental_topics_filter = false
+
       sign_in(user)
 
       get "/filter.json"
 
       expect(response.status).to eq(404)
     end
+
+    it "returns category definition topics if `show_category_definitions_in_topic_lists` site setting is enabled" do
+      category_topic = Fabricate(:topic, category: category)
+      category.update!(topic: category_topic)
+
+      SiteSetting.show_category_definitions_in_topic_lists = true
+
+      sign_in(user)
+
+      get "/filter.json"
+
+      expect(response.status).to eq(200)
+
+      parsed = response.parsed_body
+
+      expect(parsed["topic_list"]["topics"].length).to eq(4)
+
+      expect(parsed["topic_list"]["topics"].map { |topic| topic["id"] }).to contain_exactly(
+        topic.id,
+        topic_with_tag.id,
+        topic2_with_tag.id,
+        category_topic.id,
+      )
+    end
+
+    it "does not return category definition topics if `show_category_definitions_in_topic_lists` site setting is disabled" do
+      category_topic = Fabricate(:topic, category: category)
+      category.update!(topic: category_topic)
+
+      SiteSetting.show_category_definitions_in_topic_lists = false
+
+      sign_in(user)
+
+      get "/filter.json"
+
+      expect(response.status).to eq(200)
+
+      parsed = response.parsed_body
+
+      expect(parsed["topic_list"]["topics"].length).to eq(3)
+
+      expect(parsed["topic_list"]["topics"].map { |topic| topic["id"] }).to contain_exactly(
+        topic.id,
+        topic_with_tag.id,
+        topic2_with_tag.id,
+      )
+    end
+
+    it "should accept the `page` query parameter" do
+      stub_const(TopicQuery, "DEFAULT_PER_PAGE_COUNT", 1) do
+        sign_in(user)
+
+        get "/filter.json?q=tags:tag1"
+
+        expect(response.status).to eq(200)
+
+        parsed = response.parsed_body
+
+        expect(parsed["topic_list"]["topics"].length).to eq(1)
+        expect(parsed["topic_list"]["topics"].first["id"]).to eq(topic2_with_tag.id)
+
+        get "/filter.json?q=tags:tag1&page=1"
+
+        expect(response.status).to eq(200)
+
+        parsed = response.parsed_body
+
+        expect(parsed["topic_list"]["topics"].length).to eq(1)
+        expect(parsed["topic_list"]["topics"].first["id"]).to eq(topic_with_tag.id)
+      end
+    end
   end
 end

spec/system/filtering_topics_spec.rb

@@ -2,13 +2,16 @@
 
 describe "Filtering topics", type: :system, js: true do
   fab!(:user) { Fabricate(:user) }
-  fab!(:topic) { Fabricate(:topic) }
-  fab!(:closed_topic) { Fabricate(:topic, closed: true) }
   let(:topic_list) { PageObjects::Components::TopicList.new }
+  let(:topic_query_filter) { PageObjects::Components::TopicQueryFilter.new }
 
   before { SiteSetting.experimental_topics_filter = true }
 
-  it "should allow users to input a custom query string to filter through topics" do
+  describe "when filtering by status" do
+    fab!(:topic) { Fabricate(:topic) }
+    fab!(:closed_topic) { Fabricate(:topic, closed: true) }
+
+    it "should display the right topics when the status filter is used in the query string" do
       sign_in(user)
 
       visit("/filter")
@@ -16,26 +19,48 @@ describe "Filtering topics", type: :system, js: true do
       expect(topic_list).to have_topic(topic)
       expect(topic_list).to have_topic(closed_topic)
 
-    topic_query_filter = PageObjects::Components::TopicQueryFilter.new
       topic_query_filter.fill_in("status:open")
 
       expect(topic_list).to have_topic(topic)
       expect(topic_list).to have_no_topic(closed_topic)
-    expect(page).to have_current_path("/filter?status=open")
 
       topic_query_filter.fill_in("status:closed")
 
       expect(topic_list).to have_no_topic(topic)
       expect(topic_list).to have_topic(closed_topic)
-    expect(page).to have_current_path("/filter?status=closed")
+    end
   end
 
-  it "should filter topics when 'status' query params is present" do
+  describe "when filtering by tags" do
+    fab!(:tag) { Fabricate(:tag, name: "tag1") }
+    fab!(:tag2) { Fabricate(:tag, name: "tag2") }
+    fab!(:topic_with_tag) { Fabricate(:topic, tags: [tag]) }
+    fab!(:topic_with_tag2) { Fabricate(:topic, tags: [tag2]) }
+    fab!(:topic_with_tag_and_tag2) { Fabricate(:topic, tags: [tag, tag2]) }
+
+    it "should display the right topics when tags filter is used in the query string" do
       sign_in(user)
 
-    visit("/filter?status=open")
+      visit("/filter")
 
-    expect(topic_list).to have_topic(topic)
+      expect(topic_list).to have_topics(count: 3)
-    expect(topic_list).to have_no_topic(closed_topic)
+      expect(topic_list).to have_topic(topic_with_tag)
+      expect(topic_list).to have_topic(topic_with_tag2)
+      expect(topic_list).to have_topic(topic_with_tag_and_tag2)
+
+      topic_query_filter.fill_in("tags:tag1")
+
+      expect(topic_list).to have_topics(count: 2)
+      expect(topic_list).to have_topic(topic_with_tag)
+      expect(topic_list).to have_topic(topic_with_tag_and_tag2)
+      expect(topic_list).to have_no_topic(topic_with_tag2)
+
+      topic_query_filter.fill_in("tags:tag1+tag2")
+
+      expect(topic_list).to have_topics(count: 1)
+      expect(topic_list).to have_no_topic(topic_with_tag)
+      expect(topic_list).to have_no_topic(topic_with_tag2)
+      expect(topic_list).to have_topic(topic_with_tag_and_tag2)
+    end
   end
 end