Skip to content

Commit 6c80d61

Browse files
adding error handling for settings tab
1 parent 36bde61 commit 6c80d61

8 files changed

+146
-18
lines changed

ui/ducks/app/app.test.js

+19
Original file line numberDiff line numberDiff line change
@@ -339,4 +339,23 @@ describe('App State', () => {
339339

340340
expect(state.showDataDeletionErrorModal).toStrictEqual(false);
341341
});
342+
343+
it('displays error in settings', () => {
344+
const state = reduceApp(metamaskState, {
345+
type: actions.SHOW_SETTINGS_PAGE_ERROR,
346+
payload: 'settings page error',
347+
});
348+
349+
expect(state.errorInSettings).toStrictEqual('settings page error');
350+
});
351+
352+
it('hides error in settings', () => {
353+
const displayErrorInSettings = { errorInSettings: 'settings page error' };
354+
const oldState = { ...metamaskState, ...displayErrorInSettings };
355+
const state = reduceApp(oldState, {
356+
type: actions.HIDE_SETTINGS_PAGE_ERROR,
357+
});
358+
359+
expect(state.errorInSettings).toBeNull();
360+
});
342361
});

ui/ducks/app/app.ts

+33
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ type AppState = {
105105
snapsInstallPrivacyWarningShown: boolean;
106106
isAddingNewNetwork: boolean;
107107
isMultiRpcOnboarding: boolean;
108+
errorInSettings: string | null;
108109
};
109110

