Extend webhook registration to support metafield_namespaces #1186
Conversation
This file contains 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
|
I have signed the CLA! |
mllemango
reviewed
Aug 11, 2023
There was a problem hiding this comment.
Hi there! Just wanted to say we totally see your PR and appreciate the thoughtful contribution. There are some changes that may affect how we deal with webhooks coming soon™️ and I'm also going to go through some of the other issues here first. All to say, we're going to have to come back to this one, thanks for you patience!
|
Hi @mllemango thank you for the update, even if it's no progress for now it's good to have that communicated. |
fourseven
force-pushed
the
metafield-namespaces-webhooks
branch
2 times, most recently
from
eb16086 to
a702464
Compare
|
Hi @mllemango how is the coming soon™️ coming? I've recently rebased this change (we still need this functionality) and I see #1189 was merged - are we likely to see any further updates soon? |
mllemango
approved these changes
Oct 6, 2023
I've added the ability to specify metafield_namespaces in webhook registration to allow these to be received from apps using this API. This is mostly following the patterns already there for fields. Have updated: - docs, adding in mention of the new parameter as well as ensuring it's referenced if using `fields` as well - Updated the base Registration and child classes to have another keyword argument in the initializer - Extended the tests, there's more complexity now as before it was just a boolean "are fields present" - but happy to adjust as needed. - Changelog for this PR shopify_app will also need adjusting to pass through the `metafield_namespaces` parameter but I can create that PR if there's appetite from this one.
fourseven
force-pushed
the
metafield-namespaces-webhooks
branch
from
a702464 to
0761230
Compare
fourseven
added a commit
to AMPeCommerce/shopify_app
that referenced
this pull request
Nov 8, 2023
This is an extension on Shopify/shopify-api-ruby#1186, allowing users of the shopify_app ruby/rails library to register webhooks and expect them to return with metafields back.
4 tasks
fourseven
added a commit
to AMPeCommerce/shopify_app
that referenced
this pull request
Nov 21, 2023
This is an extension on Shopify/shopify-api-ruby#1186, allowing users of the shopify_app ruby/rails library to register webhooks and expect them to return with metafields back.
garethson
pushed a commit
to garethson/shopify-api-ruby
that referenced
this pull request
Dec 1, 2023
…1186) I've added the ability to specify metafield_namespaces in webhook registration to allow these to be received from apps using this API. This is mostly following the patterns already there for fields. Have updated: - docs, adding in mention of the new parameter as well as ensuring it's referenced if using `fields` as well - Updated the base Registration and child classes to have another keyword argument in the initializer - Extended the tests, there's more complexity now as before it was just a boolean "are fields present" - but happy to adjust as needed. - Changelog for this PR shopify_app will also need adjusting to pass through the `metafield_namespaces` parameter but I can create that PR if there's appetite from this one.
fourseven
added a commit
to AMPeCommerce/shopify_app
that referenced
this pull request
Dec 14, 2023
This is an extension on Shopify/shopify-api-ruby#1186, allowing users of the shopify_app ruby/rails library to register webhooks and expect them to return with metafields back.
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.
I've added the ability to specify metafield_namespaces in webhook registration to allow these to be received from apps using this API. This is mostly following the patterns already there for fields.
The tests have gotten more complex, as before it was just fields or !fields, but I'm open to feedback (there; and anywhere in general).
This will also need minor updates in the shopify_app codebase to pass through the parameter.
Description
This change allows
ShopifyApp.configuration.webhooksto set ametafield_namespacesparameter and pass that through to webhook registration.This allows anyone using the
shopify_appgem (once that's also improved) to have webhooks with metafields without needing to write custom code. The alternative is to manually register webhooks through graphql and manage it (removing all the benefits of the shopify_app features), or to update a webhook registered this way to add themetafieldNamespacesmanually.How has this been tested?
I have added automated tests around the areas I've changed, and included this branch in a shopify app I'm building which needs these features, and tested it through that as well for HTTP webhook registration. In more detail I:
ShopifyApp.configuration.webhookswith the new param,Checklist: