Shopify / shopify-api-ruby

ShopifyAPI is a lightweight gem for accessing the Shopify admin REST and GraphQL web services.
MIT License
1.06k stars 473 forks source link

skip mandatory webhook topics during registration/unregistration #1237

Closed nelsonwittwer closed 1 year ago

nelsonwittwer commented 1 year ago

Description

Friction with mandatory webhooks was identified in the shopify app gem. Since Mandatory webhooks aren't subscribed to via API, we need to gracefully skip them during registrations without breaking handlers.

This skips registration/unregistration for mandatory topics that are managed in the partner dashboard

How has this been tested?

Unit tests within this library. I've also created a new app using our ruby app template pointing to this branch and my fix to the app gem's branch to verify I could recreate webhook without error.

Checklist:

mironov commented 1 year ago

I think this breaks webhooks processing for mandatory webhooks.

https://github.com/Shopify/shopify-api-ruby/blob/82e4caf30c21a67a68384b5abcc897884dbc5065/lib/shopify_api/webhooks/registry.rb#L179-L183

The @registry[request.topic] will be nil for mandatory webhooks because they were skipped in https://github.com/Shopify/shopify-api-ruby/blob/82e4caf30c21a67a68384b5abcc897884dbc5065/lib/shopify_api/webhooks/registry.rb#L25-L27

kirillplatonov commented 12 months ago

Yep, I can confirm that too. Mandatory webhooks in v13.3.0 are broken. Please rollback this PR.

nelsonwittwer commented 11 months ago

Fixed with https://github.com/Shopify/shopify-api-ruby/pull/1249

kirillplatonov commented 11 months ago

Thanks for the quick fix! 🙏