Fix CustomProperties plugin calls renamed in @octokit/plugin-rest-endpoint-methods v17#1032
Open
breymander wants to merge 1 commit into
Conversation
@octokit/plugin-rest-endpoint-methods v17.0.0 (pulled in transitively
by octokit@^5 / probot@^14) renamed the repository custom-properties
methods as a documented breaking change:
- repos.getCustomPropertiesValues
-> repos.customPropertiesForReposGetRepositoryValues
- repos.createOrUpdateCustomPropertiesValues
-> repos.customPropertiesForReposCreateOrUpdateRepositoryValues
The CustomProperties plugin still called the old names. The read side
fell back to paginating 'GET /' (the API root), so existing values
always came back empty and every configured property was treated as a
new addition; the write side then threw
'createOrUpdateCustomPropertiesValues is not a function' on every repo
on every sync. In nop mode the .endpoint() access on the undefined
method threw synchronously and crashed the process.
Rename the three call sites to the v17 method names and update the
unit-test mocks to match.
Fixes github-community-projects#981
Contributor
There was a problem hiding this comment.
Pull request overview
Restores CustomProperties plugin compatibility with @octokit/plugin-rest-endpoint-methods v17 by updating renamed .rest.repos.* method calls, preventing silent read failures and crashes in nop/validation mode.
Changes:
- Updated
CustomProperties.find()to paginate usingrepos.customPropertiesForReposGetRepositoryValues. - Updated
CustomProperties.modifyProperty()(nop + apply paths) to userepos.customPropertiesForReposCreateOrUpdateRepositoryValues. - Updated unit test mocks and expectations to match the new Octokit method names.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| lib/plugins/custom_properties.js | Renames Octokit REST endpoint method usages to the v17 operation IDs for repo custom-properties read/write. |
| test/unit/lib/plugins/custom_properties.test.js | Aligns test mocks and assertions with the renamed Octokit REST endpoint methods. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #981.
@octokit/plugin-rest-endpoint-methodsv17.0.0 (resolved transitively viaoctokit@^5/probot@^14) renamed the repository custom-properties methods as a documented breaking change:repos.getCustomPropertiesValuesrepos.customPropertiesForReposGetRepositoryValuesrepos.createOrUpdateCustomPropertiesValuesrepos.customPropertiesForReposCreateOrUpdateRepositoryValueslib/plugins/custom_properties.jsstill called the old names, which broke the plugin in three ways:find()):paginate(undefined, …)falls back toGET https://api.github.com/(the API root), so existing property values always read as empty and every configured property diffs as a brand-new addition on every repo, every sync.add()fails with…createOrUpdateCustomPropertiesValues is not a function. The error is caught and logged per property, so nothing crashes; custom properties are silently never applied, with one error line per repo per sync cycle..endpoint(params)on the undefined method throws synchronously, which escapesPromise.allinSettings.updateReposand crashes the process, leaving the check-run stuckIN_PROGRESS.This is the same root cause as #971; the #978 fix patched
normalize()but the renamed-method calls remained.Changes
lib/plugins/custom_properties.jsto the v17 method namestest/unit/lib/plugins/custom_properties.test.jsto matchDeliberately kept minimal per the contributing guidelines: no behavior changes beyond restoring the intended API calls. I'm aware #990 rebuilds a broader set of fixes from 2.1.18; this PR just unblocks custom-properties sync in the meantime and should be trivial to carry or drop in that effort.
Verification
npm run test:unit:cipasses (134 passed, 14 skipped, same as base)undefinedonProbotOctokit().rest.reposand the two v17 names above are present;request.endpoint(undefined, {owner, repo, per_page})resolves toGET https://api.github.com/, matching the silent-read failure described in 2.1.21: CustomProperties plugin still crashes — modifyProperty() calls renamed octokit methods #981Note for maintainers
These endpoint operation IDs have now changed twice upstream. If you'd prefer the plugin to be immune to future renames, I'm happy to switch the calls to raw routes instead (
octokit.request('GET /repos/{owner}/{repo}/properties/values')/('PATCH …')). I went with the.restnamespace here to stay consistent with #949.