Skip to content

Only copy any_approver rule, if one does not already exist

Marc Shaw requested to merge add_additional_check_copying_rules into master

What does this MR do and why?

We are attempting to fix the finalizing of approval rules when merging, by taking into account the rule_type, however there was a problem, as a merge request can only have one any_approver merge request rule, sometimes we had one on the project level, and one on the merge request level, but with different names, this lead to a validation error when trying to save the additional rule. This MR change is that when we are copying over the project rules into merge request rules, we ensure that a any_approver rule does not already exist on the merge request, if so, ignore it as we only care about the rules that were on the merge request at the time.

Reproduction Steps:

  • FF copy_additional_properties_approval_rules on
  • Be on master branch
  • My project has these settings: -Screenshot_2023-09-15_at_15.11.24
  • I create a new Merge Request
  • I send an API request to update the approval rule (I used bundle exec rails c), and MergeRequest.last.approval_rules to get the id
    • curl --request PUT -H "PRIVATE-TOKEN: TOKEN" "http://localhost:3000/api/v4/projects/62/merge_requests/6/approval_rules/604?name=jim&approvals_required=1"
    • This gives me a MergeApprovalRule with a different name to the ProjectApprovalRule
  • I then approved the MR, and merged it
  • In my logs, I get the debug error, and my merged merge request looks like:
    • Screenshot_2023-09-15_at_15.31.12
    • {"severity":"DEBUG","time":"2023-09-15T13:25:48.903Z","correlation_id":"01HACG9PX1W9R2E9GB2GDKRN46","meta.caller_id":"MergeWorker","meta.remote_ip":"127.0.0.1","meta.feature_category":"source_code_management","meta.user":"root","meta.user_id":1,"meta.project":"root/candelete","meta.root_namespace":"root","meta.client_id":"user/1","meta.root_caller_id":"Projects::MergeRequestsController#merge","message":"Failed to persist approval rule: ["Rule type any-approver for the merge request already exists"]. Defaulting to original rules"}
  • Switch to this branch, rerun the process, but this time check no logging and the MR only has one rule
Edited by Allison Browne

Merge request reports