From 3cbfd0be8ca1bb65f9bc2f017517bb2806fb3c4c Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Sat, 31 Aug 2019 23:20:19 +0800 Subject: [PATCH] Add projects parameter to IssuableFinder --- app/finders/issuable_finder.rb | 23 ++++++++-- spec/finders/issues_finder_spec.rb | 50 ++++++++++++++++++++++ spec/finders/merge_requests_finder_spec.rb | 20 ++++++++- 3 files changed, 88 insertions(+), 5 deletions(-) diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index b735f9ff3b8..8ed6ff56e2b 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -193,15 +193,30 @@ class IssuableFinder projects = if current_user && params[:authorized_only].presence && !current_user_related? current_user.authorized_projects(min_access_level) - elsif group - find_group_projects else - Project.public_or_visible_to_user(current_user, min_access_level) + projects_public_or_visible_to_user end @projects = projects.with_feature_available_for_user(klass, current_user).reorder(nil) # rubocop: disable CodeReuse/ActiveRecord end + def projects_public_or_visible_to_user + projects = + if group + if params[:projects] + find_group_projects.id_in(params[:projects]) + else + find_group_projects + end + elsif params[:projects] + Project.id_in(params[:projects]) + else + Project + end + + projects.public_or_visible_to_user(current_user, min_access_level) + end + def find_group_projects return Project.none unless group @@ -209,7 +224,7 @@ class IssuableFinder Project.where(namespace_id: group.self_and_descendants) # rubocop: disable CodeReuse/ActiveRecord else group.projects - end.public_or_visible_to_user(current_user, min_access_level) + end end def search diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb index 879ff01f294..ef8749be0be 100644 --- a/spec/finders/issues_finder_spec.rb +++ b/spec/finders/issues_finder_spec.rb @@ -42,6 +42,24 @@ describe IssuesFinder do end end + context 'filtering by projects' do + context 'when projects are passed in a list of ids' do + let(:params) { { projects: [project1.id] } } + + it 'returns the issue belonging to the projects' do + expect(issues).to contain_exactly(issue1) + end + end + + context 'when projects are passed in a subquery' do + let(:params) { { projects: Project.id_in(project1.id) } } + + it 'returns the issue belonging to the projects' do + expect(issues).to contain_exactly(issue1) + end + end + end + context 'filtering by group_id' do let(:params) { { group_id: group.id } } @@ -49,6 +67,30 @@ describe IssuesFinder do it 'returns all group issues' do expect(issues).to contain_exactly(issue1) end + + context 'when projects outside the group are passed' do + let(:params) { { group_id: group.id, projects: [project2.id] } } + + it 'returns no issues' do + expect(issues).to be_empty + end + end + + context 'when projects of the group are passed' do + let(:params) { { group_id: group.id, projects: [project1.id] } } + + it 'returns the issue within the group and projects' do + expect(issues).to contain_exactly(issue1) + end + end + + context 'when projects of the group are passed as a subquery' do + let(:params) { { group_id: group.id, projects: Project.id_in(project1.id) } } + + it 'returns the issue within the group and projects' do + expect(issues).to contain_exactly(issue1) + end + end end context 'when include_subgroup param is true' do @@ -59,6 +101,14 @@ describe IssuesFinder do it 'returns all group and subgroup issues' do expect(issues).to contain_exactly(issue1, issue4) end + + context 'when mixed projects are passed' do + let(:params) { { group_id: group.id, projects: [project2.id, project3.id] } } + + it 'returns the issue within the group and projects' do + expect(issues).to contain_exactly(issue4) + end + end end end diff --git a/spec/finders/merge_requests_finder_spec.rb b/spec/finders/merge_requests_finder_spec.rb index 78224f0b9da..6c0bbeff4f4 100644 --- a/spec/finders/merge_requests_finder_spec.rb +++ b/spec/finders/merge_requests_finder_spec.rb @@ -13,7 +13,7 @@ describe MergeRequestsFinder do expect(merge_requests).to contain_exactly(merge_request1, merge_request4, merge_request5) end - it 'filters by project' do + it 'filters by project_id' do params = { project_id: project1.id, scope: 'authored', state: 'opened' } merge_requests = described_class.new(user, params).execute @@ -21,6 +21,14 @@ describe MergeRequestsFinder do expect(merge_requests).to contain_exactly(merge_request1) end + it 'filters by projects' do + params = { projects: [project2.id, project3.id] } + + merge_requests = described_class.new(user, params).execute + + expect(merge_requests).to contain_exactly(merge_request3, merge_request4) + end + it 'filters by commit sha' do merge_requests = described_class.new( user, @@ -49,6 +57,16 @@ describe MergeRequestsFinder do expect(merge_requests).to contain_exactly(merge_request1, merge_request2, merge_request5) end + + it 'filters by group projects including subgroups' do + # project3 is not in the group, so it should not return merge_request4 + projects = [project3.id, project4.id] + params = { group_id: group.id, include_subgroups: true, projects: projects } + + merge_requests = described_class.new(user, params).execute + + expect(merge_requests).to contain_exactly(merge_request5) + end end it 'filters by non_archived' do