110111
export type AppSliceState = {
@@ -192,6 +193,7 @@ const initialState: AppState = {
192193
snapsInstallPrivacyWarningShown: false,
193194
isAddingNewNetwork: false,
194195
isMultiRpcOnboarding: false,
196+
errorInSettings: null,
195197
};
196198

197199
export default function reduceApp(
@@ -632,6 +634,16 @@ export default function reduceApp(
632634
...appState,
633635
showDataDeletionErrorModal: false,
634636
};
637+
case actionConstants.SHOW_SETTINGS_PAGE_ERROR:
638+
return {
639+
...appState,
640+
errorInSettings: action.payload,
641+
};
642+
case actionConstants.HIDE_SETTINGS_PAGE_ERROR:
643+
return {
644+
...appState,
645+
errorInSettings: null,
646+
};
635647
///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps)
636648
case actionConstants.SHOW_KEYRING_SNAP_REMOVAL_RESULT:
637649
return {
@@ -720,6 +732,27 @@ export function setCustomTokenAmount(payload: string): PayloadAction<string> {
720732
return { type: actionConstants.SET_CUSTOM_TOKEN_AMOUNT, payload };
721733
}
722734

735+
/**
736+
* An action creator for display a error to the user in various places in the
737+
* UI. It will not be cleared until a new warning replaces it or `hideWarning`
738+
* is called.
739+
*
740+
* @param payload - The warning to show.
741+
* @returns The action to display the warning.
742+
*/
743+
export function displayErrorInSettings(payload: string): PayloadAction<string> {
744+
return {
745+
type: actionConstants.SHOW_SETTINGS_PAGE_ERROR,
746+
payload,
747+
};
748+
}
749+
750+
export function hideErrorInSettings() {
751+
return {
752+
type: actionConstants.HIDE_SETTINGS_PAGE_ERROR,
753+
};
754+
}
755+
723756
// Selectors
724757
export function getQrCodeData(state: AppSliceState): {
725758
type?: string | null;

ui/pages/settings/advanced-tab/__snapshots__/advanced-tab.component.test.js.snap

+1
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ exports[`AdvancedTab Component should match snapshot 1`] = `
2929
>
3030
<button
3131
class="button btn--rounded btn-secondary btn--large"
32+
data-testid="advanced-setting-state-logs-button"
3233
>
3334
Download state logs
3435
</button>

ui/pages/settings/advanced-tab/advanced-tab.component.js

+22-12
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,10 @@ export default class AdvancedTab extends PureComponent {
4040
setUseNonceField: PropTypes.func,
4141
useNonceField: PropTypes.bool,
4242
setHexDataFeatureFlag: PropTypes.func,
43-
displayWarning: PropTypes.func,
43+
displayErrorInSettings: PropTypes.func,
44+
hideErrorInSettings: PropTypes.func,
4445
showResetAccountConfirmationModal: PropTypes.func,
45-
warning: PropTypes.string,
46+
errorInSettings: PropTypes.string,
4647
sendHexData: PropTypes.bool,
4748
showFiatInTestnets: PropTypes.bool,
4849
showTestNetworks: PropTypes.bool,
@@ -80,7 +81,9 @@ export default class AdvancedTab extends PureComponent {
8081

8182
componentDidMount() {
8283
const { t } = this.context;
84+
const { hideErrorInSettings } = this.props;
8385
handleSettingsRefs(t, t('advanced'), this.settingsRefs);
86+
hideErrorInSettings();
8487
}
8588

8689
async getTextFromFile(file) {
@@ -112,7 +115,7 @@ export default class AdvancedTab extends PureComponent {
112115

113116
renderStateLogs() {
114117
const { t } = this.context;
115-
const { displayWarning } = this.props;
118+
const { displayErrorInSettings } = this.props;
116119

117120
return (
118121
<Box
@@ -133,16 +136,21 @@ export default class AdvancedTab extends PureComponent {
133136
<Button
134137
type="secondary"
135138
large
139+
data-testid="advanced-setting-state-logs-button"
136140
onClick={() => {
137-
window.logStateString((err, result) => {
141+
window.logStateString(async (err, result) => {
138142
if (err) {
139-
displayWarning(t('stateLogError'));
143+
displayErrorInSettings(t('stateLogError'));
140144
} else {
141-
exportAsFile(
142-
`${t('stateLogFileName')}.json`,
143-
result,
144-
ExportableContentType.JSON,
145-
);
145+
try {
146+
await exportAsFile(
147+
`${t('stateLogFileName')}.json`,
148+
result,
149+
ExportableContentType.JSON,
150+
);
151+
} catch (error) {
152+
displayErrorInSettings(error.message);
153+
}
146154
}
147155
});
148156
}}
@@ -576,11 +584,13 @@ export default class AdvancedTab extends PureComponent {
576584
}
577585

578586
render() {
579-
const { warning } = this.props;
587+
const { errorInSettings } = this.props;
580588
// When adding/removing/editing the order of renders, double-check the order of the settingsRefs. This affects settings-search.js
581589
return (
582590
<div className="settings-page__body">
583-
{warning ? <div className="settings-tab__error">{warning}</div> : null}
591+
{errorInSettings ? (
592+
<div className="settings-tab__error">{errorInSettings}</div>
593+
) : null}
584594
{this.renderStateLogs()}
585595
{this.renderResetAccount()}
586596
{this.renderToggleStxOptIn()}

ui/pages/settings/advanced-tab/advanced-tab.component.test.js

+58-1
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,17 @@
11
import React from 'react';
2-
import { fireEvent } from '@testing-library/react';
2+
import { fireEvent, waitFor } from '@testing-library/react';
33
import configureMockStore from 'redux-mock-store';
44
import thunk from 'redux-thunk';
55
import mockState from '../../../../test/data/mock-state.json';
66
import { renderWithProvider } from '../../../../test/lib/render-helpers';
7+
import { exportAsFile } from '../../../helpers/utils/export-utils';
78
import AdvancedTab from '.';
89

910
const mockSetAutoLockTimeLimit = jest.fn().mockReturnValue({ type: 'TYPE' });
1011
const mockSetShowTestNetworks = jest.fn();
1112
const mockSetShowFiatConversionOnTestnetsPreference = jest.fn();
1213
const mockSetStxPrefEnabled = jest.fn();
14+
const mockDisplayErrorInSettings = jest.fn();
1315

1416
jest.mock('../../../store/actions.ts', () => {
1517
return {
@@ -21,6 +23,32 @@ jest.mock('../../../store/actions.ts', () => {
2123
};
2224
});
2325

26+
jest.mock('../../../ducks/app/app.ts', () => ({
27+
displayErrorInSettings: () => mockDisplayErrorInSettings,
28+
hideErrorInSettings: () => jest.fn(),
29+
}));
30+
31+
jest.mock('../../../helpers/utils/export-utils', () => ({
32+
...jest.requireActual('../../../helpers/utils/export-utils'),
33+
exportAsFile: jest
34+
.fn()
35+
.mockResolvedValueOnce({})
36+
.mockImplementationOnce(new Error('state file error')),
37+
}));
38+
39+
jest.mock('webextension-polyfill', () => ({
40+
runtime: {
41+
getPlatformInfo: jest.fn().mockResolvedValue('mac'),
42+
},
43+
}));
44+
45+
Object.defineProperty(window, 'stateHooks', {
46+
value: {
47+
getCleanAppState: () => mockState,
48+
getLogs: () => [],
49+
},
50+
});
51+
2452
describe('AdvancedTab Component', () => {
2553
const mockStore = configureMockStore([thunk])(mockState);
2654

@@ -105,4 +133,33 @@ describe('AdvancedTab Component', () => {
105133
expect(mockSetStxPrefEnabled).toHaveBeenCalled();
106134
});
107135
});
136+
137+
describe('renderStateLogs', () => {
138+
it('should render the toggle button for state log download', () => {
139+
const { queryByTestId } = renderWithProvider(<AdvancedTab />, mockStore);
140+
const stateLogButton = queryByTestId('advanced-setting-state-logs');
141+
expect(stateLogButton).toBeInTheDocument();
142+
});
143+
144+
it('should call exportAsFile when the toggle button is clicked', async () => {
145+
const { queryByTestId } = renderWithProvider(<AdvancedTab />, mockStore);
146+
const stateLogButton = queryByTestId(
147+
'advanced-setting-state-logs-button',
148+
);
149+
fireEvent.click(stateLogButton);
150+
await waitFor(() => {
151+
expect(exportAsFile).toHaveBeenCalledTimes(1);
152+
});
153+
});
154+
it('should call displayErrorInSettings when the state file download fails', async () => {
155+
const { queryByTestId } = renderWithProvider(<AdvancedTab />, mockStore);
156+
const stateLogButton = queryByTestId(
157+
'advanced-setting-state-logs-button',
158+
);
159+
fireEvent.click(stateLogButton);
160+
await waitFor(() => {
161+
expect(mockDisplayErrorInSettings).toHaveBeenCalledTimes(1);
162+
});
163+
});
164+
});
108165
});

ui/pages/settings/advanced-tab/advanced-tab.container.js

+9-4
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import { DEFAULT_AUTO_LOCK_TIME_LIMIT } from '../../../../shared/constants/prefe
55
import { getPreferences } from '../../../selectors';
66
import {
77
backupUserData,
8-
displayWarning,
98
setAutoLockTimeLimit,
109
setDismissSeedBackUpReminder,
1110
setFeatureFlag,
@@ -17,11 +16,15 @@ import {
1716
showModal,
1817
} from '../../../store/actions';
1918
import { getSmartTransactionsPreferenceEnabled } from '../../../../shared/modules/selectors';
19+
import {
20+
displayErrorInSettings,
21+
hideErrorInSettings,
22+
} from '../../../ducks/app/app';
2023
import AdvancedTab from './advanced-tab.component';
2124

2225
export const mapStateToProps = (state) => {
2326
const {
24-
appState: { warning },
27+
appState: { errorInSettings },
2528
metamask,
2629
} = state;
2730
const {
@@ -37,7 +40,7 @@ export const mapStateToProps = (state) => {
3740
} = getPreferences(state);
3841

3942
return {
40-
warning,
43+
errorInSettings,
4144
sendHexData,
4245
showFiatInTestnets,
4346
showTestNetworks,
@@ -54,7 +57,9 @@ export const mapDispatchToProps = (dispatch) => {
5457
backupUserData: () => backupUserData(),
5558
setHexDataFeatureFlag: (shouldShow) =>
5659
dispatch(setFeatureFlag('sendHexData', shouldShow)),
57-
displayWarning: (warning) => dispatch(displayWarning(warning)),
60+
displayErrorInSettings: (errorInSettings) =>
61+
dispatch(displayErrorInSettings(errorInSettings)),
62+
hideErrorInSettings: () => dispatch(hideErrorInSettings()),
5863
showResetAccountConfirmationModal: () =>
5964
dispatch(showModal({ name: 'CONFIRM_RESET_ACCOUNT' })),
6065
setUseNonceField: (value) => dispatch(setUseNonceField(value)),

ui/pages/settings/advanced-tab/advanced-tab.stories.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ export default {
2222
setDismissSeedBackUpReminder: { action: 'setDismissSeedBackUpReminder' },
2323
setUseNonceField: { action: 'setUseNonceField' },
2424
setHexDataFeatureFlag: { action: 'setHexDataFeatureFlag' },
25-
displayWarning: { action: 'displayWarning' },
25+
displayErrorInSettings: { action: 'displayErrorInSettings' },
26+
hideErrorInSettings: { action: 'hideErrorInSettings' },
2627
history: { action: 'history' },
2728
showResetAccountConfirmationModal: {
2829
action: 'showResetAccountConfirmationModal',

ui/store/actionConstants.ts

+2
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ export const LOCK_METAMASK = 'LOCK_METAMASK';
4949
// error handling
5050
export const DISPLAY_WARNING = 'DISPLAY_WARNING';
5151
export const HIDE_WARNING = 'HIDE_WARNING';
52+
export const SHOW_SETTINGS_PAGE_ERROR = 'SHOW_SETTINGS_PAGE_ERROR';
53+
export const HIDE_SETTINGS_PAGE_ERROR = 'HIDE_SETTINGS_PAGE_ERROR';
5254
export const CAPTURE_SINGLE_EXCEPTION = 'CAPTURE_SINGLE_EXCEPTION';
5355
// accounts screen
5456
export const SHOW_ACCOUNTS_PAGE = 'SHOW_ACCOUNTS_PAGE';

0 commit comments

Comments
 (0)