From 66c50547b4ef89969241a1f8d27bb4248ace6c07 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Fri, 3 Mar 2023 09:46:21 +0800 Subject: [PATCH] DEV: Experimental /filter route to filter through topics (#20494) This commit introduces an experimental `/filter` route which allows a user to input a query string to filter through topics. Internal Ref: /t/92833 --- .../discourse/app/components/d-navigation.hbs | 43 +++++++++++----- .../discourse/app/components/d-navigation.js | 4 ++ .../app/controllers/discovery-sortable.js | 4 ++ .../dynamic-route-builders.js | 13 ++++- .../discourse/app/routes/app-route-map.js | 2 + .../discourse/app/routes/build-topic-route.js | 8 +++ .../stylesheets/common/components/_index.scss | 1 + .../common/components/topic-query-filter.scss | 10 ++++ app/controllers/list_controller.rb | 5 ++ config/locales/client.en.yml | 6 +++ config/routes.rb | 2 + config/site_settings.yml | 4 ++ lib/topic_query.rb | 38 ++++++-------- lib/topics_filter.rb | 51 +++++++++++++++++++ spec/lib/topics_filter_spec.rb | 49 ++++++++++++++++++ spec/requests/list_controller_spec.rb | 19 +++++++ spec/system/filtering_topics_spec.rb | 41 +++++++++++++++ .../page_objects/components/topic_list.rb | 18 ++++++- .../components/topic_query_filter.rb | 16 ++++++ 19 files changed, 297 insertions(+), 37 deletions(-) create mode 100644 app/assets/stylesheets/common/components/topic-query-filter.scss create mode 100644 lib/topics_filter.rb create mode 100644 spec/lib/topics_filter_spec.rb create mode 100644 spec/system/filtering_topics_spec.rb create mode 100644 spec/system/page_objects/components/topic_query_filter.rb diff --git a/app/assets/javascripts/discourse/app/components/d-navigation.hbs b/app/assets/javascripts/discourse/app/components/d-navigation.hbs index 9a33a368930..c328d2349f8 100644 --- a/app/assets/javascripts/discourse/app/components/d-navigation.hbs +++ b/app/assets/javascripts/discourse/app/components/d-navigation.hbs @@ -1,19 +1,36 @@ - +{{#if this.isQueryFilterMode}} +
+ -{{#unless this.additionalTags}} - {{! nav bar doesn't work with tag intersections }} - +
+{{else}} + -{{/unless}} + + {{#unless this.additionalTags}} + {{! nav bar doesn't work with tag intersections }} + + {{/unless}} +{{/if}}
{{#if (and this.notCategoriesRoute this.site.mobileView this.canBulk)}} diff --git a/app/assets/javascripts/discourse/app/components/d-navigation.js b/app/assets/javascripts/discourse/app/components/d-navigation.js index 73e63a5d21e..26af2c8eedc 100644 --- a/app/assets/javascripts/discourse/app/components/d-navigation.js +++ b/app/assets/javascripts/discourse/app/components/d-navigation.js @@ -6,6 +6,7 @@ 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"; export default Component.extend(FilterModeMixin, { router: service(), @@ -140,6 +141,9 @@ export default Component.extend(FilterModeMixin, { return controller.canBulkSelect; }, + isQueryFilterMode: equal("filterMode", "filter"), + queryString: alias("router.currentRoute.queryParams.q"), + actions: { changeCategoryNotificationLevel(notificationLevel) { this.category.setNotification(notificationLevel); diff --git a/app/assets/javascripts/discourse/app/controllers/discovery-sortable.js b/app/assets/javascripts/discourse/app/controllers/discovery-sortable.js index 6730ed46ef7..bb0bb74a4b4 100644 --- a/app/assets/javascripts/discourse/app/controllers/discovery-sortable.js +++ b/app/assets/javascripts/discourse/app/controllers/discovery-sortable.js @@ -34,6 +34,10 @@ 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/pre-initializers/dynamic-route-builders.js b/app/assets/javascripts/discourse/app/pre-initializers/dynamic-route-builders.js index 635f39f481e..9c11c4c4248 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,7 +10,18 @@ export default { after: "inject-discourse-objects", name: "dynamic-route-builders", - initialize(registry, app) { + initialize(container, app) { + const siteSettings = container.lookup("service:site-settings"); + + if (siteSettings.experimental_topics_filter) { + app.register( + "controller:discovery.filter", + DiscoverySortableController.extend() + ); + + app.register("route:discovery.filter", buildTopicRoute("filter")); + } + app.register( "controller:discovery.category", DiscoverySortableController.extend() diff --git a/app/assets/javascripts/discourse/app/routes/app-route-map.js b/app/assets/javascripts/discourse/app/routes/app-route-map.js index b30a365ca68..e1bd3d2f172 100644 --- a/app/assets/javascripts/discourse/app/routes/app-route-map.js +++ b/app/assets/javascripts/discourse/app/routes/app-route-map.js @@ -52,6 +52,8 @@ export default function () { }); }); + this.route("filter", { path: "/filter" }); + this.route("categories"); // default filter for a 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 4717641724e..0e3b344023f 100644 --- a/app/assets/javascripts/discourse/app/routes/build-topic-route.js +++ b/app/assets/javascripts/discourse/app/routes/build-topic-route.js @@ -1,4 +1,5 @@ import { + changeQueryString, changeSort, queryParams, resetParams, @@ -146,6 +147,7 @@ export default function (filter, extras) { }; this.controllerFor("discovery/topics").setProperties(topicOpts); + this.controllerFor("navigation/default").set( "canCreateTopic", model.get("can_create_topic") @@ -154,6 +156,7 @@ export default function (filter, extras) { renderTemplate() { this.render("navigation/default", { outlet: "navigation-bar" }); + this.render("discovery/topics", { controller: "discovery/topics", outlet: "list-container", @@ -165,6 +168,11 @@ 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/stylesheets/common/components/_index.scss b/app/assets/stylesheets/common/components/_index.scss index f9201fa36ce..bd9a319233c 100644 --- a/app/assets/stylesheets/common/components/_index.scss +++ b/app/assets/stylesheets/common/components/_index.scss @@ -30,6 +30,7 @@ @import "tap-tile"; @import "time-input"; @import "time-shortcut-picker"; +@import "topic-query-filter"; @import "user-card"; @import "user-info"; @import "user-status-message"; diff --git a/app/assets/stylesheets/common/components/topic-query-filter.scss b/app/assets/stylesheets/common/components/topic-query-filter.scss new file mode 100644 index 00000000000..cda0cc2b855 --- /dev/null +++ b/app/assets/stylesheets/common/components/topic-query-filter.scss @@ -0,0 +1,10 @@ +.topic-query-filter { + display: flex; + flex-direction: row; + margin-right: auto; + margin-bottom: var(--nav-space); + + .topic-query-filter__input { + margin: 0 0.5em 0 0; + } +} diff --git a/app/controllers/list_controller.rb b/app/controllers/list_controller.rb index b9e827190f8..cb90a196407 100644 --- a/app/controllers/list_controller.rb +++ b/app/controllers/list_controller.rb @@ -117,6 +117,11 @@ class ListController < ApplicationController end end + def filter + raise Discourse::NotFound if !SiteSetting.experimental_topics_filter + latest + end + def category_default canonical_url "#{Discourse.base_url_no_prefix}#{@category.url}" view_method = @category.default_view diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 515abec9467..d000fa72a32 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -2871,6 +2871,7 @@ en: bookmarks: "You have no bookmarked topics yet." category: "There are no %{category} topics." top: "There are no top topics." + filter: "There are no topics." educate: new: '

Your new topics will appear here. By default, topics are considered new and will show a indicator if they were created in the last 2 days.

Visit your preferences to change this.

' unread: '

Your unread topics appear here.

By default, topics are considered unread and will show unread counts 1 if you:

  • Created the topic
  • Replied to the topic
  • Read the topic for more than 4 minutes

Or if you have explicitly set the topic to Tracked or Watched via the 🔔 in each topic.

Visit your preferences to change this.

' @@ -2885,6 +2886,7 @@ en: tag: "There are no more %{tag} topics." top: "There are no more top topics." bookmarks: "There are no more bookmarked topics." + filter: "There are no more topics." topic: filter_to: @@ -3931,6 +3933,10 @@ en: filters: with_topics: "%{filter} topics" with_category: "%{filter} %{category} topics" + filter: + title: "Filter" + button: + label: "Filter" latest: title: "Latest" title_with_count: diff --git a/config/routes.rb b/config/routes.rb index 225b535393b..d1aa0b5b9b0 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1207,6 +1207,8 @@ Discourse::Application.routes.draw do Discourse.filters.each { |filter| get "#{filter}" => "list##{filter}" } + get "filter" => "list#filter" + get "search/query" => "search#query" get "search" => "search#show" post "search/click" => "search#click" diff --git a/config/site_settings.yml b/config/site_settings.yml index ed6505d2799..8a5082f32fc 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -2080,6 +2080,10 @@ developer: default: "" allow_any: false refresh: true + experimental_topics_filter: + client: true + default: false + hidden: true navigation: navigation_menu: diff --git a/lib/topic_query.rb b/lib/topic_query.rb index 7c364e53d6a..fe5dc2ff500 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -260,6 +260,10 @@ class TopicQuery create_list(:latest, {}, latest_results) end + def list_filter + list_latest + end + def list_read create_list(:read, unordered: true) do |topics| topics.where("tu.last_visited_at IS NOT NULL").order("tu.last_visited_at DESC") @@ -663,7 +667,7 @@ class TopicQuery end # Start with a list of all topics - result = Topic.unscoped.includes(:category) + result = Topic.includes(:category) if @user result = @@ -821,8 +825,6 @@ class TopicQuery ) end - require_deleted_clause = true - if before = options[:before] if (before = before.to_i) > 0 result = result.where("topics.created_at < ?", before.to_i.days.ago) @@ -836,24 +838,17 @@ class TopicQuery end if status = options[:status] - case status - when "open" - result = result.where("NOT topics.closed AND NOT topics.archived") - when "closed" - result = result.where("topics.closed") - when "archived" - result = result.where("topics.archived") - when "listed" - result = result.where("topics.visible") - when "unlisted" - result = result.where("NOT topics.visible") - when "deleted" - category = Category.find_by(id: options[:category]) - if @guardian.can_see_deleted_topics?(category) - result = result.where("topics.deleted_at IS NOT NULL") - require_deleted_clause = false - end - end + 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]) end if (filter = (options[:filter] || options[:f])) && @user @@ -876,7 +871,6 @@ class TopicQuery result = TopicQuery.tracked_filter(result, @user.id) if filter == "tracked" end - result = result.where("topics.deleted_at IS NULL") if require_deleted_clause result = result.where("topics.posts_count <= ?", options[:max_posts]) if options[ :max_posts ].present? diff --git a/lib/topics_filter.rb b/lib/topics_filter.rb new file mode 100644 index 00000000000..af80382e12f --- /dev/null +++ b/lib/topics_filter.rb @@ -0,0 +1,51 @@ +# 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 + + @scope + end +end diff --git a/spec/lib/topics_filter_spec.rb b/spec/lib/topics_filter_spec.rb new file mode 100644 index 00000000000..778bbbb9fc0 --- /dev/null +++ b/spec/lib/topics_filter_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +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, + ) + 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 + + 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 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) + end + end +end diff --git a/spec/requests/list_controller_spec.rb b/spec/requests/list_controller_spec.rb index 547e0573139..29c66262ebe 100644 --- a/spec/requests/list_controller_spec.rb +++ b/spec/requests/list_controller_spec.rb @@ -1084,4 +1084,23 @@ RSpec.describe ListController do expect(parsed["topic_list"]["topics"].first["id"]).to eq(welcome_topic.id) end end + + describe "#filter" do + it "should respond with 403 response code for an anonymous user" do + SiteSetting.experimental_topics_filter = true + + get "/filter.json" + + expect(response.status).to eq(403) + end + + 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 + end end diff --git a/spec/system/filtering_topics_spec.rb b/spec/system/filtering_topics_spec.rb new file mode 100644 index 00000000000..fcc8513a411 --- /dev/null +++ b/spec/system/filtering_topics_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +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 } + + before { SiteSetting.experimental_topics_filter = true } + + it "should allow users to enter a custom query string to filter through topics" do + sign_in(user) + + visit("/filter") + + 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?q=status%3Aopen") + + 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") + end + + it "should filter topics when 'q' query params is present" do + sign_in(user) + + visit("/filter?q=status:open") + + expect(topic_list).to have_topic(topic) + expect(topic_list).to have_no_topic(closed_topic) + end +end diff --git a/spec/system/page_objects/components/topic_list.rb b/spec/system/page_objects/components/topic_list.rb index 83be6a93c27..1c1b86c74ff 100644 --- a/spec/system/page_objects/components/topic_list.rb +++ b/spec/system/page_objects/components/topic_list.rb @@ -3,13 +3,29 @@ module PageObjects module Components class TopicList < PageObjects::Components::Base + TOPIC_LIST_BODY_CLASS = ".topic-list-body" + def topic_list - ".topic-list-body" + TOPIC_LIST_BODY_CLASS + end + + def has_topic?(topic) + page.has_css?(topic_list_item_class(topic)) + end + + def has_no_topic?(topic) + page.has_no_css?(topic_list_item_class(topic)) end def visit_topic_with_title(title) find(".topic-list-body a", text: title).click end + + private + + def topic_list_item_class(topic) + "#{TOPIC_LIST_BODY_CLASS} .topic-list-item[data-topic-id='#{topic.id}']" + end end end end diff --git a/spec/system/page_objects/components/topic_query_filter.rb b/spec/system/page_objects/components/topic_query_filter.rb new file mode 100644 index 00000000000..75a9b3c5a7e --- /dev/null +++ b/spec/system/page_objects/components/topic_query_filter.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module PageObjects + module Components + class TopicQueryFilter < PageObjects::Components::Base + def fill_in(text) + page.fill_in(class: "topic-query-filter__input", with: text) + + page.click_button( + I18n.t("js.filters.filter.button.label"), + class: "topic-query-filter__button", + ) + end + end + end +end