diff --git a/app/assets/javascripts/discourse/app/components/d-navigation.hbs b/app/assets/javascripts/discourse/app/components/d-navigation.hbs index 04092dc19ec..2b7badf2092 100644 --- a/app/assets/javascripts/discourse/app/components/d-navigation.hbs +++ b/app/assets/javascripts/discourse/app/components/d-navigation.hbs @@ -3,11 +3,11 @@ { + 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; diff --git a/app/assets/javascripts/discourse/app/controllers/navigation/default.js b/app/assets/javascripts/discourse/app/controllers/navigation/default.js index 2e46e7abb17..18e62a93709 100644 --- a/app/assets/javascripts/discourse/app/controllers/navigation/default.js +++ b/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") diff --git a/app/assets/javascripts/discourse/app/pre-initializers/dynamic-route-builders.js b/app/assets/javascripts/discourse/app/pre-initializers/dynamic-route-builders.js index 9c11c4c4248..99ea7796dd9 100644 --- a/app/assets/javascripts/discourse/app/pre-initializers/dynamic-route-builders.js +++ b/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) { - const siteSettings = container.lookup("service:site-settings"); + initialize(_container, app) { + app.register( + "controller:discovery.filter", + DiscoverySortableController.extend() + ); - if (siteSettings.experimental_topics_filter) { - app.register( - "controller:discovery.filter", - DiscoverySortableController.extend() - ); - - app.register("route:discovery.filter", buildTopicRoute("filter")); - } + app.register("route:discovery.filter", buildTopicRoute("filter")); app.register( "controller:discovery.category", diff --git a/app/assets/javascripts/discourse/app/routes/build-topic-route.js b/app/assets/javascripts/discourse/app/routes/build-topic-route.js index 0e3b344023f..feea4a2a098 100644 --- a/app/assets/javascripts/discourse/app/routes/build-topic-route.js +++ b/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); diff --git a/app/assets/javascripts/discourse/app/templates/navigation/default.hbs b/app/assets/javascripts/discourse/app/templates/navigation/default.hbs index 9848aadf5fd..cfa4ee3536b 100644 --- a/app/assets/javascripts/discourse/app/templates/navigation/default.hbs +++ b/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}} /> \ No newline at end of file diff --git a/lib/topic_query.rb b/lib/topic_query.rb index 4be35415753..977db1fe7b2 100644 --- a/lib/topic_query.rb +++ b/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 diff --git a/lib/topics_filter.rb b/lib/topics_filter.rb index af80382e12f..e15d707e15e 100644 --- a/lib/topics_filter.rb +++ b/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) - input - .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 - + def filter(status: nil) + filter_status(@scope, status) if status @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 diff --git a/spec/lib/topics_filter_spec.rb b/spec/lib/topics_filter_spec.rb index 778bbbb9fc0..13b3cfd6664 100644 --- a/spec/lib/topics_filter_spec.rb +++ b/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 - expect( - TopicsFilter.new(guardian: Guardian.new).filter("randomstring").pluck(:id), - ).to contain_exactly(topic.id, closed_topic.id, archived_topic.id) - end + context "when filtering by topic's status" do + it "should only return topics that have not been closed or archived when input is `status:open`" do + expect( + TopicsFilter.new(guardian: Guardian.new).filter(status: "open").pluck(:id), + ).to contain_exactly(topic.id) + end - it "should only return topics that have not been closed or archived when input is `status:open`" do - expect( - TopicsFilter.new(guardian: Guardian.new).filter("status:open").pluck(:id), - ).to contain_exactly(topic.id) - end + it "should only return topics that have been deleted when input is `status: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 only return topics that have been deleted when input is `status: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 status filter when input is `status: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 - it "should status filter when input is `status: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 - - 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) + 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 end diff --git a/spec/system/filtering_topics_spec.rb b/spec/system/filtering_topics_spec.rb index fcc8513a411..81f305fd32c 100644 --- a/spec/system/filtering_topics_spec.rb +++ b/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)