MR: gitlab.com/gitlab-org/gitlab/-/merge_requests/153400

Adds a flag for merge request reviews that have changes requested
to be overriden.

Changelog: added

#430728

Changelog: fixed
EE: true

Martin Čavoj authored 1 year ago and Martin Čavoj committed 1 year ago

Changelog: other
EE: true

Changelog: other
EE: true

MR: gitlab.com/gitlab-org/gitlab/-/merge_requests/138154
Issue: gitlab.com/gitlab-org/gitlab/-/issues/432961

Martin Čavoj authored 1 year ago and Andy Schoenen committed 1 year ago

With this change, a policy bot comment will be created for policy rules
that could not be enforced due to missing scanners in the pipeline,
or missing pipeline setup altogether. A new worker will be executed
only if there are approval rules with scan result policies in place.

Martin Čavoj authored 1 year ago and Andy Schoenen committed 1 year ago

With this change, approval rules for any_merge_request policies
are correctly applied based on the merge request's target branch
and no violation is created for branches not covered by policy.

Nothing else calls `MergeRequests::BaseService#reset_approvals`
aside from `MergeRequests::ResetApprovalsService` so we move the
method to that service along with other methods it needs that are
also not called elsewhere.

This also includes changes to how we call `#delete_approvals` in
`MergeRequests::UpdateService`. Outside EE, we don't really reset
approvals when target branch changes but we are currently overriding
`#create_branch_change_note` to add that behavior in EE which is
not really the responsibility of that said method.

To improve this, we introduce a new method called
`#delete_approvals_on_target_branch_change` and we call it in
`#handle_target_branch_change`. It doesn't do anything outside EE
and we override it to delete approvals when target branch changes
in EE.

Patrick Bajao authored 1 year ago and GitLab Release Tools Bot committed 1 year ago

Merge branch 'security-reset-approvals-target-branch-change' into 'master'

See merge request gitlab-org/security/gitlab!3577

Changelog: security

Alan (Maciej) Paruszewski authored 1 year ago and drew stachon committed 1 year ago

Changelog: removed

Move handle_label_changes call to
app/services/issuable_base_service.rb

Add specs for work items update service

We're passing in a merge request object when the underlying worker
expects an merge request id.

Changelog: fixed

When we're only updating a single attribute of a merge request, it can
be MUCH faster to use a specialized update service for that specific
attribute (or relationship) than the "update the world" approach of
MergeRequests::UpdateService#execute does by default.

This is to reduce the number of queries that get executed by
running `MergeRequests::UpdateService`.

This is based on the solution made in
!57523.

Added a new worker/service to extend the capability of the worker
added in that merge request. It now accepts a `execute_hooks`
option so it can be re-used by both `UpdateService` and
`UpdateAssigneesService`.

Since the API endpoints have been deprecated
and removed, we can clean up the code that they
used

This ensures that we have an automation
in erroring condition

That will help us to show correct code owners rules when
target branch is updated

A new `reset_approval_rules_to_defaults` param that will be sent
by the frontend when approval rules are set to be reset to project
defaults.

When that param is set to `true`, the backend will delete all
existing any_approver/regular rules of the merge request. It will
also append inapplicable rules so it'll behave like creating rules
on MR creation.

It will be used at code review analytics

Robert May authored 5 years ago and Sean McGivern committed 5 years ago

Ensures that all approvers are set a new todo when the MR
is updated, which will have cleared old approvals.

GitLab has model-level support for the idea of one merge request
"blocking" another from being merged - the second MR can only be merged
once the first one has, itself, been merged. We also show any existing
blocks in the merge request widget.

This commit adds support for adding and removing these blocks when
creating or editing a merge request.

Frontend changes mostly by Sam Bigelow <sbigelow@gitlab.com>

Remove spec for haml rendering which are moved to vue

Remove unused overall_approer_groups

Signed-off-by: Rémy Coutable <remy@rymai.me>

Allow single tasks to be updated quickly

This is to recompute the new overall_approvers
In this method, the following calls would cause a change in the
database which needs resetting the cache:

- super (which updates MR)
- sync_approval_rules (which updates rule record's approvals_required)
- cleanup_approvers (which removes any approver not mentioned in params)

resetting cache must be done after those three calls,
so moving some calls earlier is required.
However this would not affect outcome.

as it is no longer needed.

MR#approvals_before_merge can be set to nil,
and in UX it means use project setting.
Simulate this behavior by updating with project setting.

When project/MR's approvals_before_merge is changed,
update its rule's approvals_required.

Enables frozen string in the following:

* ee/app/policies/**/*.rb
* ee/app/presenters/**/*.rb
* ee/app/serializers/**/*.rb
* ee/app/services/**/*.rb
* ee/app/uploaders/**/*.rb
* ee/app/validators/**/*.rb
* ee/app/workers/**/*.rb

Partially addresses gitlab-org/gitlab-ce/#47424.

Code owner was included as part of approvers.
However this would cause notifications and todos,
which can spam code owners.
Remove notifications/todo for new code owners as approvers.

Its return type is change from `Approver` to `User`,
as code owner is type `User`, and it does not make sense
to wrap them in `Approver`.

When approvers are overriden, code owner is not included,
because the persisted approvers will include code owners already.

Add endpoint to delete approver by user_id,
since overall_approvers now returns User,
and this avoids one query for fetching user's approver association.

Previously if the project had approvals_before_merge setting at 0
the approvals would not be cleared even if the "Pushing to a merge
request clears approvals" project setting was enabled.

This commit removes the approvals_before_merge out of the check of
whether we should or not reset the merge request approvals.