From 0162f0ccb0d648eafb0fe1b0faf6145dd6d3a14a Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Fri, 31 Mar 2023 14:32:12 +0800 Subject: [PATCH] DEV: Update experimental `/filter` route with categories support (#20911) On the `/filter` route, the categories filtering query language is now supported in the input per the example provided below: ``` category:bug => topics in the bug category AND all subcategories =category:bug => topics in the bug category excluding subcategories category:bug,feature => allow for categories either in bug or feature =category:bug,feature => allow for exact categories match excluding sub cats categories: => alias for category ``` Currently composing multiple category filters is not supported as we have yet to determine what behaviour it should result in. For example, `category:bug category:feature` would now return topics that are in both the `bug` and `feature` category but it is not possible for a topic to belong to two categories. --- lib/topics_filter.rb | 44 ++++++++++- spec/lib/topics_filter_spec.rb | 138 +++++++++++++++++++++++++++++++++ 2 files changed, 178 insertions(+), 4 deletions(-) diff --git a/lib/topics_filter.rb b/lib/topics_filter.rb index 8b6044a0eea..2c3995f93be 100644 --- a/lib/topics_filter.rb +++ b/lib/topics_filter.rb @@ -9,14 +9,18 @@ class TopicsFilter def filter_from_query_string(query_string) return @scope if query_string.blank? - query_string.scan(/(?-)?(?\w+):(?[^:\s]+)/) do |exclude, key, value| + query_string.scan( + /(?[-=])?(?\w+):(?[^:\s]+)/, + ) do |key_prefix, key, value| case key when "status" @scope = filter_status(status: value) when "tags" value.scan( - /^(?([a-zA-Z0-9\-]+)(?[,+])?([a-zA-Z0-9\-]+)?(\k[a-zA-Z0-9\-]+)*)$/, - ) do |value, delimiter| + /^(?([a-zA-Z0-9\-]+)(?[,+])?([a-zA-Z0-9\-]+)?(\k[a-zA-Z0-9\-]+)*)$/, + ) do |tag_names, delimiter| + break if key_prefix && key_prefix != "-" + match_all = if delimiter == "," false @@ -25,7 +29,23 @@ class TopicsFilter end @scope = - filter_tags(tag_names: value.split(delimiter), exclude: exclude, match_all: match_all) + filter_tags( + tag_names: tag_names.split(delimiter), + exclude: key_prefix.presence, + match_all: match_all, + ) + end + when "category", "categories" + value.scan( + /^(?([a-zA-Z0-9\-]+)(?[,])?([a-zA-Z0-9\-]+)?(\k[a-zA-Z0-9\-]+)*)$/, + ) do |category_slugs, delimiter| + break if key_prefix && key_prefix != "=" + + @scope = + filter_categories( + category_slugs: category_slugs.split(delimiter), + exclude_subcategories: key_prefix.presence, + ) end end end @@ -60,6 +80,22 @@ class TopicsFilter private + def filter_categories(category_slugs:, exclude_subcategories: false) + category_ids = + Category + .where(slug: category_slugs) + .filter { |category| @guardian.can_see_category?(category) } + .map(&:id) + + return @scope.none if category_ids.length != category_slugs.length + + if !exclude_subcategories + category_ids = category_ids.flat_map { |category_id| Category.subcategory_ids(category_id) } + end + + @scope = @scope.joins(:category).where("categories.id IN (?)", category_ids) + end + def filter_tags(tag_names:, match_all: true, exclude: false) return @scope if !SiteSetting.tagging_enabled? return @scope if tag_names.blank? diff --git a/spec/lib/topics_filter_spec.rb b/spec/lib/topics_filter_spec.rb index e837c49c249..d8dbfa8b741 100644 --- a/spec/lib/topics_filter_spec.rb +++ b/spec/lib/topics_filter_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true RSpec.describe TopicsFilter do + fab!(:user) { Fabricate(:user) } fab!(:admin) { Fabricate(:admin) } fab!(:group) { Fabricate(:group) } @@ -23,6 +24,143 @@ RSpec.describe TopicsFilter do end end + describe "when filtering by categories" do + fab!(:category) { Fabricate(:category, name: "category") } + + fab!(:category_subcategory) do + Fabricate(:category, parent_category: category, name: "category subcategory") + end + + fab!(:category2) { Fabricate(:category, name: "category2") } + + fab!(:category2_subcategory) do + Fabricate(:category, parent_category: category2, name: "category2 subcategory") + end + + fab!(:private_category) do + Fabricate(:private_category, group: group, slug: "private-category") + end + + fab!(:topic_in_category) { Fabricate(:topic, category: category) } + fab!(:topic_in_category_subcategory) { Fabricate(:topic, category: category_subcategory) } + fab!(:topic_in_category2) { Fabricate(:topic, category: category2) } + fab!(:topic_in_category2_subcategory) { Fabricate(:topic, category: category2_subcategory) } + fab!(:topic_in_private_category) { Fabricate(:topic, category: private_category) } + + describe "when query string is `-category:category`" do + it "ignores the filter because the prefix is invalid" do + expect( + TopicsFilter + .new(guardian: Guardian.new) + .filter_from_query_string("-category:category") + .pluck(:id), + ).to contain_exactly( + topic_in_category.id, + topic_in_category_subcategory.id, + topic_in_category2.id, + topic_in_category2_subcategory.id, + topic_in_private_category.id, + ) + end + end + + describe "when query string is `category:private-category`" do + it "should not return any topics when user does not have access to specified category" do + expect( + TopicsFilter + .new(guardian: Guardian.new) + .filter_from_query_string("category:private-category") + .pluck(:id), + ).to eq([]) + end + + it "should return topics from specified category when user has access to specified category" do + group.add(user) + + expect( + TopicsFilter + .new(guardian: Guardian.new(user)) + .filter_from_query_string("category:private-category") + .pluck(:id), + ).to contain_exactly(topic_in_private_category.id) + end + end + + describe "when query string is `category:category`" do + it "should return topics from specified category and its subcategories" do + expect( + TopicsFilter + .new(guardian: Guardian.new) + .filter_from_query_string("category:category") + .pluck(:id), + ).to contain_exactly(topic_in_category.id, topic_in_category_subcategory.id) + end + + it "should return topics from specified category, its subcategories and sub-subcategories" do + SiteSetting.max_category_nesting = 3 + + category_subcategory_subcategory = + Fabricate( + :category, + parent_category: category_subcategory, + name: "category subcategory subcategory", + ) + + topic_in_category_subcategory_subcategory = + Fabricate(:topic, category: category_subcategory_subcategory) + + expect( + TopicsFilter + .new(guardian: Guardian.new) + .filter_from_query_string("category:category") + .pluck(:id), + ).to contain_exactly( + topic_in_category.id, + topic_in_category_subcategory.id, + topic_in_category_subcategory_subcategory.id, + ) + end + end + + describe "when query string is `category:category,category2`" do + it "should return topics from any of the specified categories and its subcategories" do + expect( + TopicsFilter + .new(guardian: Guardian.new) + .filter_from_query_string("category:category,category2") + .pluck(:id), + ).to contain_exactly( + topic_in_category.id, + topic_in_category_subcategory.id, + topic_in_category2.id, + topic_in_category2_subcategory.id, + ) + end + end + + describe "when query string is `=category:category`" do + it "should not return topics from subcategories`" do + expect( + TopicsFilter + .new(guardian: Guardian.new) + .filter_from_query_string("=category:category") + .pluck(:id), + ).to contain_exactly(topic_in_category.id) + end + end + + describe "when query string is `=category:category,category2`" do + it "should not return topics from subcategories" do + expect( + TopicsFilter + .new(guardian: Guardian.new) + .filter_from_query_string("=category:category,category2") + .pluck(:id), + ).to contain_exactly(topic_in_category.id, topic_in_category2.id) + end + end + end + describe "when filtering by status" do fab!(:topic) { Fabricate(:topic) } fab!(:closed_topic) { Fabricate(:topic, closed: true) }