DEV: Remove experimental support for query string on /filter route (#20632)

app/assets/javascripts/discourse/app/components/d-navigation.hbs

@@ -3,11 +3,11 @@
     <Input
       class="topic-query-filter__input"
       @value={{this.queryString}}
-      @enter={{route-action "changeQueryString" this.queryString}}
+      @enter={{action @updateTopicsListQueryParams this.queryString}}
     />
 
     <DButton
-      @action={{route-action "changeQueryString" this.queryString}}
+      @action={{action @updateTopicsListQueryParams this.queryString}}
       @icon="filter"
       @class="btn-primary topic-query-filter__button"
       @label="filters.filter.button.label"

app/assets/javascripts/discourse/app/components/d-navigation.js

@@ -6,12 +6,18 @@ import { NotificationLevels } from "discourse/lib/notification-levels";
 import { getOwner } from "discourse-common/lib/get-owner";
 import { htmlSafe } from "@ember/template";
 import { inject as service } from "@ember/service";
-import { alias, equal } from "@ember/object/computed";
+import { equal } from "@ember/object/computed";
 
 export default Component.extend(FilterModeMixin, {
   router: service(),
   dialog: service(),
   tagName: "",
+  queryString: "",
+
+  init() {
+    this._super(...arguments);
+    this.queryString = this.filterQueryString;
+  },
 
   // Should be a `readOnly` instead but some themes/plugins still pass
   // the `categories` property into this component
@@ -142,7 +148,6 @@ export default Component.extend(FilterModeMixin, {
   },
 
   isQueryFilterMode: equal("filterMode", "filter"),
-  queryString: alias("router.currentRoute.queryParams.q"),
 
   actions: {
     changeCategoryNotificationLevel(notificationLevel) {

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

@@ -1,4 +1,6 @@
 import Controller, { inject as controller } from "@ember/controller";
+import discourseComputed from "discourse-common/utils/decorators";
+import { action } from "@ember/object";
 
 // Just add query params here to have them automatically passed to topic list filters.
 export const queryParams = {
@@ -27,6 +29,31 @@ export const queryParams = {
 const controllerOpts = {
   discoveryTopics: controller("discovery/topics"),
   queryParams: Object.keys(queryParams),
+
+  @discourseComputed(...Object.keys(queryParams))
+  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)) {
+      const key = match[1];
+      const value = match[2];
+
+      if (controllerOpts.queryParams.includes(key)) {
+        this.set(key, value);
+      }
+    }
+  },
 };
 
 // Default to `undefined`
@@ -34,10 +61,6 @@ controllerOpts.queryParams.forEach((p) => {
   controllerOpts[p] = queryParams[p].default;
 });
 
-export function changeQueryString(queryString) {
-  this.controller.set("q", queryString);
-}
-
 export function changeSort(sortBy) {
   let model = this.controllerFor("discovery.topics").model;
 

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

@@ -6,6 +6,7 @@ import { TRACKED_QUERY_PARAM_VALUE } from "discourse/lib/topic-list-tracked-filt
 
 export default Controller.extend(FilterModeMixin, {
   discovery: controller(),
+  discoveryFilter: controller("discovery.filter"),
   router: service(),
 
   @discourseComputed("router.currentRoute.queryParams.f")

app/assets/javascripts/discourse/app/pre-initializers/dynamic-route-builders.js

@@ -10,17 +10,13 @@ export default {
   after: "inject-discourse-objects",
   name: "dynamic-route-builders",
 
-  initialize(container, app) {
+  initialize(_container, app) {
-    const siteSettings = container.lookup("service:site-settings");
+    app.register(
+      "controller:discovery.filter",
+      DiscoverySortableController.extend()
+    );
 
-    if (siteSettings.experimental_topics_filter) {
+    app.register("route:discovery.filter", buildTopicRoute("filter"));
-      app.register(
-        "controller:discovery.filter",
-        DiscoverySortableController.extend()
-      );
-
-      app.register("route:discovery.filter", buildTopicRoute("filter"));
-    }
 
     app.register(
       "controller:discovery.category",

app/assets/javascripts/discourse/app/routes/build-topic-route.js

@@ -1,5 +1,4 @@
 import {
-  changeQueryString,
   changeSort,
   queryParams,
   resetParams,
@@ -168,11 +167,6 @@ export default function (filter, extras) {
         changeSort.call(this, sortBy);
       },
 
-      @action
-      changeQueryString(queryString) {
-        changeQueryString.call(this, queryString);
-      },
-
       @action
       resetParams(skipParams = []) {
         resetParams.call(this, skipParams);

app/assets/javascripts/discourse/app/templates/navigation/default.hbs

@@ -9,5 +9,7 @@
     @hasDraft={{this.currentUser.has_topic_draft}}
     @createTopic={{route-action "createTopic"}}
     @skipCategoriesNavItem={{this.skipCategoriesNavItem}}
+    @filterQueryString={{this.discoveryFilter.queryString}}
+    @updateTopicsListQueryParams={{this.discoveryFilter.updateTopicsListQueryParams}}
   />
 </DSection>

lib/topic_query.rb

@@ -848,17 +848,12 @@ class TopicQuery
     end
 
     if status = options[:status]
-      options[:q] ||= +""
-      options[:q] << " status:#{status}"
-    end
-
-    if options[:q].present?
       result =
         TopicsFilter.new(
           scope: result,
           guardian: @guardian,
           category_id: options[:category],
-        ).filter(options[:q])
+        ).filter(status: options[:status])
     end
 
     if (filter = (options[:filter] || options[:f])) && @user

lib/topics_filter.rb

@@ -1,51 +1,31 @@
 # frozen_string_literal: true
 
 class TopicsFilter
-  def self.register_filter(matcher, &block)
-    self.filters[matcher] = block
-  end
-
-  def self.filters
-    @@filters ||= {}
-  end
-
-  register_filter(/\Astatus:([a-zA-Z]+)\z/i) do |topics, match|
-    case match
-    when "open"
-      topics.where("NOT topics.closed AND NOT topics.archived")
-    when "closed"
-      topics.where("topics.closed")
-    when "archived"
-      topics.where("topics.archived")
-    when "deleted"
-      if @guardian.can_see_deleted_topics?(@category)
-        topics.unscope(where: :deleted_at).where("topics.deleted_at IS NOT NULL")
-      end
-    end
-  end
-
   def initialize(guardian:, scope: Topic, category_id: nil)
     @guardian = guardian
     @scope = scope
     @category = category_id.present? ? Category.find_by(id: category_id) : nil
   end
 
-  def filter(input)
+  def filter(status: nil)
-    input
+    filter_status(@scope, status) if status
-      .to_s
-      .scan(/(([^" \t\n\x0B\f\r]+)?(("[^"]+")?))/)
-      .to_a
-      .map do |(word, _)|
-        next if word.blank?
-
-        self.class.filters.each do |matcher, block|
-          cleaned = word.gsub(/["']/, "")
-
-          new_scope = instance_exec(@scope, $1, &block) if cleaned =~ matcher
-          @scope = new_scope if !new_scope.nil?
-        end
-      end
-
     @scope
   end
+
+  private
+
+  def filter_status(scope, status)
+    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 "deleted"
+      if @guardian.can_see_deleted_topics?(@category)
+        @scope = @scope.unscope(where: :deleted_at).where("topics.deleted_at IS NOT NULL")
+      end
+    end
+  end
 end

spec/lib/topics_filter_spec.rb

@@ -9,41 +9,37 @@ RSpec.describe TopicsFilter do
 
   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(
+      expect(TopicsFilter.new(guardian: Guardian.new).filter.pluck(:id)).to contain_exactly(
         topic.id,
         closed_topic.id,
         archived_topic.id,
       )
     end
 
-    it "should return all topics when input does not match any filters" do
+    context "when filtering by topic's status" do
-      expect(
+      it "should only return topics that have not been closed or archived when input is `status:open`" do
-        TopicsFilter.new(guardian: Guardian.new).filter("randomstring").pluck(:id),
+        expect(
-      ).to contain_exactly(topic.id, closed_topic.id, archived_topic.id)
+          TopicsFilter.new(guardian: Guardian.new).filter(status: "open").pluck(:id),
-    end
+        ).to contain_exactly(topic.id)
+      end
 
-    it "should only return topics that have not been closed or archived when input is `status:open`" do
+      it "should only return topics that have been deleted when input is `status:deleted` and user can see deleted topics" do
-      expect(
+        expect(
-        TopicsFilter.new(guardian: Guardian.new).filter("status:open").pluck(:id),
+          TopicsFilter.new(guardian: Guardian.new(admin)).filter(status: "deleted").pluck(:id),
-      ).to contain_exactly(topic.id)
+        ).to contain_exactly(deleted_topic_id)
-    end
+      end
 
-    it "should only return topics that have been deleted when input is `status:deleted` and user can see deleted topics" do
+      it "should status filter when input is `status:deleted` and user cannot see deleted topics" do
-      expect(
+        expect(
-        TopicsFilter.new(guardian: Guardian.new(admin)).filter("status:deleted").pluck(:id),
+          TopicsFilter.new(guardian: Guardian.new).filter(status: "deleted").pluck(:id),
-      ).to contain_exactly(deleted_topic_id)
+        ).to contain_exactly(topic.id, closed_topic.id, archived_topic.id)
-    end
+      end
 
-    it "should status filter when input is `status:deleted` and user cannot see deleted topics" do
+      it "should only return topics that have been archived when input is `status:archived`" do
-      expect(
+        expect(
-        TopicsFilter.new(guardian: Guardian.new).filter("status:deleted").pluck(:id),
+          TopicsFilter.new(guardian: Guardian.new).filter(status: "archived").pluck(:id),
-      ).to contain_exactly(topic.id, closed_topic.id, archived_topic.id)
+        ).to contain_exactly(archived_topic.id)
-    end
+      end
-
-    it "should only return topics that have been archived when input is `status:archived`" do
-      expect(
-        TopicsFilter.new(guardian: Guardian.new).filter("status:archived").pluck(:id),
-      ).to contain_exactly(archived_topic.id)
     end
   end
 end

spec/system/filtering_topics_spec.rb

@@ -8,7 +8,7 @@ describe "Filtering topics", type: :system, js: true do
 
   before { SiteSetting.experimental_topics_filter = true }
 
-  it "should allow users to enter a custom query string to filter through topics" do
+  it "should allow users to input a custom query string to filter through topics" do
     sign_in(user)
 
     visit("/filter")
@@ -21,19 +21,19 @@ describe "Filtering topics", type: :system, js: true do
 
     expect(topic_list).to have_topic(topic)
     expect(topic_list).to have_no_topic(closed_topic)
-    expect(page).to have_current_path("/filter?q=status%3Aopen")
+    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?q=status%3Aclosed")
+    expect(page).to have_current_path("/filter?status=closed")
   end
 
-  it "should filter topics when 'q' query params is present" do
+  it "should filter topics when 'status' query params is present" do
     sign_in(user)
 
-    visit("/filter?q=status:open")
+    visit("/filter?status=open")
 
     expect(topic_list).to have_topic(topic)
     expect(topic_list).to have_no_topic(closed_topic)