From 7976853c724f2d6952d1de44fcba4fb6cac7b618 Mon Sep 17 00:00:00 2001 From: Timur Karimov Date: Tue, 26 Mar 2024 12:11:22 +0100 Subject: [PATCH] Payments settings improvements (#8428) Co-authored-by: Timur Karimov --- .../add-8428-payment-settings-ux-improvements | 4 ++ .../payment-methods-list/payment-method.tsx | 2 - client/data/settings/actions.js | 6 +- client/data/settings/test/actions.js | 61 ++++++++++++++----- ...s-wc-rest-payments-settings-controller.php | 2 +- ...s-wc-rest-payments-settings-controller.php | 23 ++++--- 6 files changed, 73 insertions(+), 25 deletions(-) create mode 100644 changelog/add-8428-payment-settings-ux-improvements diff --git a/changelog/add-8428-payment-settings-ux-improvements b/changelog/add-8428-payment-settings-ux-improvements new file mode 100644 index 00000000000..dd8aa8c1c3b --- /dev/null +++ b/changelog/add-8428-payment-settings-ux-improvements @@ -0,0 +1,4 @@ +Significance: minor +Type: add + +Improve payment settings UX. diff --git a/client/components/payment-methods-list/payment-method.tsx b/client/components/payment-methods-list/payment-method.tsx index 7213407e66c..cb41ffaa34d 100644 --- a/client/components/payment-methods-list/payment-method.tsx +++ b/client/components/payment-methods-list/payment-method.tsx @@ -270,8 +270,6 @@ const PaymentMethod = ( { checked={ checked } disabled={ disabled || locked } onChange={ handleChange } - delayMsOnCheck={ 1500 } - delayMsOnUncheck={ 0 } hideLabel isAllowingManualCapture={ isAllowingManualCapture } isSetupRequired={ isSetupRequired } diff --git a/client/data/settings/actions.js b/client/data/settings/actions.js index d2d4728997d..ef467116d79 100644 --- a/client/data/settings/actions.js +++ b/client/data/settings/actions.js @@ -217,12 +217,16 @@ export function* saveSettings() { yield updateIsSavingSettings( true, null ); - yield apiFetch( { + const response = yield apiFetch( { path: `${ NAMESPACE }/settings`, method: 'post', data: settings, } ); + yield updateSettingsValues( { + payment_method_statuses: response.data.payment_method_statuses, + } ); + yield dispatch( 'core/notices' ).createSuccessNotice( __( 'Settings saved.', 'woocommerce-payments' ) ); diff --git a/client/data/settings/test/actions.js b/client/data/settings/test/actions.js index 1c337e73ca3..0c629dfe4bb 100644 --- a/client/data/settings/test/actions.js +++ b/client/data/settings/test/actions.js @@ -3,7 +3,6 @@ */ import { dispatch, select } from '@wordpress/data'; import { apiFetch } from '@wordpress/data-controls'; -import { findIndex } from 'lodash'; /** * Internal dependencies @@ -57,30 +56,64 @@ describe( 'Settings actions tests', () => { } ); test( 'before saving sets isSaving to true, and after - to false', () => { - apiFetch.mockReturnValue( 'api request' ); - - const yielded = [ ...saveSettings() ]; + const apiResponse = { + data: { + payment_method_statuses: { + bancontact: 'active', + }, + }, + }; + apiFetch.mockReturnValue( { ...apiResponse } ); - const apiRequestIndex = yielded.indexOf( 'api request' ); + const saveGenerator = saveSettings(); - const isSavingStartIndex = findIndex( - yielded, + // Assert the first yield is updating isSaving to true + let next = saveGenerator.next(); + expect( next.value ).toEqual( updateIsSavingSettings( true, null ) ); - const isSavingEndIndex = findIndex( - yielded, + // Execute the next step, which should be the apiFetch call + next = saveGenerator.next(); + expect( next.value ).toEqual( apiResponse ); + + // Simulate the response from the apiFetch call and proceed to the next yield + // Since the actual fetching process is mocked, pass the apiResponse to the next saveGenerator step directly + next = saveGenerator.next( apiResponse ); + expect( next.value ).toEqual( { + type: 'SET_SETTINGS_VALUES', + payload: { + payment_method_statuses: + apiResponse.data.payment_method_statuses, + }, + } ); + + next = saveGenerator.next(); // Skip the success notice + next = saveGenerator.next(); // Move to updateIsSavingSettings(false) + expect( next.value ).toEqual( updateIsSavingSettings( false, null ) ); - expect( apiRequestIndex ).not.toEqual( -1 ); - expect( isSavingStartIndex ).toBeLessThan( apiRequestIndex ); - expect( isSavingEndIndex ).toBeGreaterThan( apiRequestIndex ); + // Check if the saveGenerator is complete + expect( saveGenerator.next().done ).toBeTruthy(); } ); test( 'displays success notice after saving', () => { - // eslint-disable-next-line no-unused-expressions - [ ...saveSettings() ]; + const apiResponse = { + data: { + payment_method_statuses: { + bancontact: 'active', + }, + }, + }; + apiFetch.mockReturnValue( { ...apiResponse } ); + + // Execute the generator until the end + const saveGenerator = saveSettings(); + while ( ! saveGenerator.next( apiResponse ).done ) { + // Intentionally empty + } + expect( saveGenerator.next().done ).toBeTruthy(); expect( dispatch( 'core/notices' ).createSuccessNotice diff --git a/includes/admin/class-wc-rest-payments-settings-controller.php b/includes/admin/class-wc-rest-payments-settings-controller.php index c20fcd80208..9137df7062d 100644 --- a/includes/admin/class-wc-rest-payments-settings-controller.php +++ b/includes/admin/class-wc-rest-payments-settings-controller.php @@ -563,7 +563,7 @@ public function update_settings( WP_REST_Request $request ) { return new WP_REST_Response( [ 'server_error' => $update_account_result->get_error_message() ], 400 ); } - return new WP_REST_Response( [], 200 ); + return new WP_REST_Response( $this->get_settings(), 200 ); } /** diff --git a/tests/unit/admin/test-class-wc-rest-payments-settings-controller.php b/tests/unit/admin/test-class-wc-rest-payments-settings-controller.php index 7acffe22944..79486949b83 100644 --- a/tests/unit/admin/test-class-wc-rest-payments-settings-controller.php +++ b/tests/unit/admin/test-class-wc-rest-payments-settings-controller.php @@ -86,6 +86,13 @@ class WC_REST_Payments_Settings_Controller_Test extends WCPAY_UnitTestCase { */ private $mock_session_service; + /** + * Domestic currency. + * + * @var string + */ + private $domestic_currency = 'usd'; + /** * Pre-test setup */ @@ -142,6 +149,10 @@ public function set_up() { $mock_payment_methods[ $mock_payment_method->get_id() ] = $mock_payment_method; } + $this->mock_wcpay_account + ->method( 'get_account_default_currency' ) + ->willReturn( $this->domestic_currency ); + $this->gateway = new WC_Payment_Gateway_WCPay( $this->mock_api_client, $this->mock_wcpay_account, @@ -350,7 +361,7 @@ public function test_update_settings_returns_error_on_non_bool_is_wcpay_enabled_ $this->assertEquals( 400, $response->get_status() ); } - public function test_timur_testing() { + public function test_update_settings_saves_enabled_payment_methods() { WC_Payments::get_gateway()->update_option( 'upe_enabled_payment_method_ids', [ Payment_Method::CARD ] ); $request = new WP_REST_Request(); @@ -693,10 +704,9 @@ public function test_get_settings_card_eligible_flag(): void { } public function test_get_settings_domestic_currency(): void { - $mock_domestic_currency = 'usd'; $this->mock_localization_service->method( 'get_country_locale_data' )->willReturn( [ - 'currency_code' => $mock_domestic_currency, + 'currency_code' => $this->domestic_currency, ] ); $this->mock_wcpay_account @@ -706,20 +716,19 @@ public function test_get_settings_domestic_currency(): void { $response = $this->controller->get_settings(); $this->assertArrayHasKey( 'account_domestic_currency', $response->get_data() ); - $this->assertSame( $mock_domestic_currency, $response->get_data()['account_domestic_currency'] ); + $this->assertSame( $this->domestic_currency, $response->get_data()['account_domestic_currency'] ); } public function test_get_settings_domestic_currency_fallbacks_to_default_currency(): void { - $mock_domestic_currency = 'usd'; $this->mock_localization_service->method( 'get_country_locale_data' )->willReturn( [] ); $this->mock_wcpay_account ->expects( $this->once() ) ->method( 'get_account_default_currency' ) - ->willReturn( $mock_domestic_currency ); + ->willReturn( $this->domestic_currency ); $response = $this->controller->get_settings(); $this->assertArrayHasKey( 'account_domestic_currency', $response->get_data() ); - $this->assertSame( $mock_domestic_currency, $response->get_data()['account_domestic_currency'] ); + $this->assertSame( $this->domestic_currency, $response->get_data()['account_domestic_currency'] ); } /**