Skip to content

Commit

Permalink
skip mandatory webhook topics during registration/unregistration (Sho…
Browse files Browse the repository at this point in the history
…pify#1237)

* Do not allow registration to mandatory webhooks

* skip unregister calls for mandatory webhooks

* changelog
  • Loading branch information
nelsonwittwer authored and garethson committed Dec 1, 2023
1 parent 71fb48b commit c4d6b27
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
Note: For changes to the API, see https://shopify.dev/changelog?filter=api

## Unreleased
- [#1237](https://github.com/Shopify/shopify-api-ruby/pull/1237) Skip mandatory webhook topic registration/unregistrations

- [#1205](https://github.com/Shopify/shopify-api-ruby/pull/1205) Fixes invalid typing of ShopifyAPI::DiscountCode#errors.
## 13.2.0
Expand Down
16 changes: 16 additions & 0 deletions lib/shopify_api/webhooks/registry.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ module ShopifyAPI
module Webhooks
class Registry
@registry = T.let({}, T::Hash[String, Registration])
MANDATORY_TOPICS = T.let([
"shop/redact",
"customers/redact",
"customers/data_request",
].freeze, T::Array[String])

class << self
extend T::Sig
Expand All @@ -17,6 +22,8 @@ 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,
Expand Down Expand Up @@ -101,6 +108,8 @@ def register_all(session:)
).returns(T::Hash[String, T.untyped])
end
def unregister(topic:, session:)
return { "response": nil } if mandatory_webhook_topic?(topic)

client = Clients::Graphql::Admin.new(session: session)

webhook_id = get_webhook_id(topic: topic, client: client)
Expand Down Expand Up @@ -213,6 +222,13 @@ def send_register_request(client, registration, webhook_id)
def registration_sucessful?(body, mutation_name)
!body.dig("data", mutation_name, "webhookSubscription").nil?
end

# Mandatory webhooks are subscribed to via the partner dashboard not the API
# https://shopify.dev/docs/apps/webhooks/configuration/mandatory-webhooks
sig { params(topic: String).returns(T::Boolean) }
def mandatory_webhook_topic?(topic)
MANDATORY_TOPICS.include?(topic)
end
end
end
end
Expand Down
29 changes: 29 additions & 0 deletions test/webhooks/registry_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,17 @@ def test_unregister_fail_with_user_errors
assert_equal("Failed to delete webhook from Shopify: some error", exception.message)
end

def test_unregister_to_mandatory_topics_are_skipped
ShopifyAPI::Clients::Graphql::Admin.expects(:new).never

ShopifyAPI::Webhooks::Registry::MANDATORY_TOPICS.each do |topic|
ShopifyAPI::Webhooks::Registry.unregister(
topic: topic,
session: @session,
)
end
end

def test_get_webhook_id_success
stub_request(:post, @url)
.with(body: JSON.dump({ query: queries[:fetch_id_query], variables: nil }))
Expand Down Expand Up @@ -262,6 +273,24 @@ def test_get_webhook_id_with_graphql_errors
assert_equal("Failed to fetch webhook from Shopify: some error", exception.message)
end

def test_registrations_to_mandatory_topics_are_ignored
ShopifyAPI::Webhooks::Registry.clear

ShopifyAPI::Webhooks::Registrations::Http.expects(:new).never

ShopifyAPI::Webhooks::Registry::MANDATORY_TOPICS.each do |topic|
ShopifyAPI::Webhooks::Registry.add_registration(
topic: topic,
delivery_method: :http,
path: "some_path_under_the_rainbow",
handler: TestHelpers::FakeWebhookHandler.new(
lambda do |topic, shop, body|
end,
),
)
end
end

private

def do_registration_test(delivery_method, path, fields: nil, metafield_namespaces: nil)
Expand Down

0 comments on commit c4d6b27

Please sign in to comment.