Prevent modification of MR-level approval rules after MR is merged
What does this MR do and why?
We need to prevent editing and deleting of merge request rules after a merge request is merged.
To Test:
- Find a merged MR with approval rules (
MergeRequest.merged.last.approval_rules
) - Try to delete/edit/create a rule through the UI
- Try to delete/edit/create a rule through the API
- Try to delete/edit/create a rule through graphql ??
curl -X PUT -d 'approvals_required=1' -H "PRIVATE-TOKEN: TOKEN" "http://localhost:3000/api/v4/projects/PROJECT_ID/merge_requests/MR_IID/approval_rules/APPROVAL_RULE_ID"
curl -X DELETE -H "PRIVATE-TOKEN: TOKEN" "http://localhost:3000/api/v4/projects/PROJECT_ID/merge_requests/MR_IID/approval_rules/APPROVAL_RULE_ID"
curl -X POST -d 'name=a' -d 'approvals_required=1' -H "PRIVATE-TOKEN: TOKEN" "http://localhost:3000/api/v4/projects/PROJECT_ID/merge_requests/MR_IID/approval_rules"
Query Postgres.ai
https://console.postgres.ai/gitlab/gitlab-production-tunnel-pg12/sessions/24524/commands/78234
Related to #432961 (closed)
Merge request reports
Activity
assigned to @marc_shaw
- A deleted user
added backend label
- Resolved by Rutger Wessels
2 Messages CHANGELOG missing: If this merge request needs a changelog entry, add the
Changelog
trailer to the commit message you want to add to the changelog.If this merge request doesn't need a CHANGELOG entry, feel free to ignore this message.
This merge request adds or changes files that require a review from the Database team. This merge request requires a database review. To make sure these changes are reviewed, take the following steps:
- Ensure the merge request has database and databasereview pending labels. If the merge request modifies database files, Danger will do this for you.
- Prepare your MR for database review according to the docs.
- Assign and mention the database reviewer suggested by Reviewer Roulette.
If you no longer require a database review, you can remove this suggestion by removing the database label and re-running the
danger-review
job.Reviewer roulette
Changes that require review have been detected!
Please refer to the table below for assigning reviewers and maintainers suggested by Danger in the specified category:
Category Reviewer Maintainer analytics instrumentation No engineer is available for automated assignment, please reach out to the #g_analyze_analytics_instrumentation
Slack channel or mention@gitlab-org/analytics-section/analytics-instrumentation/engineers
for assistance.Maintainer review is optional for analytics instrumentation backend @akotte
(UTC+5.5, 4.5 hours ahead of author)
@jessieay
(UTC+9, 8 hours ahead of author)
Please check reviewer's status!
Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.
To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.
Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.
If needed, you can retry the
danger-review
job that generated this comment.Generated by
DangerEdited by Ghost User- Resolved by Rutger Wessels
- Resolved by Rutger Wessels
E2E Test Result Summary
allure-report-publisher
generated test report!e2e-test-on-gdk:
test report for 991dbb1dexpand test summary
+------------------------------------------------------------------+ | suites summary | +-------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +-------------+--------+--------+---------+-------+-------+--------+ | Plan | 55 | 0 | 0 | 0 | 55 | ✅ | | Verify | 31 | 0 | 0 | 0 | 31 | ✅ | | Data Stores | 23 | 0 | 0 | 0 | 23 | ✅ | | Govern | 72 | 0 | 0 | 0 | 72 | ✅ | | Release | 5 | 0 | 0 | 0 | 5 | ✅ | | Create | 54 | 0 | 7 | 0 | 61 | ✅ | | Manage | 0 | 0 | 1 | 0 | 1 | ➖ | | Package | 15 | 0 | 1 | 0 | 16 | ✅ | | Analytics | 2 | 0 | 0 | 0 | 2 | ✅ | | Monitor | 7 | 0 | 0 | 0 | 7 | ✅ | +-------------+--------+--------+---------+-------+-------+--------+ | Total | 264 | 0 | 9 | 0 | 273 | ✅ | +-------------+--------+--------+---------+-------+-------+--------+
e2e-package-and-test:
test report for 991dbb1dexpand test summary
+------------------------------------------------------------------+ | suites summary | +-------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +-------------+--------+--------+---------+-------+-------+--------+ | Create | 550 | 0 | 52 | 8 | 602 | ✅ | | Govern | 6 | 0 | 0 | 0 | 6 | ✅ | | Data Stores | 4 | 0 | 0 | 0 | 4 | ✅ | | Plan | 8 | 0 | 0 | 0 | 8 | ✅ | | Package | 0 | 0 | 2 | 0 | 2 | ➖ | | Monitor | 8 | 0 | 0 | 0 | 8 | ✅ | +-------------+--------+--------+---------+-------+-------+--------+ | Total | 576 | 0 | 54 | 8 | 630 | ✅ | +-------------+--------+--------+---------+-------+-------+--------+
Edited by Ghost User
mentioned in merge request !133665 (merged)
mentioned in issue #432961 (closed)
mentioned in merge request !138466 (merged)
added 654 commits
-
c8c0426b...a4d4503e - 653 commits from branch
master
- 1255aa6f - wip
-
c8c0426b...a4d4503e - 653 commits from branch
added 1 commit
- 1308151e - Prevent moditfication of MR approval rules after merge
- Resolved by Marc Shaw
- Resolved by Marc Shaw
added 1 commit
- cc99095d - Prevent moditfication of MR approval rules after merge