diff options
author | Bob Van Landuyt <bob@vanlanduyt.co> | 2018-04-06 16:02:36 +0200 |
---|---|---|
committer | Bob Van Landuyt <bob@vanlanduyt.co> | 2018-04-11 10:51:15 +0200 |
commit | 12dd2b0cc0cf6dd8dc43ff8b8df8687268ba4af5 (patch) | |
tree | 65dd7c5467b931dc1777214dc1416c78add7891a | |
parent | 8ad9c4e873f2185e8bbfa6f363db32dbbba27141 (diff) | |
download | gitlab-ce-12dd2b0cc0cf6dd8dc43ff8b8df8687268ba4af5.tar.gz |
Share collaboration check between view and presenter
-rw-r--r-- | app/controllers/concerns/checks_collaboration.rb | 21 | ||||
-rw-r--r-- | app/controllers/projects/application_controller.rb | 18 | ||||
-rw-r--r-- | app/helpers/commits_helper.rb | 2 | ||||
-rw-r--r-- | app/presenters/merge_request_presenter.rb | 15 | ||||
-rw-r--r-- | app/views/projects/commit/_commit_box.html.haml | 2 | ||||
-rw-r--r-- | app/views/projects/commit/show.html.haml | 2 | ||||
-rw-r--r-- | spec/controllers/concerns/checks_collaboration_spec.rb | 55 | ||||
-rw-r--r-- | spec/views/projects/tree/show.html.haml_spec.rb | 1 |
8 files changed, 84 insertions, 32 deletions
diff --git a/app/controllers/concerns/checks_collaboration.rb b/app/controllers/concerns/checks_collaboration.rb new file mode 100644 index 00000000000..81367663a06 --- /dev/null +++ b/app/controllers/concerns/checks_collaboration.rb @@ -0,0 +1,21 @@ +module ChecksCollaboration + def can_collaborate_with_project?(project, ref: nil) + return true if can?(current_user, :push_code, project) + + can_create_merge_request = + can?(current_user, :create_merge_request_in, project) && + current_user.already_forked?(project) + + can_create_merge_request || + user_access(project).can_push_to_branch?(ref) + end + + # rubocop:disable Gitlab/ModuleWithInstanceVariables + # enabling this so we can easily cache the user access value as it might be + # used across multiple calls in the view + def user_access(project) + @user_access ||= {} + @user_access[project] ||= Gitlab::UserAccess.new(current_user, project: project) + end + # rubocop:enable Gitlab/ModuleWithInstanceVariables +end diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb index 53a0c712e49..032bb2267e7 100644 --- a/app/controllers/projects/application_controller.rb +++ b/app/controllers/projects/application_controller.rb @@ -1,5 +1,6 @@ class Projects::ApplicationController < ApplicationController include RoutableActions + include ChecksCollaboration skip_before_action :authenticate_user! before_action :project @@ -31,18 +32,6 @@ class Projects::ApplicationController < ApplicationController @repository ||= project.repository end - def can_collaborate_with_project?(project = nil, ref: nil) - project ||= @project - - can_create_merge_request = - can?(current_user, :create_merge_request_in, project) && - current_user.already_forked?(project) - - can?(current_user, :push_code, project) || - can_create_merge_request || - user_access(project).can_push_to_branch?(ref) - end - def authorize_action!(action) unless can?(current_user, action, project) return access_denied! @@ -95,9 +84,4 @@ class Projects::ApplicationController < ApplicationController def check_issues_available! return render_404 unless @project.feature_available?(:issues, current_user) end - - def user_access(project) - @user_access ||= {} - @user_access[project] ||= Gitlab::UserAccess.new(current_user, project: project) - end end diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb index 7cc56de24e4..98894b86551 100644 --- a/app/helpers/commits_helper.rb +++ b/app/helpers/commits_helper.rb @@ -163,7 +163,7 @@ module CommitsHelper tooltip = "#{action.capitalize} this #{commit.change_type_title(current_user)} in a new merge request" if has_tooltip btn_class = "btn btn-#{btn_class}" unless btn_class.nil? - if can_collaborate_with_project? + if can_collaborate_with_project?(@project) link_to action.capitalize, "#modal-#{action}-commit", 'data-toggle' => 'modal', 'data-container' => 'body', title: (tooltip if has_tooltip), class: "#{btn_class} #{'has-tooltip' if has_tooltip}" elsif can?(current_user, :fork_project, @project) continue_params = { diff --git a/app/presenters/merge_request_presenter.rb b/app/presenters/merge_request_presenter.rb index a55af1422e8..4b4132af2d0 100644 --- a/app/presenters/merge_request_presenter.rb +++ b/app/presenters/merge_request_presenter.rb @@ -3,6 +3,7 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated include GitlabRoutingHelper include MarkupHelper include TreeHelper + include ChecksCollaboration include Gitlab::Utils::StrongMemoize presents :merge_request @@ -152,11 +153,11 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated end def can_revert_on_current_merge_request? - user_can_collaborate_with_project? && cached_can_be_reverted? + can_collaborate_with_project?(project) && cached_can_be_reverted? end def can_cherry_pick_on_current_merge_request? - user_can_collaborate_with_project? && can_be_cherry_picked? + can_collaborate_with_project?(project) && can_be_cherry_picked? end def can_push_to_source_branch? @@ -195,16 +196,6 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated end.sort.to_sentence end - def user_can_collaborate_with_project? - can_create_merge_request = - can?(current_user, :create_merge_request_in, project) && - current_user.already_forked?(project) - - can?(current_user, :push_code, project) || - can_create_merge_request || - can_push_to_source_branch? - end - def user_can_fork_project? can?(current_user, :fork_project, project) end diff --git a/app/views/projects/commit/_commit_box.html.haml b/app/views/projects/commit/_commit_box.html.haml index aec58a91a71..e391f08acca 100644 --- a/app/views/projects/commit/_commit_box.html.haml +++ b/app/views/projects/commit/_commit_box.html.haml @@ -35,7 +35,7 @@ - unless @commit.has_been_reverted?(current_user) %li.clearfix = revert_commit_link(@commit, project_commit_path(@project, @commit.id), has_tooltip: false) - - if can_collaborate_with_project? + - if can_collaborate_with_project?(@project) %li.clearfix = cherry_pick_commit_link(@commit, project_commit_path(@project, @commit.id), has_tooltip: false) - if can?(current_user, :push_code, @project) diff --git a/app/views/projects/commit/show.html.haml b/app/views/projects/commit/show.html.haml index abb292f8f27..541ae905246 100644 --- a/app/views/projects/commit/show.html.haml +++ b/app/views/projects/commit/show.html.haml @@ -17,6 +17,6 @@ .limited-width-notes = render "shared/notes/notes_with_form", :autocomplete => true - - if can_collaborate_with_project? + - if can_collaborate_with_project?(@project) - %w(revert cherry-pick).each do |type| = render "projects/commit/change", type: type, commit: @commit, title: @commit.title diff --git a/spec/controllers/concerns/checks_collaboration_spec.rb b/spec/controllers/concerns/checks_collaboration_spec.rb new file mode 100644 index 00000000000..1bd764290ae --- /dev/null +++ b/spec/controllers/concerns/checks_collaboration_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' + +describe ChecksCollaboration do + include ProjectForksHelper + + let(:helper) do + fake_class = Class.new(ApplicationController) do + include ChecksCollaboration + end + + fake_class.new + end + + describe '#can_collaborate_with_project?' do + let(:user) { create(:user) } + let(:project) { create(:project, :public) } + + before do + allow(helper).to receive(:current_user).and_return(user) + allow(helper).to receive(:can?) do |user, ability, subject| + Ability.allowed?(user, ability, subject) + end + end + + it 'is true if the user can push to the project' do + project.add_developer(user) + + expect(helper.can_collaborate_with_project?(project)).to be_truthy + end + + it 'is true when the user can push to a branch of the project' do + fake_access = double('Gitlab::UserAccess') + expect(fake_access).to receive(:can_push_to_branch?).with('a-branch').and_return(true) + expect(Gitlab::UserAccess).to receive(:new).with(user, project: project).and_return(fake_access) + + expect(helper.can_collaborate_with_project?(project, ref: 'a-branch')).to be_truthy + end + + context 'when the user has forked the project' do + before do + fork_project(project, user, namespace: user.namespace) + end + + it 'is true' do + expect(helper.can_collaborate_with_project?(project)).to be_truthy + end + + it 'is false when the project is archived' do + project.archived = true + + expect(helper.can_collaborate_with_project?(project)).to be_falsy + end + end + end +end diff --git a/spec/views/projects/tree/show.html.haml_spec.rb b/spec/views/projects/tree/show.html.haml_spec.rb index 3b098320ad7..6fa878eceaf 100644 --- a/spec/views/projects/tree/show.html.haml_spec.rb +++ b/spec/views/projects/tree/show.html.haml_spec.rb @@ -12,6 +12,7 @@ describe 'projects/tree/show' do assign(:lfs_blob_ids, []) allow(view).to receive(:can?).and_return(true) + allow(view).to receive(:can_contribute_to_project?).and_return(true) allow(view).to receive(:can_collaborate_with_project?).and_return(true) allow(view).to receive_message_chain('user_access.can_push_to_branch?').and_return(true) allow(view).to receive(:current_application_settings).and_return(Gitlab::CurrentSettings.current_application_settings) |