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

Add T::Array[String] support for webhook fields #1189

Merged
merged 1 commit into from
Aug 10, 2023

Conversation

SheldonNunes
Copy link
Contributor

@SheldonNunes SheldonNunes commented Aug 1, 2023

Description

This PR updates the sorbet signature for the Webhooks::Registry to support T::Array[String].

ShopifyApp.configure do |config|
  config.webhooks = [
    {topic: 'products/update', path: 'webhooks/products_update', fields: ['title', 'vendor']}
  ]
end

The examples currently described in ShopifyApp above do not work as the following error is encountered:

Parameter 'fields': Expected type T.nilable(String), got type Array with value ["title", "vendor"] (TypeError)

This PR updates the signature to match the underlying registration code which accepts both String and T::Array[String]

https://github.com/Shopify/shopify-api-ruby/blob/77ab36ca816516a677b736a46eec59aeed36a055/lib/shopify_api/webhooks/registration.rb#L24C19-L24C61

How has this been tested?

Added additional tests asserting the behaviour remains unchanged when passing in array.

Checklist:

  • My commit message follow the pattern described in here
  • I have performed a self-review of my own code.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have updated the project documentation.
  • I have added a changelog line.

@SheldonNunes SheldonNunes force-pushed the webhook-registry-support-fields-array branch from 52914f0 to 24a5d23 Compare August 1, 2023 22:31
@SheldonNunes SheldonNunes force-pushed the webhook-registry-support-fields-array branch from 24a5d23 to 185438b Compare August 1, 2023 22:32
Copy link
Contributor

@mllemango mllemango left a comment

Choose a reason for hiding this comment

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

LGTM, thank you

@SheldonNunes SheldonNunes merged commit 30833ce into main Aug 10, 2023
@SheldonNunes SheldonNunes deleted the webhook-registry-support-fields-array branch August 10, 2023 21:41
@shopify-shipit shopify-shipit bot temporarily deployed to rubygems October 10, 2023 16:44 Inactive
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