diff --git a/app/services/protected_branches/base_service.rb b/app/services/protected_branches/base_service.rb index f8741fcb3d5..a5896587ded 100644 --- a/app/services/protected_branches/base_service.rb +++ b/app/services/protected_branches/base_service.rb @@ -1,6 +1,15 @@ module ProtectedBranches class BaseService < ::BaseService + include API::Helpers + + def initialize(project, current_user, params = {}) + super(project, current_user, params) + @allowed_to_push = params[:allowed_to_push] + @allowed_to_merge = params[:allowed_to_merge] + end + def set_access_levels! + translate_api_params! set_merge_access_levels! set_push_access_levels! end @@ -8,7 +17,7 @@ module ProtectedBranches protected def set_merge_access_levels! - case params[:allowed_to_merge] + case @allowed_to_merge when 'masters' @protected_branch.merge_access_level.masters! when 'developers' @@ -17,7 +26,7 @@ module ProtectedBranches end def set_push_access_levels! - case params[:allowed_to_push] + case @allowed_to_push when 'masters' @protected_branch.push_access_level.masters! when 'developers' @@ -26,5 +35,26 @@ module ProtectedBranches @protected_branch.push_access_level.no_one! end end + + # The `branches` API still uses `developers_can_push` and `developers_can_merge`, + # which need to be translated to `allowed_to_push` and `allowed_to_merge`, + # respectively. + def translate_api_params! + @allowed_to_push ||= + case to_boolean(params[:developers_can_push]) + when true + 'developers' + when false + 'masters' + end + + @allowed_to_merge ||= + case to_boolean(params[:developers_can_merge]) + when true + 'developers' + when false + 'masters' + end + end end end diff --git a/lib/api/branches.rb b/lib/api/branches.rb index 66b853eb342..4133a1f7a6b 100644 --- a/lib/api/branches.rb +++ b/lib/api/branches.rb @@ -35,6 +35,10 @@ module API # Protect a single branch # + # Note: The internal data model moved from `developers_can_{merge,push}` to `allowed_to_{merge,push}` + # in `gitlab-org/gitlab-ce!5081`. The API interface has not been changed (to maintain compatibility), + # but it works with the changed data model to infer `developers_can_merge` and `developers_can_push`. + # # Parameters: # id (required) - The ID of a project # branch (required) - The name of the branch @@ -49,18 +53,19 @@ module API @branch = user_project.repository.find_branch(params[:branch]) not_found!('Branch') unless @branch protected_branch = user_project.protected_branches.find_by(name: @branch.name) - developers_can_push = to_boolean(params[:developers_can_push]) - developers_can_merge = to_boolean(params[:developers_can_merge]) + protected_branch_params = { + name: @branch.name, + developers_can_push: params[:developers_can_push], + developers_can_merge: params[:developers_can_merge] + } - if protected_branch - protected_branch.developers_can_push = developers_can_push unless developers_can_push.nil? - protected_branch.developers_can_merge = developers_can_merge unless developers_can_merge.nil? - protected_branch.save - else - user_project.protected_branches.create(name: @branch.name, - developers_can_push: developers_can_push || false, - developers_can_merge: developers_can_merge || false) - end + service = if protected_branch + ProtectedBranches::UpdateService.new(user_project, current_user, protected_branch.id, protected_branch_params) + else + ProtectedBranches::CreateService.new(user_project, current_user, protected_branch_params) + end + + service.execute present @branch, with: Entities::RepoBranch, project: user_project end diff --git a/lib/api/entities.rb b/lib/api/entities.rb index e76e7304674..e51bee5c846 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -126,11 +126,13 @@ module API end expose :developers_can_push do |repo_branch, options| - options[:project].developers_can_push_to_protected_branch? repo_branch.name + project = options[:project] + project.protected_branches.matching(repo_branch.name).any? { |protected_branch| protected_branch.push_access_level.developers? } end expose :developers_can_merge do |repo_branch, options| - options[:project].developers_can_merge_to_protected_branch? repo_branch.name + project = options[:project] + project.protected_branches.matching(repo_branch.name).any? { |protected_branch| protected_branch.merge_access_level.developers? } end end diff --git a/spec/requests/api/branches_spec.rb b/spec/requests/api/branches_spec.rb index 719da27f919..e8fd697965f 100644 --- a/spec/requests/api/branches_spec.rb +++ b/spec/requests/api/branches_spec.rb @@ -112,7 +112,7 @@ describe API::API, api: true do before do project.repository.add_branch(user, protected_branch, 'master') - create(:protected_branch, project: project, name: protected_branch, developers_can_push: true, developers_can_merge: true) + create(:protected_branch, :developers_can_push, :developers_can_merge, project: project, name: protected_branch) end it 'updates that a developer can push' do