Use `Gitlab::Access` to protected branch access levels.

1. It makes sense to reuse these constants since we had them duplicated
   in the previous enum implementation. This also simplifies our
   `check_access` implementation, because we can use
   `project.team.max_member_access` directly.

2. Use `accepts_nested_attributes_for` to create push/merge access
   levels. This was a bit fiddly to set up, but this simplifies our code
   by quite a large amount. We can even get rid of
   `ProtectedBranches::BaseService`.

3. Move API handling back into the API (previously in
   `ProtectedBranches::BaseService#translate_api_params`.

4. The protected branch services now return a `ProtectedBranch` rather
   than `true/false`.

5. Run `load_protected_branches` on-demand in the `create` action, to
   prevent it being called unneccessarily.

6. "Masters" is pre-selected as the default option for "Allowed to Push"
   and "Allowed to Merge".

7. These changes were based on a review from @rymai in !5081.

3 files changed, 23 insertions, 17 deletions

diff --git a/spec/factories/protected_branches.rb b/spec/factories/protected_branches.rb
index 24a9b78f0c2..5575852c2d7 100644
--- a/spec/factories/protected_branches.rb
+++ b/spec/factories/protected_branches.rb
@@ -4,20 +4,26 @@ FactoryGirl.define do
     project
 
     after(:create) do |protected_branch|
-      protected_branch.create_push_access_level!(access_level: :masters)
-      protected_branch.create_merge_access_level!(access_level: :masters)
+      protected_branch.create_push_access_level!(access_level: Gitlab::Access::MASTER)
+      protected_branch.create_merge_access_level!(access_level: Gitlab::Access::MASTER)
     end
 
     trait :developers_can_push do
-      after(:create) { |protected_branch| protected_branch.push_access_level.developers! }
+      after(:create) do |protected_branch|
+        protected_branch.push_access_level.update!(access_level: Gitlab::Access::DEVELOPER)
+      end
     end
 
     trait :developers_can_merge do
-      after(:create) { |protected_branch| protected_branch.merge_access_level.developers! }
+      after(:create) do |protected_branch|
+        protected_branch.merge_access_level.update!(access_level: Gitlab::Access::DEVELOPER)
+      end
     end
 
     trait :no_one_can_push do
-      after(:create) { |protected_branch| protected_branch.push_access_level.no_one! }
+      after(:create) do |protected_branch|
+        protected_branch.push_access_level.update!(access_level: Gitlab::Access::NO_ACCESS)
+      end
     end
   end
 end
diff --git a/spec/features/protected_branches_spec.rb b/spec/features/protected_branches_spec.rb
index dac2bcf9efd..57734b33a44 100644
--- a/spec/features/protected_branches_spec.rb
+++ b/spec/features/protected_branches_spec.rb
@@ -91,12 +91,12 @@ feature 'Projected Branches', feature: true, js: true do
         set_protected_branch_name('master')
         within('.new_protected_branch') do
           find(".allowed-to-push").click
-          click_on access_type_name
+          within(".dropdown.open .dropdown-menu") { click_on access_type_name }
         end
         click_on "Protect"
 
         expect(ProtectedBranch.count).to eq(1)
-        expect(ProtectedBranch.last.allowed_to_push).to eq(access_type_id)
+        expect(ProtectedBranch.last.push_access_level.access_level).to eq(access_type_id)
       end
 
       it "allows updating protected branches so that #{access_type_name} can push to them" do
@@ -112,7 +112,7 @@ feature 'Projected Branches', feature: true, js: true do
         end
 
         wait_for_ajax
-        expect(ProtectedBranch.last.allowed_to_push).to eq(access_type_id)
+        expect(ProtectedBranch.last.push_access_level.access_level).to eq(access_type_id)
       end
     end
 
@@ -122,12 +122,12 @@ feature 'Projected Branches', feature: true, js: true do
         set_protected_branch_name('master')
         within('.new_protected_branch') do
           find(".allowed-to-merge").click
-          click_on access_type_name
+          within(".dropdown.open .dropdown-menu") { click_on access_type_name }
         end
         click_on "Protect"
 
         expect(ProtectedBranch.count).to eq(1)
-        expect(ProtectedBranch.last.allowed_to_merge).to eq(access_type_id)
+        expect(ProtectedBranch.last.merge_access_level.access_level).to eq(access_type_id)
       end
 
       it "allows updating protected branches so that #{access_type_name} can merge to them" do
@@ -143,7 +143,7 @@ feature 'Projected Branches', feature: true, js: true do
         end
 
         wait_for_ajax
-        expect(ProtectedBranch.last.allowed_to_merge).to eq(access_type_id)
+        expect(ProtectedBranch.last.merge_access_level.access_level).to eq(access_type_id)
       end
     end
   end
diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb
index 621eced83f6..ffa998dffc3 100644
--- a/spec/services/git_push_service_spec.rb
+++ b/spec/services/git_push_service_spec.rb
@@ -227,8 +227,8 @@ describe GitPushService, services: true do
         expect(project.default_branch).to eq("master")
         execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' )
         expect(project.protected_branches).not_to be_empty
-        expect(project.protected_branches.first.allowed_to_push).to eq('masters')
-        expect(project.protected_branches.first.allowed_to_merge).to eq('masters')
+        expect(project.protected_branches.first.push_access_level.access_level).to eq(Gitlab::Access::MASTER)
+        expect(project.protected_branches.first.merge_access_level.access_level).to eq(Gitlab::Access::MASTER)
       end
 
       it "when pushing a branch for the first time with default branch protection disabled" do
@@ -249,8 +249,8 @@ describe GitPushService, services: true do
         execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' )
 
         expect(project.protected_branches).not_to be_empty
-        expect(project.protected_branches.last.allowed_to_push).to eq('developers')
-        expect(project.protected_branches.last.allowed_to_merge).to eq('masters')
+        expect(project.protected_branches.last.push_access_level.access_level).to eq(Gitlab::Access::DEVELOPER)
+        expect(project.protected_branches.last.merge_access_level.access_level).to eq(Gitlab::Access::MASTER)
       end
 
       it "when pushing a branch for the first time with default branch protection set to 'developers can merge'" do
@@ -260,8 +260,8 @@ describe GitPushService, services: true do
         expect(project.default_branch).to eq("master")
         execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' )
         expect(project.protected_branches).not_to be_empty
-        expect(project.protected_branches.first.allowed_to_push).to eq('masters')
-        expect(project.protected_branches.first.allowed_to_merge).to eq('developers')
+        expect(project.protected_branches.first.push_access_level.access_level).to eq(Gitlab::Access::MASTER)
+        expect(project.protected_branches.first.merge_access_level.access_level).to eq(Gitlab::Access::DEVELOPER)
       end
 
       it "when pushing new commits to existing branch" do