From 7dab3e3f3b65a838bf6b1bcaf5b278a75f7222b2 Mon Sep 17 00:00:00 2001 From: Ravinder Kumar Date: Wed, 7 Feb 2024 11:56:38 +0530 Subject: [PATCH 1/7] add: handle stripe connect redirect on specific pages --- .../NewStripeAccountOnBoardingController.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/PaymentGateways/Stripe/Controllers/NewStripeAccountOnBoardingController.php b/src/PaymentGateways/Stripe/Controllers/NewStripeAccountOnBoardingController.php index 60d7dd779b..96fd3455a3 100644 --- a/src/PaymentGateways/Stripe/Controllers/NewStripeAccountOnBoardingController.php +++ b/src/PaymentGateways/Stripe/Controllers/NewStripeAccountOnBoardingController.php @@ -30,6 +30,10 @@ public function __construct(Settings $settings) } /** + * @unreleased Handle Stripe connect account on-boarding redirect on specific pages. + * Admin redirect to following page: + * 1. GiveWP stripe settings page. + * 2. Legacy donation form edit form. * @since 2.13.0 */ public function __invoke() @@ -38,6 +42,15 @@ public function __invoke() return; } + $isDonationFormPage = isset($_GET['give_tab']) && $_GET['give_tab'] !== 'stripe_form_account_options'; + $isSettingPage = isset($_GET['tab'], $_GET['tab']) + && Give_Admin_Settings::is_setting_page('gateways', 'stripe-settings'); + + // Exit if admin is not redirect to the GiveWP settings page or donation form page. + if (! $isDonationFormPage && ! $isSettingPage) { + return; + } + $requestedData = NewStripeAccountOnBoardingDto::fromArray(give_clean($_GET)); if (!$requestedData->hasValidateData()) { From 966a9675885580b46f46d484bc8dffad17162b10 Mon Sep 17 00:00:00 2001 From: Ravinder Kumar Date: Wed, 7 Feb 2024 12:01:07 +0530 Subject: [PATCH 2/7] doc: upadet function document --- .../Stripe/Controllers/NewStripeAccountOnBoardingController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PaymentGateways/Stripe/Controllers/NewStripeAccountOnBoardingController.php b/src/PaymentGateways/Stripe/Controllers/NewStripeAccountOnBoardingController.php index 96fd3455a3..5a676a9f0b 100644 --- a/src/PaymentGateways/Stripe/Controllers/NewStripeAccountOnBoardingController.php +++ b/src/PaymentGateways/Stripe/Controllers/NewStripeAccountOnBoardingController.php @@ -33,7 +33,7 @@ public function __construct(Settings $settings) * @unreleased Handle Stripe connect account on-boarding redirect on specific pages. * Admin redirect to following page: * 1. GiveWP stripe settings page. - * 2. Legacy donation form edit form. + * 2. V2 donation form edit form. * @since 2.13.0 */ public function __invoke() From 2193d1569ff6e446a83d3111ae7bea968b913880 Mon Sep 17 00:00:00 2001 From: Ravinder Kumar Date: Wed, 7 Feb 2024 21:48:09 +0530 Subject: [PATCH 3/7] fix: improve logic to detect v2 donation form edit page --- .../Controllers/NewStripeAccountOnBoardingController.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/PaymentGateways/Stripe/Controllers/NewStripeAccountOnBoardingController.php b/src/PaymentGateways/Stripe/Controllers/NewStripeAccountOnBoardingController.php index 5a676a9f0b..8326f2a241 100644 --- a/src/PaymentGateways/Stripe/Controllers/NewStripeAccountOnBoardingController.php +++ b/src/PaymentGateways/Stripe/Controllers/NewStripeAccountOnBoardingController.php @@ -42,8 +42,12 @@ public function __invoke() return; } - $isDonationFormPage = isset($_GET['give_tab']) && $_GET['give_tab'] !== 'stripe_form_account_options'; - $isSettingPage = isset($_GET['tab'], $_GET['tab']) + $isDonationFormPage = isset($_GET['give_tab'], $_GET['post_type']) + && $_GET['post_type'] === 'give_forms' + && $_GET['give_tab'] !== 'stripe_form_account_options'; + $isSettingPage = isset($_GET['post_type'], $_GET['page'], $_GET['tab'], $_GET['section']) + && $_GET['post_type'] === 'give_forms' + && $_GET['page'] === 'give-settings' && Give_Admin_Settings::is_setting_page('gateways', 'stripe-settings'); // Exit if admin is not redirect to the GiveWP settings page or donation form page. From 9ef8cba6dd7d9ca55376636cdffcc465f720f7af Mon Sep 17 00:00:00 2001 From: Ravinder Kumar Date: Wed, 7 Feb 2024 21:52:49 +0530 Subject: [PATCH 4/7] refactor: add a function to return whether we can process request on current page --- .../NewStripeAccountOnBoardingController.php | 50 ++++++++++++------- 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/src/PaymentGateways/Stripe/Controllers/NewStripeAccountOnBoardingController.php b/src/PaymentGateways/Stripe/Controllers/NewStripeAccountOnBoardingController.php index 8326f2a241..0d7a517b91 100644 --- a/src/PaymentGateways/Stripe/Controllers/NewStripeAccountOnBoardingController.php +++ b/src/PaymentGateways/Stripe/Controllers/NewStripeAccountOnBoardingController.php @@ -31,38 +31,27 @@ public function __construct(Settings $settings) /** * @unreleased Handle Stripe connect account on-boarding redirect on specific pages. - * Admin redirect to following page: - * 1. GiveWP stripe settings page. - * 2. V2 donation form edit form. + * * @since 2.13.0 */ public function __invoke() { - if (!current_user_can('manage_give_settings')) { + if (! current_user_can('manage_give_settings')) { return; } - $isDonationFormPage = isset($_GET['give_tab'], $_GET['post_type']) - && $_GET['post_type'] === 'give_forms' - && $_GET['give_tab'] !== 'stripe_form_account_options'; - $isSettingPage = isset($_GET['post_type'], $_GET['page'], $_GET['tab'], $_GET['section']) - && $_GET['post_type'] === 'give_forms' - && $_GET['page'] === 'give-settings' - && Give_Admin_Settings::is_setting_page('gateways', 'stripe-settings'); - - // Exit if admin is not redirect to the GiveWP settings page or donation form page. - if (! $isDonationFormPage && ! $isSettingPage) { + if (! $this->canProcessRequestOnCurrentPage()) { return; } $requestedData = NewStripeAccountOnBoardingDto::fromArray(give_clean($_GET)); - if (!$requestedData->hasValidateData()) { + if (! $requestedData->hasValidateData()) { return; } $stripe_accounts = give_stripe_get_all_accounts(); - $secret_key = !give_is_test_mode() ? $requestedData->stripeAccessToken : $requestedData->stripeAccessTokenTest; + $secret_key = ! give_is_test_mode() ? $requestedData->stripeAccessToken : $requestedData->stripeAccessTokenTest; Stripe::setApiKey($secret_key); @@ -86,7 +75,7 @@ public function __invoke() return; } - $account_name = !empty($account_details->business_profile->name) ? + $account_name = ! empty($account_details->business_profile->name) ? $account_details->business_profile->name : $account_details->settings->dashboard->display_name; $account_slug = $account_details->id; @@ -94,7 +83,7 @@ public function __invoke() $account_country = $account_details->country; // Set first Stripe account as default. - if (!$stripe_accounts) { + if (! $stripe_accounts) { give_update_option('_give_stripe_default_account', $account_slug); } @@ -118,7 +107,7 @@ public function __invoke() $this->settings->addNewStripeAccount($accountDetailModel); if ($requestedData->formId) { - if (!Settings::getDefaultStripeAccountSlugForDonationForm($requestedData->formId)) { + if (! Settings::getDefaultStripeAccountSlugForDonationForm($requestedData->formId)) { Settings::setDefaultStripeAccountSlugForDonationForm( $requestedData->formId, $accountDetailModel->accountSlug @@ -163,4 +152,27 @@ public function __invoke() return; } } + + /** + * Check if the request can be processed on the current page. + * + * Admin redirect to following page: + * 1. GiveWP stripe settings page. + * 2. V2 donation form edit form. + * + * @unreleased + * @return bool + */ + private function canProcessRequestOnCurrentPage(): bool + { + $isDonationFormPage = isset($_GET['give_tab'], $_GET['post_type']) + && $_GET['post_type'] === 'give_forms' + && $_GET['give_tab'] !== 'stripe_form_account_options'; + $isSettingPage = isset($_GET['post_type'], $_GET['page'], $_GET['tab'], $_GET['section']) + && $_GET['post_type'] === 'give_forms' + && $_GET['page'] === 'give-settings' + && Give_Admin_Settings::is_setting_page('gateways', 'stripe-settings'); + + return $isDonationFormPage || $isSettingPage; + } } From 85f26691a809114d9e7d946ad3e4ab1236f74ca7 Mon Sep 17 00:00:00 2001 From: Ravinder Kumar Date: Wed, 7 Feb 2024 21:54:33 +0530 Subject: [PATCH 5/7] change: remove duplicate check is_setting_page function handle this check internally. --- .../Controllers/NewStripeAccountOnBoardingController.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/PaymentGateways/Stripe/Controllers/NewStripeAccountOnBoardingController.php b/src/PaymentGateways/Stripe/Controllers/NewStripeAccountOnBoardingController.php index 0d7a517b91..e06d8886ea 100644 --- a/src/PaymentGateways/Stripe/Controllers/NewStripeAccountOnBoardingController.php +++ b/src/PaymentGateways/Stripe/Controllers/NewStripeAccountOnBoardingController.php @@ -168,8 +168,7 @@ private function canProcessRequestOnCurrentPage(): bool $isDonationFormPage = isset($_GET['give_tab'], $_GET['post_type']) && $_GET['post_type'] === 'give_forms' && $_GET['give_tab'] !== 'stripe_form_account_options'; - $isSettingPage = isset($_GET['post_type'], $_GET['page'], $_GET['tab'], $_GET['section']) - && $_GET['post_type'] === 'give_forms' + $isSettingPage = isset($_GET['page'], $_GET['tab'], $_GET['section']) && $_GET['page'] === 'give-settings' && Give_Admin_Settings::is_setting_page('gateways', 'stripe-settings'); From 587c8cd5ee4057c1317e7b802fed69d492f12f67 Mon Sep 17 00:00:00 2001 From: Ravinder Kumar Date: Wed, 7 Feb 2024 22:59:50 +0530 Subject: [PATCH 6/7] tests: add test to test page validation function --- .../NewStripeAccountOnBoardingController.php | 40 +++++++--- ...wStripeAccountOnBoardingControllerTest.php | 74 +++++++++++++++++++ 2 files changed, 105 insertions(+), 9 deletions(-) create mode 100644 tests/Feature/Controllers/NewStripeAccountOnBoardingControllerTest.php diff --git a/src/PaymentGateways/Stripe/Controllers/NewStripeAccountOnBoardingController.php b/src/PaymentGateways/Stripe/Controllers/NewStripeAccountOnBoardingController.php index e06d8886ea..8ca6e3d0af 100644 --- a/src/PaymentGateways/Stripe/Controllers/NewStripeAccountOnBoardingController.php +++ b/src/PaymentGateways/Stripe/Controllers/NewStripeAccountOnBoardingController.php @@ -40,7 +40,7 @@ public function __invoke() return; } - if (! $this->canProcessRequestOnCurrentPage()) { + if (wp_doing_ajax() || ! $this->canProcessRequestOnCurrentPage($_SERVER['REQUEST_URI'])) { return; } @@ -161,16 +161,38 @@ public function __invoke() * 2. V2 donation form edit form. * * @unreleased - * @return bool */ - private function canProcessRequestOnCurrentPage(): bool + protected function canProcessRequestOnCurrentPage($url): bool { - $isDonationFormPage = isset($_GET['give_tab'], $_GET['post_type']) - && $_GET['post_type'] === 'give_forms' - && $_GET['give_tab'] !== 'stripe_form_account_options'; - $isSettingPage = isset($_GET['page'], $_GET['tab'], $_GET['section']) - && $_GET['page'] === 'give-settings' - && Give_Admin_Settings::is_setting_page('gateways', 'stripe-settings'); + // Check if request is from edit.php or post.php page. + if (false === strpos($url, 'wp-admin/post.php') && false === strpos($url, 'wp-admin/edit.php')) { + return false; + } + + $path = wp_parse_url($url); + + // Result should be in array and should have query string. + if (! is_array($path) || ! isset($path['query'])) { + return false; + } + + $queryParams = wp_parse_args($path['query']); + + if (empty($queryParams)) { + return false; + } + + // Check if request is from V2 donation form edit page. + $isDonationFormPage = isset($queryParams['post'], $queryParams['give_tab']) + && get_post_type(absint($queryParams['post'])) === 'give_forms' + && $queryParams['give_tab'] === 'stripe_form_account_options'; + + // Check if request is from GiveWP stripe settings page. + $isSettingPage = isset($queryParams['page'], $queryParams['post_type'], $queryParams['tab'], $queryParams['section']) + && $queryParams['post_type'] === 'give_forms' + && $queryParams['page'] === 'give-settings' + && $queryParams['tab'] === 'gateways' + && $queryParams['section'] === 'stripe-settings'; return $isDonationFormPage || $isSettingPage; } diff --git a/tests/Feature/Controllers/NewStripeAccountOnBoardingControllerTest.php b/tests/Feature/Controllers/NewStripeAccountOnBoardingControllerTest.php new file mode 100644 index 0000000000..1d4ead1baf --- /dev/null +++ b/tests/Feature/Controllers/NewStripeAccountOnBoardingControllerTest.php @@ -0,0 +1,74 @@ +newStripeAccountOnBoardingController = give(NewStripeAccountOnBoardingController::class); + $this->canProcessRequestOnCurrentPageMethod = (new \ReflectionObject( + $this->newStripeAccountOnBoardingController + ))->getMethod('canProcessRequestOnCurrentPage'); + $this->canProcessRequestOnCurrentPageMethod->setAccessible(true); + } + + public function testShouldReturnTrueForV2DonationFomEditPage(): void + { + /** @var DonationForm $form */ + $form = DonationForm::factory()->create(); + $url = "wp-admin/post.php?post=$form->id&action=edit&give_tab=stripe_form_account_options"; + $result = $this->canProcessRequestOnCurrentPageMethod->invokeArgs( + $this->newStripeAccountOnBoardingController, + [$url] + ); + $this->assertTrue($result); + } + + public function testShouldReturnTrueGlobalStripeSettingPage(): void + { + $url = 'wp-admin/edit.php?post_type=give_forms&page=give-settings&tab=gateways§ion=stripe-settings'; + $result = $this->canProcessRequestOnCurrentPageMethod->invokeArgs( + $this->newStripeAccountOnBoardingController, + [$url] + ); + $this->assertTrue($result); + } + + public function testShouldReturnFalseOtherPage(): void + { + $url = 'wp-admin/edit.php?post_type=give_forms&page=give-settings&tab=gateways§ion=paypal-settings'; + $result = $this->canProcessRequestOnCurrentPageMethod->invokeArgs( + $this->newStripeAccountOnBoardingController, + [$url] + ); + $this->assertFalse($result); + + /** @var Donation $donation */ + $donation = Donation::factory()->create(); + $url = "wp-admin/post.php?post=$donation->id&action=edit&give_tab=stripe_form_account_options"; + $result = $this->canProcessRequestOnCurrentPageMethod->invokeArgs( + $this->newStripeAccountOnBoardingController, + [$url] + ); + $this->assertFalse($result); + } +} From 864f1a1234d11aca2d3c7ba3e4442c79420d7558 Mon Sep 17 00:00:00 2001 From: Ravinder Kumar Date: Wed, 7 Feb 2024 23:03:29 +0530 Subject: [PATCH 7/7] add: add type to function param --- .../Stripe/Controllers/NewStripeAccountOnBoardingController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PaymentGateways/Stripe/Controllers/NewStripeAccountOnBoardingController.php b/src/PaymentGateways/Stripe/Controllers/NewStripeAccountOnBoardingController.php index 8ca6e3d0af..843f4a94bf 100644 --- a/src/PaymentGateways/Stripe/Controllers/NewStripeAccountOnBoardingController.php +++ b/src/PaymentGateways/Stripe/Controllers/NewStripeAccountOnBoardingController.php @@ -162,7 +162,7 @@ public function __invoke() * * @unreleased */ - protected function canProcessRequestOnCurrentPage($url): bool + protected function canProcessRequestOnCurrentPage(string $url): bool { // Check if request is from edit.php or post.php page. if (false === strpos($url, 'wp-admin/post.php') && false === strpos($url, 'wp-admin/edit.php')) {