Commit 00b81c87 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Add OR filtering for author username

This adds OR filtering of authors for issues, MRs, and epics
parent 7b52ec04
...@@ -88,7 +88,7 @@ class IssuableFinder ...@@ -88,7 +88,7 @@ class IssuableFinder
end end
def valid_params def valid_params
@valid_params ||= scalar_params + [array_params.merge(not: {})] @valid_params ||= scalar_params + [array_params.merge(or: {}, not: {})]
end end
end end
...@@ -377,6 +377,7 @@ class IssuableFinder ...@@ -377,6 +377,7 @@ class IssuableFinder
Issuables::AuthorFilter.new( Issuables::AuthorFilter.new(
items, items,
params: original_params, params: original_params,
or_filters_enabled: or_filters_enabled?,
not_filters_enabled: not_filters_enabled? not_filters_enabled: not_filters_enabled?
).filter ).filter
end end
...@@ -498,6 +499,12 @@ class IssuableFinder ...@@ -498,6 +499,12 @@ class IssuableFinder
params[:non_archived].present? ? items.non_archived : items params[:non_archived].present? ? items.non_archived : items
end end
def or_filters_enabled?
strong_memoize(:or_filters_enabled) do
Feature.enabled?(:or_issuable_queries, feature_flag_scope, default_enabled: :yaml)
end
end
def not_filters_enabled? def not_filters_enabled?
strong_memoize(:not_filters_enabled) do strong_memoize(:not_filters_enabled) do
Feature.enabled?(:not_issuable_queries, feature_flag_scope, default_enabled: :yaml) Feature.enabled?(:not_issuable_queries, feature_flag_scope, default_enabled: :yaml)
......
# frozen_string_literal: true
module Issuables module Issuables
class AuthorFilter < BaseFilter class AuthorFilter < BaseFilter
def filter def filter
filtered = by_author(issuables) filtered = by_author(issuables)
filtered = by_author_union(filtered)
by_negated_author(filtered) by_negated_author(filtered)
end end
private private
def by_author(issuables) def by_author(issuables)
if no_author? if params[:author_id].present?
issuables.where(author_id: nil) issuables.authored(params[:author_id])
elsif params[:author_id].present?
issuables.where(author_id: params[:author_id])
elsif params[:author_username].present? elsif params[:author_username].present?
issuables.where(author_id: authors_by_username(params[:author_username])) issuables.authored(User.by_username(params[:author_username]))
else else
issuables issuables
end end
end end
def by_author_union(issuables)
return issuables unless or_filters_enabled? && or_params&.fetch(:author_username).present?
issuables.authored(User.by_username(or_params[:author_username]))
end
def by_negated_author(issuables) def by_negated_author(issuables)
return issuables unless not_params.present? && not_filters_enabled? return issuables unless not_filters_enabled? && not_params.present?
if not_params[:author_id].present? if not_params[:author_id].present?
issuables.where.not(author_id: not_params[:author_id]) issuables.not_authored(not_params[:author_id])
elsif not_params[:author_username].present? elsif not_params[:author_username].present?
issuables.where.not(author_id: authors_by_username(not_params[:author_username])) issuables.not_authored(User.by_username(not_params[:author_username]))
else else
issuables issuables
end end
end end
def no_author?
# author_id takes precedence over author_username
params[:author_id] == NONE || params[:author_username] == NONE
end
def authors_by_username(usernames)
User.where(username: usernames)
end
end end
end end
# frozen_string_literal: true
module Issuables module Issuables
class BaseFilter class BaseFilter
# This is used as a common filter for None / Any
FILTER_NONE = 'none'
FILTER_ANY = 'any'
# This is used in unassigning users
NONE = '0'
attr_reader :issuables, :params attr_reader :issuables, :params
def initialize(issuables, params:, not_filters_enabled: false) def initialize(issuables, params:, or_filters_enabled: false, not_filters_enabled: false)
@issuables = issuables @issuables = issuables
@params = params @params = params
@or_filters_enabled = or_filters_enabled
@not_filters_enabled = not_filters_enabled @not_filters_enabled = not_filters_enabled
end end
...@@ -21,10 +17,18 @@ module Issuables ...@@ -21,10 +17,18 @@ module Issuables
private private
def or_params
params[:or]
end
def not_params def not_params
params[:not] params[:not]
end end
def or_filters_enabled?
@or_filters_enabled
end
def not_filters_enabled? def not_filters_enabled?
@not_filters_enabled @not_filters_enabled
end end
......
...@@ -86,6 +86,7 @@ module Issuable ...@@ -86,6 +86,7 @@ module Issuable
before_validation :truncate_description_on_import! before_validation :truncate_description_on_import!
scope :authored, ->(user) { where(author_id: user) } scope :authored, ->(user) { where(author_id: user) }
scope :not_authored, ->(user) { where.not(author_id: user) }
scope :recent, -> { reorder(id: :desc) } scope :recent, -> { reorder(id: :desc) }
scope :of_projects, ->(ids) { where(project_id: ids) } scope :of_projects, ->(ids) { where(project_id: ids) }
scope :opened, -> { with_state(:opened) } scope :opened, -> { with_state(:opened) }
......
---
name: or_issuable_queries
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/54444
rollout_issue_url:
milestone: '13.10'
type: development
group: group::project management
default_enabled: false
...@@ -105,6 +105,22 @@ RSpec.describe EpicsFinder do ...@@ -105,6 +105,22 @@ RSpec.describe EpicsFinder do
it 'returns all epics authored by the given user' do it 'returns all epics authored by the given user' do
expect(epics(author_id: user.id)).to contain_exactly(epic2) expect(epics(author_id: user.id)).to contain_exactly(epic2)
end end
context 'using OR' do
it 'returns all epics authored by any of the given users' do
expect(epics(or: { author_username: [epic2.author.username, epic3.author.username] })).to contain_exactly(epic2, epic3)
end
context 'when feature flag is disabled' do
before do
stub_feature_flags(or_issuable_queries: false)
end
it 'does not add any filter' do
expect(epics(or: { author_username: [epic2.author.username, epic3.author.username] })).to contain_exactly(epic1, epic2, epic3)
end
end
end
end end
context 'by label' do context 'by label' do
......
...@@ -179,33 +179,54 @@ RSpec.describe IssuesFinder do ...@@ -179,33 +179,54 @@ RSpec.describe IssuesFinder do
end end
end end
context 'filtering by author ID' do context 'filtering by author' do
let(:params) { { author_id: user2.id } } context 'by author ID' do
let(:params) { { author_id: user2.id } }
it 'returns issues created by that user' do it 'returns issues created by that user' do
expect(issues).to contain_exactly(issue3) expect(issues).to contain_exactly(issue3)
end
end end
end
context 'filtering by not author ID' do context 'using OR' do
let(:params) { { not: { author_id: user2.id } } } let(:issue6) { create(:issue, project: project2) }
let(:params) { { or: { author_username: [issue3.author.username, issue6.author.username] } } }
it 'returns issues created by any of the given users' do
expect(issues).to contain_exactly(issue3, issue6)
end
context 'when feature flag is disabled' do
before do
stub_feature_flags(or_issuable_queries: false)
end
it 'returns issues not created by that user' do it 'does not add any filter' do
expect(issues).to contain_exactly(issue1, issue2, issue4, issue5) expect(issues).to contain_exactly(issue1, issue2, issue3, issue4, issue5, issue6)
end
end
end end
end
context 'filtering by nonexistent author ID and issue term using CTE for search' do context 'filtering by NOT author ID' do
let(:params) do let(:params) { { not: { author_id: user2.id } } }
{
author_id: 'does-not-exist', it 'returns issues not created by that user' do
search: 'git', expect(issues).to contain_exactly(issue1, issue2, issue4, issue5)
attempt_group_search_optimizations: true end
}
end end
it 'returns no results' do context 'filtering by nonexistent author ID and issue term using CTE for search' do
expect(issues).to be_empty let(:params) do
{
author_id: 'does-not-exist',
search: 'git',
attempt_group_search_optimizations: true
}
end
it 'returns no results' do
expect(issues).to be_empty
end
end end
end end
......
...@@ -41,30 +41,51 @@ RSpec.describe MergeRequestsFinder do ...@@ -41,30 +41,51 @@ RSpec.describe MergeRequestsFinder do
expect(merge_requests).to contain_exactly(merge_request1) expect(merge_requests).to contain_exactly(merge_request1)
end end
it 'filters by nonexistent author ID and MR term using CTE for search' do context 'filtering by author' do
params = { subject(:merge_requests) { described_class.new(user, params).execute }
author_id: 'does-not-exist',
search: 'git',
attempt_group_search_optimizations: true
}
merge_requests = described_class.new(user, params).execute context 'using OR' do
let(:params) { { or: { author_username: [merge_request1.author.username, merge_request2.author.username] } } }
expect(merge_requests).to be_empty before do
end merge_request1.update!(author: create(:user))
merge_request2.update!(author: create(:user))
end
it 'returns merge requests created by any of the given users' do
expect(merge_requests).to contain_exactly(merge_request1, merge_request2)
end
context 'filtering by not author ID' do context 'when feature flag is disabled' do
let(:params) { { not: { author_id: user2.id } } } before do
stub_feature_flags(or_issuable_queries: false)
end
before do it 'does not add any filter' do
merge_request2.update!(author: user2) expect(merge_requests).to contain_exactly(merge_request1, merge_request2, merge_request3, merge_request4, merge_request5)
merge_request3.update!(author: user2) end
end
end end
it 'returns merge requests not created by that user' do context 'with nonexistent author ID and MR term using CTE for search' do
merge_requests = described_class.new(user, params).execute let(:params) { { author_id: 'does-not-exist', search: 'git', attempt_group_search_optimizations: true } }
it 'returns no results' do
expect(merge_requests).to be_empty
end
end
expect(merge_requests).to contain_exactly(merge_request1, merge_request4, merge_request5) context 'filtering by not author ID' do
let(:params) { { not: { author_id: user2.id } } }
before do
merge_request2.update!(author: user2)
merge_request3.update!(author: user2)
end
it 'returns merge requests not created by that user' do
expect(merge_requests).to contain_exactly(merge_request1, merge_request4, merge_request5)
end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment