Skip to content

Commit

Permalink
feat: display error on invalid bank statement (#2384)
Browse files Browse the repository at this point in the history
  • Loading branch information
frosso authored Jul 9, 2021
1 parent 198d504 commit 4d3e7bd
Show file tree
Hide file tree
Showing 11 changed files with 161 additions and 25 deletions.
14 changes: 7 additions & 7 deletions client/data/settings/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,11 @@ export function updateEnabledPaymentMethodIds( methodIds ) {
} );
}

export function updateIsSavingSettings( isSaving ) {
export function updateIsSavingSettings( isSaving, error ) {
return {
type: ACTION_TYPES.SET_IS_SAVING_SETTINGS,
isSaving,
error,
};
}

Expand All @@ -79,11 +80,11 @@ export function updateAccountStatementDescriptor( accountStatementDescriptor ) {
}

export function* saveSettings() {
let isSuccess = false;
let error = null;
try {
const settings = select( STORE_NAME ).getSettings();

yield updateIsSavingSettings( true );
yield updateIsSavingSettings( true, null );

yield apiFetch( {
path: `${ NAMESPACE }/settings`,
Expand All @@ -94,17 +95,16 @@ export function* saveSettings() {
yield dispatch( 'core/notices' ).createSuccessNotice(
__( 'Settings saved.', 'woocommerce-payments' )
);

isSuccess = true;
} catch ( e ) {
error = e;
yield dispatch( 'core/notices' ).createErrorNotice(
__( 'Error saving settings.', 'woocommerce-payments' )
);
} finally {
yield updateIsSavingSettings( false );
yield updateIsSavingSettings( false, error );
}

return isSuccess;
return null === error;
}

export function updatePaymentRequestLocations( locations ) {
Expand Down
8 changes: 8 additions & 0 deletions client/data/settings/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -197,3 +197,11 @@ export const usePaymentRequestButtonTheme = () => {
];
} );
};

export const useGetSavingError = () => {
return useSelect( ( select ) => {
const { getSavingError } = select( STORE_NAME );

return getSavingError();
}, [] );
};
3 changes: 3 additions & 0 deletions client/data/settings/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import ACTION_TYPES from './action-types';

const defaultState = {
isSaving: false,
savingError: null,
data: {},
};

Expand All @@ -24,6 +25,7 @@ export const receiveSettings = (
case ACTION_TYPES.SET_SETTINGS_VALUES:
return {
...state,
savingError: null,
data: {
...state.data,
...action.payload,
Expand All @@ -34,6 +36,7 @@ export const receiveSettings = (
return {
...state,
isSaving: action.isSaving,
savingError: action.error,
};
}

Expand Down
4 changes: 4 additions & 0 deletions client/data/settings/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,7 @@ export const getPaymentRequestButtonSize = ( state ) => {
export const getPaymentRequestButtonTheme = ( state ) => {
return getSettings( state ).payment_request_button_theme || '';
};

export const getSavingError = ( state ) => {
return getSettingsState( state ).savingError;
};
13 changes: 10 additions & 3 deletions client/data/settings/test/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,12 @@ describe( 'Settings actions tests', () => {

const isSavingStartIndex = findIndex(
yielded,
updateIsSavingSettings( true )
updateIsSavingSettings( true, null )
);

const isSavingEndIndex = findIndex(
yielded,
updateIsSavingSettings( false )
updateIsSavingSettings( false, null )
);

expect( apiRequestIndex ).not.toEqual( -1 );
Expand Down Expand Up @@ -117,7 +117,14 @@ describe( 'Settings actions tests', () => {
expect(
dispatch( 'core/notices' ).createErrorNotice
).toHaveBeenCalled();
expect( yielded ).toContainEqual( updateIsSavingSettings( false ) );
expect( yielded ).toEqual(
expect.arrayContaining( [
expect.objectContaining( {
type: 'SET_IS_SAVING_SETTINGS',
isSaving: false,
} ),
] )
);
} );
} );
} );
31 changes: 29 additions & 2 deletions client/data/settings/test/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ describe( 'Settings reducer tests', () => {
expect( defaultState ).toEqual( {
isSaving: false,
data: {},
savingError: null,
} );
} );

Expand Down Expand Up @@ -56,6 +57,7 @@ describe( 'Settings reducer tests', () => {
data: {
baz: 'quux',
},
savingError: {},
};

const newSettings = {
Expand All @@ -69,6 +71,7 @@ describe( 'Settings reducer tests', () => {
data: {
quuz: 'corge',
},
savingError: {},
} );
} );
} );
Expand All @@ -77,23 +80,33 @@ describe( 'Settings reducer tests', () => {
test( 'toggles isSaving', () => {
const oldState = {
isSaving: false,
savingError: null,
};

const state = reducer( oldState, updateIsSavingSettings( true ) );
const state = reducer(
oldState,
updateIsSavingSettings( true, {} )
);

expect( state.isSaving ).toBeTruthy();
expect( state.savingError ).toEqual( {} );
} );

test( 'leaves other fields unchanged', () => {
const oldState = {
foo: 'bar',
isSaving: false,
savingError: {},
};

const state = reducer( oldState, updateIsSavingSettings( true ) );
const state = reducer(
oldState,
updateIsSavingSettings( true, null )
);

expect( state ).toEqual( {
foo: 'bar',
savingError: null,
isSaving: true,
} );
} );
Expand All @@ -105,6 +118,7 @@ describe( 'Settings reducer tests', () => {
data: {
enabled_payment_method_ids: [],
},
savingError: null,
};

const methodIds = [ 'foo', 'bar' ];
Expand All @@ -126,6 +140,7 @@ describe( 'Settings reducer tests', () => {
enabled_payment_method_ids: [],
quuz: 'corge',
},
savingError: {},
};

const methodIds = [ 'foo', 'bar' ];
Expand All @@ -141,6 +156,7 @@ describe( 'Settings reducer tests', () => {
enabled_payment_method_ids: methodIds,
quuz: 'corge',
},
savingError: null,
} );
} );
} );
Expand All @@ -165,6 +181,7 @@ describe( 'Settings reducer tests', () => {
is_wcpay_enabled: false,
baz: 'quux',
},
savingError: {},
};

