DEV: Support `in:<notification level>` filter on `/filter` route (#21038)
This commit adds support for the `in:<topic notification level>` query filter. As an example, `in:tracking` will filter for topics that the user is watching. Filtering for multiple topic notification levels can be done by comma separating the topic notification level keys. For example, `in:muted,tracking` or `in:muted,tracking,watching`. Alternatively, the user can also compose multiple filters with `in:muted in:tracking` which translates to the same behaviour as `in:muted,tracking`.
This commit is contained in:
parent
c68497159f
commit
2809d7ba8e
|
@ -269,10 +269,14 @@ class TopicQuery
|
|||
end
|
||||
|
||||
def list_filter
|
||||
results =
|
||||
TopicsFilter.new(guardian: @guardian, scope: latest_results).filter_from_query_string(
|
||||
@options[:q],
|
||||
)
|
||||
topics_filter =
|
||||
TopicsFilter.new(guardian: @guardian, scope: latest_results(include_muted: false))
|
||||
|
||||
results = topics_filter.filter_from_query_string(@options[:q])
|
||||
|
||||
if !topics_filter.topic_notification_levels.include?(NotificationLevels.all[:muted])
|
||||
results = remove_muted_topics(results, @user)
|
||||
end
|
||||
|
||||
create_list(:filter, {}, results)
|
||||
end
|
||||
|
@ -846,7 +850,11 @@ class TopicQuery
|
|||
end
|
||||
|
||||
def remove_muted(list, user, options)
|
||||
list = remove_muted_topics(list, user) unless options && options[:state] == "muted"
|
||||
if options && (options[:include_muted].nil? || options[:include_muted]) &&
|
||||
options[:state] != "muted"
|
||||
list = remove_muted_topics(list, user)
|
||||
end
|
||||
|
||||
list = remove_muted_categories(list, user, exclude: options[:category])
|
||||
TopicQuery.remove_muted_tags(list, user, options)
|
||||
end
|
||||
|
|
|
@ -4,11 +4,16 @@ class TopicsFilter
|
|||
def initialize(guardian:, scope: Topic)
|
||||
@guardian = guardian
|
||||
@scope = scope
|
||||
@notification_levels = Set.new
|
||||
@registered_scopes = {}
|
||||
end
|
||||
|
||||
def topic_notification_levels
|
||||
@registered_scopes.dig(:topic_notification_levels, :params, :notification_levels) || []
|
||||
end
|
||||
|
||||
def filter_from_query_string(query_string)
|
||||
return @scope if query_string.blank?
|
||||
category_or_clause = false
|
||||
|
||||
query_string.scan(
|
||||
/(?<key_prefix>[-=])?(?<key>\w+):(?<value>[^\s]+)/,
|
||||
|
@ -48,14 +53,13 @@ class TopicsFilter
|
|||
filter_categories(
|
||||
category_slugs: category_slugs.split(delimiter),
|
||||
exclude_subcategories: key_prefix.presence,
|
||||
or_clause: category_or_clause,
|
||||
)
|
||||
|
||||
category_or_clause = true
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@registered_scopes.each_value { |value| @scope = value[:block].call(*value[:params].values) }
|
||||
|
||||
@scope
|
||||
end
|
||||
|
||||
|
@ -101,11 +105,36 @@ class TopicsFilter
|
|||
@guardian.user.id,
|
||||
)
|
||||
else
|
||||
return @scope.none if !@guardian.user
|
||||
|
||||
topic_notification_levels = Set.new
|
||||
|
||||
state
|
||||
.split(",")
|
||||
.each do |topic_notification_level|
|
||||
if level = TopicUser.notification_levels[topic_notification_level.to_sym]
|
||||
topic_notification_levels << level
|
||||
end
|
||||
end
|
||||
|
||||
register_scope(
|
||||
key: :topic_notification_levels,
|
||||
params: {
|
||||
notification_levels: topic_notification_levels.to_a,
|
||||
},
|
||||
) do |notification_levels|
|
||||
@scope.joins(:topic_users).where(
|
||||
"topic_users.notification_level IN (:topic_notification_levels) AND topic_users.user_id = :user_id",
|
||||
topic_notification_levels: notification_levels,
|
||||
user_id: @guardian.user.id,
|
||||
)
|
||||
end
|
||||
|
||||
@scope
|
||||
end
|
||||
end
|
||||
|
||||
def filter_categories(category_slugs:, exclude_subcategories: false, or_clause: false)
|
||||
def filter_categories(category_slugs:, exclude_subcategories: false)
|
||||
category_ids = Category.ids_from_slugs(category_slugs)
|
||||
|
||||
category_ids =
|
||||
|
@ -121,11 +150,14 @@ class TopicsFilter
|
|||
category_ids = category_ids.flat_map { |category_id| Category.subcategory_ids(category_id) }
|
||||
end
|
||||
|
||||
if or_clause
|
||||
@scope.or(Topic.where("categories.id IN (?)", category_ids))
|
||||
else
|
||||
@scope.joins(:category).where("categories.id IN (?)", category_ids)
|
||||
register_scope(key: :categories, params: { category_ids: category_ids }) do |in_category_ids|
|
||||
@scope.joins(:category).where(
|
||||
"categories.id IN (:category_ids)",
|
||||
category_ids: in_category_ids,
|
||||
)
|
||||
end
|
||||
|
||||
@scope
|
||||
end
|
||||
|
||||
def filter_tags(tag_names:, match_all: true, exclude: false)
|
||||
|
@ -197,4 +229,18 @@ class TopicsFilter
|
|||
def include_topics_with_any_tags(tag_ids)
|
||||
@scope = @scope.joins(:topic_tags).where("topic_tags.tag_id IN (?)", tag_ids).distinct(:id)
|
||||
end
|
||||
|
||||
def register_scope(key:, params:, &block)
|
||||
if registered_scope = @registered_scopes[key]
|
||||
registered_scope[:params].each do |param_name, param_value|
|
||||
if param_value.is_a?(Array)
|
||||
registered_scope[:params][param_name].concat(params[param_name]).tap(&:uniq!)
|
||||
else
|
||||
registered_scope[:params][param_name] = params[param_name]
|
||||
end
|
||||
end
|
||||
else
|
||||
@registered_scopes[key] = { block: block, params: params }
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -90,6 +90,103 @@ RSpec.describe TopicsFilter do
|
|||
).to contain_exactly(topic.id)
|
||||
end
|
||||
end
|
||||
|
||||
TopicUser.notification_levels.keys.each do |notification_level|
|
||||
describe "when query string is `in:#{notification_level}`" do
|
||||
fab!("user_#{notification_level}_topic".to_sym) do
|
||||
Fabricate(:topic).tap do |topic|
|
||||
TopicUser.change(
|
||||
user.id,
|
||||
topic.id,
|
||||
notification_level: TopicUser.notification_levels[notification_level],
|
||||
)
|
||||
end
|
||||
end
|
||||
|
||||
it "should not return any topics if the user is anonymous" do
|
||||
expect(
|
||||
TopicsFilter
|
||||
.new(guardian: Guardian.new)
|
||||
.filter_from_query_string("in:#{notification_level}")
|
||||
.pluck(:id),
|
||||
).to eq([])
|
||||
end
|
||||
|
||||
it "should return topics that the user has notification level set to #{notification_level}" do
|
||||
expect(
|
||||
TopicsFilter
|
||||
.new(guardian: Guardian.new(user))
|
||||
.filter_from_query_string("in:#{notification_level}")
|
||||
.pluck(:id),
|
||||
).to contain_exactly(self.public_send("user_#{notification_level}_topic").id)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe "when filtering by multiple topic notification levels" do
|
||||
fab!(:user_muted_topic) do
|
||||
Fabricate(:topic).tap do |topic|
|
||||
TopicUser.change(
|
||||
user.id,
|
||||
topic.id,
|
||||
notification_level: TopicUser.notification_levels[:muted],
|
||||
)
|
||||
end
|
||||
end
|
||||
|
||||
fab!(:user_watching_topic) do
|
||||
Fabricate(:topic).tap do |topic|
|
||||
TopicUser.change(
|
||||
user.id,
|
||||
topic.id,
|
||||
notification_level: TopicUser.notification_levels[:watching],
|
||||
)
|
||||
end
|
||||
end
|
||||
|
||||
fab!(:user_tracking_topic) do
|
||||
Fabricate(:topic).tap do |topic|
|
||||
TopicUser.change(
|
||||
user.id,
|
||||
topic.id,
|
||||
notification_level: TopicUser.notification_levels[:tracking],
|
||||
)
|
||||
end
|
||||
end
|
||||
|
||||
describe "when query string is `in:muted,invalid`" do
|
||||
it "should ignore the invalid notification level" do
|
||||
expect(
|
||||
TopicsFilter
|
||||
.new(guardian: Guardian.new(user))
|
||||
.filter_from_query_string("in:muted,invalid")
|
||||
.pluck(:id),
|
||||
).to contain_exactly(user_muted_topic.id)
|
||||
end
|
||||
end
|
||||
|
||||
describe "when query string is `in:muted in:tracking`" do
|
||||
it "should return topics that the user is tracking or has muted" do
|
||||
expect(
|
||||
TopicsFilter
|
||||
.new(guardian: Guardian.new(user))
|
||||
.filter_from_query_string("in:muted in:tracking")
|
||||
.pluck(:id),
|
||||
).to contain_exactly(user_muted_topic.id, user_tracking_topic.id)
|
||||
end
|
||||
end
|
||||
|
||||
describe "when query string is `in:muted,tracking" do
|
||||
it "should return topics that the user is tracking or has muted" do
|
||||
expect(
|
||||
TopicsFilter
|
||||
.new(guardian: Guardian.new(user))
|
||||
.filter_from_query_string("in:muted,tracking")
|
||||
.pluck(:id),
|
||||
).to contain_exactly(user_muted_topic.id, user_tracking_topic.id)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe "when filtering by categories" do
|
||||
|
|
|
@ -1212,6 +1212,52 @@ RSpec.describe ListController do
|
|||
end
|
||||
end
|
||||
|
||||
describe "when filtering with the `in:<topic_notification_level>` filter" do
|
||||
fab!(:user_muted_topic) do
|
||||
Fabricate(:topic).tap do |topic|
|
||||
TopicUser.change(
|
||||
user.id,
|
||||
topic.id,
|
||||
notification_level: TopicUser.notification_levels[:muted],
|
||||
)
|
||||
end
|
||||
end
|
||||
|
||||
fab!(:user_tracking_topic) do
|
||||
Fabricate(:topic).tap do |topic|
|
||||
TopicUser.change(
|
||||
user.id,
|
||||
topic.id,
|
||||
notification_level: TopicUser.notification_levels[:tracking],
|
||||
)
|
||||
end
|
||||
end
|
||||
|
||||
it "does not return topics that are muted by the user when `q` query param does not include `in:muted`" do
|
||||
sign_in(user)
|
||||
|
||||
get "/filter.json", params: { q: "in:tracking" }
|
||||
|
||||
expect(response.status).to eq(200)
|
||||
|
||||
expect(response.parsed_body["topic_list"]["topics"].map { |topic| topic["id"] }).to eq(
|
||||
[user_tracking_topic.id],
|
||||
)
|
||||
end
|
||||
|
||||
it "only return topics that are muted by the user when `q` query param is `in:muted`" do
|
||||
sign_in(user)
|
||||
|
||||
get "/filter.json", params: { q: "in:muted" }
|
||||
|
||||
expect(response.status).to eq(200)
|
||||
|
||||
expect(response.parsed_body["topic_list"]["topics"].map { |topic| topic["id"] }).to eq(
|
||||
[user_muted_topic.id],
|
||||
)
|
||||
end
|
||||
end
|
||||
|
||||
describe "when filtering by status" do
|
||||
fab!(:group) { Fabricate(:group) }
|
||||
fab!(:private_category) { Fabricate(:private_category, group: group) }
|
||||
|
|
Loading…
Reference in New Issue