webhooks icon indicating copy to clipboard operation
webhooks copied to clipboard

[BUG]: PullRequest's `$.pull_request.{base,head}.repo` does not have `custom_properties` field

Open w568w opened this issue 1 year ago • 5 comments

What happened?

Currently $.pull_request.{base,head}.repo in Pull Request-related events (e.g. pull_request reopened event) is marked as #/definitions/repository, which has a mandatory custom_properties field.

It is a mistake because in GitHub docs, pull_request events do not have such a field there. And this schema will not pass validation with a strict validator.

It is strange because #900 reported the opposite behavior recently. I suppose there might have been an API regression from GitHub side.

Here is an example payload.

Versions

@octokit/[email protected] with octokit-rs.

Relevant log output

`Error("missing field `custom_properties`", line: 195, column: 17)`

Code of Conduct

  • [X] I agree to follow this project's Code of Conduct

w568w avatar Mar 20 '24 10:03 w568w

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

github-actions[bot] avatar Mar 20 '24 10:03 github-actions[bot]

That's odd that one event has the property for repository and another doesn't.

It's usually always present

wolfy1339 avatar Mar 21 '24 00:03 wolfy1339

Yes, I believe it's GitHub that should be responsible for this issue, but so far I haven't seen any public issue reports, regression tracking, or changelogs mentioning the removal of this field in all PR-related events.

I'm not sure when (and whether) this will eventually get fixed. But for the sake of consistency with the official documentation (and to not break the workflow for all those using a strict validator), I think it's necessary to fix this for now, temporarily, in terms of the schema definition.

Maybe define a new pullrequest_repository type for now?

w568w avatar Mar 21 '24 04:03 w568w

I cross-checked with the OpenAPI spec, and indeed this problem is not present there.

The best thing to do is add a new repository type for PRs

wolfy1339 avatar Apr 22 '24 18:04 wolfy1339

Thanks for taking the time to notify the downstream repository.

I cross-checked with the OpenAPI spec, and indeed this problem is not present there.

Do you mean the OpenAPI spec has no such problem? I checked https://github.com/octokit/openapi-webhooks/releases/download/v8.2.0/api.github.com.json: its #/components/schemas/webhook-pull-request-reopened refers to #/components/schemas/repository-webhooks, which does have custom_properties property.

Thus the problem still exists.

w568w avatar Apr 23 '24 03:04 w568w