summaryrefslogtreecommitdiff
path: root/lib
Commit message (Collapse)AuthorAgeFilesLines
* Implement last round of review comments from !4892.18193-developers-can-mergeTimothy Andrew2016-07-131-2/+4
| | | | | | | | 1. Fix typos, minor styling errors. 2. Use single quotes rather than double quotes in `user_access_spec`. 3. Test formatting.
* Move all "checks" under `GitLab::Checks`.Timothy Andrew2016-07-134-21/+21
| | | | | - https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/4892#note_12892160 - This is more consistent.
* Appease rubocop.Timothy Andrew2016-07-131-1/+0
|
* Refactor `Gitlab::GitAccess`Timothy Andrew2016-07-135-134/+173
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Enforce "developers can merge" during `pre-receive`.Timothy Andrew2016-07-132-0/+25
| | | | | | | | | | | | | | 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.
* Added "developers can merge" setting to protected branchesMathias Vestergaard2016-07-132-3/+15
| | | | | - Cherry-picked from `mvestergaard:branch-protection-dev-merge` - https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/4220
* Merge branch 'multi-line-inline-diff' into 'master' Douwe Maan2016-07-131-14/+38
|\ | | | | | | | | | | | | | | | | | | | | | | | | Render inline diffs for multiple changed lines following eachother Before: ![Screen_Shot_2016-07-11_at_00.08.27](/uploads/b14664211e0f5cef6e77a78eadfcbcdf/Screen_Shot_2016-07-11_at_00.08.27.png) After: ![Screen_Shot_2016-07-11_at_00.07.34](/uploads/567be631869a4867a2edf6ff7eda6369/Screen_Shot_2016-07-11_at_00.07.34.png) See merge request !5174
| * Rename constant to be more descriptiveDouwe Maan2016-07-131-2/+2
| |
| * Extended regexes ignore whitespace, so use \smulti-line-inline-diffDouwe Maan2016-07-121-2/+2
| |
| * Render inline diffs for multiple changed lines following eachotherDouwe Maan2016-07-111-14/+38
| |
* | Merge branch 'jacobvosmaer-gitlab/gitlab-ce-strict-host-headers'Douwe Maan2016-07-122-2/+12
|\ \
| * | Defend against 'Host' header injectionJacob Vosmaer2016-07-122-2/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
* | | Merge branch 'fix-gh-rate-limit' into 'master' Douwe Maan2016-07-121-1/+12
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
| * | | Memoize response from `has_rate_limit?` to avoid extra API callfix-gh-rate-limitDouglas Barbosa Alexandre2016-07-121-1/+3
| | | |
| * | | Checks if rate limit is enabled instead of stubbing responseDouglas Barbosa Alexandre2016-07-121-2/+6
| | | |
| * | | Fix GItHub client requests when rate limit is disabledDouglas Barbosa Alexandre2016-07-121-0/+5
| |/ /
* | | Object renderer read_multi rendered entries from Cacheread-multi-rendered-objectsPaco Guzman2016-07-123-11/+72
|/ /
* | Merge branch 'remove-branch-api' into 'master' Rémy Coutable2016-07-121-0/+2
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
| * | api: expose {should,force}_remove_source_branchBen Boeckel2016-07-111-0/+2
| | | | | | | | | | | | | | | Workflows which use a bot to merge should remove branches if requested. Expose the flag so that bots can request know this.
* | | Optimize system note visibility checking by hiding notes thatStan Hu2016-07-112-18/+29
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* | | Merge branch 'fix/import-event-error' into 'master' Robert Speicher2016-07-111-0/+9
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | Fix problems with events under notes importing GitLab projects Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/19588 See merge request !5154
| * \ \ Merge branch 'master' of gitlab.com:gitlab-org/gitlab-ce into ↵James Lopez2016-07-1196-319/+1321
| |\ \ \ | | |/ / | | | | | | | | fix/import-event-error
| * | | squashed - refactor to cope with sub sub N relations probably using the ↵James Lopez2016-07-111-0/+9
| | | | | | | | | | | | | | | | sub_relations method recursively.
* | | | Merge branch 'single-file-diffs' into 'master' Douwe Maan2016-07-111-0/+4
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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. ![Expand_and_collapse_diffs](/uploads/a4072029085082b85c47006f67ac531c/Expand_and_collapse_diffs.gif) See merge request !4990
| * \ \ \ Merge branch 'master' into single-file-diffsSean McGivern2016-07-118-16/+131
| |\ \ \ \ | | | |/ / | | |/| |
| * | | | Collapse large diffs by defaultSean McGivern2016-07-081-0/+4
| | | | | | | | | | | | | | | | | | | | | | | | | When rendering a list of diff files, skip those where the diff is over 10 KB and provide an endpoint to render individually instead.
* | | | | Merge branch 'remove-duplicate-api-field' into 'master' Rémy Coutable2016-07-111-1/+0
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
| * | | | | entities: remove :description from MergeRequestBen Boeckel2016-07-111-1/+0
| | |/ / / | |/| | | | | | | | | | | | | It is already part of ProjectEntity.
* | | | | Merge branch 'send-entry-via-Gitlab-Workhorse-Send-Data' into 'master' Douwe Maan2016-07-111-0/+12
|\ \ \ \ \ | |/ / / / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
| * | | | Use Gitlab-Workhorse-Send-Data to send entry:Lin Jen-Shin2016-07-051-0/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Closes #19224, Closes #19128 Also requires this MR to work: https://gitlab.com/gitlab-org/gitlab-workhorse/merge_requests/53
* | | | | Merge branch 'fix/remove-import-url-migration' into 'master' Rémy Coutable2016-07-111-0/+2
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
| * | | | | spec and fix for sanitize methodJames Lopez2016-07-111-0/+2
| | |/ / / | |/| | |
* | | | | Merge branch 'blockquote-fence-filter' into 'master' Robert Speicher2016-07-102-1/+73
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add blockquote fence syntax to Markdown Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/16564 Before Markdown rendering happens, this will transform this: ``` Let me quote this here email: >>> Dear friend, How are you? Greetings, Me >>> ``` Into this, saving me from having to prefix all of those lines with `>` manually when I copy some multiline text from another medium: ``` Let me quote this here email: > Dear friend, > > How are you? > > Greetings, > > Me ``` See merge request !3954
| * | | | | Add more comments to regexDouwe Maan2016-07-101-11/+32
| | | | | |
| * | | | | Add blockquote fence syntax to MarkdownDouwe Maan2016-07-092-1/+52
| | |_|_|/ | |/| | |
* | | | | Re-use queries in reference parsersreuse-queries-in-reference-parsersYorick Peterse2016-07-082-4/+37
|/ / / / | | | | | | | | | | | | | | | | This caches various queries to ensure that multiple reference extraction runs re-use any objects queried in previous runs.
* | | | Merge branch 'rubocop/enable-identical-conditional-branches-cop' into 'master' Robert Speicher2016-07-081-10/+6
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Enable Style/IdenticalConditionalBranches Rubocop cop ## What does this MR do? This MR enables Rubocop cop that checks for identical lines at the end of each branch of a conditional statement. Examples: ```ruby @bad if condition do_x do_z else do_y do_z end @good if condition do_x else do_y end do_z ``` ## What are the relevant issue numbers? #17478 See merge request !5011
| * | | | Enable Style/IdenticalConditionalBranches Rubocop copGrzegorz Bizon2016-07-081-10/+6
| |/ / /
* | | | Merge branch '1548-average-commits-per-day' into 'master' Rémy Coutable2016-07-081-1/+1
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fix: Infinity Bug in Commit Statistics ## What does this MR do? It fixes a logic bug in the commits statistics: The code assumed that the amount of days involved in a commit range is equal to the difference between the first and last date. This is not true, though, as (from a human standpoint), a commit yesterday and a commit today involve two days, not one. Similarly, a fresh project with only commits made today already 'used' one day. Since the number of involved days used to be zero for new projects, the result for commits per day quite often amounted to `Infinity`… ## Are there points in the code the reviewer needs to double check? The test file. I hope it is up to the standards of GitLab. ## Why was this MR needed? The bug occurres especially for new users with their first project while exploring GitLab. ## What are the relevant issue numbers? This bug was reported as #1548. ## Screenshots (if relevant) See merge request !4231
| * | | | Infinity Bug in Commit StatisticsJonas Weber2016-05-201-1/+1
| | | | | | | | | | | | | | | | | | | | fixes #1548
* | | | | Merge branch 'api-shared-projects' into 'master' Rémy Coutable2016-07-081-0/+12
|\ \ \ \ \ | |_|/ / / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Api shared projects ## What does this MR do? Exposes the shared projects in the group endpoint ## What are the relevant issue numbers? Builds upon !5148 and closes #18780 ## 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 - [x] All builds are passing - [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides) - [x] 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 !5150
| * | | | API: Expose shared projects in a groupapi-shared-projectsRobert Schilling2016-07-081-0/+1
| | | | |
| * | | | Expose shared groups for projectsapi-shared-groupsRobert Schilling2016-07-081-0/+11
| | | | |
* | | | | Merge branch 'feature/option-set-new-users-external' into 'master' Rémy Coutable2016-07-081-0/+1
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Added setting to set new users by default as external ## What does this MR do? This implements the feature request #14508. It adds an option in the application settings to set new users by default as external. ## Are there points in the code the reviewer needs to double check? Everything. Like I mentioned in the discussion of the issue my knowledge of Ruby basically doesn't exists. I tested it on my machine and it seems to work, but as I am very unexperienced in Ruby I highly recommend to take a close look at the code. ## Why was this MR needed? It was requested by @DouweM to work on the issue with the proposed changes by me. ## What are the relevant issue numbers? This MR is for the issue #14508 that followed up after the implementation of #4009. See merge request !4545
| * | | | | Added setting to set new users by default as externalDravere2016-07-071-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | As requested by the issue #14508 this adds an option in the application settings to set newly registered users by default as external. The default setting is set to false to stay backward compatible.
* | | | | | Merge branch '18627-wildcard-branch-protection' into 'master' Douwe Maan2016-07-071-6/+3
|\ \ \ \ \ \ | |_|/ / / / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Allow specifying protected branches using wildcards Closes #18627 # Tasks - [ ] #18627 !4665 Allow specifying protected branches using wildcards - [x] Find existing usages of protected branches - Protecting branches - `ProtectedBranchesController` is used to mark a branch protected/unprotected - `API::Branches` can be used to mark a branch protected/unprotected - Enforcing branch protection - `Gitlab::GitAccess` has helpers (`can_push_to_branch?`, `check`) that are used to deny pushes if a branch is protected - Over SSH: `gitlab-shell` receives a push, and calls `/allowed` on the GitLab API, which calls `GitAccess.check` - Over HTTP: - `gitlab-workhorse` receives the request, and forwards it to rails - Rails (in the `GitHttpController#git-recieve-pack`) runs basic checks (is the user logged in, not protected branch checks) and returns ok with `GL_ID` and `RepoPath` - `gitlab-workhorse` looks at the response, and calls the relevant `gitlab-shell` action from `git-http/handlePostRPC` - Rest of this flow is the same as the SSH flow above - [x] Implementation - [x] Backend - [x] Change `project#protected_branch?` to look at wildcard protected branches - [x] Change `project#developers_can_push_to_protected_branch?` - [x] Change `project#open_branches` - [x] Better error message when creating a disallowed branch from the Web UI - [x] Frontend - [x] Protected branches page should allow typing out a wildcard pattern - [x] Add help text explaining the use of wildcards - [x] Show matching branches for each protected branch - [x] ~~On the index page~~ - [x] On a show page - [x] Index? - [x] Can't have the "last commit" column for wildcard protected branches - [x] Fix / write tests - [x] What happens if a hook is missing in dev? - [x] Refactor - [x] Test workflows - Create a branch matching a wildcard pattern - Push to a branch matching a wildcard pattern - Force push to a branch matching a wildcard pattern - Delete a branch matching a wildcard pattern - [x] Test using Web UI - [x] Test over SSH - [x] Test over HTTP - [x] Test as developer and master - [x] Investigate performance - [x] Test with a large number of protected branches / branches - [x] Paginate list of protected branches - [x] ~~Possibly rewrite `open_branches`~~ - [x] Add `iid`s to existing `ProtectedBranch`es - [x] Add documentation - [x] Add CHANGELOG entry - [x] Add screenshots - [x] Make sure [build](https://gitlab.com/gitlab-org/gitlab-ce/commit/2f753e3ed2ce681b4444944d521f4419e8ed37f7/builds) passes - [x] Assign to endboss for review - [x] Address @DouweM's comments - [x] `protected_branch_params` - [x] `exact_match` instead of `explicit_match` - [x] When would self.name be blank? - [x] Move `protected_branches.each` to a partial - [x] Move `matching_branches.each` to a partial - [x] If the branch is in @matching_branches, it's not been removed - [x] move this regex to a method and memoize it - [x] `commit_sha` directly for exact matches - [x] Number of matches for wildcard matches, with a link - [x] Wait for [build](https://gitlab.com/gitlab-org/gitlab-ce/commit/43f9ce0e88194b8f719bb1c1e656b7fc13278d56/builds) to pass - [x] Respond to @DouweM's comments - [x] Don't use iid - [x] Controller should use `@project.protected_branches.new` - [x] move the memoization to `def wildcard_regex` - [x] render with `collection: @protected_branches` - [x] Wait for [build](https://gitlab.com/gitlab-org/gitlab-ce/commit/f7beedf122fa0c7aa89e86181fe7499321fb10ca/builds) to pass - [x] Wait for @DouweM's review - [x] Wait for @jschatz1's review - [x] Respond to @jschatz1's comments - [x] Use the new dropdown style - [x] description should be moved to the description section without the styling - [x] Protect button should be disabled when no branch is selected - [x] Update screenshots - [x] Merge conflicts - [x] Make sure [build](https://gitlab.com/gitlab-org/gitlab-ce/commit/20f3cfe8d5540eab64c2ba548043d600b28c61ba/builds) passes - [ ] Revisit performance, possibly with staging/production data - [ ] Get a dump of staging / run against staging live - [ ] Get SSH access to staging - [ ] Wait for review/merge # Screenshots ## Creating wildcard protected branches ![1](/uploads/9446afccfdf6fa381e00c800dd2cc82e/1.png) ![2](/uploads/0b154503b297a818d3577488c575d845/2.png) ![3](/uploads/36217f79df9e41cc1550601f02627fe8/3.png) ![4](/uploads/041ca9bd529bcfa5373fca67e917cbcb/4.png) ### Using the `GLDropdown` component ![2016-06-30_14-16-15](/uploads/508afc2a5e2463c2954641409a560d88/2016-06-30_14-16-15.gif) ## Enforcing wildcard protected branches ### From the Web UI ![Screen_Shot_2016-06-20_at_1.21.18_PM](/uploads/8b5d4b1911e9152698a0488daf1880bc/Screen_Shot_2016-06-20_at_1.21.18_PM.png) ### Over SSH ![SSH](/uploads/7365989d7e4c406ef37b6ae5106442c9/SSH.gif) ### Over HTTPS ![HTTPS](/uploads/a7c0f56ae58efcffc75e6700fa2f4ac0/HTTPS.gif) ## Listing matching branches ![Screen_Shot_2016-06-20_at_1.33.44_PM](/uploads/d054113022f5d7ec64c0e57e501ac104/Screen_Shot_2016-06-20_at_1.33.44_PM.png) See merge request !4665
| * | | | | Improve the error message displayed when branch creation fails.Timothy Andrew2016-07-051-6/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Note: This feature was developed independently on master while this was in review. I've removed the conflicting bits and left the relevant additions, mainly a test for `Gitlab::Git::Hook`. The original commit message follows: 1. `gitlab-shell` outputs errors to `stderr`, but we weren't using this information, prior to this commit. Now we capture the `stderr`, and display it in the flash message when branch creation fails. 2. This can be used to display better errors for other git operation failures with small tweaks. 3. The return value of `Gitlab::Git::Hook#trigger` is changed from a simple `true`/`false` to a tuple of `[status, errors]`. All usages and tests have been updated to reflect this change. 4. This is only relevant to branch creation _from the Web UI_, since SSH and HTTP pushes access `gitlab-shell` either directly or through `gitlab-workhorse`. 5. A few minor changes need to be made on the `gitlab-shell` end. Right now, the `stderr` message it outputs is prefixed by "GitLab: ", which shows up in our flash message. This is better removed.
* | | | | | Merge branch 'new-diff-notes' into 'master' Douwe Maan2016-07-0715-83/+580
|\ \ \ \ \ \ | |_|/ / / / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | New diff notes Fixes #12732, #14731, #19375, #14783 Builds on https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/4110 To do: - [x] Get it mostly working - [x] Validate position validity - [x] Fix: Don’t link to `#` - [x] Fix: Base ref can be `nil`, potentially, when the MR has an oprhan source branch => Yep, doesn’t work. We need to store a `start_id` - [x] Optimize: Fewer duplicate `git diff` compares - [x] Optimize: Pass paths to `PositionTracer#diff` for faster diffs - [x] Refactor: Use `head_id` in `MergeRequest`/`MergeRequestDiff` instead of `source_sha` - [x] Refactor: Convert existing array-based diff refs to the DiffRefs model - [x] Tweak: Use `note_type` in `Autosave` key - [x] Tweak: Remove `line_code: note.line_code` from `link_to_reply_discussion` - [x] Update: `SentNotifications` and reply-by-email receiver - [x] Update: MR diff notification email - [x] Update: API (MR, Commit note creation and entity) - [x] Update: GitHub importer - [x] Address any other TODO comments - [x] Fix: Suppress "edited 4 minutes ago" - [x] Write tests - [x] `LineMapper` - [x] `PositionTracer` - [x] `Position` - [x] `DiffPositionUpdateService` - [x] `DiffNote` - [x] `MergeRequests::RefreshService` / `MergeRequest#update_diff_notes_positions` - [x] Make sure commits with diff notes don't get cleaned up, since this would prevent the diff notes from being rendered (https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5062) Future improvements: - Display unresolved comments on files outside the diff, if the comment was added when that file _was_ part of the diff - Allow commenting on sections between hunks, when expanding the diff using `...` - (We'd need to generate line code based on Position if we have it, even if it falls outside bounds of diff) - `diff_hunk` on diff note API entity - Show diff hunk in notification email - Resolved line notes would have a boolean, and be inactive through `notes.any? { !active? || resolved? }` - Multi line notes would store a number of positions, and do the right thing (™) in grouping and then rendering if the first item is multiline? => true - Image diff notes could store x,y,width,height instead of old_line,new_line for similar grouping. Does it need a reference to say if it's on old or new? These can't have line_codes, clearly. Rendering would be interesting. - Show commit line comments in the MR diff - Comment on specific selected words - Comment on file header - Unfold top of discussion diff note - New diff notes API for commits and MRs /cc @rspeicher See merge request !4101
| * | | | | Remove duplication, unused methods, and some other style thingsDouwe Maan2016-07-062-9/+9
| | | | | |
| * | | | | Add some more code comments.Douwe Maan2016-07-063-1/+16
| | | | | |