Commit 79c42110 authored by Igor Drozdov's avatar Igor Drozdov Committed by Yorick Peterse

Display the correct number of MRs a user has access to

parent 42d3117f
...@@ -46,13 +46,6 @@ module Milestoneish ...@@ -46,13 +46,6 @@ module Milestoneish
end end
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 issue_participants_visible_by_user(user) def issue_participants_visible_by_user(user)
User.joins(:issue_assignees) User.joins(:issue_assignees)
.where('issue_assignees.issue_id' => issues_visible_to_user(user).select(:id)) .where('issue_assignees.issue_id' => issues_visible_to_user(user).select(:id))
...@@ -73,6 +66,13 @@ module Milestoneish ...@@ -73,6 +66,13 @@ module Milestoneish
merge_requests_visible_to_user(user).sort_by_attribute('label_priority') merge_requests_visible_to_user(user).sort_by_attribute('label_priority')
end 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? def upcoming?
start_date && start_date.future? start_date && start_date.future?
end end
......
...@@ -76,7 +76,7 @@ class ProjectFeature < ActiveRecord::Base ...@@ -76,7 +76,7 @@ class ProjectFeature < ActiveRecord::Base
# This feature might not be behind a feature flag at all, so default to true # 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) return false unless ::Feature.enabled?(feature, user, default_enabled: true)
get_permission(user, access_level(feature)) get_permission(user, feature)
end end
def access_level(feature) def access_level(feature)
...@@ -134,12 +134,12 @@ class ProjectFeature < ActiveRecord::Base ...@@ -134,12 +134,12 @@ class ProjectFeature < ActiveRecord::Base
(FEATURES - %i(pages)).each {|f| validator.call("#{f}_access_level")} (FEATURES - %i(pages)).each {|f| validator.call("#{f}_access_level")}
end end
def get_permission(user, level) def get_permission(user, feature)
case level case access_level(feature)
when DISABLED when DISABLED
false false
when PRIVATE when PRIVATE
user && (project.team.member?(user) || user.full_private_access?) team_access?(user, feature)
when ENABLED when ENABLED
true true
when PUBLIC when PUBLIC
...@@ -148,4 +148,11 @@ class ProjectFeature < ActiveRecord::Base ...@@ -148,4 +148,11 @@ class ProjectFeature < ActiveRecord::Base
true true
end end
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 end
...@@ -465,7 +465,7 @@ class ProjectPolicy < BasePolicy ...@@ -465,7 +465,7 @@ class ProjectPolicy < BasePolicy
when ProjectFeature::DISABLED when ProjectFeature::DISABLED
false false
when ProjectFeature::PRIVATE when ProjectFeature::PRIVATE
guest? || admin? admin? || team_access_level >= ProjectFeature.required_minimum_access_level(feature)
else else
true true
end end
......
...@@ -32,7 +32,7 @@ ...@@ -32,7 +32,7 @@
= milestone_progress_bar(milestone) = milestone_progress_bar(milestone)
= link_to pluralize(milestone.total_issues_count(current_user), 'Issue'), issues_path = link_to pluralize(milestone.total_issues_count(current_user), 'Issue'), issues_path
&middot; &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 .float-lg-right.light #{milestone.percent_complete(current_user)}% complete
.col-sm-2 .col-sm-2
.milestone-actions.d-flex.justify-content-sm-start.justify-content-md-end .milestone-actions.d-flex.justify-content-sm-start.justify-content-md-end
......
...@@ -12,7 +12,7 @@ ...@@ -12,7 +12,7 @@
%li.nav-item %li.nav-item
= link_to '#tab-merge-requests', class: 'nav-link', 'data-toggle' => 'tab', 'data-endpoint': milestone_merge_request_tab_path(milestone) do = link_to '#tab-merge-requests', class: 'nav-link', 'data-toggle' => 'tab', 'data-endpoint': milestone_merge_request_tab_path(milestone) do
Merge Requests Merge Requests
%span.badge.badge-pill= milestone.merge_requests.size %span.badge.badge-pill= milestone.merge_requests_visible_to_user(current_user).size
- else - else
%li.nav-item %li.nav-item
= link_to '#tab-merge-requests', class: 'nav-link active', 'data-toggle' => 'tab', 'data-endpoint': milestone_merge_request_tab_path(milestone) do = link_to '#tab-merge-requests', class: 'nav-link active', 'data-toggle' => 'tab', 'data-endpoint': milestone_merge_request_tab_path(milestone) do
......
---
title: Display the correct number of MRs a user has access to
merge_request:
author:
type: security
...@@ -13,6 +13,7 @@ describe MergeRequestsFinder do ...@@ -13,6 +13,7 @@ describe MergeRequestsFinder do
end end
end end
context "multiple projects with merge requests" do
let(:user) { create :user } let(:user) { create :user }
let(:user2) { create :user } let(:user2) { create :user }
...@@ -55,7 +56,7 @@ describe MergeRequestsFinder do ...@@ -55,7 +56,7 @@ describe MergeRequestsFinder do
project6.add_developer(user) project6.add_developer(user)
end end
describe "#execute" do describe '#execute' do
it 'filters by scope' do it 'filters by scope' do
params = { scope: 'authored', state: 'opened' } params = { scope: 'authored', state: 'opened' }
merge_requests = described_class.new(user, params).execute merge_requests = described_class.new(user, params).execute
...@@ -278,6 +279,49 @@ describe MergeRequestsFinder do ...@@ -278,6 +279,49 @@ describe MergeRequestsFinder do
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 end
end
describe '#row_count', :request_store do
it 'returns the number of rows for the default state' do
finder = described_class.new(user)
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(finder.row_count).to eq(1)
end
end
end
context 'when projects require different access levels for merge requests' do
let(:user) { create(:user) }
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) }
let(:merge_requests) { described_class.new(user, {}).execute }
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) }
context 'with admin user' do
let(:user) { create(:user, :admin) }
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
context 'when project restricts merge requests' do context 'when project restricts merge requests' do
let(:non_member) { create(:user) } let(:non_member) { create(:user) }
...@@ -293,19 +337,85 @@ describe MergeRequestsFinder do ...@@ -293,19 +337,85 @@ describe MergeRequestsFinder do
expect(merge_requests).to be_empty expect(merge_requests).to be_empty
end end
end end
context 'with external user' do
let(:user) { create(:user, :external) }
it 'returns only public merge requests' do
expect(merge_requests).to eq([mr_public])
end
end end
describe '#row_count', :request_store do context 'with authenticated user' do
it 'returns the number of rows for the default state' do it 'returns public and internal merge requests' do
finder = described_class.new(user) expect(merge_requests).to eq([mr_internal, mr_public])
end
expect(finder.row_count).to eq(7) context 'being added to the private project' do
context 'as a guest' do
before do
private_project.add_guest(user)
end end
it 'returns the number of rows for a given state' do it 'does not return merge requests from the private project' do
finder = described_class.new(user, state: 'closed') expect(merge_requests).to eq([mr_internal, mr_public])
end
end
expect(finder.row_count).to eq(1) 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
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 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