From 4bc754754604f1c0282bc024180e2824623d61db Mon Sep 17 00:00:00 2001 From: Onat Buyukakkus <55088871+onbuyuka@users.noreply.github.com> Date: Tue, 19 May 2026 19:39:49 +0200 Subject: [PATCH 1/5] [Shopify] Refresh cached plan before order sync Auto-refresh Shop."Advanced Shopify Plan" via the existing Shop.GetShopSettings() helper before both order-import entry points so that a Shopify plan downgrade doesn't leave the connector requesting the staffMember field that the new plan can no longer grant access to (ACCESS_DENIED on read_users scope). Entry points covered: - Bulk sync: report 30104 "Shpfy Sync Orders from Shopify" (also the scheduled auto-sync path). One refresh call per shop per batch run. - Single-order reimport: ShpfyImportOrder .ReimportExistingOrderConfirmIfConflicting. Tests added to ShpfyOrdersAPITest.Codeunit.al: - TestGetShopSettingsClearsStaleAdvancedShopifyPlanFlag - TestSyncOrdersFromShopifyReportRefreshesAdvancedShopifyPlanFlag Fixes AB#635878 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Codeunits/ShpfyImportOrder.Codeunit.al | 5 ++ .../ShpfySyncOrdersfromShopify.Report.al | 2 + .../ShpfyOrdersAPITest.Codeunit.al | 83 +++++++++++++++++++ 3 files changed, 90 insertions(+) diff --git a/src/Apps/W1/Shopify/App/src/Order handling/Codeunits/ShpfyImportOrder.Codeunit.al b/src/Apps/W1/Shopify/App/src/Order handling/Codeunits/ShpfyImportOrder.Codeunit.al index 5d08f434e6..463714292d 100644 --- a/src/Apps/W1/Shopify/App/src/Order handling/Codeunits/ShpfyImportOrder.Codeunit.al +++ b/src/Apps/W1/Shopify/App/src/Order handling/Codeunits/ShpfyImportOrder.Codeunit.al @@ -35,9 +35,14 @@ codeunit 30161 "Shpfy Import Order" internal procedure ReimportExistingOrderConfirmIfConflicting(OrderHeader: Record "Shpfy Order Header") var + ShopToRefresh: Record "Shpfy Shop"; OrderMapping: Codeunit "Shpfy Order Mapping"; begin OrderHeader.Get(OrderHeader."Shopify Order Id"); + if ShopToRefresh.Get(OrderHeader."Shop Code") then begin + ShopToRefresh.GetShopSettings(); + ShopToRefresh.Modify(); + end; ImportOrderAndCreateOrUpdate(OrderHeader."Shop Code", OrderHeader."Shopify Order Id"); OrderHeader.Get(OrderHeader."Shopify Order Id"); if OrderMapping.DoMapping(OrderHeader) then; diff --git a/src/Apps/W1/Shopify/App/src/Order handling/Reports/ShpfySyncOrdersfromShopify.Report.al b/src/Apps/W1/Shopify/App/src/Order handling/Reports/ShpfySyncOrdersfromShopify.Report.al index a467101d79..7ac0a25470 100644 --- a/src/Apps/W1/Shopify/App/src/Order handling/Reports/ShpfySyncOrdersfromShopify.Report.al +++ b/src/Apps/W1/Shopify/App/src/Order handling/Reports/ShpfySyncOrdersfromShopify.Report.al @@ -90,6 +90,8 @@ report 30104 "Shpfy Sync Orders from Shopify" trigger OnAfterGetRecord() begin + Shop.GetShopSettings(); + Shop.Modify(); Clear(OrdersAPI); OrdersAPI.GetOrdersToImport(Shop); end; diff --git a/src/Apps/W1/Shopify/Test/Order Handling/ShpfyOrdersAPITest.Codeunit.al b/src/Apps/W1/Shopify/Test/Order Handling/ShpfyOrdersAPITest.Codeunit.al index ca5f7beca8..7d550167f7 100644 --- a/src/Apps/W1/Shopify/Test/Order Handling/ShpfyOrdersAPITest.Codeunit.al +++ b/src/Apps/W1/Shopify/Test/Order Handling/ShpfyOrdersAPITest.Codeunit.al @@ -32,10 +32,13 @@ codeunit 139608 "Shpfy Orders API Test" Any: Codeunit Any; CompanyLocationId: BigInteger; IsInitialized: Boolean; + PlanRefreshExpected: Boolean; + PlanRefreshCallCount: Integer; OrdersToImportChannelLiableMismatchTxt: Label 'Orders to import Channel Liable Taxes mismatch when %1.', Locked = true; OrderLevelTaxLineExpectedTxt: Label 'An order-level tax line should exist when %1.', Locked = true; ChannelLiableFlagMismatchTxt: Label 'Channel Liable flag mismatch when %1.', Locked = true; OrderHeaderChannelLiableMismatchTxt: Label 'Order header Channel Liable Taxes mismatch when %1.', Locked = true; + DowngradedPlanShopResponseTok: Label '{"data":{"shop":{"name":"Test","plan":{"publicDisplayName":"Basic Shopify","partnerDevelopment":false,"shopifyPlus":false},"weightUnit":"KILOGRAMS"}},"extensions":{"cost":{"requestedQueryCost":1,"actualQueryCost":1,"throttleStatus":{"maximumAvailable":2000.0,"currentlyAvailable":1999,"restoreRate":100.0}}}}', Locked = true; [Test] procedure UnitTestExtractShopifyOrdersToImport() @@ -1536,6 +1539,80 @@ codeunit 139608 "Shpfy Orders API Test" LibraryAssert.IsFalse(OrderLine.Tip, 'Tip flag must not be propagated to the subsequent regular order line'); end; + [Test] + [HandlerFunctions('OrdersAPIHttpHandler')] + procedure TestGetShopSettingsClearsStaleAdvancedShopifyPlanFlag() + var + LocalShop: Record "Shpfy Shop"; + begin + // [SCENARIO] Bug 635878: when the merchant downgrades from a Plus/Advanced plan to a + // standard plan, Shop.GetShopSettings() must clear the cached "Advanced Shopify Plan" + // flag based on the live response from Shopify so the order GraphQL query stops + // requesting the staffMember field (which would otherwise fail with ACCESS_DENIED). + Initialize(); + + // [GIVEN] Shop has a stale "Advanced Shopify Plan" flag set to true + LocalShop.Get(Shop.Code); + LocalShop."Advanced Shopify Plan" := true; + LocalShop.Modify(false); + + // [GIVEN] The HTTP handler is primed to return a downgraded-plan response + PlanRefreshExpected := true; + PlanRefreshCallCount := 0; + + // [WHEN] GetShopSettings is called + LocalShop.GetShopSettings(); + + // [THEN] The Advanced Shopify Plan flag is cleared based on the live response + LibraryAssert.IsFalse(LocalShop."Advanced Shopify Plan", 'Stale Advanced Shopify Plan flag should be refreshed to false after plan downgrade.'); + LibraryAssert.AreEqual(1, PlanRefreshCallCount, 'GetShopSettings should issue exactly one plan-refresh query.'); + + PlanRefreshExpected := false; + end; + + [Test] + [HandlerFunctions('OrdersAPIHttpHandler')] + procedure TestSyncOrdersFromShopifyReportRefreshesAdvancedShopifyPlanFlag() + var + ShopFilter: Record "Shpfy Shop"; + OrdersToImport: Record "Shpfy Orders to Import"; + SyncOrdersFromShopify: Report "Shpfy Sync Orders from Shopify"; + begin + // [SCENARIO] Bug 635878: the bulk "Sync Orders from Shopify" report must refresh + // the cached "Advanced Shopify Plan" flag before importing orders, so a plan + // downgrade does not leave the connector requesting the staffMember field that + // the new plan can no longer grant access to. + Initialize(); + + // [GIVEN] Shop has a stale "Advanced Shopify Plan" flag set to true + Shop.Get(Shop.Code); + Shop."Advanced Shopify Plan" := true; + Shop."Auto Create Orders" := false; + Shop.Modify(false); + Commit(); + + // [GIVEN] No orders are queued so the report only exercises the Shop dataitem trigger + OrdersToImport.SetRange("Shop Code", Shop.Code); + OrdersToImport.DeleteAll(false); + + // [GIVEN] The HTTP handler is primed to return a downgraded-plan response + PlanRefreshExpected := true; + PlanRefreshCallCount := 0; + + // [WHEN] The "Sync Orders from Shopify" report runs against this shop + ShopFilter.SetRange(Code, Shop.Code); + SyncOrdersFromShopify.SetTableView(ShopFilter); + SyncOrdersFromShopify.UseRequestPage(false); + SyncOrdersFromShopify.Run(); + + // [THEN] The Advanced Shopify Plan flag is refreshed on the persisted Shop record + Shop.Get(Shop.Code); + LibraryAssert.IsFalse(Shop."Advanced Shopify Plan", 'Bulk sync report should refresh Advanced Shopify Plan flag before importing orders.'); + LibraryAssert.IsTrue(PlanRefreshCallCount >= 1, 'Bulk sync report should issue a plan-refresh query.'); + + PlanRefreshExpected := false; + end; + local procedure CreateTaxArea(var TaxArea: Record "Tax Area"; var ShopifyTaxArea: Record "Shpfy Tax Area"; ShopParam: Record "Shpfy Shop") var ShopifyCustomerTemplate: Record "Shpfy Customer Template"; @@ -1657,6 +1734,12 @@ codeunit 139608 "Shpfy Orders API Test" if not InitializeTest.VerifyRequestUrl(Request.Path, Shop."Shopify URL") then exit(true); + if PlanRefreshExpected and (PlanRefreshCallCount = 0) then begin + PlanRefreshCallCount += 1; + Response.Content.WriteFrom(DowngradedPlanShopResponseTok); + exit(false); + end; + if CompanyLocationId <> 0 then begin Body := NavApp.GetResourceAsText('Order Handling/CompanyLocationResult.txt', TextEncoding::UTF8); Response.Content.WriteFrom(Body.Replace('{{LocationId}}', Format(CompanyLocationId))); From b3b31c256001ae05b618d2aa504d415a0378ec18 Mon Sep 17 00:00:00 2001 From: Onat Buyukakkus <55088871+onbuyuka@users.noreply.github.com> Date: Tue, 19 May 2026 20:30:11 +0200 Subject: [PATCH 2/5] [Shopify] Reset plan-refresh mock state in Initialize Address PR review feedback: reset PlanRefreshExpected / PlanRefreshCallCount at the start of Initialize() so an assertion failure in one test cannot leak the plan-refresh flag into the next test and corrupt unrelated HTTP calls. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Order Handling/ShpfyOrdersAPITest.Codeunit.al | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/Apps/W1/Shopify/Test/Order Handling/ShpfyOrdersAPITest.Codeunit.al b/src/Apps/W1/Shopify/Test/Order Handling/ShpfyOrdersAPITest.Codeunit.al index 7d550167f7..55a4b2f067 100644 --- a/src/Apps/W1/Shopify/Test/Order Handling/ShpfyOrdersAPITest.Codeunit.al +++ b/src/Apps/W1/Shopify/Test/Order Handling/ShpfyOrdersAPITest.Codeunit.al @@ -1558,7 +1558,6 @@ codeunit 139608 "Shpfy Orders API Test" // [GIVEN] The HTTP handler is primed to return a downgraded-plan response PlanRefreshExpected := true; - PlanRefreshCallCount := 0; // [WHEN] GetShopSettings is called LocalShop.GetShopSettings(); @@ -1566,8 +1565,6 @@ codeunit 139608 "Shpfy Orders API Test" // [THEN] The Advanced Shopify Plan flag is cleared based on the live response LibraryAssert.IsFalse(LocalShop."Advanced Shopify Plan", 'Stale Advanced Shopify Plan flag should be refreshed to false after plan downgrade.'); LibraryAssert.AreEqual(1, PlanRefreshCallCount, 'GetShopSettings should issue exactly one plan-refresh query.'); - - PlanRefreshExpected := false; end; [Test] @@ -1597,7 +1594,6 @@ codeunit 139608 "Shpfy Orders API Test" // [GIVEN] The HTTP handler is primed to return a downgraded-plan response PlanRefreshExpected := true; - PlanRefreshCallCount := 0; // [WHEN] The "Sync Orders from Shopify" report runs against this shop ShopFilter.SetRange(Code, Shop.Code); @@ -1609,8 +1605,6 @@ codeunit 139608 "Shpfy Orders API Test" Shop.Get(Shop.Code); LibraryAssert.IsFalse(Shop."Advanced Shopify Plan", 'Bulk sync report should refresh Advanced Shopify Plan flag before importing orders.'); LibraryAssert.IsTrue(PlanRefreshCallCount >= 1, 'Bulk sync report should issue a plan-refresh query.'); - - PlanRefreshExpected := false; end; local procedure CreateTaxArea(var TaxArea: Record "Tax Area"; var ShopifyTaxArea: Record "Shpfy Tax Area"; ShopParam: Record "Shpfy Shop") @@ -1714,6 +1708,11 @@ codeunit 139608 "Shpfy Orders API Test" CommunicationMgt: Codeunit "Shpfy Communication Mgt."; AccessToken: SecretText; begin + // Reset per-test mock state so a previous test's assertion failure cannot leak + // the plan-refresh flag into the next test and corrupt unrelated HTTP calls. + PlanRefreshExpected := false; + PlanRefreshCallCount := 0; + if IsInitialized then exit; From 8f0d9619ef70c534b5909c02ce59c526227150d9 Mon Sep 17 00:00:00 2001 From: Onat Buyukakkus <55088871+onbuyuka@users.noreply.github.com> Date: Tue, 19 May 2026 21:48:59 +0200 Subject: [PATCH 3/5] Update src/Apps/W1/Shopify/Test/Order Handling/ShpfyOrdersAPITest.Codeunit.al Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../Shopify/Test/Order Handling/ShpfyOrdersAPITest.Codeunit.al | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Apps/W1/Shopify/Test/Order Handling/ShpfyOrdersAPITest.Codeunit.al b/src/Apps/W1/Shopify/Test/Order Handling/ShpfyOrdersAPITest.Codeunit.al index 55a4b2f067..c1ae0cc45b 100644 --- a/src/Apps/W1/Shopify/Test/Order Handling/ShpfyOrdersAPITest.Codeunit.al +++ b/src/Apps/W1/Shopify/Test/Order Handling/ShpfyOrdersAPITest.Codeunit.al @@ -1606,7 +1606,7 @@ codeunit 139608 "Shpfy Orders API Test" LibraryAssert.IsFalse(Shop."Advanced Shopify Plan", 'Bulk sync report should refresh Advanced Shopify Plan flag before importing orders.'); LibraryAssert.IsTrue(PlanRefreshCallCount >= 1, 'Bulk sync report should issue a plan-refresh query.'); end; - +LibraryAssert.AreEqual(1, PlanRefreshCallCount, 'Bulk sync report should issue exactly one plan-refresh query.'); local procedure CreateTaxArea(var TaxArea: Record "Tax Area"; var ShopifyTaxArea: Record "Shpfy Tax Area"; ShopParam: Record "Shpfy Shop") var ShopifyCustomerTemplate: Record "Shpfy Customer Template"; From d31bdda9275e6d7633e698cb72234db7dce7ee2c Mon Sep 17 00:00:00 2001 From: Onat Buyukakkus <55088871+onbuyuka@users.noreply.github.com> Date: Tue, 19 May 2026 22:47:58 +0200 Subject: [PATCH 4/5] [Shopify] Suppress AA0214 around ShopToRefresh.Modify CI lint fails with AA0214 ("record should be modified before saving") because the analyzer cannot see through GetShopSettings() to detect the field assignments it performs on the record. This mirrors the established suppression pattern used elsewhere in the codebase (e.g., ShpfyProductCollectionTest, Subscription Billing tables, EDocument line matching) for the same false-positive scenario. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/Order handling/Codeunits/ShpfyImportOrder.Codeunit.al | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Apps/W1/Shopify/App/src/Order handling/Codeunits/ShpfyImportOrder.Codeunit.al b/src/Apps/W1/Shopify/App/src/Order handling/Codeunits/ShpfyImportOrder.Codeunit.al index 463714292d..91682b1d2a 100644 --- a/src/Apps/W1/Shopify/App/src/Order handling/Codeunits/ShpfyImportOrder.Codeunit.al +++ b/src/Apps/W1/Shopify/App/src/Order handling/Codeunits/ShpfyImportOrder.Codeunit.al @@ -41,7 +41,9 @@ codeunit 30161 "Shpfy Import Order" OrderHeader.Get(OrderHeader."Shopify Order Id"); if ShopToRefresh.Get(OrderHeader."Shop Code") then begin ShopToRefresh.GetShopSettings(); +#pragma warning disable AA0214 ShopToRefresh.Modify(); +#pragma warning restore AA0214 end; ImportOrderAndCreateOrUpdate(OrderHeader."Shop Code", OrderHeader."Shopify Order Id"); OrderHeader.Get(OrderHeader."Shopify Order Id"); From 69fd01fb2f9329512b0382a7b5e19d221efd3590 Mon Sep 17 00:00:00 2001 From: Onat Buyukakkus <55088871+onbuyuka@users.noreply.github.com> Date: Tue, 19 May 2026 22:48:57 +0200 Subject: [PATCH 5/5] [Shopify] Fix orphaned assertion left outside procedure Commit 8f0d9619 (a suggestion accepted via the GitHub UI) placed a LibraryAssert call between two procedure bodies, breaking the file syntax. Moving the intended tightened assertion ("exactly one" plan-refresh query, matching the GetShopSettings test) into its correct location inside the report test, replacing the previous ">= 1" check. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Test/Order Handling/ShpfyOrdersAPITest.Codeunit.al | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Apps/W1/Shopify/Test/Order Handling/ShpfyOrdersAPITest.Codeunit.al b/src/Apps/W1/Shopify/Test/Order Handling/ShpfyOrdersAPITest.Codeunit.al index c1ae0cc45b..0a136f7c91 100644 --- a/src/Apps/W1/Shopify/Test/Order Handling/ShpfyOrdersAPITest.Codeunit.al +++ b/src/Apps/W1/Shopify/Test/Order Handling/ShpfyOrdersAPITest.Codeunit.al @@ -1604,9 +1604,9 @@ codeunit 139608 "Shpfy Orders API Test" // [THEN] The Advanced Shopify Plan flag is refreshed on the persisted Shop record Shop.Get(Shop.Code); LibraryAssert.IsFalse(Shop."Advanced Shopify Plan", 'Bulk sync report should refresh Advanced Shopify Plan flag before importing orders.'); - LibraryAssert.IsTrue(PlanRefreshCallCount >= 1, 'Bulk sync report should issue a plan-refresh query.'); + LibraryAssert.AreEqual(1, PlanRefreshCallCount, 'Bulk sync report should issue exactly one plan-refresh query.'); end; -LibraryAssert.AreEqual(1, PlanRefreshCallCount, 'Bulk sync report should issue exactly one plan-refresh query.'); + local procedure CreateTaxArea(var TaxArea: Record "Tax Area"; var ShopifyTaxArea: Record "Shpfy Tax Area"; ShopParam: Record "Shpfy Shop") var ShopifyCustomerTemplate: Record "Shpfy Customer Template";