| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| |\
| |
| |
| |
| | |
Actually render old and new sections of parallel diff next to each other
See merge request !5173
|
| | | |
|
| |\ \
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Dropdown loading time preformance fix
## What does this MR do?
Optimizes the performance of the dropdown load time by just sending the required data to load the dropdown instead of the full object
This MR aims to fix #17474
See merge request !5113
|
| | | | |
|
| | | | |
|
| | | | |
|
| | | |
| | |
| | |
| | | |
basicprojectsdetails and changes the url to a more reader friendly format
|
| | | | |
|
| | | | |
|
| | | | |
|
| | | |
| | |
| | |
| | | |
format
|
| | | | |
|
| | | |
| | |
| | |
| | | |
basicprojectsdetails and changes the url to a more reader friendly format
|
| | | | |
|
| | | | |
|
| | | |
| | |
| | |
| | |
| | | |
With these changes we don’t lost the issue references when importing
from `GitLab.com`.
|
| |\ \ \
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
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
|
| | | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
1. Fix typos, minor styling errors.
2. Use single quotes rather than double quotes in `user_access_spec`.
3. Test formatting.
|
| | | | |
| | | |
| | | |
| | | |
| | | | |
- https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/4892#note_12892160
- This is more consistent.
|
| | | | | |
|
| | | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
1. Don't use case statements for dispatch anymore. This leads to a lot
of duplication, and makes the logic harder to follow.
2. Remove duplicated logic.
- For example, the `can_push_to_branch?` exists, but we also have a
different way of checking the same condition within `change_access_check`.
- This kind of duplication is removed, and the `can_push_to_branch?`
method is used in both places.
3. Move checks returning true/false to `UserAccess`.
- All public methods in `GitAccess` now return an instance of
`GitAccessStatus`. Previously, some methods would return
true/false as well, which was confusing.
- It makes sense for these kinds of checks to be at the level of a
user, so the `UserAccess` class was repurposed for this. The prior
`UserAccess.allowed?` classmethod is converted into an instance
method.
- All external uses of these checks have been migrated to use the
`UserAccess` class
4. Move the "change_access_check" into a separate class.
- Create the `GitAccess::ChangeAccessCheck` class to run these
checks, which are quite substantial.
- `ChangeAccessCheck` returns an instance of `GitAccessStatus` as
well.
5. Break out the boolean logic in `ChangeAccessCheck` into `if/else`
chains - this seems more readable.
6. I can understand that this might look like overkill for !4892, but I
think this is a good opportunity to clean it up.
- http://martinfowler.com/bliki/OpportunisticRefactoring.html
|
| | | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
1. When a merge request is being merged, save the merge commit SHA in
the `in_progress_merge_commit_sha` database column.
2. The `pre-receive` hook looks for any locked (in progress) merge
request with `in_progress_merge_commit_sha` matching the `newrev` it
is passed.
3. If it finds a matching MR, the merge is legitimate.
4. Update `git_access_spec` to test the behaviour we added here. Also
refactored this spec a bit to make it easier to add more contexts / conditions.
|
| | | | |
| | | |
| | | |
| | | |
| | | | |
- Cherry-picked from `mvestergaard:branch-protection-dev-merge`
- https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/4220
|
| |/ / /
| | |
| | |
| | | |
Rails.cache.read_multi
|
| |\ \ \
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
Render inline diffs for multiple changed lines following eachother
Before:
After:
See merge request !5174
|
| | | | | |
|
| | | | | |
|
| | | |/
| |/| |
|
| |\ \ \ |
|
| | | |/
| |/|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/17877 .
This change adds 'defense in depth' against 'Host' HTTP header
injection. It affects normal users in the following way. Suppose your
GitLab server has IP address 1.2.3.4 and hostname gitlab.example.com.
Currently, if you enter 1.2.3.4 in your browser, you get redirected to
1.2.3.4/users/sign_in. After this change, you get redirected from
1.2.3.4 to gitlab.example.com/users/sign_in. This is because the
address you typed in the address bar of your browser ('1.2.3.4'),
which gets stored in the 'Host' header, is now being overwritten to
'gitlab.example.com' in NGINX.
In this change we also make NGINX clear the 'X-Forwarded-Host' header
because Ruby on Rails also uses that header the same wayas the 'Host'
header.
We think that for most GitLab servers this is the right behavior, and
if not then administrators can change this behavior themselves at the
NGINX level.
|
| |\ \ \
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
Fix GitHub client requests when rate limit is disabled
## What does this MR do?
GitHub Rate Limit API returns 404 when the rate limit is disabled. In this case we just want to return gracefully instead of spitting out an error.
## Are there points in the code the reviewer needs to double check?
No.
## Why was this MR needed?
GitHub importer fails when rate limit is disabled in GitHub Enterprise.
## What are the relevant issue numbers?
https://gitlab.com/gitlab-org/gitlab-ee/issues/697
## Does this MR meet the acceptance criteria?
- [X] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- Tests
- [X] Added for this feature/bug
- [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [ ] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [ ] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
See merge request !5191
|
| | | | | |
|
| | | | | |
|
| | |/ / |
|
| |/ / |
|
| |\ \
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
api: expose {should,force}_remove_source_branch
## What does this MR do?
Exposes the `should_remove_source_branch` and `force_remove_source_branch` booleans via the API.
## Are there points in the code the reviewer needs to double check?
I don't think so.
## Why was this MR needed?
See the commit message.
## What are the relevant issue numbers?
N/A
## Screenshots (if relevant)
N/A
## Does this MR meet the acceptance criteria?
- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [x] API support added
- Tests
- [x] Added for this feature/bug
- [ ] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [ ] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
See merge request !5184
|
| | | |
| | |
| | |
| | |
| | | |
Workflows which use a bot to merge should remove branches if requested.
Expose the flag so that bots can request know this.
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
have been fully redacted and contain cross-project references.
The previous implementation relied on Note#cross_reference_not_visible_for?,
which essentially tries to render all the Markdown references in a system note
and only displays the note if the user can see the referring project. But this
duplicated the work that Banzai::NotesRenderer was doing already. Instead, for
each note we render, we memoize the number of visible user references and
use it later if it is available.
Improves #19273
|
| |\ \ \
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
Fix problems with events under notes importing GitLab projects
Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/19588
See merge request !5154
|
| | |\ \ \
| | |/ /
| | | |
| | | | |
fix/import-event-error
|
| | | | |
| | | |
| | | |
| | | | |
sub_relations method recursively.
|
| |\ \ \ \
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
Single file diffs
Closes #14103. Related (not part of this MR): #19553.
This adds a `diff_for_path` action to each place we render diffs (commit, compare, new MR, existing MR) which renders the diff for a single path. The action is always available with the same params as the 'parent' action, to make it simpler to generate the URIs.
If a diff is bigger than 10 KB, it will be collapsed by default and have a data attribute added. You can then click the message or the filename to expand that diff. For expanded files, you can collapse and expand them, but they won't make any AJAX requests.
See merge request !4990
|
| | |\ \ \ \
| | | |/ /
| | |/| | |
|
| | | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
When rendering a list of diff files, skip those where the diff is over
10 KB and provide an endpoint to render individually instead.
|
| |\ \ \ \ \
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | | |
entities: remove :description from MergeRequest
## What does this MR do?
Removes a duplicate `description` key from API JSON objects.
## Are there points in the code the reviewer needs to double check?
No.
## Why was this MR needed?
`description` is already exported via `ProjectEntity` and doesn't need to be listed in `MergeRequest` as well.
## What are the relevant issue numbers?
N/A
## Screenshots (if relevant)
N/A
## Does this MR meet the acceptance criteria?
- [ ] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [ ] API support added
- Tests
- [ ] Added for this feature/bug
- [ ] All builds are passing
- [ ] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [ ] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [ ] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
See merge request !5099
|
| | | |/ / /
| |/| | |
| | | | |
| | | | | |
It is already part of ProjectEntity.
|
| |\ \ \ \ \
| |/ / / /
|/| | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
Use Gitlab-Workhorse-Send-Data to send entry
## What does this MR do?
Use Gitlab-Workhorse-Send-Data to send entry:
Closes #19224, Closes #19128
Also requires this MR to work:
https://gitlab.com/gitlab-org/gitlab-workhorse/merge_requests/53
## Are there points in the code the reviewer needs to double check?
Do we have a test for this?
## Why was this MR needed?
This way gitlab-workhorse does not have to call any API.
See merge request !5094
|
| | | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
Closes #19224, Closes #19128
Also requires this MR to work:
https://gitlab.com/gitlab-org/gitlab-workhorse/merge_requests/53
|
| |\ \ \ \ \
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | | |
Remove slow migration and add fix to sanitize errors
Getting rid of the slow migration and preventing the Error to occur
Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/19608
See merge request !5176
|
| | | |/ / /
| |/| | | |
|
