Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Payments settings improvements #8428

Merged
merged 8 commits into from
Mar 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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() {
timur27 marked this conversation as resolved.
Show resolved Hide resolved
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
Loading