Have the `branches` API work with the new protected branches data model.

1. The new data model moves from `developers_can_{push,merge}` to
   `allowed_to_{push,merge}`.

2. The API interface has not been changed. It still accepts
   `developers_can_push` and `developers_can_merge` as options. These
   attributes are inferred from the new data model.

3. Modify the protected branch create/update services to translate from
   the API interface to our current data model.

4 files changed, 54 insertions, 17 deletions

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])
-
-        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
+        protected_branch_params = {
+          name: @branch.name,
+          developers_can_push: params[:developers_can_push],
+          developers_can_merge: params[:developers_can_merge]
+        }
+
+        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