fix(api): fix saving merge request approval rules

[Sjord]

Closes #2548

This seems to work when testing against a GitLab Ultimate test project. I assume that the integration tests run against a Gitlab CE instance, so adding a test for this is not feasible. I am not very sure of the changes I made, but it does seem to work correctly.

Changes

Remove custom code and correctly set identifiers for merge request approvals.

Documentation and testing

Please consider whether this PR needs documentation and tests. This is not required, but highly appreciated:

• [ ] Documentation in the matching docs section
• [x] Unit tests and/or functional tests

[JohnVillalovos]

This seems to work when testing against a GitLab Ultimate test project. I assume that the integration tests run against a Gitlab CE instance, so adding a test for this is not feasible.

FYI: The functional tests run against a GitLab EE instance. So tests should be feasible.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 96.48%. Comparing base (7ec3189) to head (0866a12). Report is 44 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2773      +/-   ##
==========================================
- Coverage   96.52%   96.48%   -0.05%     
==========================================
  Files          90       90              
  Lines        5872     5862      -10     
==========================================
- Hits         5668     5656      -12     
- Misses        204      206       +2     

Flag	Coverage Δ
api_func_v4	82.39% <100.00%> (+0.15%)	:arrow_up:
cli_func_v4	83.65% <100.00%> (+0.07%)	:arrow_up:
unit	88.24% <100.00%> (-0.06%)	:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
gitlab/v4/objects/merge_request_approvals.py	98.48% <100.00%> (-0.27%)	:arrow_down:

... and 1 file with indirect coverage changes

[Sjord]

I at least changed the unit test to have different ids for different entities, so that a mixup in ids would become noticable.

[david-vana-conrad]

What's the plan with this fix? How can I help to finish it?

[Sjord]

I consider this ready to be merged.

@JohnVillalovos suggested adding functional tests, but I updated the unit tests instead to test this functionality. Is that sufficient?

@david-vana-conrad, perhaps you can test this and confirm that it works correctly, if you have access to a Gitlab Ultimate instance.

[david-vana-conrad]

@Sjord Your fix works correctly on our GitLab Ultimate instance. Script has changed the Merge Request approval settings as requested.

[jarodwilson]

I've verified that this patch gets our .save() calls to gitlab.com hosted projects working again as well, would like to see it merged so we don't have to run a patched build to support our needs. :)