feat(api): allow updating protected branches

[Sjord]

Closes #2390

Changes

p_b.allow_force_push = True
p_b.save()

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
• Unit tests and/or functional tests

[Sjord]

I don't understand why the integration test fails. It works on my local instance, and the code seems to make sense:

p_b.allow_force_push = True
p_b.save()

p_b = project.protectedbranches.get("*-stable")
assert p_b.allow_force_push

[JohnVillalovos]

I don't understand why the integration test fails. It works on my local instance, and the code seems to make sense:

p_b.allow_force_push = True
p_b.save()

p_b = project.protectedbranches.get("*-stable")
assert p_b.allow_force_push

Just a guess, but it may need to use the wait_for_sidekiq() function. As the GitLab server may still be processing the save() asynchronously while doing the get().

functional/api/test_merge_requests.py has usage of wait_for_sidekiq()

[nejch]

Sadly our functional tests still run against 15.4 (yes, I know!) because we had issues with flaky and failing tests when upgrading. This was introduced in 15.6 so this is sort of expected. We need to take the time to fix that soon I guess.

I would suggest adding a skip or xfail (I forget if we still have a decorator for gitlab versions - that would be ideal. At least we have a gitlab_version fixture so you could conditionally test those few lines as well.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (48726fd) 96.48% compared to head (cef02d2) 96.52%.
Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2771      +/-   ##
==========================================
+ Coverage   96.48%   96.52%   +0.03%     
==========================================
  Files          90       90              
  Lines        5866     5873       +7     
==========================================
+ Hits         5660     5669       +9     
+ Misses        206      204       -2     

Flag	Coverage Δ
api_func_v4	82.24% <100.00%> (-0.09%)	⬇️
cli_func_v4	83.63% <100.00%> (+0.05%)	⬆️
unit	88.30% <100.00%> (+0.04%)	⬆️

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

Files	Coverage Δ
gitlab/v4/objects/branches.py	100.00% <100.00%> (ø)

... and 13 files with indirect coverage changes

[nejch]

Thanks @Sjord and sorry for the delays! I'll go ahead and merge this now, but I'll squash the commits if that's ok with you as the other commits aren't user-facing fixes. Thanks again!

[nickbroon]

Is this expected to also work with protectedbranch.allowed_to_push or protectedbranch.push_access_levels ?

The PATCH api requires to send allowed_to_push attribute, but the returned attribute is push_access_levels, as documented in the API and examples: https://docs.gitlab.com/ee/api/protected_branches.html#example-update-a-push_access_level-record

So using the python-gitlab library to modify the push_access_levels attribute that the object has and then calling .save() appears to do nothing, and the python object has no allowed_to_push attribute to modify.

[Sjord]

I made this to support allow_force_push, and if anything else worked that would be a bonus. I expect allowed_to_push to work.

It is indeed a pain that the API expects different information than it returns. I have been bitten by this before, and I think I saw an issue about this in some other context, but I can't find it right now. Unfortunately, there is not really a clear solution for this.

Perhaps it's better to create a new issue for your problem. This pull request is already closed so comments here will get out of sight.