-
Notifications
You must be signed in to change notification settings - Fork 176
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
extend:webhook-deliveries #364
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi there 👋
Thanks for your contribution!
We will need to keep the functionality were the webhook subscription is either created or updated, and we will need to use the getMutationName so that we call the correct mutation based on the type of subscription we are creating (pubsub, http ect).
We should also update the tests to include these new parameters.
Should be good now, also updated php-docs @lizkenyon |
@admirsaheta You will need to update the tests for the register method now that we have changed the request to create the subscriptions. |
Not really well-versed with tests, would appreciate co-authoring here. :) |
src/Webhooks/DeliveryMethod.php
Outdated
$fieldsQuery = !empty($fields) ? 'fields: [' . implode(',', $fields) . ']' : ''; | ||
$metafieldNamespacesQuery = !empty($metafieldNamespaces) ? 'metafieldNamespaces: [' . implode(',', $metafieldNamespaces) . ']' : ''; | ||
|
||
// Assemble the webhook subscription arguments | ||
$webhookSubscriptionArgs = $this->queryEndpoint($callbackAddress); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to use this method to properly build the callback URL, for each of the webhook cases Pubsub, HTTP, eventbridge.
To run the tests you can run |
…tafield namespaces
"I have signed the CLA!" |
WHY are these changes introduced?
Fixes #362
This pull request addresses the lack of support for specifying fields and metafieldNamespaces in webhook registrations.
The feature aligns with the Ruby implementation by allowing more granular control over which fields and metafields are included
in the webhook payload.
WHAT is this pull request doing?
fields
andmetafieldNamespaces
in thebuildRegisterQuery
method of theRegistry
class.buildRegisterQuery
method to construct GraphQL mutations with these new parameters.Type of change
Checklist