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

drop.entitlement.grant subscription must have is_batching_enabled param #150

Merged

Conversation

alexschastny
Copy link
Contributor

in case of subscription drop.entitlement.grant we must use is_batching_enabled: true param https://dev.twitch.tv/docs/eventsub/eventsub-subscription-types/#drop-entitlement-grant-notification-example

otherwise will get an exception

{
    "error":"Bad Request",
    "status":400,
    "message":"is_batching_enabled must be true for drop.entitlement.grant subscriptions"
}

@Brandin
Copy link
Collaborator

Brandin commented May 12, 2023

Hi @alexschastny,

Thank you for your PR. It looks like your tests failed as the test for Drop Entitlements needs to be updated to include the change. Please update and push.

Thank you,
Brandin.

@alexschastny alexschastny force-pushed the add-batching-enabled-param-support branch from d367531 to d0f17e8 Compare May 12, 2023 19:01
@alexschastny
Copy link
Contributor Author

Yeah, wrong place in the test for the additional params, fixed!

Copy link
Collaborator

@Brandin Brandin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @alexschastny,

Thank you for the changes. After considering this update, where this parameter is only applicable for 1 out of all of the EventSub Subscriptions at this time, we'll default the param to null and only include it in the body if it is set. You can see some examples of how we do this in some of the other Classes if you need any examples.

Thank you!
Brandin.

spec/TwitchApi/Resources/EventSubApiSpec.php Outdated Show resolved Hide resolved
spec/TwitchApi/Resources/EventSubApiSpec.php Outdated Show resolved Hide resolved
src/Resources/EventSubApi.php Outdated Show resolved Hide resolved
src/Resources/EventSubApi.php Outdated Show resolved Hide resolved
@Brandin Brandin merged commit 54c9ae2 into nicklaw5:master May 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants