Repository navigation
refactor!: Rename InstallationTokenOptions and InstallationTokenListRepoOptions to ...Request types and pass them by value - #4700
Conversation
…stRepoOptions` to `...Request` types and pass them by value
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4700 +/- ##
=======================================
Coverage 98.64% 98.64%
=======================================
Files 203 203
Lines 19027 19027
=======================================
Hits 18769 18769
Misses 258 258 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| # although they are absent from the pinned repository creation request schemas. | ||
| # | ||
| # CreateInstallationTokenListReposRequest.RepositoryIDs has no omit option on purpose: | ||
| # CreateInstallationTokenListRepos exists to send repository_ids even when it is nil or |
There was a problem hiding this comment.
omitemptytag onInstallationAccessTokenOptions.Repositoriesmasking functionality of GitHub APIWhen creating an app installation access token through the REST API (POST /app/installations/{installation_id}/access_tokens), if the repositories parameter is set to an empty array, the API returns a list of all accessible repositories. However, the client currently strips empty arrays (omitempty) when marshaling the InstallationAccessTokenOptions.Repositories value making that functionality inaccessible through the client.
Can we use omitzero since it doesn't strip empty arrays?
There was a problem hiding this comment.
Done in d8e60bd. omitzero still sends [] and now omits nil instead of sending null, so I removed the exception and added a test for both. I had already checked with a throwaway GitHub App that null, [], and no field all return the same 201, and updated the PR description.
Updates #3644.
BREAKING CHANGE:
InstallationTokenOptionsis renamed toCreateInstallationTokenRequestandInstallationTokenListRepoOptionsis renamed toCreateInstallationTokenListReposRequest;AppsService.CreateInstallationTokenandAppsService.CreateInstallationTokenListReposnow take them by value.Both methods use
POST /app/installations/{installation_id}/access_tokens, whose request body schema has only the optionalrepositories,repository_ids, andpermissions, so no fields are added or removed.CreateInstallationTokenListReposRequest.RepositoryIDschanges fromjson:"repository_ids"tojson:"repository_ids,omitzero": an empty array is still sent, which is why the method was added in #3155 (for #3106), and nil is now omitted instead of being sent asnull, which the schema does not allow.With a value parameter, the zero value now sends a body instead of no body:
{}for both methods. ForCreateInstallationTokenListRepos, passing an empty struct used to send{"repository_ids":null}. I checked with a throwaway GitHub App (Metadata read, one selected repository) that no body,{},{"repository_ids":null}, and{"repository_ids":[]}all return the same 201 response, and that each token can access the same repositories (GET /installation/repositories).InstallationTokenOptionsandInstallationTokenListRepoOptionsare removed frombody-allowed-pointer-typesandbody-allowed-wrong-names, and the tests check the request body withtestJSONBodyandtestJSONBodyRaw.The method names already start with the verb of the GitHub docs operation name ("Create an installation access token for an app"), so they are not renamed.
Verified locally:
go build ./...,go vet -tags integration ./test/integration/,examplemodule build,go test ./github/, generated files idempotent,custom-gcl0 issues, andscript/check-schema-fields.shreports nothing for these types. (It still reports the 4EnterpriseUpdateBudgetwarnings that master has since #4693; #4699 fixes them.)