From 7178df49ad8b3297c30613d6ba78404dc32864ff Mon Sep 17 00:00:00 2001 From: Nelson Wittwer Date: Mon, 24 Apr 2023 06:52:22 -0400 Subject: [PATCH 1/4] add session to block --- .../shopify_app/add_webhook/templates/webhook_job.rb.tt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/generators/shopify_app/add_webhook/templates/webhook_job.rb.tt b/lib/generators/shopify_app/add_webhook/templates/webhook_job.rb.tt index 2c7021355..ec32ea4db 100644 --- a/lib/generators/shopify_app/add_webhook/templates/webhook_job.rb.tt +++ b/lib/generators/shopify_app/add_webhook/templates/webhook_job.rb.tt @@ -12,11 +12,11 @@ class <%= @job_class_name %> < ActiveJob::Base if shop.nil? logger.error("#{self.class} failed: cannot find shop with domain '#{shop_domain}'") - + raise ActiveRecord::RecordNotFound, "Shop Not Found" end - shop.with_shopify_session do + shop.with_shopify_session do |session| end end end From 81097f326f1f49a248d286eaee3c7c00c49e5163 Mon Sep 17 00:00:00 2001 From: Nelson Wittwer Date: Mon, 24 Apr 2023 10:11:09 -0400 Subject: [PATCH 2/4] Use ShopifyApp::WebhooksManager methods to recreate_webhooks --- lib/shopify_app/managers/webhooks_manager.rb | 4 +--- .../managers/webhooks_manager_test.rb | 23 ++----------------- 2 files changed, 3 insertions(+), 24 deletions(-) diff --git a/lib/shopify_app/managers/webhooks_manager.rb b/lib/shopify_app/managers/webhooks_manager.rb index b1ca01203..09acc85fc 100644 --- a/lib/shopify_app/managers/webhooks_manager.rb +++ b/lib/shopify_app/managers/webhooks_manager.rb @@ -24,10 +24,8 @@ def recreate_webhooks!(session:) destroy_webhooks(session: session) return unless ShopifyApp.configuration.has_webhooks? - add_registrations - ShopifyApp::Logger.debug("Recreating webhooks") - ShopifyAPI::Webhooks::Registry.register_all(session: session) + add_registrations end def destroy_webhooks(session:) diff --git a/test/shopify_app/managers/webhooks_manager_test.rb b/test/shopify_app/managers/webhooks_manager_test.rb index 86e6fc1d7..716e588ca 100644 --- a/test/shopify_app/managers/webhooks_manager_test.rb +++ b/test/shopify_app/managers/webhooks_manager_test.rb @@ -80,31 +80,12 @@ class ShopifyApp::WebhooksManagerTest < ActiveSupport::TestCase 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", session: session) - ShopifyApp::WebhooksManager.expects(:add_registrations).twice + ShopifyApp::WebhooksManager.expects(:destroy_webhooks).with(session: session) + ShopifyApp::WebhooksManager.expects(:add_registrations) - ShopifyApp.configure do |config| - config.webhooks = [ - { topic: "orders/updated", path: "webhooks" }, - ] - end - ShopifyApp::WebhooksManager.add_registrations ShopifyApp::WebhooksManager.recreate_webhooks!(session: session) end - test "#recreate_webhooks! does not call unregister if there is no webhook" do - ShopifyAPI::Webhooks::Registry.expects(:register_all).never - ShopifyAPI::Webhooks::Registry.expects(:unregister).never - ShopifyAPI::Webhooks::Registry.expects(:add_registration).never - - ShopifyApp.configure do |config| - config.webhooks = [] - end - 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) From 8d3676f5d2c4ffedeed924cb45029593ddf2b9fb Mon Sep 17 00:00:00 2001 From: Nelson Wittwer Date: Thu, 27 Apr 2023 12:41:11 -0400 Subject: [PATCH 3/4] properly setup test --- test/shopify_app/managers/webhooks_manager_test.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/shopify_app/managers/webhooks_manager_test.rb b/test/shopify_app/managers/webhooks_manager_test.rb index 716e588ca..2e47f76ee 100644 --- a/test/shopify_app/managers/webhooks_manager_test.rb +++ b/test/shopify_app/managers/webhooks_manager_test.rb @@ -78,9 +78,10 @@ class ShopifyApp::WebhooksManagerTest < ActiveSupport::TestCase end test "#recreate_webhooks! destroys all webhooks and recreates" do + ShopifyApp.configuration.expects(:has_webhooks?).returns(true) session = ShopifyAPI::Auth::Session.new(shop: "shop.myshopify.com") - ShopifyApp::WebhooksManager.expects(:destroy_webhooks).with(session: session) + ShopifyApp::WebhooksManager.expects(:destroy_webhooks) ShopifyApp::WebhooksManager.expects(:add_registrations) ShopifyApp::WebhooksManager.recreate_webhooks!(session: session) From f069a6a830d34588687cc3113fb915b390a12de2 Mon Sep 17 00:00:00 2001 From: Nelson Wittwer Date: Thu, 27 Apr 2023 14:05:07 -0400 Subject: [PATCH 4/4] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 88fd4f74e..eac45c405 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ Unreleased * Set `access_scopes` column to string by default [#1636](https://github.com/Shopify/shopify_app/pull/1636) * Fixes a bug with `EnsureBilling` causing infinite redirect in embedded apps [#1578](https://github.com/Shopify/shopify_app/pull/1578) * Modifies SessionStorage#with_shopify_session to call a block with a supplied session instance [#1488](https://github.com/Shopify/shopify_app/pull/1488) +* Refactors `ShopifyApp::WebhhooksManager#recreate_webhooks!` to have a uniform webhook inventory that doesn't clash with the API library. Updates webhook generator to use supplied session. [#1686](https://github.com/Shopify/shopify_app/pull/1686) 21.4.1 (Feb 21, 2023) ----------