Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PATCH branch-protection updates check list even when checks are disabled #26351

Merged
merged 3 commits into from
Aug 24, 2023

Conversation

Infinoid
Copy link
Contributor

@Infinoid Infinoid commented Aug 5, 2023

Fixes: #26333.

Previously, this endpoint only updates the StatusCheckContexts field when EnableStatusCheck==true, which makes it impossible to clear the array otherwise.

This patch uses slice nil-ness to decide whether to update the list of checks. The field is ignored when either the client explicitly passes in a null, or just omits the field from the json (which causes json.Unmarshal to leave the struct field unchanged). I think this is a better measure of intent than whether the EnableStatusCheck flag was set, because it matches the semantics of other field types.

Also adds a test case. I noticed that testAPIEditBranchProtection only checks the branch name and no other fields, so I added some extra GET calls and specific checks to make sure the fields are changing properly.

I added those checks the existing integration test; is that the right place for it?

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 5, 2023
@pull-request-size pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Aug 5, 2023
@lunny lunny added modifies/api This PR adds API routes or modifies them type/enhancement An improvement of existing functionality labels Aug 24, 2023
@lunny lunny added this to the 1.21.0 milestone Aug 24, 2023
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Aug 24, 2023
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Aug 24, 2023
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Aug 24, 2023
@lunny lunny enabled auto-merge (squash) August 24, 2023 05:06
@lunny lunny merged commit 86ee5b4 into go-gitea:main Aug 24, 2023
23 checks passed
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Aug 24, 2023
zjjhot added a commit to zjjhot/gitea that referenced this pull request Aug 25, 2023
* giteaofficial/main:
  Fix review bar misalignment (go-gitea#26711)
  Use "small-loading-icon" insead of "btn-octicon is-loading" (go-gitea#26710)
  Improve Image Diff UI (go-gitea#26696)
  Make issue template field template access correct template data (go-gitea#26698)
  add Upload URL to release API (go-gitea#26663)
  Add merge files files to GetCommitFileStatus (go-gitea#20515)
  PATCH branch-protection updates check list even when checks are disabled (go-gitea#26351)
  Add `member`, `collaborator`, `contributor`, and `first-time contributor` roles and tooltips (go-gitea#26658)
  chore(actions): support cron schedule task (go-gitea#26655)
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Nov 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/api This PR adds API routes or modifies them size/S Denotes a PR that changes 10-29 lines, ignoring generated files. type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PATCH endpoint for branch protections does not clear contexts array
4 participants