Commit graph

13 commits

Author SHA1 Message Date
Phil Hughes
025a1b0175 Enabled no-one as a merge access level in protected branches
Closes #31541
2017-05-10 17:19:40 +01:00
Timothy Andrew
41bf093662 CE-specific changes gitlab-org/gitlab-ee#1137
- Extract all common {push,merge} access level model code into the
  `ProtectedBranchAccess` module

- Use the HTTP verb to define controller specs
2016-11-29 17:38:45 +05:30
Timothy Andrew
e805a64700 Backport changes from gitlab-org/gitlab-ee!581 to CE.
!581 has a lot of changes that would cause merge conflicts if not
properly backported to CE. This commit/MR serves as a better
foundation for gitlab-org/gitlab-ee!581.

= Changes =

1. Move from `has_one {merge,push}_access_level` to `has_many`, with the
   `length` of the association limited to `1`. This is _effectively_ a
   `has_one` association, but should cause less conflicts with EE, which
   is set to `has_many`. This has a number of related changes in the
   views, specs, and factories.

2. Make `gon` variable loading more consistent (with EE!581) in the
   `ProtectedBranchesController`. Also use `::` to prefix the
   `ProtectedBranches` services, because this is required in EE.

3. Extract a `ProtectedBranchAccess` concern from the two access level
   models. This concern only has a single `humanize` method here, but
   will have more methods in EE.

4. Add `form_errors` to the protected branches creation form. This is
   not strictly required for EE compatibility, but was an oversight
   nonetheless.
2016-08-16 11:05:14 +05:30
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
7b2ad2d5b9 Implement review comments from @dbalexandre.
1. Remove `master_or_greater?` and `developer_or_greater?` in favor of
   `max_member_access`, which is a lot nicer.

2. Remove a number of instances of `include Gitlab::Database::MigrationHelpers`
   in migrations that don't need this module. Also remove comments where
   not necessary.

3. Remove duplicate entry in CHANGELOG.

4. Move `ProtectedBranchAccessSelect` from Coffeescript to ES6.

5. Split the `set_access_levels!` method in two - one each for `merge` and
   `push` access levels.
2016-07-29 15:20:39 +05:30
Timothy Andrew
cc1cebdcc5 Admins count as masters too.
1. In the context of protected branches.

2. Test this behaviour.
2016-07-29 15:20:39 +05:30
Timothy Andrew
f2df2966aa Humanize protected branches' access levels at one location.
1. The model now contains this humanization data, which is the once
   source of truth.

2. Previously, this was being listed out in the dropdown component as well.
2016-07-29 15:20:39 +05:30
Timothy Andrew
c647540c10 Fix all specs related to changes in !5081.
1. Remove `Project#developers_can_push_to_protected_branch?` since it
   isn't used anymore.

2. Remove `Project#developers_can_merge_to_protected_branch?` since it
   isn't used anymore.
2016-07-29 15:20:39 +05:30
Timothy Andrew
828f6eb6e5 Enforce "No One Can Push" during git operations.
1. The crux of this change is in `UserAccess`, which looks through all
   the access levels, asking each if the user has access to push/merge
   for the current project.

2. Update the `protected_branches` factory to create access levels as
   necessary.

3. Fix and augment `user_access` and `git_access` specs.
2016-07-29 15:20:39 +05:30
Timothy Andrew
ab6096c172 Add "No One Can Push" to the protected branches UI.
1. Move to dropdowns instead of checkboxes. One each for "Allowed to
   Push" and "Allowed to Merge"

2. Refactor the `ProtectedBranches` coffeescript class into
   `ProtectedBranchesAccessSelect`.

3. Modify the backend to accept the new parameters.
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
Timothy Andrew
21bece443d Add models for the protected branch access levels.
- And hook up their associations.
2016-07-29 15:20:39 +05:30