const state = reducer( oldState, updateIsWCPayEnabled( true ) );
Expand All @@ -175,6 +192,7 @@ describe( 'Settings reducer tests', () => {
is_wcpay_enabled: true,
baz: 'quux',
},
savingError: null,
} );
} );
} );
Expand Down Expand Up @@ -202,6 +220,7 @@ describe( 'Settings reducer tests', () => {
is_manual_capture_enabled: false,
baz: 'quux',
},
savingError: {},
};

const state = reducer(
Expand All @@ -210,6 +229,7 @@ describe( 'Settings reducer tests', () => {
);

expect( state ).toEqual( {
savingError: null,
foo: 'bar',
data: {
is_manual_capture_enabled: true,
Expand Down Expand Up @@ -244,6 +264,7 @@ describe( 'Settings reducer tests', () => {
account_statement_descriptor: 'Statement',
baz: 'quux',
},
savingError: {},
};

const state = reducer(
Expand All @@ -253,6 +274,7 @@ describe( 'Settings reducer tests', () => {

expect( state ).toEqual( {
foo: 'bar',
savingError: null,
data: {
account_statement_descriptor: 'New Statement',
baz: 'quux',
Expand All @@ -267,6 +289,7 @@ describe( 'Settings reducer tests', () => {
data: {
is_payment_request_enabled: false,
},
savingError: null,
};

const state = reducer(
Expand All @@ -284,6 +307,7 @@ describe( 'Settings reducer tests', () => {
is_payment_request_enabled: false,
baz: 'quux',
},
savingError: {},
};

const state = reducer(
Expand All @@ -293,6 +317,7 @@ describe( 'Settings reducer tests', () => {

expect( state ).toEqual( {
foo: 'bar',
savingError: null,
data: {
is_payment_request_enabled: true,
baz: 'quux',
Expand Down Expand Up @@ -329,6 +354,7 @@ describe( 'Settings reducer tests', () => {
payment_request_enabled_locations: initPaymentRequestState,
baz: 'quux',
},
savingError: {},
};

const state = reducer(
Expand All @@ -342,6 +368,7 @@ describe( 'Settings reducer tests', () => {
payment_request_enabled_locations: enableAllpaymentRequestState,
baz: 'quux',
},
savingError: null,
} );
} );
} );
Expand Down
3 changes: 1 addition & 2 deletions client/settings/general-settings/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import interpolateComponents from 'interpolate-components';
/**
* Internal dependencies
*/
import './style.scss';
import { useDevMode, useIsWCPayEnabled, useTestMode } from 'data';
import CardBody from '../card-body';

Expand All @@ -19,7 +18,7 @@ const GeneralSettings = () => {
const isDevModeEnabled = useDevMode();

return (
<Card className="general-settings">
<Card>
<CardBody>
<CheckboxControl
checked={ isWCPayEnabled }
Expand Down
29 changes: 23 additions & 6 deletions client/settings/transactions-and-deposits/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,20 @@ import {
CheckboxControl,
ExternalLink,
TextControl,
Notice,
} from '@wordpress/components';

/**
* Internal dependencies
*/
import { useAccountStatementDescriptor, useManualCapture } from 'data';
import WCPaySettingsContext from '../wcpay-settings-context';
import CardBody from '../card-body';
import {
useAccountStatementDescriptor,
useManualCapture,
useGetSavingError,
} from '../../data';
import './style.scss';

const ACCOUNT_STATEMENT_MAX_LENGTH = 22;

Expand All @@ -31,9 +37,11 @@ const TransactionsAndDeposits = () => {
accountStatementDescriptor,
setAccountStatementDescriptor,
] = useAccountStatementDescriptor();
const customerBankStatementErrorMessage = useGetSavingError()?.data?.details
?.account_statement_descriptor?.message;

return (
<Card>
<Card className="transactions-and-deposits">
<CardBody>
<h4>
{ __( 'Transaction preferences', 'woocommerce-payments' ) }
Expand All @@ -51,9 +59,18 @@ const TransactionsAndDeposits = () => {
'woocommerce-payments'
) }
/>
<div className="general-settings__account-statement-wrapper">
{ customerBankStatementErrorMessage && (
<Notice status="error" isDismissible={ false }>
<span
dangerouslySetInnerHTML={ {
__html: customerBankStatementErrorMessage,
} }
/>
</Notice>
) }
<div className="transactions-and-deposits__account-statement-wrapper">
<TextControl
className="general-settings__account-statement-input"
className="transactions-and-deposits__account-statement-input"
help={ __(
"Edit the way your store name appears on your customers' bank statements.",
'woocommerce-payments'
Expand All @@ -70,14 +87,14 @@ const TransactionsAndDeposits = () => {
{ `${ accountStatementDescriptor.length } / ${ ACCOUNT_STATEMENT_MAX_LENGTH }` }
</span>
</div>
<div className="general-settings__bank-information">
<div className="transactions-and-deposits__bank-information">
<h4>
{ __(
'Bank account information',
'woocommerce-payments'
) }
</h4>
<p className="general-settings__bank-information-help">
<p className="transactions-and-deposits__bank-information-help">
{ __(
'Manage and update your deposit account information to receive payments and payouts.',
'woocommerce-payments'
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
.general-settings {
.transactions-and-deposits {
.components-notice {
margin-left: 0;
margin-right: 0;
margin-bottom: 1em;
}

&__account-statement-wrapper {
max-width: 500px;
position: relative;
Expand Down
Loading

0 comments on commit 4d3e7bd

Please sign in to comment.