summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorIgor Drozdov <idrozdov@gitlab.com>2019-02-27 18:16:19 +0000
committerYorick Peterse <yorickpeterse@gmail.com>2019-02-27 18:16:19 +0000
commitb1bc3b817635567ba08d48b7160f8a05300909ad (patch)
tree5764fde5600dc5c36ef25e0e2904b0fd1daf2989
parentb001a30f43d219f1f7b606cf8939d0ecc64bee59 (diff)
downloadgitlab-ce-b1bc3b817635567ba08d48b7160f8a05300909ad.tar.gz
Display the correct number of MRs a user has access to
-rw-r--r--app/models/concerns/milestoneish.rb14
-rw-r--r--app/models/project_feature.rb15
-rw-r--r--app/policies/project_policy.rb2
-rw-r--r--app/views/shared/milestones/_milestone.html.haml2
-rw-r--r--app/views/shared/milestones/_tabs.html.haml2
-rw-r--r--changelogs/unreleased/security-id-fix-mr-visibility.yml5
-rw-r--r--spec/finders/merge_requests_finder_spec.rb508
7 files changed, 335 insertions, 213 deletions
diff --git a/app/models/concerns/milestoneish.rb b/app/models/concerns/milestoneish.rb
index 39372c4f68b..e73a5f2fd29 100644
--- a/app/models/concerns/milestoneish.rb
+++ b/app/models/concerns/milestoneish.rb
@@ -46,13 +46,6 @@ module Milestoneish
end
end
- def merge_requests_visible_to_user(user)
- memoize_per_user(user, :merge_requests_visible_to_user) do
- MergeRequestsFinder.new(user, {})
- .execute.where(milestone_id: milestoneish_id)
- end
- end
-
def sorted_issues(user)
issues_visible_to_user(user).preload_associations.sort_by_attribute('label_priority')
end
@@ -61,6 +54,13 @@ module Milestoneish
merge_requests_visible_to_user(user).sort_by_attribute('label_priority')
end
+ def merge_requests_visible_to_user(user)
+ memoize_per_user(user, :merge_requests_visible_to_user) do
+ MergeRequestsFinder.new(user, issues_finder_params)
+ .execute.where(milestone_id: milestoneish_id)
+ end
+ end
+
def upcoming?
start_date && start_date.future?
end
diff --git a/app/models/project_feature.rb b/app/models/project_feature.rb
index f700090a493..e6787236c4e 100644
--- a/app/models/project_feature.rb
+++ b/app/models/project_feature.rb
@@ -76,7 +76,7 @@ class ProjectFeature < ActiveRecord::Base
# This feature might not be behind a feature flag at all, so default to true
return false unless ::Feature.enabled?(feature, user, default_enabled: true)
- get_permission(user, access_level(feature))
+ get_permission(user, feature)
end
def access_level(feature)
@@ -134,12 +134,12 @@ class ProjectFeature < ActiveRecord::Base
(FEATURES - %i(pages)).each {|f| validator.call("#{f}_access_level")}
end
- def get_permission(user, level)
- case level
+ def get_permission(user, feature)
+ case access_level(feature)
when DISABLED
false
when PRIVATE
- user && (project.team.member?(user) || user.full_private_access?)
+ team_access?(user, feature)
when ENABLED
true
when PUBLIC
@@ -148,4 +148,11 @@ class ProjectFeature < ActiveRecord::Base
true
end
end
+
+ def team_access?(user, feature)
+ return unless user
+ return true if user.full_private_access?
+
+ project.team.member?(user, ProjectFeature.required_minimum_access_level(feature))
+ end
end
diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
index a8270442ea9..85143c5d339 100644
--- a/app/policies/project_policy.rb
+++ b/app/policies/project_policy.rb
@@ -464,7 +464,7 @@ class ProjectPolicy < BasePolicy
when ProjectFeature::DISABLED
false
when ProjectFeature::PRIVATE
- guest? || admin?
+ admin? || team_access_level >= ProjectFeature.required_minimum_access_level(feature)
else
true
end
diff --git a/app/views/shared/milestones/_milestone.html.haml b/app/views/shared/milestones/_milestone.html.haml
index 40b8374848e..e75f0a184ea 100644
--- a/app/views/shared/milestones/_milestone.html.haml
+++ b/app/views/shared/milestones/_milestone.html.haml
@@ -32,7 +32,7 @@
= milestone_progress_bar(milestone)
= link_to pluralize(milestone.total_issues_count(current_user), 'Issue'), issues_path
&middot;
- = link_to pluralize(milestone.merge_requests.size, 'Merge Request'), merge_requests_path
+ = link_to pluralize(milestone.merge_requests_visible_to_user(current_user).size, 'Merge Request'), merge_requests_path
.float-lg-right.light #{milestone.percent_complete(current_user)}% complete
.col-sm-2
.milestone-actions.d-flex.justify-content-sm-start.justify-content-md-end
diff --git a/app/views/shared/milestones/_tabs.html.haml b/app/views/shared/milestones/_tabs.html.haml
index 55460acab8f..a75af1a7e15 100644
--- a/app/views/shared/milestones/_tabs.html.haml
+++ b/app/views/shared/milestones/_tabs.html.haml
@@ -12,7 +12,7 @@
%li.nav-item
= link_to '#tab-merge-requests', class: 'nav-link', 'data-toggle' => 'tab', 'data-endpoint': milestone_merge_request_tab_path(milestone) do
Merge Requests
- %span.badge.badge-pill= milestone.merge_requests.size
+ %span.badge.badge-pill= milestone.merge_requests_visible_to_user(current_user).size
- else
%li.nav-item
= link_to '#tab-merge-requests', class: 'nav-link active', 'data-toggle' => 'tab', 'data-endpoint': milestone_merge_request_tab_path(milestone) do
diff --git a/changelogs/unreleased/security-id-fix-mr-visibility.yml b/changelogs/unreleased/security-id-fix-mr-visibility.yml
new file mode 100644
index 00000000000..8f41d191acc
--- /dev/null
+++ b/changelogs/unreleased/security-id-fix-mr-visibility.yml
@@ -0,0 +1,5 @@
+---
+title: Display the correct number of MRs a user has access to
+merge_request:
+author:
+type: security
diff --git a/spec/finders/merge_requests_finder_spec.rb b/spec/finders/merge_requests_finder_spec.rb
index 79f854cdb96..503b88fcbad 100644
--- a/spec/finders/merge_requests_finder_spec.rb
+++ b/spec/finders/merge_requests_finder_spec.rb
@@ -13,269 +13,313 @@ describe MergeRequestsFinder do
end
end
- let(:user) { create :user }
- let(:user2) { create :user }
-
- let(:group) { create(:group) }
- let(:subgroup) { create(:group, parent: group) }
- let(:project1) { create_project_without_n_plus_1(group: group) }
- let(:project2) do
- Gitlab::GitalyClient.allow_n_plus_1_calls do
- fork_project(project1, user)
+ context "multiple projects with merge requests" do
+ let(:user) { create :user }
+ let(:user2) { create :user }
+
+ let(:group) { create(:group) }
+ let(:subgroup) { create(:group, parent: group) }
+ let(:project1) { create_project_without_n_plus_1(group: group) }
+ let(:project2) do
+ Gitlab::GitalyClient.allow_n_plus_1_calls do
+ fork_project(project1, user)
+ end
end
- end
- let(:project3) do
- Gitlab::GitalyClient.allow_n_plus_1_calls do
- p = fork_project(project1, user)
- p.update!(archived: true)
- p
+ let(:project3) do
+ Gitlab::GitalyClient.allow_n_plus_1_calls do
+ p = fork_project(project1, user)
+ p.update!(archived: true)
+ p
+ end
end
- end
- let(:project4) { create_project_without_n_plus_1(:repository, group: subgroup) }
- let(:project5) { create_project_without_n_plus_1(group: subgroup) }
- let(:project6) { create_project_without_n_plus_1(group: subgroup) }
-
- let!(:merge_request1) { create(:merge_request, :simple, author: user, source_project: project2, target_project: project1) }
- let!(:merge_request2) { create(:merge_request, :conflict, author: user, source_project: project2, target_project: project1, state: 'closed') }
- let!(:merge_request3) { create(:merge_request, :simple, author: user, source_project: project2, target_project: project2, state: 'locked', title: 'thing WIP thing') }
- let!(:merge_request4) { create(:merge_request, :simple, author: user, source_project: project3, target_project: project3, title: 'WIP thing') }
- let!(:merge_request5) { create(:merge_request, :simple, author: user, source_project: project4, target_project: project4, title: '[WIP]') }
- let!(:merge_request6) { create(:merge_request, :simple, author: user, source_project: project5, target_project: project5, title: 'WIP: thing') }
- let!(:merge_request7) { create(:merge_request, :simple, author: user, source_project: project6, target_project: project6, title: 'wip thing') }
- let!(:merge_request8) { create(:merge_request, :simple, author: user, source_project: project1, target_project: project1, title: '[wip] thing') }
- let!(:merge_request9) { create(:merge_request, :simple, author: user, source_project: project1, target_project: project2, title: 'wip: thing') }
-
- before do
- project1.add_maintainer(user)
- project2.add_developer(user)
- project3.add_developer(user)
- project2.add_developer(user2)
- project4.add_developer(user)
- project5.add_developer(user)
- project6.add_developer(user)
- end
-
- describe "#execute" do
- it 'filters by scope' do
- params = { scope: 'authored', state: 'opened' }
- merge_requests = described_class.new(user, params).execute
- expect(merge_requests.size).to eq(7)
+ let(:project4) { create_project_without_n_plus_1(:repository, group: subgroup) }
+ let(:project5) { create_project_without_n_plus_1(group: subgroup) }
+ let(:project6) { create_project_without_n_plus_1(group: subgroup) }
+
+ let!(:merge_request1) { create(:merge_request, :simple, author: user, source_project: project2, target_project: project1) }
+ let!(:merge_request2) { create(:merge_request, :conflict, author: user, source_project: project2, target_project: project1, state: 'closed') }
+ let!(:merge_request3) { create(:merge_request, :simple, author: user, source_project: project2, target_project: project2, state: 'locked', title: 'thing WIP thing') }
+ let!(:merge_request4) { create(:merge_request, :simple, author: user, source_project: project3, target_project: project3, title: 'WIP thing') }
+ let!(:merge_request5) { create(:merge_request, :simple, author: user, source_project: project4, target_project: project4, title: '[WIP]') }
+ let!(:merge_request6) { create(:merge_request, :simple, author: user, source_project: project5, target_project: project5, title: 'WIP: thing') }
+ let!(:merge_request7) { create(:merge_request, :simple, author: user, source_project: project6, target_project: project6, title: 'wip thing') }
+ let!(:merge_request8) { create(:merge_request, :simple, author: user, source_project: project1, target_project: project1, title: '[wip] thing') }
+ let!(:merge_request9) { create(:merge_request, :simple, author: user, source_project: project1, target_project: project2, title: 'wip: thing') }
+
+ before do
+ project1.add_maintainer(user)
+ project2.add_developer(user)
+ project3.add_developer(user)
+ project2.add_developer(user2)
+ project4.add_developer(user)
+ project5.add_developer(user)
+ project6.add_developer(user)
end
- it 'filters by project' do
- params = { project_id: project1.id, scope: 'authored', state: 'opened' }
- merge_requests = described_class.new(user, params).execute
- expect(merge_requests.size).to eq(2)
- end
+ describe '#execute' do
+ it 'filters by scope' do
+ params = { scope: 'authored', state: 'opened' }
+ merge_requests = described_class.new(user, params).execute
+ expect(merge_requests.size).to eq(7)
+ end
- it 'filters by commit sha' do
- merge_requests = described_class.new(
- user,
- commit_sha: merge_request5.merge_request_diff.last_commit_sha
- ).execute
+ it 'filters by project' do
+ params = { project_id: project1.id, scope: 'authored', state: 'opened' }
+ merge_requests = described_class.new(user, params).execute
+ expect(merge_requests.size).to eq(2)
+ end
- expect(merge_requests).to contain_exactly(merge_request5)
- end
+ it 'filters by commit sha' do
+ merge_requests = described_class.new(
+ user,
+ commit_sha: merge_request5.merge_request_diff.last_commit_sha
+ ).execute
+
+ expect(merge_requests).to contain_exactly(merge_request5)
+ end
+
+ context 'filtering by group' do
+ it 'includes all merge requests when user has access' do
+ params = { group_id: group.id }
+
+ merge_requests = described_class.new(user, params).execute
- context 'filtering by group' do
- it 'includes all merge requests when user has access' do
- params = { group_id: group.id }
+ expect(merge_requests.size).to eq(3)
+ end
+
+ it 'excludes merge requests from projects the user does not have access to' do
+ private_project = create_project_without_n_plus_1(:private, group: group)
+ private_mr = create(:merge_request, :simple, author: user, source_project: private_project, target_project: private_project)
+ params = { group_id: group.id }
+
+ private_project.add_guest(user)
+ merge_requests = described_class.new(user, params).execute
+
+ expect(merge_requests.size).to eq(3)
+ expect(merge_requests).not_to include(private_mr)
+ end
+
+ it 'filters by group including subgroups', :nested_groups do
+ params = { group_id: group.id, include_subgroups: true }
+
+ merge_requests = described_class.new(user, params).execute
+
+ expect(merge_requests.size).to eq(6)
+ end
+ end
+
+ it 'filters by non_archived' do
+ params = { non_archived: true }
+ merge_requests = described_class.new(user, params).execute
+ expect(merge_requests.size).to eq(8)
+ end
+
+ it 'filters by iid' do
+ params = { project_id: project1.id, iids: merge_request1.iid }
merge_requests = described_class.new(user, params).execute
- expect(merge_requests.size).to eq(3)
+ expect(merge_requests).to contain_exactly(merge_request1)
end
- it 'excludes merge requests from projects the user does not have access to' do
- private_project = create_project_without_n_plus_1(:private, group: group)
- private_mr = create(:merge_request, :simple, author: user, source_project: private_project, target_project: private_project)
- params = { group_id: group.id }
+ it 'filters by source branch' do
+ params = { source_branch: merge_request2.source_branch }
- private_project.add_guest(user)
merge_requests = described_class.new(user, params).execute
- expect(merge_requests.size).to eq(3)
- expect(merge_requests).not_to include(private_mr)
+ expect(merge_requests).to contain_exactly(merge_request2)
end
- it 'filters by group including subgroups', :nested_groups do
- params = { group_id: group.id, include_subgroups: true }
+ it 'filters by target branch' do
+ params = { target_branch: merge_request2.target_branch }
merge_requests = described_class.new(user, params).execute
- expect(merge_requests.size).to eq(6)
+ expect(merge_requests).to contain_exactly(merge_request2)
end
- end
- it 'filters by non_archived' do
- params = { non_archived: true }
- merge_requests = described_class.new(user, params).execute
- expect(merge_requests.size).to eq(8)
- end
+ it 'filters by state' do
+ params = { state: 'locked' }
- it 'filters by iid' do
- params = { project_id: project1.id, iids: merge_request1.iid }
+ merge_requests = described_class.new(user, params).execute
- merge_requests = described_class.new(user, params).execute
+ expect(merge_requests).to contain_exactly(merge_request3)
+ end
- expect(merge_requests).to contain_exactly(merge_request1)
- end
+ it 'filters by wip' do
+ params = { wip: 'yes' }
- it 'filters by source branch' do
- params = { source_branch: merge_request2.source_branch }
+ merge_requests = described_class.new(user, params).execute
- merge_requests = described_class.new(user, params).execute
+ expect(merge_requests).to contain_exactly(merge_request4, merge_request5, merge_request6, merge_request7, merge_request8, merge_request9)
+ end
- expect(merge_requests).to contain_exactly(merge_request2)
- end
+ it 'filters by not wip' do
+ params = { wip: 'no' }
- it 'filters by target branch' do
- params = { target_branch: merge_request2.target_branch }
+ merge_requests = described_class.new(user, params).execute
- merge_requests = described_class.new(user, params).execute
+ expect(merge_requests).to contain_exactly(merge_request1, merge_request2, merge_request3)
+ end
- expect(merge_requests).to contain_exactly(merge_request2)
- end
+ it 'returns all items if no valid wip param exists' do
+ params = { wip: '' }
- it 'filters by state' do
- params = { state: 'locked' }
+ merge_requests = described_class.new(user, params).execute
- merge_requests = described_class.new(user, params).execute
+ expect(merge_requests).to contain_exactly(merge_request1, merge_request2, merge_request3, merge_request4, merge_request5, merge_request6, merge_request7, merge_request8, merge_request9)
+ end
- expect(merge_requests).to contain_exactly(merge_request3)
- end
+ it 'adds wip to scalar params' do
+ scalar_params = described_class.scalar_params
- it 'filters by wip' do
- params = { wip: 'yes' }
+ expect(scalar_params).to include(:wip, :assignee_id)
+ end
- merge_requests = described_class.new(user, params).execute
+ context 'filtering by group milestone' do
+ let!(:group) { create(:group, :public) }
+ let(:group_milestone) { create(:milestone, group: group) }
+ let!(:group_member) { create(:group_member, group: group, user: user) }
+ let(:params) { { milestone_title: group_milestone.title } }
- expect(merge_requests).to contain_exactly(merge_request4, merge_request5, merge_request6, merge_request7, merge_request8, merge_request9)
- end
+ before do
+ project2.update(namespace: group)
+ merge_request2.update(milestone: group_milestone)
+ merge_request3.update(milestone: group_milestone)
+ end
- it 'filters by not wip' do
- params = { wip: 'no' }
+ it 'returns issues assigned to that group milestone' do
+ merge_requests = described_class.new(user, params).execute
- merge_requests = described_class.new(user, params).execute
+ expect(merge_requests).to contain_exactly(merge_request2, merge_request3)
+ end
+ end
- expect(merge_requests).to contain_exactly(merge_request1, merge_request2, merge_request3)
- end
+ context 'filtering by created_at/updated_at' do
+ let(:new_project) { create(:project, forked_from_project: project1) }
- it 'returns all items if no valid wip param exists' do
- params = { wip: '' }
+ let!(:new_merge_request) do
+ create(:merge_request,
+ :simple,
+ author: user,
+ created_at: 1.week.from_now,
+ updated_at: 1.week.from_now,
+ source_project: new_project,
+ target_project: new_project)
+ end
- merge_requests = described_class.new(user, params).execute
+ let!(:old_merge_request) do
+ create(:merge_request,
+ :simple,
+ author: user,
+ source_branch: 'feature_1',
+ created_at: 1.week.ago,
+ updated_at: 1.week.ago,
+ source_project: new_project,
+ target_project: new_project)
+ end
- expect(merge_requests).to contain_exactly(merge_request1, merge_request2, merge_request3, merge_request4, merge_request5, merge_request6, merge_request7, merge_request8, merge_request9)
- end
+ before do
+ new_project.add_maintainer(user)
+ end
- it 'adds wip to scalar params' do
- scalar_params = described_class.scalar_params
+ it 'filters by created_after' do
+ params = { project_id: new_project.id, created_after: new_merge_request.created_at }
- expect(scalar_params).to include(:wip, :assignee_id)
- end
+ merge_requests = described_class.new(user, params).execute
- context 'filtering by group milestone' do
- let!(:group) { create(:group, :public) }
- let(:group_milestone) { create(:milestone, group: group) }
- let!(:group_member) { create(:group_member, group: group, user: user) }
- let(:params) { { milestone_title: group_milestone.title } }
+ expect(merge_requests).to contain_exactly(new_merge_request)
+ end
- before do
- project2.update(namespace: group)
- merge_request2.update(milestone: group_milestone)
- merge_request3.update(milestone: group_milestone)
- end
+ it 'filters by created_before' do
+ params = { project_id: new_project.id, created_before: old_merge_request.created_at }
- it 'returns issues assigned to that group milestone' do
- merge_requests = described_class.new(user, params).execute
+ merge_requests = described_class.new(user, params).execute
- expect(merge_requests).to contain_exactly(merge_request2, merge_request3)
- end
- end
+ expect(merge_requests).to contain_exactly(old_merge_request)
+ end
- context 'filtering by created_at/updated_at' do
- let(:new_project) { create(:project, forked_from_project: project1) }
-
- let!(:new_merge_request) do
- create(:merge_request,
- :simple,
- author: user,
- created_at: 1.week.from_now,
- updated_at: 1.week.from_now,
- source_project: new_project,
- target_project: new_project)
- end
+ it 'filters by created_after and created_before' do
+ params = {
+ project_id: new_project.id,
+ created_after: old_merge_request.created_at,
+ created_before: new_merge_request.created_at
+ }
- let!(:old_merge_request) do
- create(:merge_request,
- :simple,
- author: user,
- source_branch: 'feature_1',
- created_at: 1.week.ago,
- updated_at: 1.week.ago,
- source_project: new_project,
- target_project: new_project)
- end
+ merge_requests = described_class.new(user, params).execute
- before do
- new_project.add_maintainer(user)
- end
+ expect(merge_requests).to contain_exactly(old_merge_request, new_merge_request)
+ end
- it 'filters by created_after' do
- params = { project_id: new_project.id, created_after: new_merge_request.created_at }
+ it 'filters by updated_after' do
+ params = { project_id: new_project.id, updated_after: new_merge_request.updated_at }
- merge_requests = described_class.new(user, params).execute
+ merge_requests = described_class.new(user, params).execute
- expect(merge_requests).to contain_exactly(new_merge_request)
- end
+ expect(merge_requests).to contain_exactly(new_merge_request)
+ end
- it 'filters by created_before' do
- params = { project_id: new_project.id, created_before: old_merge_request.created_at }
+ it 'filters by updated_before' do
+ params = { project_id: new_project.id, updated_before: old_merge_request.updated_at }
- merge_requests = described_class.new(user, params).execute
+ merge_requests = described_class.new(user, params).execute
- expect(merge_requests).to contain_exactly(old_merge_request)
- end
+ expect(merge_requests).to contain_exactly(old_merge_request)
+ end
- it 'filters by created_after and created_before' do
- params = {
- project_id: new_project.id,
- created_after: old_merge_request.created_at,
- created_before: new_merge_request.created_at
- }
+ it 'filters by updated_after and updated_before' do
+ params = {
+ project_id: new_project.id,
+ updated_after: old_merge_request.updated_at,
+ updated_before: new_merge_request.updated_at
+ }
- merge_requests = described_class.new(user, params).execute
+ merge_requests = described_class.new(user, params).execute
- expect(merge_requests).to contain_exactly(old_merge_request, new_merge_request)
+ expect(merge_requests).to contain_exactly(old_merge_request, new_merge_request)
+ end
end
+ end
- it 'filters by updated_after' do
- params = { project_id: new_project.id, updated_after: new_merge_request.updated_at }
+ describe '#row_count', :request_store do
+ it 'returns the number of rows for the default state' do
+ finder = described_class.new(user)
- merge_requests = described_class.new(user, params).execute
+ expect(finder.row_count).to eq(7)
+ end
+
+ it 'returns the number of rows for a given state' do
+ finder = described_class.new(user, state: 'closed')
- expect(merge_requests).to contain_exactly(new_merge_request)
+ expect(finder.row_count).to eq(1)
end
+ end
+ end
- it 'filters by updated_before' do
- params = { project_id: new_project.id, updated_before: old_merge_request.updated_at }
+ context 'when projects require different access levels for merge requests' do
+ let(:user) { create(:user) }
- merge_requests = described_class.new(user, params).execute
+ let(:public_project) { create(:project, :public) }
+ let(:internal) { create(:project, :internal) }
+ let(:private_project) { create(:project, :private) }
+ let(:public_with_private_repo) { create(:project, :public, :repository, :repository_private) }
+ let(:internal_with_private_repo) { create(:project, :internal, :repository, :repository_private) }
- expect(merge_requests).to contain_exactly(old_merge_request)
- end
+ let(:merge_requests) { described_class.new(user, {}).execute }
- it 'filters by updated_after and updated_before' do
- params = {
- project_id: new_project.id,
- updated_after: old_merge_request.updated_at,
- updated_before: new_merge_request.updated_at
- }
+ let!(:mr_public) { create(:merge_request, source_project: public_project) }
+ let!(:mr_private) { create(:merge_request, source_project: private_project) }
+ let!(:mr_internal) { create(:merge_request, source_project: internal) }
+ let!(:mr_private_repo_access) { create(:merge_request, source_project: public_with_private_repo) }
+ let!(:mr_internal_private_repo_access) { create(:merge_request, source_project: internal_with_private_repo) }
- merge_requests = described_class.new(user, params).execute
+ context 'with admin user' do
+ let(:user) { create(:user, :admin) }
- expect(merge_requests).to contain_exactly(old_merge_request, new_merge_request)
+ it 'returns all merge requests' do
+ expect(merge_requests).to eq(
+ [mr_internal_private_repo_access, mr_private_repo_access, mr_internal, mr_private, mr_public]
+ )
end
end
@@ -293,19 +337,85 @@ describe MergeRequestsFinder do
expect(merge_requests).to be_empty
end
end
- end
- describe '#row_count', :request_store do
- it 'returns the number of rows for the default state' do
- finder = described_class.new(user)
+ context 'with external user' do
+ let(:user) { create(:user, :external) }
- expect(finder.row_count).to eq(7)
+ it 'returns only public merge requests' do
+ expect(merge_requests).to eq([mr_public])
+ end
end
- it 'returns the number of rows for a given state' do
- finder = described_class.new(user, state: 'closed')
+ context 'with authenticated user' do
+ it 'returns public and internal merge requests' do
+ expect(merge_requests).to eq([mr_internal, mr_public])
+ end
+
+ context 'being added to the private project' do
+ context 'as a guest' do
+ before do
+ private_project.add_guest(user)
+ end
+
+ it 'does not return merge requests from the private project' do
+ expect(merge_requests).to eq([mr_internal, mr_public])
+ end
+ end
+
+ context 'as a developer' do
+ before do
+ private_project.add_developer(user)
+ end
+
+ it 'returns merge requests from the private project' do
+ expect(merge_requests).to eq([mr_internal, mr_private, mr_public])
+ end
+ end
+ end
- expect(finder.row_count).to eq(1)
+ context 'being added to the public project with private repo access' do
+ context 'as a guest' do
+ before do
+ public_with_private_repo.add_guest(user)
+ end
+
+ it 'returns merge requests from the project' do
+ expect(merge_requests).to eq([mr_internal, mr_public])
+ end
+ end
+
+ context 'as a reporter' do
+ before do
+ public_with_private_repo.add_reporter(user)
+ end
+
+ it 'returns merge requests from the project' do
+ expect(merge_requests).to eq([mr_private_repo_access, mr_internal, mr_public])
+ end
+ end
+ end
+
+ context 'being added to the internal project with private repo access' do
+ context 'as a guest' do
+ before do
+ internal_with_private_repo.add_guest(user)
+ end
+
+ it 'returns merge requests from the project' do
+ expect(merge_requests).to eq([mr_internal, mr_public])
+ end
+ end
+
+ context 'as a reporter' do
+ before do
+ internal_with_private_repo.add_reporter(user)
+ end
+
+ it 'returns merge requests from the project' do
+ expect(merge_requests).to eq([mr_internal_private_repo_access, mr_internal, mr_public])
+ end
+ end
+ end
end
end
end