| Age | Commit message (Collapse) | Author |
|---|
|
webhooks: include old revision in MR update events
## What does this MR do?
Includes the old revision for an MR in webhooks.
## Are there points in the code the reviewer needs to double check?
The docs do not give an example `update` hook; should I add one?
## Why was this MR needed?
In order to keep web hook listeners stateless, it helps to include the previous revision of the MR in the hook so that it does not need to remember the last revision (which may also be wrong in case of missed or out-of-order events due to previous errors).
## 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)
- [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 !5535
|
|
Line tooltip up with icon
## What does this MR do?
Lines up tooltip with CI icon
## What are the relevant issue numbers?
Closes #20294
## Screenshots (if relevant)
See merge request !5508
|
|
Fix search results for notes without commits
part of #20063
See merge request !5571
|
|
Pipelines/Builds updates
## What does this MR do?
Adds branch/tag icons in admin view; increase min-width of builds tables
## What are the relevant issue numbers?
#20139
## Screenshots (if relevant)
Branch icons:
Reduce `icon-container` width, before:
Reduce `icon-container` width, after:
See merge request !5527
|
|
|
|
|
|
Fix attr reader to force the intended values for source and target shas
## What does this MR do?
When importing a pull request from GitHub, the old and new branches may no longer actually exist by those names, but we need to recreate the merge
request diff with the right source and target shas. We use these `target_branch_sha` and `source_branch_sha` attributes to force these to the intended values. But the reader methods were always looking up to the target/source branch head instead of check if these values was previously set, this MR applies a memoization pattern to both of them.
## Are there points in the code the reviewer needs to double check?
This [commit](https://gitlab.com/gitlab-org/gitlab-ce/commit/6ce25e7b4caa9e94de74378729178c7060d640b2) introduced this bug in the importer.
## What are the relevant issue numbers?
gitlab-org/gitlab-ce#20385
## 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)
- [ ] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
/cc @akitaonrails @DouweM
See merge request !5573
|
|
Avoid line_code and position calculation on line partial for plain view
See merge request !5591
|
|
Sensible state specific default sort order for issues and merge requests
## What does this MR do?
It provides more sensible default sort order for issues and merge requests based on the following table:
| type | state | default sort order |
|----------------|--------|--------------------|
| issues | open | last created |
| issues | closed | last updated |
| issues | all | last created |
| merge requests | open | last created |
| merge requests | merged | last updated |
| merge requests | closed | last updated |
| merge requests | all | last created |
## Are there points in the code the reviewer needs to double check?
All the bits where `id_desc` was changed to `created_desc`.
I hope it's okay, It makes more sense in my opinion.
## Why was this MR needed?
Prior to this MR the issues and merge request were sorted based on `id_desc` by default.
This MR aims to make the interface more user-friendly by providing state specific sorting defaults most users would expect.
## What are the relevant issue numbers?
See #15064
See merge request !5453
|
|
Fix Importing labels and milestones associations - Import/Export
Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/19510 and https://gitlab.com/gitlab-org/gitlab-ce/issues/19447
- [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 !5426
|
|
|
|
Provide more sensible default sort order for issues and merge requests
based on the following table:
| type | state | default sort order |
|----------------|--------|--------------------|
| issues | open | last created |
| issues | closed | last updated |
| issues | all | last created |
| merge requests | open | last created |
| merge requests | merged | last updated |
| merge requests | closed | last updated |
| merge requests | all | last created |
|
|
Add an URL field to Enviroments
## What does this MR do?
Adds a field to the `enviroments` table to expose later in other features. Now I see the task list below, I noticed I forgot some minor things, but Ill adress those after the first review.
## Are there points in the code the reviewer needs to double check?
The field is a string on the database, thus limited to 255 chars, which seems more than enough.
## What are the relevant issue numbers?
Closes #19527
## Screenshots (if relevant)
## Does this MR meet the acceptance criteria?
- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [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] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
See merge request !5469
|
|
Make "New issue" button in Issue page less obtrusive
## What does this MR do?
Replace green background of "New issue" button on issue page by green border and text.
## Why was this MR needed?
Every issue page has two green buttons—Comment and New Issue.
- New Issue is not directly content related and less likely to be used.
- [UI Guide] says
> Use green or blue button for primary action. Primary button should be only one.
[UI Guide]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/ui_guide.md
## What are the relevant issue numbers?
fixes #20186
## Screenshots
### Before
### After
See merge request !5457
|
|
refactored reader class a bit
|
|
|
|
|
|
When importing a pull request from GitHub, the old and new branches may
no longer actually exist by those names, but we need to recreate the
merge request diff with the right source and target shas.
We use these `target_branch_sha` and `source_branch_sha` attributes to
force these to the intended values. But the reader methods were always
looking up to the target/source branch head instead of check if these
values was previously set.
|
|
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
|
|
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
|
|
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
|
|
Closes #13654
|
|
|
|
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
|
|
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
|
|
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
|
|
|
|
|
|
This MR adds a string (thus max 255 chars) field to the enviroments
table to expose it later in other features.
|
|
|
|
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.
|
|
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.
|
|
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.
|
|
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.
|
|
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.
|
|
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.
|
|
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.
|
|
- Based on feedback from @axil
- http://docs.gitlab.com/ce/development/ui_guide.html#buttons
|
|
1. In the context of protected branches.
2. Test this behaviour.
|
|
1. These versions of PhantomJS don't support `PATCH` requests, so we use
a `POST` with `_method` set to `PATCH`.
|
|
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.
|
|
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.
|
|
1. So it works with the new data model for protected branch access levels.
|
|
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.
|
|
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.
|
|
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.
|
|
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.
|
|
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`
|
|
- And hook up their associations.
|
|
Cache the commit author in RequestStore to avoid extra lookups in PostReceive
See merge request !5537
|
