Commit 550f2982 authored by David Kim's avatar David Kim Committed by charlie ablett

Load review requested MRs in assignee filter

- It loads review requested MRs as part of Assignee filter
- We would probably want a dedicated filter for it down the track, but
this is our first iteration
parent 27b3ad4a
...@@ -150,7 +150,7 @@ module IssuableCollections ...@@ -150,7 +150,7 @@ module IssuableCollections
common_attributes + [:project, project: :namespace] common_attributes + [:project, project: :namespace]
when 'MergeRequest' when 'MergeRequest'
common_attributes + [ common_attributes + [
:target_project, :latest_merge_request_diff, :approvals, :approved_by_users, :target_project, :latest_merge_request_diff, :approvals, :approved_by_users, :reviewers,
source_project: :route, head_pipeline: :project, target_project: :namespace source_project: :route, head_pipeline: :project, target_project: :namespace
] ]
end end
......
...@@ -397,7 +397,7 @@ class IssuableFinder ...@@ -397,7 +397,7 @@ class IssuableFinder
elsif params.filter_by_any_assignee? elsif params.filter_by_any_assignee?
items.assigned items.assigned
elsif params.assignee elsif params.assignee
items.assigned_to(params.assignee) items_assigned_to(items, params.assignee)
elsif params.assignee_id? || params.assignee_username? # assignee not found elsif params.assignee_id? || params.assignee_username? # assignee not found
items.none items.none
else else
...@@ -405,6 +405,10 @@ class IssuableFinder ...@@ -405,6 +405,10 @@ class IssuableFinder
end end
end end
def items_assigned_to(items, user)
items.assigned_to(user)
end
def by_negated_assignee(items) def by_negated_assignee(items)
# We want CE users to be able to say "Issues not assigned to either PersonA nor PersonB" # We want CE users to be able to say "Issues not assigned to either PersonA nor PersonB"
if not_params.assignees.present? if not_params.assignees.present?
......
...@@ -145,6 +145,10 @@ class MergeRequestsFinder < IssuableFinder ...@@ -145,6 +145,10 @@ class MergeRequestsFinder < IssuableFinder
.execute(items) .execute(items)
end end
# rubocop: enable CodeReuse/Finder # rubocop: enable CodeReuse/Finder
def items_assigned_to(items, user)
MergeRequest.from_union([super, items.reviewer_assigned_to(user)])
end
end end
MergeRequestsFinder.prepend_if_ee('EE::MergeRequestsFinder') MergeRequestsFinder.prepend_if_ee('EE::MergeRequestsFinder')
...@@ -302,6 +302,10 @@ class MergeRequest < ApplicationRecord ...@@ -302,6 +302,10 @@ class MergeRequest < ApplicationRecord
includes(:metrics) includes(:metrics)
end end
scope :reviewer_assigned_to, ->(user) do
where("EXISTS (SELECT TRUE FROM merge_request_reviewers WHERE user_id = ? AND merge_request_id = merge_requests.id)", user.id)
end
after_save :keep_around_commit, unless: :importing? after_save :keep_around_commit, unless: :importing?
alias_attribute :project, :target_project alias_attribute :project, :target_project
......
...@@ -52,20 +52,29 @@ RSpec.describe 'Dashboard Merge Requests' do ...@@ -52,20 +52,29 @@ RSpec.describe 'Dashboard Merge Requests' do
end end
context 'merge requests exist' do context 'merge requests exist' do
let_it_be(:author_user) { create(:user) }
let(:label) { create(:label) } let(:label) { create(:label) }
let!(:assigned_merge_request) do let!(:assigned_merge_request) do
create(:merge_request, create(:merge_request,
assignees: [current_user], assignees: [current_user],
source_project: project, source_project: project,
author: create(:user)) author: author_user)
end
let!(:review_requested_merge_request) do
create(:merge_request,
reviewers: [current_user],
source_branch: 'review',
source_project: project,
author: author_user)
end end
let!(:assigned_merge_request_from_fork) do let!(:assigned_merge_request_from_fork) do
create(:merge_request, create(:merge_request,
source_branch: 'markdown', assignees: [current_user], source_branch: 'markdown', assignees: [current_user],
target_project: public_project, source_project: forked_project, target_project: public_project, source_project: forked_project,
author: create(:user)) author: author_user)
end end
let!(:authored_merge_request) do let!(:authored_merge_request) do
...@@ -94,7 +103,7 @@ RSpec.describe 'Dashboard Merge Requests' do ...@@ -94,7 +103,7 @@ RSpec.describe 'Dashboard Merge Requests' do
create(:merge_request, create(:merge_request,
source_branch: 'fix', source_branch: 'fix',
source_project: project, source_project: project,
author: create(:user)) author: author_user)
end end
before do before do
...@@ -111,6 +120,10 @@ RSpec.describe 'Dashboard Merge Requests' do ...@@ -111,6 +120,10 @@ RSpec.describe 'Dashboard Merge Requests' do
expect(page).not_to have_content(labeled_merge_request.title) expect(page).not_to have_content(labeled_merge_request.title)
end end
it 'shows review requested merge requests' do
expect(page).to have_content(review_requested_merge_request.title)
end
it 'shows authored merge requests', :js do it 'shows authored merge requests', :js do
reset_filters reset_filters
input_filtered_search("author:=#{current_user.to_reference}") input_filtered_search("author:=#{current_user.to_reference}")
......
...@@ -333,6 +333,8 @@ RSpec.describe MergeRequestsFinder do ...@@ -333,6 +333,8 @@ RSpec.describe MergeRequestsFinder do
end end
context 'assignee filtering' do context 'assignee filtering' do
let_it_be(:user3) { create(:user) }
let(:issuables) { described_class.new(user, params).execute } let(:issuables) { described_class.new(user, params).execute }
it_behaves_like 'assignee ID filter' do it_behaves_like 'assignee ID filter' do
...@@ -351,7 +353,6 @@ RSpec.describe MergeRequestsFinder do ...@@ -351,7 +353,6 @@ RSpec.describe MergeRequestsFinder do
merge_request3.assignees = [user2, user3] merge_request3.assignees = [user2, user3]
end end
let_it_be(:user3) { create(:user) }
let(:params) { { assignee_username: [user2.username, user3.username] } } let(:params) { { assignee_username: [user2.username, user3.username] } }
let(:expected_issuables) { [merge_request3] } let(:expected_issuables) { [merge_request3] }
end end
...@@ -366,7 +367,6 @@ RSpec.describe MergeRequestsFinder do ...@@ -366,7 +367,6 @@ RSpec.describe MergeRequestsFinder do
end end
it_behaves_like 'no assignee filter' do it_behaves_like 'no assignee filter' do
let_it_be(:user3) { create(:user) }
let(:expected_issuables) { [merge_request4, merge_request5] } let(:expected_issuables) { [merge_request4, merge_request5] }
end end
...@@ -374,30 +374,54 @@ RSpec.describe MergeRequestsFinder do ...@@ -374,30 +374,54 @@ RSpec.describe MergeRequestsFinder do
let(:expected_issuables) { [merge_request1, merge_request2, merge_request3] } let(:expected_issuables) { [merge_request1, merge_request2, merge_request3] }
end end
context 'filtering by group milestone' do context 'with just reviewers' do
let(:group_milestone) { create(:milestone, group: group) } it_behaves_like 'assignee username filter' do
before do
merge_request4.reviewers = [user3]
merge_request4.assignees = []
end
before do let(:params) { { assignee_username: [user3.username] } }
merge_request1.update!(milestone: group_milestone) let(:expected_issuables) { [merge_request4] }
merge_request2.update!(milestone: group_milestone)
end end
end
it 'returns merge requests assigned to that group milestone' do context 'with an additional reviewer' do
params = { milestone_title: group_milestone.title } it_behaves_like 'assignee username filter' do
before do
merge_requests = described_class.new(user, params).execute merge_request3.assignees = [user3]
merge_request4.reviewers = [user3]
end
expect(merge_requests).to contain_exactly(merge_request1, merge_request2) let(:params) { { assignee_username: [user3.username] } }
let(:expected_issuables) { [merge_request3, merge_request4] }
end end
end
end
context 'using NOT' do context 'filtering by group milestone' do
let(:params) { { not: { milestone_title: group_milestone.title } } } let(:group_milestone) { create(:milestone, group: group) }
it 'returns MRs not assigned to that group milestone' do before do
merge_requests = described_class.new(user, params).execute merge_request1.update!(milestone: group_milestone)
merge_request2.update!(milestone: group_milestone)
end
expect(merge_requests).to contain_exactly(merge_request3, merge_request4, merge_request5) it 'returns merge requests assigned to that group milestone' do
end params = { milestone_title: group_milestone.title }
merge_requests = described_class.new(user, params).execute
expect(merge_requests).to contain_exactly(merge_request1, merge_request2)
end
context 'using NOT' do
let(:params) { { not: { milestone_title: group_milestone.title } } }
it 'returns MRs not assigned to that group milestone' do
merge_requests = described_class.new(user, params).execute
expect(merge_requests).to contain_exactly(merge_request3, merge_request4, merge_request5)
end 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