From b803bc7bb8ad481790d01848902e80602e77da67 Mon Sep 17 00:00:00 2001
From: Timothy Andrew <mail@timothyandrew.net>
Date: Thu, 22 Sep 2016 20:38:05 +0530
Subject: [PATCH] Implement review comments from @DouweM.

---
 .../concerns/protected_branch_access.rb       |  4 +--
 .../protected_branches/api_create_service.rb  | 25 +++++++++++--------
 lib/api/branches.rb                           |  4 +--
 3 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/app/models/concerns/protected_branch_access.rb b/app/models/concerns/protected_branch_access.rb
index dd6b34d934b..7fd0905ee81 100644
--- a/app/models/concerns/protected_branch_access.rb
+++ b/app/models/concerns/protected_branch_access.rb
@@ -2,8 +2,8 @@ module ProtectedBranchAccess
   extend ActiveSupport::Concern
 
   included do
-    scope :master, -> () { where(access_level: Gitlab::Access::MASTER) }
-    scope :developer, -> () { where(access_level: Gitlab::Access::DEVELOPER) }
+    scope :master, -> { where(access_level: Gitlab::Access::MASTER) }
+    scope :developer, -> { where(access_level: Gitlab::Access::DEVELOPER) }
   end
 
   def humanize
diff --git a/app/services/protected_branches/api_create_service.rb b/app/services/protected_branches/api_create_service.rb
index a28056035b8..cbb99ede9f3 100644
--- a/app/services/protected_branches/api_create_service.rb
+++ b/app/services/protected_branches/api_create_service.rb
@@ -11,17 +11,22 @@ module ProtectedBranches
     end
 
     def execute
-      if @developers_can_push
-        @params.merge!(push_access_levels_attributes: [{ access_level: Gitlab::Access::DEVELOPER }])
-      else
-        @params.merge!(push_access_levels_attributes: [{ access_level: Gitlab::Access::MASTER }])
-      end
+      push_access_level =
+        if @developers_can_push
+          Gitlab::Access::DEVELOPER
+        else
+          Gitlab::Access::MASTER
+        end
 
-      if @developers_can_merge
-        @params.merge!(merge_access_levels_attributes: [{ access_level: Gitlab::Access::DEVELOPER }])
-      else
-        @params.merge!(merge_access_levels_attributes: [{ access_level: Gitlab::Access::MASTER }])
-      end
+      merge_access_level =
+        if @developers_can_merge
+          Gitlab::Access::DEVELOPER
+        else
+          Gitlab::Access::MASTER
+        end
+
+      @params.merge!(push_access_levels_attributes: [{ access_level: push_access_level }],
+                     merge_access_levels_attributes: [{ access_level: merge_access_level }])
 
       service = ProtectedBranches::CreateService.new(@project, @current_user, @params)
       service.execute
diff --git a/lib/api/branches.rb b/lib/api/branches.rb
index 6941cc4aca6..7feb47784b7 100644
--- a/lib/api/branches.rb
+++ b/lib/api/branches.rb
@@ -57,11 +57,11 @@ module API
         developers_can_merge = to_boolean(params[:developers_can_merge])
         developers_can_push = to_boolean(params[:developers_can_push])
 
-        params = {
+        protected_branch_params = {
           name: @branch.name,
         }
 
-        service_args = [user_project, current_user, params,
+        service_args = [user_project, current_user, protected_branch_params,
                         developers_can_push: developers_can_push,
                         developers_can_merge: developers_can_merge]
 
-- 
GitLab