From 93c2ae585c42193af41fc85db47822cf9f73a208 Mon Sep 17 00:00:00 2001 From: Gabriel Grubba <70247653+Grubba27@users.noreply.github.com> Date: Thu, 13 Jun 2024 14:03:49 -0300 Subject: [PATCH] FEATURE: Add tag_group option in `/filter` (#27427) * FEATURE: Add tag_group option in `/filter` * DEV: Update tag_group_filter in `/filter` to use SQL * DEV: Add guardian to `/filter` by tag_group * DEV: remove unused code * DEV: Update tag_group filter implementation * DEV: Add tests to tag_group filter --- lib/topics_filter.rb | 23 ++++++++++ spec/lib/topics_filter_spec.rb | 66 +++++++++++++++++++++++++++ spec/requests/list_controller_spec.rb | 18 ++++++++ 3 files changed, 107 insertions(+) diff --git a/lib/topics_filter.rb b/lib/topics_filter.rb index c360f928dac..ae1ec5bb580 100644 --- a/lib/topics_filter.rb +++ b/lib/topics_filter.rb @@ -74,6 +74,8 @@ class TopicsFilter filter_by_number_of_posters(max: filter_values) when "status" filter_values.each { |status| @scope = filter_status(status: status) } + when "tag_group" + filter_tag_groups(values: key_prefixes.zip(filter_values)) when "tag" filter_tags(values: key_prefixes.zip(filter_values)) when "views-min" @@ -368,6 +370,27 @@ class TopicsFilter all_tag_ids end + def filter_tag_groups(values:) + values.each do |key_prefix, tag_groups| + tag_group_ids = TagGroup.visible(@guardian).where(name: tag_groups).pluck(:id) + exclude_clause = "NOT" if key_prefix == "-" + filter = + "tags.id #{exclude_clause} IN (SELECT tag_id FROM tag_group_memberships WHERE tag_group_id IN (?))" + + query = + if exclude_clause.present? + @scope + .joins("LEFT JOIN topic_tags ON topic_tags.topic_id = topics.id") + .joins("LEFT JOIN tags ON tags.id = topic_tags.tag_id") + .where("tags.id IS NULL OR #{filter}", tag_group_ids) + else + @scope.joins(:tags).where(filter, tag_group_ids) + end + + @scope = query.distinct(:id) + end + end + def filter_tags(values:) return if !SiteSetting.tagging_enabled? diff --git a/spec/lib/topics_filter_spec.rb b/spec/lib/topics_filter_spec.rb index 1ba3e496e57..5fa0a0b9ead 100644 --- a/spec/lib/topics_filter_spec.rb +++ b/spec/lib/topics_filter_spec.rb @@ -929,6 +929,72 @@ RSpec.describe TopicsFilter do end end + describe "when filtering by tag_groups" do + fab!(:tag) { Fabricate(:tag, name: "tag1") } + fab!(:tag2) { Fabricate(:tag, name: "tag2") } + fab!(:tag3) { Fabricate(:tag, name: "tag3") } + + fab!(:topic_without_tag) { Fabricate(:topic) } + fab!(:topic_with_tag) { Fabricate(:topic, tags: [tag]) } + fab!(:topic_with_tag_and_tag2) { Fabricate(:topic, tags: [tag, tag2]) } + fab!(:topic_with_tag2) { Fabricate(:topic, tags: [tag2]) } + + fab!(:tag_group) { Fabricate(:tag_group, tag_names: [tag.name, tag2.name]) } + fab!(:topic_with_tag3) { Fabricate(:topic, tags: [tag3]) } + + fab!(:staff_only_tag) { Fabricate(:tag, name: "group-only-tag") } + fab!(:group) + let!(:staff_tag_group) do + Fabricate( + :tag_group, + permissions: { + group.name => TagGroupPermission.permission_types[:full], + }, + name: "staff-only-tag-group", + tag_names: [staff_only_tag.name], + ) + end + + fab!(:topic_with_staff_only_tag) { Fabricate(:topic, tags: [staff_only_tag]) } + + it "should only return topics that are tagged with any of the specified tag_group when query string is tag_group:tag_group_name" do + expect( + TopicsFilter + .new(guardian: Guardian.new) + .filter_from_query_string("tag_group:#{tag_group.name}") + .pluck(:id), + ).to contain_exactly(topic_with_tag.id, topic_with_tag_and_tag2.id, topic_with_tag2.id) + end + + it "should only return topics that are not excluded by the specified tag_group when query string is -tag_group:tag_group_name" do + expect( + TopicsFilter + .new(guardian: Guardian.new) + .filter_from_query_string("-tag_group:#{tag_group.name}") + .pluck(:id), + ).to contain_exactly(topic_with_tag3.id, topic_without_tag.id, topic_with_staff_only_tag.id) + end + + it "should return the right topics when query string is `tag_group:staff_tag_group` and user has access to specified tag" do + group.add(admin) + + expect( + TopicsFilter + .new(guardian: Guardian.new(admin)) + .filter_from_query_string("tag_group:#{staff_tag_group.name}") + .pluck(:id), + ).to contain_exactly(topic_with_staff_only_tag.id) + end + + it "should not return any topics when query string is `tag_group:staff_tag_group` because specified tag is hidden to user" do + expect( + TopicsFilter + .new(guardian: Guardian.new) + .filter_from_query_string("tag_group:#{staff_tag_group.name}") + .pluck(:id), + ).to eq([]) + end + end describe "when filtering by topic author" do fab!(:user2) { Fabricate(:user, username: "username2") } fab!(:topic_by_user) { Fabricate(:topic, user: user) } diff --git a/spec/requests/list_controller_spec.rb b/spec/requests/list_controller_spec.rb index 6aa49cb9739..17045a88374 100644 --- a/spec/requests/list_controller_spec.rb +++ b/spec/requests/list_controller_spec.rb @@ -1283,6 +1283,24 @@ RSpec.describe ListController do end end + it "should filter with tag_group option" do + topic_with_tag = Fabricate(:topic, tags: [tag]) + topic2_with_tag = Fabricate(:topic, tags: [tag]) + tag_group = Fabricate(:tag_group, tags: [tag]) + + sign_in(user) + + get "/filter.json", params: { q: "tag_group:#{tag_group.name}" } + + parsed = response.parsed_body + expect(response.status).to eq(200) + expect(parsed["topic_list"]["topics"].length).to eq(2) + expect(parsed["topic_list"]["topics"].map { |topic| topic["id"] }).to contain_exactly( + topic_with_tag.id, + topic2_with_tag.id, + ) + end + describe "when filtering with the `created-by:` filter" do fab!(:topic2) { Fabricate(:topic, user: admin) }