Fix: ShopifyAPI::Webhooks::Registry doesn't update webhooks if metafield_namespaces has changed (#1344)

* remove dead code in test

* make the do registration test helper more granular, update how it's called

* fill out all test scenarios, temp set check to true

* remove unused comments and temporary bypass that made unit tests pass

* the dumb solution

* add no registration done tests for http, pubsub, eventbridge

* make parse type untyped and possibly sort arrays when checking for equality

* invert order of attribute arrays for testing equality test

* changelog

* make type conversions more explicit, add unit test for edgecase of comparing nil and empty array

* remove removing duplicate code

* initial unit test refactoring: put everything in a loop

* rename to registry test queries

* pull out adding and registring webhooks into seperate fn

* allow more addresses in the loop, remove hardcoding 2 other tests

* rename constant

* make test names shorter for rubocop

* bring in http/pubsub/eventbridge registration error into loop

* rename test

* refactor away no registration test helper function because it's only used once

* rename test again

* remove only instance of check error test to abstract less

* consolidate adding new webhook test, reduces redundancy

* helper function to setup tests and expected queries

* totally refactor away do registration test, and use setup queries helper for the other tests

* rename test

Author: andy-liuu

CHANGELOG.md

@@ -3,6 +3,7 @@
 Note: For changes to the API, see https://shopify.dev/changelog?filter=api
 ## Unreleased
 
