Commit Graph

5 Commits

Author SHA1 Message Date
Douwe Maan 030baf70d2 Enable Performance/RedundantMerge 2017-02-23 09:31:57 -06:00
Timothy Andrew db0182e261 Implement third round of review comments from @DouweM.
Extract/mutate `params` in the `execute` method of the API services,
rather than in `initialize`.
2016-10-24 11:33:38 +05:30
Timothy Andrew 1051087ac4 Implement second round of review comments from @DouweM.
- Pass `developers_and_merge` and `developers_can_push` in `params`
  instead of using keyword arguments.

- Refactor a slightly complex boolean check to a simple `nil?` check.
2016-10-24 11:33:38 +05:30
Timothy Andrew cef10ef7d7 Implement review comments from @dbalexandre.
1. Don't have any EE-only code in CE. Ok to have CE-only code in EE.

2. Use `case` instead of `if/elsif`
2016-10-24 11:33:38 +05:30
Timothy Andrew f79f3a1dd6 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.
2016-10-24 11:33:38 +05:30