Skip to content

Commit

Permalink
Payments settings improvements (#8428)
Browse files Browse the repository at this point in the history
Co-authored-by: Timur Karimov <[email protected]>
  • Loading branch information
timur27 and Timur Karimov authored Mar 26, 2024
1 parent d3f595e commit 7976853
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 25 deletions.
4 changes: 4 additions & 0 deletions changelog/add-8428-payment-settings-ux-improvements
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: minor
Type: add

Improve payment settings UX.
2 changes: 0 additions & 2 deletions client/components/payment-methods-list/payment-method.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -270,8 +270,6 @@ const PaymentMethod = ( {
checked={ checked }
disabled={ disabled || locked }
onChange={ handleChange }
delayMsOnCheck={ 1500 }
delayMsOnUncheck={ 0 }
hideLabel
isAllowingManualCapture={ isAllowingManualCapture }
isSetupRequired={ isSetupRequired }
Expand Down
6 changes: 5 additions & 1 deletion client/data/settings/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' )
);
Expand Down
61 changes: 47 additions & 14 deletions client/data/settings/test/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
*/
import { dispatch, select } from '@wordpress/data';
import { apiFetch } from '@wordpress/data-controls';
import { findIndex } from 'lodash';

/**
* Internal dependencies
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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
Expand All @@ -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'] );
}

/**
Expand Down

0 comments on commit 7976853

Please sign in to comment.