octokit / webhooks

machine-readable, always up-to-date GitHub Webhooks specifications
MIT License
222 stars 41 forks source link

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

Open w568w opened 6 months ago

w568w commented 6 months ago

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/webhooks-schemas@7.4.0 with octokit-rs.

Relevant log output

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

Code of Conduct

github-actions[bot] commented 6 months ago

👋 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! 🚀

wolfy1339 commented 5 months ago

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

It's usually always present

w568w commented 5 months ago

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?

wolfy1339 commented 4 months ago

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

w568w commented 4 months ago

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.