1. 03 Aug, 2016 1 commit
  2. 02 Aug, 2016 7 commits
  3. 01 Aug, 2016 4 commits
  4. 29 Jul, 2016 28 commits
    • Robert Speicher's avatar
      Merge branch 'rubocop/enable-access-modifiers-cops' into 'master' · 0fb0f060
      Robert Speicher authored
      Rubocop - enable cops access modifiers
      
      This is sync with https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5014
      
      See merge request !603
      0fb0f060
    • Grzegorz Bizon's avatar
      60e41187
    • Grzegorz Bizon's avatar
      Merge branch 'master' into rubocop/enable-access-modifiers-cops · 11efddf5
      Grzegorz Bizon authored
      * master: (3075 commits)
        Fix rubocop spec.
        Implement final review comments from @rymai.
        Use `Gitlab::Access` to protected branch access levels.
        Fix `git_push_service_spec`
        Authorize user before creating/updating a protected branch.
        Have the `branches` API work with the new protected branches data model.
        Implement review comments from @axil.
        Remove duplicate specs from `git_access_spec`
        Implement review comments from @dbalexandre.
        Favor labels like `Allowed to push` over `Allowed To Push`.
        Add changelog entry.
        Admins count as masters too.
        Make specs compatible with PhantomJS versions < 2.
        Humanize protected branches' access levels at one location.
        Fix all specs related to changes in !5081.
        Fix default branch protection.
        Update protected branches spec to work with the `select`s.
        Allow setting "Allowed To Push/Merge" while creating a protected branch.
        Enforce "No One Can Push" during git operations.
        Add "No One Can Push" to the protected branches UI.
        ...
      
      Conflicts:
      	app/services/system_note_service.rb
      11efddf5
    • Rémy Coutable's avatar
      Merge branch 'ce-18193-no-one-can-push' into 'master' · 56b36c91
      Rémy Coutable authored
      Allow creating protected branches that can't be pushed to
      
      - Mirror of this CE MR: gitlab-org/gitlab-ce!5081
      - Having an EE MR for this feature should avoid merge conflicts later
      
      
      See merge request !569
      56b36c91
    • Timothy Andrew's avatar
      Fix rubocop spec. · ab7ceb92
      Timothy Andrew authored
      ab7ceb92
    • Timothy Andrew's avatar
      Implement final review comments from @rymai. · 03e0f514
      Timothy Andrew authored
      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.
      03e0f514
    • Timothy Andrew's avatar
      Use `Gitlab::Access` to protected branch access levels. · 3bad7455
      Timothy Andrew authored
      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.
      3bad7455
    • Timothy Andrew's avatar
      Fix `git_push_service_spec` · e1986166
      Timothy Andrew authored
      1. Caused by incorrect test setup. The user wasn't added to the project,
         so protected branch creation failed authorization.
      
      2. Change setup for a different test (`Event.last` to
         `Event.find_by_action`) because our `project.team << ...` addition
         was causing a conflict.
      e1986166
    • Timothy Andrew's avatar
      Authorize user before creating/updating a protected branch. · c6ee24a7
      Timothy Andrew authored
      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.
      c6ee24a7
    • Timothy Andrew's avatar
      Have the `branches` API work with the new protected branches data model. · 3e86852b
      Timothy Andrew authored
      1. The new data model moves from `developers_can_{push,merge}` to
         `allowed_to_{push,merge}`.
      
      2. The API interface has not been changed. It still accepts
         `developers_can_push` and `developers_can_merge` as options. These
         attributes are inferred from the new data model.
      
      3. Modify the protected branch create/update services to translate from
         the API interface to our current data model.
      3e86852b
    • Timothy Andrew's avatar
      Implement review comments from @axil. · e0d002be
      Timothy Andrew authored
      1. Align "Allowed to Merge" and "Allowed to Push" dropdowns.
      
      2. Don't display a flash every time a protected branch is updated.
         Previously, we were using this so the test has something to hook onto
         before the assertion. Now we're using `wait_for_ajax` instead.
      e0d002be
    • Timothy Andrew's avatar
      Remove duplicate specs from `git_access_spec` · 969933b2
      Timothy Andrew authored
      - Likely introduced during an improper conflict resolution.
      969933b2
    • Timothy Andrew's avatar
      Implement review comments from @dbalexandre. · e85503f7
      Timothy Andrew authored
      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.
      e85503f7
    • Timothy Andrew's avatar
    • Timothy Andrew's avatar
      Add changelog entry. · 16d3622f
      Timothy Andrew authored
      16d3622f
    • Timothy Andrew's avatar
      Admins count as masters too. · 16949138
      Timothy Andrew authored
      1. In the context of protected branches.
      
      2. Test this behaviour.
      16949138
    • Timothy Andrew's avatar
      Make specs compatible with PhantomJS versions < 2. · 8cf42e63
      Timothy Andrew authored
      1. These versions of PhantomJS don't support `PATCH` requests, so we use
         a `POST` with `_method` set to `PATCH`.
      8cf42e63
    • Timothy Andrew's avatar
      Humanize protected branches' access levels at one location. · 595deddf
      Timothy Andrew authored
      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.
      595deddf
    • Timothy Andrew's avatar
      Fix all specs related to changes in !5081. · 19dd4e80
      Timothy Andrew authored
      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.
      19dd4e80
    • Timothy Andrew's avatar
      Fix default branch protection. · 8f2f6d3d
      Timothy Andrew authored
      1. So it works with the new data model for protected branch access levels.
      8f2f6d3d
    • Timothy Andrew's avatar
      Update protected branches spec to work with the `select`s. · 5ada05f6
      Timothy Andrew authored
      1. Get the existing spec passing.
      
      2. Add specs for all the access control options, both while creating and
         updating protected branches.
      
      3. Show a flash notice when updating protected branches, primarily so
         the spec knows when the update is done.
      5ada05f6
    • Timothy Andrew's avatar
      Allow setting "Allowed To Push/Merge" while creating a protected branch. · 6b73abb8
      Timothy Andrew authored
      1. Reuse the same dropdown component that we used for updating these
         settings (`ProtectedBranchesAccessSelect`). Have it accept options
         for the parent container (so we can control the elements it sees) and
         whether or not to save changes via AJAX (we need this for update, but
         not create).
      
      2. Change the "Developers" option to "Developers + Masters", which is
         clearer.
      
      3. Remove `developers_can_push` and `developers_can_merge` from the
         model, since they're not needed anymore.
      6b73abb8
    • Timothy Andrew's avatar
      Enforce "No One Can Push" during git operations. · db2ae26b
      Timothy Andrew authored
      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.
      db2ae26b
    • Timothy Andrew's avatar
      Add "No One Can Push" to the protected branches UI. · 9168f531
      Timothy Andrew authored
      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.
      9168f531
    • Timothy Andrew's avatar
      Add seeds for protected branches. · b2d62e26
      Timothy Andrew authored
      b2d62e26
    • Timothy Andrew's avatar
      Use the `{Push,Merge}AccessLevel` models in the UI. · 362e632e
      Timothy Andrew authored
      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`
      362e632e
    • Timothy Andrew's avatar
      Add models for the protected branch access levels. · f66e019b
      Timothy Andrew authored
      - And hook up their associations.
      f66e019b
    • Timothy Andrew's avatar
      Add a series of migrations changing the model-level design of protected branch access levels. · c99e5147
      Timothy Andrew authored
      1. Remove the `developers_can_push` and `developers_can_merge` boolean
         columns.
      
      2. Add two new tables, `protected_branches_push_access`, and
         `protected_branches_merge_access`. Each row of these 'access' tables is
         linked to a protected branch, and uses a `access_level` column to
         figure out settings for the protected branch.
      
      3. The `access_level` column is intended to be used with rails' `enum`,
         with `:masters` at index 0 and `:developers` at index 1.
      
      4. Doing it this way has a few advantages:
      
         - Cleaner path to planned EE features where a protected branch is
           accessible only by certain users or groups.
      
         - Rails' `enum` doesn't allow a declaration like this due to the
           duplicates. This approach doesn't have this problem.
      
             enum can_be_pushed_by: [:masters, :developers]
             enum can_be_merged_by: [:masters, :developers]
      c99e5147