diff options
author | Rémy Coutable <remy@rymai.me> | 2016-07-13 11:14:57 +0000 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2016-07-13 11:14:57 +0000 |
commit | 9ca633eb4c62231e4ddff5466c723cf8e2bdb25d (patch) | |
tree | 7147f5acd1250959814e66dc63ce9de66f4e4f2d /features | |
parent | fb229bbf7970ba908962b837b270adf56f14098f (diff) | |
parent | bb81f2afc1d82d6e88d7039025c8a8be0794aac6 (diff) | |
download | gitlab-ce-9ca633eb4c62231e4ddff5466c723cf8e2bdb25d.tar.gz |
Merge branch '18193-developers-can-merge' into 'master'
Allow developers to merge into a protected branch without having push access
## What does this MR do?
Adds a "Developers can merge" checkbox to protected branches much like the "Developers can push" checkbox. When the checkbox is enabled, a developer can merge MRs into that protected branch from the Web UI and from the command-line (any push that is entirely composed of merge commits is allowed).
## Are there points in the code the reviewer needs to double check?
- This MR refactors the `GitAccess` module, moving parts of it to `UserAccess` and the new `ChangeAccessCheck`.
- This MR refactors `GitAccessSpec`, which generates a "matrix" of tests.
- The main logic "developers can merge" should be straightforward enough.
- The commits are fairly atomic, and the commit messages are descriptive regarding the motivations behind every change.
## Why was this MR needed?
A significant portion of this feature was implemented in !4220 (thanks, @mvestergaard!) ; I'm wrapping it up.
## What are the relevant issue numbers?
#18193
Closes #967
## Screenshots



## TODO
- [ ] #18193 !4892 Add "developers can merge" as an option for protected branches
- [x] Review existing code
- [x] Fix build
- [x] Implementation / refactoring
- [x] Clean up `GitAccess`
- [x] Clean up `protected_branches.js.coffee`
- [x] Figure out authorization issue
- If we try to merge code into a protected branch for a user who doesn't have access to that branch, an auth check will fail
- We need to get around this, somehow
- [x] Try detecting merge commits and allowing those
- [x] A push with regular commits _and_ merge commits should fail
- [x] Figure out a solution
- [x] Extensive tests for `MergeCommitCheck`
- [x] Add tests
- [x] Untested parts of original MR
- [x] Improve the checks in `/allowed`
- @dzaporozhets's proposal:
- commits in push == commits in merge request
- branch to push == target branch of merge request
- merge request has required amount of approves (ee only)
- merge commit in push == merge commit we created when merged via UI
- save merge commit sha in database and compare with `newrev`
- my proposal
- /allowed finds all open merge requests with the appropriate target branch
- For each MR, compare the commit SHAs in the MR to the commit SHAs in the change set
- If we have a match, compare the diff of the matching MR to the diff of the change set
- If we still have a match, the merge is legit
- [x] Wait for replies on my proposal
- [x] Pick a strategy
- [x] Implementation
- [x] Save `in_progress_merge_commit_sha`
- [x] Check `in_progress_merge_commit_sha`
- [x] Clear `in_progress_merge_commit_sha`
- [x] Test / refactor
- [x] Merge conflicts
- [x] Verify workflows
- [x] Developer with 'developer can merge' on:
- [x] Can merge an MR from the Web UI
- [x] Error message for conflicts in the Web UI
- [x] Cannot merge an MR from the command line (HTTP)
- [x] Cannot merge an MR from the command line (SSH)
- [x] Cannot modify the branch otherwise
- [x] Developer with 'developer can merge' off:
- [x] Cannot merge an MR from the Web UI
- [x] Error message for conflicts in the Web UI
- [x] Cannot merge an MR from the command line (HTTP)
- [x] Cannot merge an MR from the command line (SSH)
- [x] Cannot modify the branch otherwise
- [x] New projects created could have have "Developers can merge" turned on automatically for the default branch
- [x] CHANGELOG
- [x] Fix build
- [x] Wait for [build](https://gitlab.com/gitlab-org/gitlab-ce/commit/42624e3d53754064186d4ae9048e310d1d3eed0b/builds) to pass
- [x] Screenshots
- [x] Assign to endboss
- [x] Respond to @dbalexandre's comments
- [x] Duplicated line, this is equals to line 26.
- [x] We aren't using any of these helpers in this migration, we can remove the include.
- [x] What do you think to add a default value for this column to avoid the Three-state Boolean Problem?
- [x] group all checks under Gitlab::Checks
- [x] You have a default value for developers_can_merge column, but your migration doesn't add it.
- [x] What do you think to rename Partially protected to anything else?
- [x] Fix conflicts
- [x] Make sure [build](https://gitlab.com/gitlab-org/gitlab-ce/commit/b1cfd42f20a78fd7f844288954e97cff32962e20/builds) passes
- [ ] Wait for merge
See merge request !4892
Diffstat (limited to 'features')
0 files changed, 0 insertions, 0 deletions