From f95e4294c179c19867badf0c9185342bf36125bb Mon Sep 17 00:00:00 2001 From: Usman Shaukat Date: Tue, 2 Aug 2022 18:27:28 +0500 Subject: [PATCH 1/7] Update BillingController.php Added a check in billing controller to redirect to the home route if no charge ID is provided. Fix issue #1174 --- src/Traits/BillingController.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Traits/BillingController.php b/src/Traits/BillingController.php index 1d24742d..35a3a402 100644 --- a/src/Traits/BillingController.php +++ b/src/Traits/BillingController.php @@ -74,7 +74,10 @@ public function process( ): RedirectResponse { // Get the shop $shop = $shopQuery->getByDomain(ShopDomain::fromNative($request->query('shop'))); - + if(!$request->has('charge_id')) + return Redirect::route(Util::getShopifyConfig('route_names.home'), [ + 'shop' => $shop->getDomain()->toNative(), + ]); // Activate the plan and save $result = $activatePlan( $shop->getId(), From 07c23ff5f928c434a93aa8dcffeb02785d464b7b Mon Sep 17 00:00:00 2001 From: Luke Walsh Date: Mon, 5 Sep 2022 11:37:44 +0100 Subject: [PATCH 2/7] Fix linting --- src/Traits/BillingController.php | 38 +++++++++++++++++--------------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/src/Traits/BillingController.php b/src/Traits/BillingController.php index aa582973..0b4d61b6 100644 --- a/src/Traits/BillingController.php +++ b/src/Traits/BillingController.php @@ -27,19 +27,20 @@ trait BillingController /** * Redirects to billing screen for Shopify. * - * @param Request $request The request object. - * @param ShopQuery $shopQuery The shop querier. - * @param GetPlanUrl $getPlanUrl The action for getting the plan URL. - * @param int|null $plan The plan's ID, if provided in route. + * @param Request $request The request object. + * @param ShopQuery $shopQuery The shop querier. + * @param GetPlanUrl $getPlanUrl The action for getting the plan URL. + * @param int|null $plan The plan's ID, if provided in route. * * @return ViewView */ public function index( - Request $request, - ShopQuery $shopQuery, + Request $request, + ShopQuery $shopQuery, GetPlanUrl $getPlanUrl, - ?int $plan = null - ): ViewView { + ?int $plan = null + ): ViewView + { // Get the shop $shop = $shopQuery->getByDomain(ShopDomain::fromNative($request->get('shop'))); @@ -59,22 +60,23 @@ public function index( /** * Processes the response from the customer. * - * @param int $plan The plan's ID. - * @param Request $request The HTTP request object. - * @param ShopQuery $shopQuery The shop querier. + * @param int $plan The plan's ID. + * @param Request $request The HTTP request object. + * @param ShopQuery $shopQuery The shop querier. * @param ActivatePlan $activatePlan The action for activating the plan for a shop. * * @return RedirectResponse */ public function process( - int $plan, - Request $request, - ShopQuery $shopQuery, + int $plan, + Request $request, + ShopQuery $shopQuery, ActivatePlan $activatePlan - ): RedirectResponse { + ): RedirectResponse + { // Get the shop $shop = $shopQuery->getByDomain(ShopDomain::fromNative($request->query('shop'))); - if(!$request->has('charge_id')) + if (!$request->has('charge_id')) return Redirect::route(Util::getShopifyConfig('route_names.home'), [ 'shop' => $shop->getDomain()->toNative(), ]); @@ -82,7 +84,7 @@ public function process( $result = $activatePlan( $shop->getId(), PlanId::fromNative($plan), - ChargeReference::fromNative((int) $request->query('charge_id')) + ChargeReference::fromNative((int)$request->query('charge_id')) ); // Go to homepage of app @@ -97,7 +99,7 @@ public function process( /** * Allows for setting a usage charge. * - * @param StoreUsageCharge $request The verified request. + * @param StoreUsageCharge $request The verified request. * @param ActivateUsageCharge $activateUsageCharge The action for activating a usage charge. * * @return RedirectResponse From 45e500637405559507ae1c4115afd47456fdca9f Mon Sep 17 00:00:00 2001 From: Luke Walsh Date: Mon, 5 Sep 2022 11:40:56 +0100 Subject: [PATCH 3/7] Fix linting - php-cs-fixer --- src/Traits/BillingController.php | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/Traits/BillingController.php b/src/Traits/BillingController.php index 0b4d61b6..5b07e29d 100644 --- a/src/Traits/BillingController.php +++ b/src/Traits/BillingController.php @@ -39,8 +39,7 @@ public function index( ShopQuery $shopQuery, GetPlanUrl $getPlanUrl, ?int $plan = null - ): ViewView - { + ): ViewView { // Get the shop $shop = $shopQuery->getByDomain(ShopDomain::fromNative($request->get('shop'))); @@ -72,19 +71,19 @@ public function process( Request $request, ShopQuery $shopQuery, ActivatePlan $activatePlan - ): RedirectResponse - { + ): RedirectResponse { // Get the shop $shop = $shopQuery->getByDomain(ShopDomain::fromNative($request->query('shop'))); - if (!$request->has('charge_id')) + if (!$request->has('charge_id')) { return Redirect::route(Util::getShopifyConfig('route_names.home'), [ 'shop' => $shop->getDomain()->toNative(), ]); + } // Activate the plan and save $result = $activatePlan( $shop->getId(), PlanId::fromNative($plan), - ChargeReference::fromNative((int)$request->query('charge_id')) + ChargeReference::fromNative((int) $request->query('charge_id')) ); // Go to homepage of app From 322017cc54684ffc479df69f82dad4e9caba2177 Mon Sep 17 00:00:00 2001 From: Luke Walsh Date: Mon, 5 Sep 2022 11:57:26 +0100 Subject: [PATCH 4/7] Test the redirect works --- tests/Traits/BillingControllerTest.php | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/Traits/BillingControllerTest.php b/tests/Traits/BillingControllerTest.php index 3b0b94b4..854b5e96 100644 --- a/tests/Traits/BillingControllerTest.php +++ b/tests/Traits/BillingControllerTest.php @@ -130,4 +130,23 @@ public function testUsageChargeSuccess(): void $response->assertRedirect('http://localhost'); $response->assertSessionHas('success'); } + + public function testReturnToSettingScreenNoPlan(){ + + // Set up a shop + $shop = factory($this->model)->create([ + 'plan_id' => null, + ]); + //Log in + $this->auth->login($shop); + $url = 'https://example.myshopify.com/billing/process/9999?shop='.$shop->name; + // Try to go to bill without a plan id which happens when you cancel the charge + $response = $this->call( + 'get', + $url, + ['shop' => $shop->name] + ); + //Confirm we get sent back to the homepage of shopify + $response->assertRedirect('https://example.myshopify.com?shop='.$shop->name); + } } From 12d93a2588d6a2157c5ec97aeaf68d97b207da80 Mon Sep 17 00:00:00 2001 From: Luke Walsh Date: Mon, 5 Sep 2022 11:59:14 +0100 Subject: [PATCH 5/7] Linting. --- tests/Traits/BillingControllerTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/Traits/BillingControllerTest.php b/tests/Traits/BillingControllerTest.php index 854b5e96..68c1a4d6 100644 --- a/tests/Traits/BillingControllerTest.php +++ b/tests/Traits/BillingControllerTest.php @@ -131,8 +131,8 @@ public function testUsageChargeSuccess(): void $response->assertSessionHas('success'); } - public function testReturnToSettingScreenNoPlan(){ - + public function testReturnToSettingScreenNoPlan() + { // Set up a shop $shop = factory($this->model)->create([ 'plan_id' => null, @@ -144,7 +144,7 @@ public function testReturnToSettingScreenNoPlan(){ $response = $this->call( 'get', $url, - ['shop' => $shop->name] + ['shop' => $shop->name] ); //Confirm we get sent back to the homepage of shopify $response->assertRedirect('https://example.myshopify.com?shop='.$shop->name); From b30035f868ba55ea48a96fda8f99b42bae2aca7c Mon Sep 17 00:00:00 2001 From: Luke Walsh Date: Mon, 5 Sep 2022 12:06:06 +0100 Subject: [PATCH 6/7] Update the url to not be the same in the test as the shopify admin as it is confusing --- tests/Traits/BillingControllerTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/Traits/BillingControllerTest.php b/tests/Traits/BillingControllerTest.php index 68c1a4d6..e9b4a394 100644 --- a/tests/Traits/BillingControllerTest.php +++ b/tests/Traits/BillingControllerTest.php @@ -139,14 +139,14 @@ public function testReturnToSettingScreenNoPlan() ]); //Log in $this->auth->login($shop); - $url = 'https://example.myshopify.com/billing/process/9999?shop='.$shop->name; + $url = 'https://example-app.com/billing/process/9999?shop='.$shop->name; // Try to go to bill without a plan id which happens when you cancel the charge $response = $this->call( 'get', $url, ['shop' => $shop->name] ); - //Confirm we get sent back to the homepage of shopify - $response->assertRedirect('https://example.myshopify.com?shop='.$shop->name); + //Confirm we get sent back to the homepage of the app + $response->assertRedirect('https://example-app.com?shop='.$shop->name); } } From 7f3ccd8fcd0c8aa8fb9af5f10be769a7a6fdc4ef Mon Sep 17 00:00:00 2001 From: Luke Walsh Date: Mon, 5 Sep 2022 12:12:34 +0100 Subject: [PATCH 7/7] Update the url to not be the same in the test as the shopify admin as it is confusing --- tests/Traits/BillingControllerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Traits/BillingControllerTest.php b/tests/Traits/BillingControllerTest.php index e9b4a394..eef18804 100644 --- a/tests/Traits/BillingControllerTest.php +++ b/tests/Traits/BillingControllerTest.php @@ -140,7 +140,7 @@ public function testReturnToSettingScreenNoPlan() //Log in $this->auth->login($shop); $url = 'https://example-app.com/billing/process/9999?shop='.$shop->name; - // Try to go to bill without a plan id which happens when you cancel the charge + // Try to go to bill without a charge id which happens when you cancel the charge $response = $this->call( 'get', $url,