+- [#1344](https://github.com/Shopify/shopify-api-ruby/pull/1344) Allow ShopifyAPI::Webhooks::Registry to update a webhook when fields or metafield_namespaces are changed.
 - [#1343](https://github.com/Shopify/shopify-api-ruby/pull/1343) Make ShopifyAPI::Context::scope parameter optional. `scope` defaults to empty list `[]`.
 
 ## 14.6.0

lib/shopify_api/webhooks/registration.rb

@@ -48,7 +48,14 @@ def mutation_name(webhook_id); end
       sig { abstract.returns(String) }
       def build_check_query; end
 
-      sig { abstract.params(body: T::Hash[String, T.untyped]).returns(T::Hash[Symbol, String]) }
+      sig do
+        abstract.params(body: T::Hash[String, T.untyped]).returns({
+          webhook_id: T.nilable(String),
+          current_address: T.nilable(String),
+          fields: T::Array[String],
+          metafield_namespaces: T::Array[String],
+        })
+      end
       def parse_check_result(body); end
 
       sig { params(webhook_id: T.nilable(String)).returns(String) }

lib/shopify_api/webhooks/registrations/event_bridge.rb

@@ -30,6 +30,8 @@ def build_check_query
                 edges {
                   node {
                     id
+                    includeFields
+                    metafieldNamespaces
                     endpoint {
                       __typename
                       ... on WebhookEventBridgeEndpoint {
@@ -43,17 +45,29 @@ def build_check_query
           QUERY
         end
 
-        sig { override.params(body: T::Hash[String, T.untyped]).returns(T::Hash[Symbol, String]) }
+        sig do
+          override.params(body: T::Hash[String, T.untyped]).returns({
+            webhook_id: T.nilable(String),
+            current_address: T.nilable(String),
+            fields: T::Array[String],
+            metafield_namespaces: T::Array[String],
+          })
+        end
         def parse_check_result(body)
           edges = body.dig("data", "webhookSubscriptions", "edges") || {}
           webhook_id = nil
+          fields = []
+          metafield_namespaces = []
           current_address = nil
           unless edges.empty?
             node = edges[0]["node"]
             webhook_id = node["id"].to_s
             current_address = node["endpoint"]["arn"].to_s
+            fields = node["includeFields"] || []
+            metafield_namespaces = node["metafieldNamespaces"] || []
           end
-          { webhook_id: webhook_id, current_address: current_address }
+          { webhook_id: webhook_id, current_address: current_address, fields: fields,
+            metafield_namespaces: metafield_namespaces, }
         end
       end
     end

lib/shopify_api/webhooks/registrations/http.rb

@@ -36,6 +36,8 @@ def build_check_query
                 edges {
                   node {
                     id
+                    includeFields
+                    metafieldNamespaces
                     endpoint {
                       __typename
                       ... on WebhookHttpEndpoint {
@@ -49,10 +51,19 @@ def build_check_query
           QUERY
         end
 
-        sig { override.params(body: T::Hash[String, T.untyped]).returns(T::Hash[Symbol, String]) }
+        sig do
+          override.params(body: T::Hash[String, T.untyped]).returns({
+            webhook_id: T.nilable(String),
+            current_address: T.nilable(String),
+            fields: T::Array[String],
+            metafield_namespaces: T::Array[String],
+          })
+        end
         def parse_check_result(body)
           edges = body.dig("data", "webhookSubscriptions", "edges") || {}
           webhook_id = nil
+          fields = []
+          metafield_namespaces = []
           current_address = nil
           unless edges.empty?
             node = edges[0]["node"]
@@ -63,8 +74,11 @@ def parse_check_result(body)
               else
                 node["callbackUrl"].to_s
               end
+            fields = node["includeFields"] || []
+            metafield_namespaces = node["metafieldNamespaces"] || []
           end
-          { webhook_id: webhook_id, current_address: current_address }
+          { webhook_id: webhook_id, current_address: current_address, fields: fields,
+            metafield_namespaces: metafield_namespaces, }
         end
       end
     end

lib/shopify_api/webhooks/registrations/pub_sub.rb

@@ -34,6 +34,8 @@ def build_check_query
                 edges {
                   node {
                     id
+                    includeFields
+                    metafieldNamespaces
                     endpoint {
                       __typename
                       ... on WebhookPubSubEndpoint {
@@ -48,17 +50,30 @@ def build_check_query
           QUERY
         end
 
-        sig { override.params(body: T::Hash[String, T.untyped]).returns(T::Hash[Symbol, String]) }
+        sig do
+          override.params(body: T::Hash[String, T.untyped]).returns({
+            webhook_id: T.nilable(String),
+            current_address: T.nilable(String),
+            fields: T::Array[String],
+            metafield_namespaces: T::Array[String],
+          })
+        end
         def parse_check_result(body)
           edges = body.dig("data", "webhookSubscriptions", "edges") || {}
           webhook_id = nil
+          fields = []
+          metafield_namespaces = []
           current_address = nil
           unless edges.empty?
             node = edges[0]["node"]
             webhook_id = node["id"].to_s
-            current_address = "pubsub://#{node["endpoint"]["pubSubProject"]}:#{node["endpoint"]["pubSubTopic"]}"
+            current_address =
+              "pubsub://#{node["endpoint"]["pubSubProject"]}:#{node["endpoint"]["pubSubTopic"]}"
+            fields = node["includeFields"] || []
+            metafield_namespaces = node["metafieldNamespaces"] || []
           end
-          { webhook_id: webhook_id, current_address: current_address }
+          { webhook_id: webhook_id, current_address: current_address, fields: fields,
+            metafield_namespaces: metafield_namespaces, }
         end
       end
     end

lib/shopify_api/webhooks/registry.rb

@@ -219,8 +219,14 @@ def webhook_registration_needed?(client, registration)
           check_response = client.query(query: registration.build_check_query, response_as_struct: false)
           raise Errors::WebhookRegistrationError,
             "Failed to check if webhook was already registered" unless check_response.ok?
+
           parsed_check_result = registration.parse_check_result(T.cast(check_response.body, T::Hash[String, T.untyped]))
-          must_register = parsed_check_result[:current_address] != registration.callback_address
+          registration_fields = registration.fields || []
+          registration_metafield_namespaces = registration.metafield_namespaces || []
+
+          must_register = parsed_check_result[:current_address] != registration.callback_address ||
+            parsed_check_result[:fields].sort != registration_fields.sort ||
+            parsed_check_result[:metafield_namespaces].sort != registration_metafield_namespaces.sort
 
           { webhook_id: parsed_check_result[:webhook_id], must_register: must_register }
         end