diff options
author | Bob Van Landuyt <bob@vanlanduyt.co> | 2018-04-06 15:18:58 +0300 |
---|---|---|
committer | Bob Van Landuyt <bob@vanlanduyt.co> | 2018-04-11 11:51:15 +0300 |
commit | 8ad9c4e873f2185e8bbfa6f363db32dbbba27141 (patch) | |
tree | cefefd09ed7f5b03307f31d240f7a20d757ac185 | |
parent | ec43e3644436e0ae90f6fb6a64256c38ceb68cc9 (diff) |
Rename `create_merge_request` permissions
So we can distinguish between the permissions on the source and the
target project.
- `create_merge_request_from` indicates a user can create a merge
request with the project as a source_project
- `create_merge_request_in` indicates a user can create a merge
request with the project as a target_project
-rw-r--r-- | app/controllers/projects/application_controller.rb | 2 | ||||
-rw-r--r-- | app/controllers/projects/issues_controller.rb | 2 | ||||
-rw-r--r-- | app/controllers/projects/merge_requests/creations_controller.rb | 2 | ||||
-rw-r--r-- | app/helpers/blob_helper.rb | 6 | ||||
-rw-r--r-- | app/helpers/compare_helper.rb | 2 | ||||
-rw-r--r-- | app/policies/project_policy.rb | 11 | ||||
-rw-r--r-- | app/presenters/merge_request_presenter.rb | 2 | ||||
-rw-r--r-- | app/services/merge_requests/create_service.rb | 4 | ||||
-rw-r--r-- | app/views/projects/branches/_branch.html.haml | 2 | ||||
-rw-r--r-- | app/views/projects/issues/_new_branch.html.haml | 2 | ||||
-rw-r--r-- | app/views/projects/merge_requests/index.html.haml | 2 | ||||
-rw-r--r-- | db/fixtures/development/10_merge_requests.rb | 8 | ||||
-rw-r--r-- | spec/policies/project_policy_spec.rb | 13 |
13 files changed, 32 insertions, 26 deletions
diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb index cbb14b55399..53a0c712e49 100644 --- a/app/controllers/projects/application_controller.rb +++ b/app/controllers/projects/application_controller.rb @@ -35,7 +35,7 @@ class Projects::ApplicationController < ApplicationController project ||= @project can_create_merge_request = - can?(current_user, :create_merge_request_in_project, project) && + can?(current_user, :create_merge_request_in, project) && current_user.already_forked?(project) can?(current_user, :push_code, project) || diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index b14939c4216..9dcd13b9385 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -20,7 +20,7 @@ class Projects::IssuesController < Projects::ApplicationController before_action :authorize_update_issuable!, only: [:edit, :update, :move] # Allow create a new branch and empty WIP merge request from current issue - before_action :authorize_create_merge_request!, only: [:create_merge_request] + before_action :authorize_create_merge_request_in!, only: [:create_merge_request] respond_to :html diff --git a/app/controllers/projects/merge_requests/creations_controller.rb b/app/controllers/projects/merge_requests/creations_controller.rb index a90030a8312..4a377fefc62 100644 --- a/app/controllers/projects/merge_requests/creations_controller.rb +++ b/app/controllers/projects/merge_requests/creations_controller.rb @@ -5,7 +5,7 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap skip_before_action :merge_request before_action :whitelist_query_limiting, only: [:create] - before_action :authorize_create_merge_request! + before_action :authorize_create_merge_request_from! before_action :apply_diff_view_cookie!, only: [:diffs, :diff_for_path] before_action :build_merge_request, except: [:create] diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index ac7b8d6672e..a5f7a7204df 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -59,7 +59,7 @@ module BlobHelper button_tag label, class: "#{common_classes} disabled has-tooltip", title: "It is not possible to #{action} files that are stored in LFS using the web interface", data: { container: 'body' } elsif can_modify_blob?(blob, project, ref) button_tag label, class: "#{common_classes}", 'data-target' => "#modal-#{modal_type}-blob", 'data-toggle' => 'modal' - elsif can?(current_user, :create_merge_request_in_project, project) + elsif can?(current_user, :create_merge_request_in, project) edit_fork_button_tag(common_classes, project, label, edit_modify_file_fork_params(action), action) end end @@ -280,7 +280,7 @@ module BlobHelper options << link_to("submit an issue", new_project_issue_path(project)) end - merge_project = can?(current_user, :create_merge_request, project) ? project : (current_user && current_user.fork_of(project)) + merge_project = can?(current_user, :create_merge_request_from, project) ? project : (current_user && current_user.fork_of(project)) if merge_project options << link_to("create a merge request", project_new_merge_request_path(project)) end @@ -334,7 +334,7 @@ module BlobHelper # Web IDE (Beta) requires the user to have this feature enabled elsif !current_user || (current_user && can_modify_blob?(blob, project, ref)) edit_link_tag(text, edit_path, common_classes) - elsif can?(current_user, :create_merge_request_in_project, project) + elsif can?(current_user, :fork_project, project) && can?(current_user, :create_merge_request_in, project) edit_fork_button_tag(common_classes, project, text, edit_blob_fork_params(edit_path)) end end diff --git a/app/helpers/compare_helper.rb b/app/helpers/compare_helper.rb index 8bf96c0905f..2df5b5d1695 100644 --- a/app/helpers/compare_helper.rb +++ b/app/helpers/compare_helper.rb @@ -3,7 +3,7 @@ module CompareHelper from.present? && to.present? && from != to && - can?(current_user, :create_merge_request, project) && + can?(current_user, :create_merge_request_from, project) && project.repository.branch_exists?(from) && project.repository.branch_exists?(to) end diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 72e30c932a9..ac154270236 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -140,7 +140,7 @@ class ProjectPolicy < BasePolicy rule { can?(:guest_access) }.policy do enable :read_project - enable :create_merge_request_in_project + enable :create_merge_request_in enable :read_board enable :read_list enable :read_wiki @@ -212,7 +212,7 @@ class ProjectPolicy < BasePolicy enable :create_pipeline enable :update_pipeline enable :create_pipeline_schedule - enable :create_merge_request + enable :create_merge_request_from enable :create_wiki enable :push_code enable :resolve_note @@ -251,7 +251,8 @@ class ProjectPolicy < BasePolicy prevent :request_access prevent :upload_file prevent :resolve_note - prevent :create_merge_request_in_project + prevent :create_merge_request_from + prevent :create_merge_request_in READONLY_FEATURES_WHEN_ARCHIVED.each do |feature| prevent(*create_update_admin_destroy(feature)) @@ -263,7 +264,8 @@ class ProjectPolicy < BasePolicy end rule { merge_requests_disabled | repository_disabled }.policy do - prevent :create_merge_request_in_project + prevent :create_merge_request_in + prevent :create_merge_request_from prevent(*create_read_update_admin_destroy(:merge_request)) end @@ -309,7 +311,6 @@ class ProjectPolicy < BasePolicy rule { can?(:public_access) }.policy do enable :read_project - enable :create_merge_request_in_project enable :read_board enable :read_list enable :read_wiki diff --git a/app/presenters/merge_request_presenter.rb b/app/presenters/merge_request_presenter.rb index 950d3fde2ea..a55af1422e8 100644 --- a/app/presenters/merge_request_presenter.rb +++ b/app/presenters/merge_request_presenter.rb @@ -197,7 +197,7 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated def user_can_collaborate_with_project? can_create_merge_request = - can?(current_user, :create_merge_request_in_project, project) && + can?(current_user, :create_merge_request_in, project) && current_user.already_forked?(project) can?(current_user, :push_code, project) || diff --git a/app/services/merge_requests/create_service.rb b/app/services/merge_requests/create_service.rb index 4c2c8398461..fe1ac70781e 100644 --- a/app/services/merge_requests/create_service.rb +++ b/app/services/merge_requests/create_service.rb @@ -71,8 +71,8 @@ module MergeRequests params.delete(:source_project_id) params.delete(:target_project_id) - unless can?(current_user, :read_project, @source_project) && - can?(current_user, :create_merge_request_in_project, @project) + unless can?(current_user, :create_merge_request_from, @source_project) && + can?(current_user, :create_merge_request_in, @project) raise Gitlab::Access::AccessDeniedError end diff --git a/app/views/projects/branches/_branch.html.haml b/app/views/projects/branches/_branch.html.haml index 8bc146cbaf4..6ca33c7524d 100644 --- a/app/views/projects/branches/_branch.html.haml +++ b/app/views/projects/branches/_branch.html.haml @@ -4,7 +4,7 @@ - diverging_commit_counts = @repository.diverging_commit_counts(branch) - number_commits_behind = diverging_commit_counts[:behind] - number_commits_ahead = diverging_commit_counts[:ahead] -- merge_project = can?(current_user, :create_merge_request, @project) ? @project : (current_user && current_user.fork_of(@project)) +- merge_project = can?(current_user, :create_merge_request_from, @project) ? @project : (current_user && current_user.fork_of(@project)) %li{ class: "branch-item js-branch-#{branch.name}" } .branch-info .branch-title diff --git a/app/views/projects/issues/_new_branch.html.haml b/app/views/projects/issues/_new_branch.html.haml index 36e24037214..02ea3dc74fe 100644 --- a/app/views/projects/issues/_new_branch.html.haml +++ b/app/views/projects/issues/_new_branch.html.haml @@ -1,4 +1,4 @@ -- can_create_merge_request = can?(current_user, :create_merge_request, @project) +- can_create_merge_request = can?(current_user, :create_merge_request_from, @project) - data_action = can_create_merge_request ? 'create-mr' : 'create-branch' - value = can_create_merge_request ? 'Create merge request' : 'Create branch' diff --git a/app/views/projects/merge_requests/index.html.haml b/app/views/projects/merge_requests/index.html.haml index b2c0d9e1cfa..f9ab0f5709b 100644 --- a/app/views/projects/merge_requests/index.html.haml +++ b/app/views/projects/merge_requests/index.html.haml @@ -1,6 +1,6 @@ - @no_container = true - @can_bulk_update = can?(current_user, :admin_merge_request, @project) -- merge_project = can?(current_user, :create_merge_request, @project) ? @project : (current_user && current_user.fork_of(@project)) +- merge_project = can?(current_user, :create_merge_request_from, @project) ? @project : (current_user && current_user.fork_of(@project)) - new_merge_request_path = project_new_merge_request_path(merge_project) if merge_project - page_title "Merge Requests" diff --git a/db/fixtures/development/10_merge_requests.rb b/db/fixtures/development/10_merge_requests.rb index 30244ee4431..bcfdd058a1c 100644 --- a/db/fixtures/development/10_merge_requests.rb +++ b/db/fixtures/development/10_merge_requests.rb @@ -4,7 +4,7 @@ Gitlab::Seeder.quiet do # Limit the number of merge requests per project to avoid long seeds MAX_NUM_MERGE_REQUESTS = 10 - Project.all.reject(&:empty_repo?).each do |project| + Project.non_archived.with_merge_requests_enabled.reject(&:empty_repo?).each do |project| branches = project.repository.branch_names.sample(MAX_NUM_MERGE_REQUESTS * 2) branches.each do |branch_name| @@ -21,7 +21,11 @@ Gitlab::Seeder.quiet do assignee: project.team.users.sample } - MergeRequests::CreateService.new(project, project.team.users.sample, params).execute + # Only create MRs with users that are allowed to create MRs + developer = project.team.developers.sample + break unless developer + + MergeRequests::CreateService.new(project, developer, params).execute print '.' end end diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index 4c01c320841..33d5a61c563 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -14,7 +14,7 @@ describe ProjectPolicy do read_project read_board read_list read_wiki read_issue read_project_for_iids read_issue_iid read_merge_request_iid read_label read_milestone read_project_snippet read_project_member read_note - create_project create_issue create_note upload_file create_merge_request_in_project + create_project create_issue create_note upload_file create_merge_request_in ] end @@ -35,7 +35,7 @@ describe ProjectPolicy do %i[ admin_milestone admin_merge_request update_merge_request create_commit_status update_commit_status create_build update_build create_pipeline - update_pipeline create_merge_request create_wiki push_code + update_pipeline create_merge_request_from create_wiki push_code resolve_note create_container_image update_container_image create_environment create_deployment ] @@ -142,9 +142,9 @@ describe ProjectPolicy do it 'disallows all permissions when the feature is disabled' do project.project_feature.update(merge_requests_access_level: ProjectFeature::DISABLED) - mr_permissions = [:create_merge_request, :read_merge_request, + mr_permissions = [:create_merge_request_from, :read_merge_request, :update_merge_request, :admin_merge_request, - :create_merge_request_in_project] + :create_merge_request_in] expect_disallowed(*mr_permissions) end @@ -159,7 +159,8 @@ describe ProjectPolicy do let(:other_write_abilities) do %i[ - create_merge_request_in_project + create_merge_request_in + create_merge_request_from push_to_delete_protected_branch push_code request_access @@ -192,7 +193,7 @@ describe ProjectPolicy do context 'when a project has pending invites' do let(:group) { create(:group, :public) } let(:project) { create(:project, :public, namespace: group) } - let(:user_permissions) { [:create_project, :create_issue, :create_note, :upload_file] } + let(:user_permissions) { [:create_merge_request_in, :create_project, :create_issue, :create_note, :upload_file] } let(:anonymous_permissions) { guest_permissions - user_permissions } subject { described_class.new(nil, project) } |