From eba3010f5fb06561b0816077c3f23c5bb9f8ebdb Mon Sep 17 00:00:00 2001 From: Nelson Wittwer Date: Wed, 29 Nov 2023 13:44:27 -0500 Subject: [PATCH 1/4] refactor where mandatory webhooks are skipped --- lib/shopify_api/webhooks/registry.rb | 11 +++++++++-- test/webhooks/registry_test.rb | 11 +++-------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/lib/shopify_api/webhooks/registry.rb b/lib/shopify_api/webhooks/registry.rb index e52204cc..17353c04 100644 --- a/lib/shopify_api/webhooks/registry.rb +++ b/lib/shopify_api/webhooks/registry.rb @@ -22,8 +22,6 @@ class << self metafield_namespaces: T.nilable(T::Array[String])).void end def add_registration(topic:, delivery_method:, path:, handler: nil, fields: nil, metafield_namespaces: nil) - return if mandatory_webhook_topic?(topic) - @registry[topic] = case delivery_method when :pub_sub Registrations::PubSub.new(topic: topic, path: path, fields: fields, @@ -56,6 +54,8 @@ def clear ).returns(RegisterResult) end def register(topic:, session:) + return mandatory_registration_result(topic) if mandatory_webhook_topic?(topic) + registration = @registry[topic] unless registration @@ -83,6 +83,13 @@ def register(topic:, session:) RegisterResult.new(topic: topic, success: registered, body: register_body) end + sig do + params(topic: String).returns(RegisterResult) + end + def mandatory_registration_result(topic) + RegisterResult.new(topic: topic, success: true, body: nil) + end + sig do params( session: Auth::Session, diff --git a/test/webhooks/registry_test.rb b/test/webhooks/registry_test.rb index 1d0ab478..147ff7f9 100644 --- a/test/webhooks/registry_test.rb +++ b/test/webhooks/registry_test.rb @@ -276,17 +276,12 @@ def test_get_webhook_id_with_graphql_errors def test_registrations_to_mandatory_topics_are_ignored ShopifyAPI::Webhooks::Registry.clear - ShopifyAPI::Webhooks::Registrations::Http.expects(:new).never + ShopifyAPI::Clients::Graphql::Admin.expects(:new).never ShopifyAPI::Webhooks::Registry::MANDATORY_TOPICS.each do |topic| - ShopifyAPI::Webhooks::Registry.add_registration( + ShopifyAPI::Webhooks::Registry.register( topic: topic, - delivery_method: :http, - path: "some_path_under_the_rainbow", - handler: TestHelpers::FakeWebhookHandler.new( - lambda do |topic, shop, body| - end, - ), + session: @session, ) end end From c5fe6e1790efef6d1ec4d59fe60bb59dba89db4d Mon Sep 17 00:00:00 2001 From: Nelson Wittwer Date: Wed, 29 Nov 2023 16:10:08 -0500 Subject: [PATCH 2/4] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 328bd9c0..86ab6587 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ Note: For changes to the API, see https://shopify.dev/changelog?filter=api ## Unreleased - [#1244](https://github.com/Shopify/shopify-api-ruby/pull/1244) Add `expired?` to `ShopifyAPI::Auth::Session` to check if the session is expired (mainly for user sessions) +- [#1249](https://github.com/Shopify/shopify-api-ruby/pull/1249) Fix bug where mandatory webhooks could not be processed ## 13.3.0 From 291ab9130f92fb2f639a312128c6b5532d51e3ab Mon Sep 17 00:00:00 2001 From: Nelson Wittwer Date: Thu, 30 Nov 2023 12:51:15 -0500 Subject: [PATCH 3/4] more explicit mandatory registration result --- lib/shopify_api/webhooks/registry.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/shopify_api/webhooks/registry.rb b/lib/shopify_api/webhooks/registry.rb index 17353c04..53a80a40 100644 --- a/lib/shopify_api/webhooks/registry.rb +++ b/lib/shopify_api/webhooks/registry.rb @@ -87,7 +87,7 @@ def register(topic:, session:) params(topic: String).returns(RegisterResult) end def mandatory_registration_result(topic) - RegisterResult.new(topic: topic, success: true, body: nil) + RegisterResult.new(topic: topic, success: false, body: "Mandatory webhooks are to be registered in the partners dashboard") end sig do From fb4a7675e3905620e06c1a96f22c3aa815206bc6 Mon Sep 17 00:00:00 2001 From: Nelson Wittwer Date: Thu, 30 Nov 2023 13:09:02 -0500 Subject: [PATCH 4/4] lint --- lib/shopify_api/webhooks/registry.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/shopify_api/webhooks/registry.rb b/lib/shopify_api/webhooks/registry.rb index 53a80a40..b357730e 100644 --- a/lib/shopify_api/webhooks/registry.rb +++ b/lib/shopify_api/webhooks/registry.rb @@ -87,7 +87,11 @@ def register(topic:, session:) params(topic: String).returns(RegisterResult) end def mandatory_registration_result(topic) - RegisterResult.new(topic: topic, success: false, body: "Mandatory webhooks are to be registered in the partners dashboard") + RegisterResult.new( + topic: topic, + success: false, + body: "Mandatory webhooks are to be registered in the partners dashboard", + ) end sig do