refactor where mandatory webhooks are skipped (#1249)

* refactor where mandatory webhooks are skipped * changelog * more explicit mandatory registration result * lint
Shopify · Nov 30, 2023 · 98a3345 · 98a3345
1 parent 459b04e
commit 98a3345
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 10 deletions.
diff --git 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
 

diff --git 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,17 @@ 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: false,
+            body: "Mandatory webhooks are to be registered in the partners dashboard",
+          )
+        end
+
         sig do
           params(
             session: Auth::Session,

diff --git 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