Skip to content

Commit

Permalink
Merge pull request #1569 from m11o/feature/fix-recreate-webhook-bug
Browse files Browse the repository at this point in the history
Add session argument in ShopifyApp::WebhooksManager.destroy_webhooks
  • Loading branch information
klenotiw authored Dec 15, 2022

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
2 parents c3e89a9 + f4f5358 commit 82616dd
Showing 3 changed files with 29 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
Unreleased
----------
* Fixes a bug with `ShopifyApp::WebhooksManager.destroy_webhooks` causing not passing session arguments to [unregister](https://github.com/Shopify/shopify-api-ruby/blob/main/lib/shopify_api/webhooks/registry.rb#L99) method [#1569](https://github.com/Shopify/shopify_app/pull/1569)
* Validates shop's offline session token is still valid when using `EnsureInstalled`[#1612](https://github.com/Shopify/shopify_app/pull/1612)

21.3.1 (Dec 12, 2022)
6 changes: 3 additions & 3 deletions lib/shopify_app/managers/webhooks_manager.rb
Original file line number Diff line number Diff line change
@@ -21,7 +21,7 @@ def create_webhooks(session:)
end

def recreate_webhooks!(session:)
destroy_webhooks
destroy_webhooks(session: session)
return unless ShopifyApp.configuration.has_webhooks?

add_registrations
@@ -30,12 +30,12 @@ def recreate_webhooks!(session:)
ShopifyAPI::Webhooks::Registry.register_all(session: session)
end

def destroy_webhooks
def destroy_webhooks(session:)
return unless ShopifyApp.configuration.has_webhooks?

ShopifyApp::Logger.debug("Destroying webhooks")
ShopifyApp.configuration.webhooks.each do |attributes|
ShopifyAPI::Webhooks::Registry.unregister(topic: attributes[:topic])
ShopifyAPI::Webhooks::Registry.unregister(topic: attributes[:topic], session: session)
end
end

27 changes: 25 additions & 2 deletions test/shopify_app/managers/webhooks_manager_test.rb
Original file line number Diff line number Diff line change
@@ -78,8 +78,10 @@ class ShopifyApp::WebhooksManagerTest < ActiveSupport::TestCase
end

test "#recreate_webhooks! destroys all webhooks and recreates" do
session = ShopifyAPI::Auth::Session.new(shop: "shop.myshopify.com")

ShopifyAPI::Webhooks::Registry.expects(:register_all)
ShopifyAPI::Webhooks::Registry.expects(:unregister).with(topic: "orders/updated")
ShopifyAPI::Webhooks::Registry.expects(:unregister).with(topic: "orders/updated", session: session)
ShopifyApp::WebhooksManager.expects(:add_registrations).twice

ShopifyApp.configure do |config|
@@ -88,7 +90,7 @@ class ShopifyApp::WebhooksManagerTest < ActiveSupport::TestCase
]
end
ShopifyApp::WebhooksManager.add_registrations
ShopifyApp::WebhooksManager.recreate_webhooks!(session: ShopifyAPI::Auth::Session.new(shop: "shop.myshopify.com"))
ShopifyApp::WebhooksManager.recreate_webhooks!(session: session)
end

test "#recreate_webhooks! does not call unregister if there is no webhook" do
@@ -102,4 +104,25 @@ class ShopifyApp::WebhooksManagerTest < ActiveSupport::TestCase
ShopifyApp::WebhooksManager.add_registrations
ShopifyApp::WebhooksManager.recreate_webhooks!(session: ShopifyAPI::Auth::Session.new(shop: "shop.myshopify.com"))
end

test "#destroy_webhooks destroy all webhooks" do
session = ShopifyAPI::Auth::Session.new(shop: "shop.myshopify.com")
ShopifyAPI::Webhooks::Registry.expects(:unregister).with(topic: "orders/updated", session: session)

ShopifyApp.configure do |config|
config.webhooks = [
{ topic: "orders/updated", path: "webhooks" },
]
end
ShopifyApp::WebhooksManager.destroy_webhooks(session: session)
end

test "#destroy_webhooks does not call unregister if there is no webhook" do
ShopifyAPI::Webhooks::Registry.expects(:unregister).never

ShopifyApp.configure do |config|
config.webhooks = []
end
ShopifyApp::WebhooksManager.destroy_webhooks(session: ShopifyAPI::Auth::Session.new(shop: "shop.myshopify.com"))
end
end

0 comments on commit 82616dd

Please sign in to comment.