- 01 Aug, 2016 3 commits
-
-
Stan Hu authored
Improve spinach test to be more specific about link to click If you add another branch to gitlab-test that includes the word 'test', browse_files.feature will fail with an ambiguous match. See merge request !5589
-
Stan Hu authored
If you add another branch to gitlab-test that includes the word 'test', browse_files.feature will fail with an ambiguous match.
-
Stan Hu authored
Ignore invalid IPs in X-Forwarded-For when trusted proxies are configured. ## What does this MR do? Catches IPAddr::InvalidAddressError exceptions in `trusted_proxy?` when a) a trusted proxy is set up in the gitlab config and b) an invalid IP address is passed to the method (e.g. one with a port attached). When caught, returns `false` from the method. Prevents a 500 error in this situation. ## What are the relevant issue numbers? Closes gitlab-org/gitlab-ce#20466. ## Does this MR meet the acceptance criteria? - [X] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added - [N/A] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md) - [N/A] 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 !5584
-
- 31 Jul, 2016 2 commits
-
-
lookatmike authored
-
lookatmike authored
-
- 30 Jul, 2016 4 commits
-
-
Robert Speicher authored
Clarify which project is deleted to avoid alarm Some users were alarmed when navigating after deleting a project. Add the project name to avoid cause for alarm. Closes #13654 See merge request !5574
-
Robert Speicher authored
Properly abort a merge when merge conflicts occur If somehow a user attempted to accept a merge request that had conflicts (e.g. the "Accept Merge Request" button or the MR itself was not updated), `MergeService` did not properly detect that a conflict occurred. It would assume that the MR went through without any issues and close the MR as though everything was fine. This could cause data loss if the source branch were removed. Closes #20425 See merge request !5569
-
Yorick Peterse authored
Improve diff performance by eliminating redundant checks for text blobs See merge request !5575
-
Stan Hu authored
On a merge request with over 1000 changed files, there were redundant calls to blob_text_viewable?, which incurred about 7% of the time. Improves #14775
-
- 29 Jul, 2016 31 commits
-
-
Stan Hu authored
Closes #13654
-
Stan Hu authored
If somehow a user attempted to accept a merge request that had conflicts (e.g. the "Accept Merge Request" button or the MR itself was not updated), `MergeService` did not properly detect that a conflict occurred. It would assume that the MR went through without any issues and close the MR as though everything was fine. This could cause data loss if the source branch were removed. Closes #20425
-
Douwe Maan authored
fix repo hooks missing on import Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/19556 Repo hooks are missing from imported projects - [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) - [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 !5549
-
Robert Speicher authored
Optimize checking if a user can read multiple issues ## What does this MR do? This optimizes various parts of the code so it can more efficiently check if a user can read a list of issues. ## Are there points in the code the reviewer needs to double check? Yes, in particular `Ability.issues_readable_by_user` should be checked to make sure it correctly allows/restricts access to issues. ## Why was this MR needed? Currently the general approach to checking if one can read an issue is to iterate over the issues to check and call `can?(user, :read_issue, issue)` for every issue. This is not efficient as the same work has to be done for every issue. ## What are the relevant issue numbers? * #15607 * #17463 See merge request !5370
-
Robert Speicher authored
Enable Rubocop cops that check access modifiers ## What does this MR do? This MR enables Rubocop cops that detect methods that should be restricted but are the part of public API because of access modifiers used improperly. This also fixes existing offenses. ## Why was this MR needed? Some method in our codebase are public instead of being private because it is sometimes difficult to get it right without static analysis. ## What are the relevant issue numbers? See #17478 Closes #17372 See merge request !5014
-
Rémy Coutable authored
Move CI job config entries from legacy to new config ## What does this MR do? This MR extracts jobs configuration logic from legacy CI config processor to the new code. ## What are the relevant issue numbers? #15060 ## Does this MR meet the acceptance criteria? - 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) See merge request !5087
-
Rémy Coutable authored
Check for Ci::Build artifacts at database level ## What does this MR do? Check for the presence of artifacts not expired at database level ## Are there points in the code the reviewer needs to double check? N/A ## Why was this MR needed? To try to improve a little the performance of the pipelines page ## What are the relevant issue numbers? Closes #20371 ## 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 - ~~[ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)~~ - ~~[ ] 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) - [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 !5543
-
Grzegorz Bizon authored
-
Paco Guzman authored
-
Yorick Peterse authored
Bump gitlab_git to speedup DiffCollection iterations See merge request !5564
-
Paco Guzman authored
-
Yorick Peterse authored
The method Ability.issues_readable_by_user takes a list of users and an optional user and returns an Array of issues readable by said user. This method in turn is used by Banzai::ReferenceParser::IssueParser#nodes_visible_to_user so this method no longer needs to get all the available abilities just to check if a user has the "read_issue" ability. To test this I benchmarked an issue with 222 comments on my development environment. Using these changes the time spent in nodes_visible_to_user was reduced from around 120 ms to around 40 ms.
-
Rémy Coutable authored
Allow creating protected branches that can't be pushed to ## What does this MR do? - Add "No one can push" as a setting to protected branches. - This applies to Masters (as well as all other users) ## What are the relevant issue numbers? Closes #18193 ## Does this need an EE merge request? Yes. gitlab-org/gitlab-ee!569 ## Screenshots ![Screen_Shot_2016-07-29_at_3.14.59_PM](/uploads/2e8774c311763bc6e570501a2e6cabf7/Screen_Shot_2016-07-29_at_3.14.59_PM.png) ## TODO - [ ] #18193 !5081 No one can push to protected branches - [x] Implementation - [x] Model changes - [x] Remove "developers_can_merge" and "developers_can_push" - [x] Replace with `ProtectedBranchPushAccess` and `ProtectedBranchMergeAccess` - [x] Reversible migration - [x] Raise error on failure - [x] MySQL - [x] Backend changes - [x] Creating a protected branch creates access rows - [x] Add `no_one` as an access level - [x] Enforce "no one can push" - [x] Allow setting levels while creating protected branches? - [x] Frontend - [x] Replace checkboxes with `select`s - [x] Add tests - [x] `GitPushService` -> new projects' default branch protection - [x] Fix existing tests - [x] Refactor - [x] Test workflows by hand - [x] from the Web UI - [x] When "Allowed to Push" is "No one" - [x] Developers can't push - [x] Masters can't push - [x] When "Allowed to Push" is "Developers + Masters" - [x] Developers can push - [x] Masters can push - [x] When "Allowed to Push" is "Masters" - [x] Developers can't push - [x] Masters can push - [x] When "Allowed to Merge" is "Masters" and "Allowed to Push" is "Masters" - [x] Developers can't push - [x] Developers can't merge - [x] Masters can merge - [x] Masters can push - [x] When "Allowed to Merge" is "Developers + Masters" and "Allowed to Push" is "Masters" - [x] Developers can't push - [x] Developers can merge - [x] Masters can merge - [x] Masters can push - [x] When "Allowed to Merge" is "Developers + Masters" and "Allowed to Push" is "No one" - [x] Developers can't push - [x] Developers can merge - [x] Masters can merge - [x] Masters can't push - [x] When "Allowed to Merge" is "Masters" and "Allowed to Push" is "No one" - [x] Developers can't push - [x] Developers can't merge - [x] Masters can merge - [x] Masters can't push - [x] from CLI - [x] When "Allowed to Push" is "No one" - [x] Developers can't push - [x] Masters can't push - [x] When "Allowed to Push" is "Developers + Masters" - [x] Developers can push - [x] Masters can push - [x] When "Allowed to Push" is "Masters" - [x] Developers can't push - [x] Masters can push - [x] When "Allowed to Merge" is "Masters" and "Allowed to Push" is "Masters" - [x] Developers can't push - [x] Developers can't merge - [x] Masters can merge - [x] Masters can push - [x] When "Allowed to Merge" is "Developers + Masters" and "Allowed to Push" is "Masters" - [x] Developers can't push - [x] Developers can't merge - [x] Masters can merge - [x] Masters can push - [x] When "Allowed to Merge" is "Developers + Masters" and "Allowed to Push" is "No one" - [x] Developers can't push - [x] Developers can't merge - [x] Masters can't merge - [x] Masters can't push - [x] When "Allowed to Merge" is "Masters" and "Allowed to Push" is "No one" - [x] Developers can't push - [x] Developers can't merge - [x] Masters can't merge - [x] Masters can't push - [x] Add tests for owners and admins - [x] CHANGELOG - [x] Screenshots - [x] Documentation - [x] Wait for ~~!4665~~ to be merged in - [x] Wait for ~~gitlab-org/gitlab-ce#19872~~ and ~~gitlab-org/gitlab-ee!564~~ to be closed - [x] Rebase against master instead of !4892 - [x] Make sure [build](https://gitlab.com/gitlab-org/gitlab-ce/commit/a4ca206fd1cc0332d1e385ddbc0f2e4065c3ae73/builds) is green - [x] Create EE MR - [x] Cherry pick commits - [x] Make sure [build](https://gitlab.com/gitlab-org/gitlab-ee/commit/4e17190d7dc546c1f977edcafd1cbcea4bdb4043/builds) is green - [x] Address @axil's comments - [x] Assign to endboss - [x] Wait for @dbalexandre's review - [x] Address @dbalexandre's comments - [x] Address @axil's comments - [x] Align dropdowns - [x] No flash when protected branch is updated - [x] Resolve conflicts - [x] Implement protect/unprotect API - [x] Address @dbalexandre's comments - [x] Update EE MR - [x] Address @rymai's comments - [x] Create/Update service should return a `ProtectedBranch` - [x] Successfuly protected branch creation shouldn't `load_protected_branches` - [x] Rename `allowed_to_merge` as #minimum_access_level_for_merge - [x] Rename `allowed_to_push` as #minimum_access_level_for_push - [x] Use `inclusion` and `Gitlab::Access` instead of an `enum` - [x] Modify `check_access` to work with `Gitlab::Access` - [x] Pass `@protected_branch` to `#execute` in `UpdateService` - [x] simplify with a nested field `protected_branch[push_access_level][access_level]` - [x] `developers_can_{merge,push}` should be handled in the API - [x] Use `can?(current_user, ...)` instead of `current_user.can?(...)` - [x] Instantiate `ProtectedBranchesAccessSelect` in `dispatcher.js` - [x] constants regarding downtime migrations - [x] Explicit `#down` for columns with default - [x] Update EE MR - [ ] Wait for CE merge - [ ] Wait for EE merge - [ ] Create issue for UI changes proposed by @zyv See merge request !5081
-
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.
-
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.
-
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.
-
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.
-
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.
-
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.
-
Timothy Andrew authored
- Likely introduced during an improper conflict resolution.
-
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.
-
Timothy Andrew authored
- Based on feedback from @axil - http://docs.gitlab.com/ce/development/ui_guide.html#buttons
-
Timothy Andrew authored
-
Timothy Andrew authored
1. In the context of protected branches. 2. Test this behaviour.
-
Timothy Andrew authored
1. These versions of PhantomJS don't support `PATCH` requests, so we use a `POST` with `_method` set to `PATCH`.
-
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.
-
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.
-
Timothy Andrew authored
1. So it works with the new data model for protected branch access levels.
-
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.
-
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.
-
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.
-