Commit graph

6 commits

Author SHA1 Message Date
James Edwards-Jones
f16377e7dc Protected Tags backend review changes
Added changelog
2017-04-06 10:56:21 +01:00
Timothy Andrew
cebcc417ed Implement final review comments from @rymai.
1. Instantiate `ProtectedBranchesAccessSelect` from `dispatcher`

2. Use `can?(user, ...)` instead of `user.can?(...)`

3. Add `DOWNTIME` notes to all migrations added in !5081.

4. Add an explicit `down` method for migrations removing the
   `developers_can_push` and `developers_can_merge` columns, ensuring that
   the columns created (on rollback) have the appropriate defaults.

5. Remove duplicate CHANGELOG entries.

6. Blank lines after guard clauses.
2016-07-29 15:20:39 +05:30
Timothy Andrew
0a8aeb46dc 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.
2016-07-29 15:20:39 +05:30
Timothy Andrew
6d841eaadc Authorize user before creating/updating a protected branch.
1. This is a third line of defence (first in the view, second in the
   controller).

2. Duplicate the `API::Helpers.to_boolean` method in `BaseService`. The
   other alternative is to `include API::Helpers`, but this brings with it
   a number of other methods that might cause conflicts.

3. Return a 403 if authorization fails.
2016-07-29 15:20:39 +05:30
Timothy Andrew
a9958ddc7c Fix default branch protection.
1. So it works with the new data model for protected branch access levels.
2016-07-29 15:20:39 +05:30
Timothy Andrew
134fe5af83 Use the {Push,Merge}AccessLevel models in the UI.
1. Improve error handling while creating protected branches.

2. Modify coffeescript code so that the "Developers can *" checkboxes
   send a '1' or '0' even when using AJAX. This lets us keep the backend
   code simpler.

3. Use services for both creating and updating protected branches.
   Destruction is taken care of with `dependent: :destroy`
2016-07-29 15:20:39 +05:30