Fix branch protection API.

1. Previously, we were not removing existing access levels before
   creating new ones. This is not a problem for EE, but _is_ for CE,
   since we restrict the number of access levels in CE to 1.

2. The correct approach is:

    CE -> delete all access levels before updating a protected branch
    EE -> delete developer access levels if "developers_can_{merge,push}" is switched off

3. The dispatch is performed by checking if a "length: 1" validation is
   present on the access levels or not.

4. Another source of problems was that we didn't put multiple queries in
   a transaction. If the `destroy_all` passes, but the `update` fails,
   we should have a rollback.

5. Modifying the API to provide users direct access to CRUD access
   levels will make things a lot simpler.

6. Create `create/update` services separately for this API, which
   perform the necessary data translation, before calling the regular
   `create/update` services. The translation code was getting too large
   for the API endpoint itself, so this move makes sense.

1 files changed, 5 insertions, 0 deletions

diff --git a/app/models/concerns/protected_branch_access.rb b/app/models/concerns/protected_branch_access.rb
index 5a7b36070e7..dd6b34d934b 100644
--- a/app/models/concerns/protected_branch_access.rb
+++ b/app/models/concerns/protected_branch_access.rb
@@ -1,6 +1,11 @@
 module ProtectedBranchAccess
   extend ActiveSupport::Concern
 
+  included do
+    scope :master, -> () { where(access_level: Gitlab::Access::MASTER) }
+    scope :developer, -> () { where(access_level: Gitlab::Access::DEVELOPER) }
+  end
+
   def humanize
     self.class.human_access_levels[self.access_level]
   end