- 29 Jul, 2016 27 commits
-
-
Paco Guzman authored
-
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.
-
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.
-
Timothy Andrew authored
-
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`
-
Timothy Andrew authored
- And hook up their associations.
-
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]
-
Yorick Peterse authored
Cache the commit author in RequestStore to avoid extra lookups in PostReceive See merge request !5537
-
Yorick Peterse authored
Merge branch '17073-tagscontroller-index-is-terrible-response-time-goes-up-to-5-seconds' into 'master' Update to gitlab_git 10.4.1 and take advantage of preserved Ref objects See merge request !5536
-
- 28 Jul, 2016 13 commits
-
-
Douwe Maan authored
Reduce number of queries made for merge_requests/:id/diffs ## What does this MR do? It reduces the number of DB queries made while processing and rendering MR notes. ## Are there points in the code the reviewer needs to double check? N/A ## Why was this MR needed? For `https://staging.gitlab.com/gitlab-org/gitlab-ce/merge_requests/3142/diffs.json`, for each note we make number of DB queries, almost all of them are handled by the AR caching layer, but they seem to add up a few seconds. Testing on staging, calling `merge_requests/3142/diffs.json` was reduced to ~5.5 seconds from ~8 seconds. ## 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 - [ ] ~~[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~~ - [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 !5551
-
Alejandro Rodríguez authored
-
Jacob Schatz authored
Replace ci icons with slightly thicker ones ## What does this MR do? Adds updated CI icons ## Why was this MR needed? New CI icons were a tad too thin ## What are the relevant issue numbers? Closes #20089 ## Screenshots (if relevant) ![Screen_Shot_2016-07-26_at_8.59.28_AM](/uploads/35671085829fd64f8a9cf98704661ece/Screen_Shot_2016-07-26_at_8.59.28_AM.png) See merge request !5475
-
Robert Speicher authored
Remove project which can't be pulled while seeding This project can't be pulled from GH when running `SIZE=20 rake dev:setup` [ci skip] See merge request !5559
-
Z.J. van de Weg authored
[ci skip]
-
Stan Hu authored
Add a log message when a project is scheduled for destruction for debugging We have a lot of projects that are in `pending_delete` state. It's not clear whether they were ever scheduled for destruction, or whether Sidekiq just dropped the job due to `MemoryKiller` or some other reason. Also this will provide a record of which user destroys a project. #20365 See merge request !5540
-
Douwe Maan authored
Implement #3243 New Issue by email So we extend Gitlab::Email::Receiver for this new behaviour, however we might want to split it into another class for better testing it. Another issue is that, currently it's using this to parse project identifier: Gitlab::IncomingEmail.key_from_address Which is using: Gitlab.config.incoming_email.address for the receiver name. This is probably `reply` because it's used for replying to a specific issue. We might want to introduce another config for this, or just use `reply` instead of `incoming`. I'll prefer to introduce a new config for this, or just change `reply` to `incoming` because it would make sense for replying to there, too. The email template used in tests were copied and modified from: `emails/valid_reply.eml` which I hope is ok. /cc @DouweM #3243 See merge request !3363
-
Stan Hu authored
-
Alejandro Rodríguez authored
-
Rémy Coutable authored
Merge branch '20308-fix-title-that-is-show-in-the-dropdown-toggle-button-on-projects-branches' into 'master' Fix the title of the toggle dropdown button ## What does this MR do? Fix the dropdown-button toggle displaying the correctly title. ## Are there points in the code the reviewer needs to double check? See if the title is displaying correctly and if the code is acceptable. ## Why was this MR needed? When you choose `Last updated` it should display the title `Last updated` instead of `Recently updated`. This fix makes this correctly displays the title. ## What are the relevant issue numbers? Closes #20308. ## Screenshots (if relevant) ![filter-gitlab-ce](/uploads/52838679d134d57c6ff6120260806fda/filter-gitlab-ce.gif) ## Does this MR meet the acceptance criteria? - [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added - Tests - [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 !5515
-
Rémy Coutable authored
Reduce instrumentation overhead ## What does this MR do? This MR reduces the overhead of instrumented methods. See the commit message of 905f8d76 for more information. ## Are there points in the code the reviewer needs to double check? Not that I can think of. ## Why was this MR needed? The overhead of method call instrumentation was too great. ## 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) - [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 !5550
-
Herminio Torres authored
Before when you choose the way of `sort` instead it display the title correctly it was just apply the humanize helper in sort value. E.g. When you choose `Last updated` it should display the title `Last updated` instead of `Recently updated`. This fix makes this correctly displays the title. Change the implementation of the `link_to` `filter_branches_path` - Change the value of the `params[:sort]` in `link_to`. E.g. instead of using `'recently_updated'` is now using `sort_value_recently_updated`. - Change the values of the case in the `branches_sorted_by` method for the values it receives in the `params[:sort]` that are: `nil`, `'name'`, `'updated_desc'`, `'updated_asc'`.
-
Yorick Peterse authored
This reduces the overhead of the method instrumentation code primarily by reducing the number of method calls. There are also some other small optimisations such as not casting timing values to Floats (there's no particular need for this), using Symbols for method call metric names, and reducing the number of Hash lookups for instrumented methods. The exact impact depends on the code being executed. For example, for a method that's only called once the difference won't be very noticeable. However, for methods that are called many times the difference can be more significant. For example, the loading time of a large commit (nrclark/dummy_project@81ebdea5df2fb42e59257cb3eaad671a5c53ca36) was reduced from around 19 seconds to around 15 seconds using these changes.
-