diff --git a/lib/topic_query.rb b/lib/topic_query.rb index 2e8c0b35982..2d93e743925 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -799,7 +799,7 @@ class TopicQuery scope: result, guardian: @guardian, category_id: options[:category], - ).filter(status: options[:status]) + ).filter_status(status: options[:status]) end if (filter = (options[:filter] || options[:f])) && @user diff --git a/lib/topics_filter.rb b/lib/topics_filter.rb index eebc1cef0c7..c5fc97eed50 100644 --- a/lib/topics_filter.rb +++ b/lib/topics_filter.rb @@ -7,14 +7,32 @@ class TopicsFilter @category = category_id.present? ? Category.find_by(id: category_id) : nil end - def filter(status: nil) - filter_status(@scope, status) if status + def filter_tags(tag_names:, match_all: true, exclude: false) + return @scope if !SiteSetting.tagging_enabled? + return @scope if tag_names.blank? + + tag_ids = + DiscourseTagging + .filter_visible(Tag, @guardian) + .where_name(tag_names) + .pluck(:id, :target_tag_id) + + tag_ids.flatten! + tag_ids.uniq! + tag_ids.compact! + + return @scope.none if match_all && tag_ids.length != tag_names.length + return @scope if tag_ids.empty? + + self.send( + "#{exclude ? "exclude" : "include"}_topics_with_#{match_all ? "all" : "any"}_tags", + tag_ids, + ) + @scope end - private - - def filter_status(scope, status) + def filter_status(status:) case status when "open" @scope = @scope.where("NOT topics.closed AND NOT topics.archived") @@ -31,5 +49,47 @@ class TopicsFilter @scope = @scope.unscope(where: :deleted_at).where("topics.deleted_at IS NOT NULL") end end + + @scope + end + + private + + def exclude_topics_with_all_tags(tag_ids) + where_clause = [] + + tag_ids.each_with_index do |tag_id, index| + sql_alias = "tt#{index}" + + @scope = + @scope.joins( + "LEFT JOIN topic_tags #{sql_alias} ON #{sql_alias}.topic_id = topics.id AND #{sql_alias}.tag_id = #{tag_id}", + ) + + where_clause << "#{sql_alias}.topic_id IS NULL" + end + + @scope = @scope.where(where_clause.join(" OR ")) + end + + def exclude_topics_with_any_tags(tag_ids) + @scope = + @scope + .left_joins(:topic_tags) + .where("topic_tags.tag_id IS NULL OR topic_tags.tag_id NOT IN (?)", tag_ids) + .distinct(:id) + end + + def include_topics_with_all_tags(tag_ids) + tag_ids.each_with_index do |tag_id, index| + @scope = + @scope.joins( + "INNER JOIN topic_tags tt#{index} ON tt#{index}.topic_id = topics.id AND tt#{index}.tag_id = #{tag_id}", + ) + end + end + + def include_topics_with_any_tags(tag_ids) + @scope = @scope.joins(:topic_tags).where("topic_tags.tag_id IN (?)", tag_ids).distinct(:id) end end diff --git a/spec/lib/topics_filter_spec.rb b/spec/lib/topics_filter_spec.rb index 6ca92f3a40f..3779d4fee09 100644 --- a/spec/lib/topics_filter_spec.rb +++ b/spec/lib/topics_filter_spec.rb @@ -2,62 +2,177 @@ 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, + describe "#filter_status" do + 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 } + + it "should only return topics that have not been closed or archived when status is `open`" do + expect( + TopicsFilter.new(guardian: Guardian.new).filter_status(status: "open").pluck(:id), + ).to contain_exactly(topic.id) + end + + it "should only return topics that have been deleted when status is `deleted` and user can see deleted topics" do + expect( + TopicsFilter.new(guardian: Guardian.new(admin)).filter_status(status: "deleted").pluck(:id), + ).to contain_exactly(deleted_topic_id) + end + + it "should status filter when status is `deleted` and user cannot see deleted topics" do + expect( + TopicsFilter.new(guardian: Guardian.new).filter_status(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 status is `archived`" do + expect( + TopicsFilter.new(guardian: Guardian.new).filter_status(status: "archived").pluck(:id), + ).to contain_exactly(archived_topic.id) + end + + it "should only return topics that are visible when status is `listed`" do + Topic.update_all(visible: false) + topic.update!(visible: true) + + expect( + TopicsFilter.new(guardian: Guardian.new).filter_status(status: "listed").pluck(:id), + ).to contain_exactly(topic.id) + end + + it "should only return topics that are not visible when status is `unlisted`" do + Topic.update_all(visible: true) + topic.update!(visible: false) + + expect( + TopicsFilter.new(guardian: Guardian.new).filter_status(status: "unlisted").pluck(:id), + ).to contain_exactly(topic.id) + end + end + + describe "#filter_tags" do + fab!(:tag) { Fabricate(:tag) } + fab!(:tag2) { Fabricate(:tag) } + + fab!(:group_only_tag) { Fabricate(:tag) } + fab!(:group) { Fabricate(:group) } + + let!(:staff_tag_group) do + Fabricate( + :tag_group, + permissions: { + group.name => TagGroupPermission.permission_types[:full], + }, + tag_names: [group_only_tag.name], ) end - context "when filtering by topic's status" do - it "should only return topics that have not been closed or archived when status is `open`" do - expect( - TopicsFilter.new(guardian: Guardian.new).filter(status: "open").pluck(:id), - ).to contain_exactly(topic.id) - end + 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!(:topic_with_group_only_tag) { Fabricate(:topic, tags: [group_only_tag]) } - it "should only return topics that have been deleted when status is `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 not filter any topics by tags when tagging is disabled" do + SiteSetting.tagging_enabled = false - it "should status filter when status is `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 + expect( + TopicsFilter + .new(guardian: Guardian.new) + .filter_tags(tag_names: [tag.name, tag2.name], match_all: true, exclude: false) + .pluck(:id), + ).to contain_exactly( + topic_without_tag.id, + topic_with_tag.id, + topic_with_tag_and_tag2.id, + topic_with_tag2.id, + topic_with_group_only_tag.id, + ) + end - it "should only return topics that have been archived when status is `archived`" do - expect( - TopicsFilter.new(guardian: Guardian.new).filter(status: "archived").pluck(:id), - ).to contain_exactly(archived_topic.id) - end + it "should only return topics that are tagged with all of the specified tags when `match_all` is `true`" do + expect( + TopicsFilter + .new(guardian: Guardian.new) + .filter_tags(tag_names: [tag.name, tag2.name], match_all: true, exclude: false) + .pluck(:id), + ).to contain_exactly(topic_with_tag_and_tag2.id) + end - it "should only return topics that are visible when status is `listed`" do - Topic.update_all(visible: false) - topic.update!(visible: true) + it "should only return topics that are tagged with any of the specified tags when `match_all` is `false`" do + expect( + TopicsFilter + .new(guardian: Guardian.new) + .filter_tags(tag_names: [tag2.name], match_all: false, exclude: false) + .pluck(:id), + ).to contain_exactly(topic_with_tag_and_tag2.id, topic_with_tag2.id) + end - expect( - TopicsFilter.new(guardian: Guardian.new).filter(status: "listed").pluck(:id), - ).to contain_exactly(topic.id) - end + it "should not return any topics when `match_all` is `true` and one of specified tags is invalid" do + expect( + TopicsFilter + .new(guardian: Guardian.new) + .filter_tags(tag_names: ["invalid", tag.name, tag2.name], match_all: true, exclude: false) + .pluck(:id), + ).to eq([]) + end - it "should only return topics that are not visible when status is `unlisted`" do - Topic.update_all(visible: true) - topic.update!(visible: false) + it "should still filter topics by specificed tags when `match_all` is `false` even if one of the tags is invalid" do + expect( + TopicsFilter + .new(guardian: Guardian.new) + .filter_tags( + tag_names: ["invalid", tag.name, tag2.name], + match_all: false, + exclude: false, + ) + .pluck(:id), + ).to contain_exactly(topic_with_tag_and_tag2.id, topic_with_tag.id, topic_with_tag2.id) + end - expect( - TopicsFilter.new(guardian: Guardian.new).filter(status: "unlisted").pluck(:id), - ).to contain_exactly(topic.id) - end + it "should not return any topics when user tries to filter topics by tags that are hidden" do + expect( + TopicsFilter + .new(guardian: Guardian.new) + .filter_tags(tag_names: [group_only_tag.name], match_all: true, exclude: false) + .pluck(:id), + ).to eq([]) + end + + it "should allow user with permission to filter topics by tags that are hidden" do + group.add(admin) + + expect( + TopicsFilter + .new(guardian: Guardian.new(admin)) + .filter_tags(tag_names: [group_only_tag.name]) + .pluck(:id), + ).to contain_exactly(topic_with_group_only_tag.id) + end + + it "should only return topics that are not tagged with all of the specified tags when `match_all` is `true` and `exclude` is `true`" do + expect( + TopicsFilter + .new(guardian: Guardian.new) + .filter_tags(tag_names: [tag.name, tag2.name], match_all: true, exclude: true) + .pluck(:id), + ).to contain_exactly( + topic_without_tag.id, + topic_with_tag.id, + topic_with_tag2.id, + topic_with_group_only_tag.id, + ) + end + + it "should only return topics that are not tagged with any of the specified tags when `match_all` is `false` and `exclude` is `true`" do + expect( + TopicsFilter + .new(guardian: Guardian.new) + .filter_tags(tag_names: [tag.name, tag2.name], match_all: false, exclude: true) + .pluck(:id), + ).to contain_exactly(topic_without_tag.id, topic_with_group_only_tag.id) end end end