Skip to content

Commit 05bc2bb

Browse files
tofumatttofumatt
authored
Merge pull request #6047 from google/enhancement/5494-followup
Add "Retry" button to regular JS error notices in the plugin - followup
2 parents b053f9b + 7371e97 commit 05bc2bb

File tree

10 files changed

+448
-208
lines changed

10 files changed

+448
-208
lines changed

‎assets/js/components/ErrorNotice.js

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,21 +35,30 @@ import { isPermissionScopeError, isErrorRetryable } from '../util/errors';
3535
import ErrorText from './ErrorText';
3636
import Button from './Button';
3737

38-
const { useDispatch } = Data;
38+
const { useSelect, useDispatch } = Data;
3939

4040
export default function ErrorNotice( {
4141
error,
42+
storeName,
43+
message = error.message,
4244
shouldDisplayError = () => true,
4345
} ) {
4446
const dispatch = useDispatch();
4547

48+
const selectorData = useSelect( ( select ) => {
49+
if ( ! storeName ) {
50+
return null;
51+
}
52+
53+
return select( storeName ).getSelectorDataForError( error );
54+
} );
55+
4656
const handleRetry = useCallback( () => {
47-
const { selectorData } = error;
4857
dispatch( selectorData.storeName ).invalidateResolution(
4958
selectorData.name,
5059
selectorData.args
5160
);
52-
}, [ dispatch, error ] );
61+
}, [ dispatch, selectorData ] );
5362

5463
// Do not display if there is no error, or if the error is for missing scopes.
5564
if (
@@ -60,12 +69,12 @@ export default function ErrorNotice( {
6069
return null;
6170
}
6271

63-
const shouldDisplayRetry = isErrorRetryable( error );
72+
const shouldDisplayRetry = isErrorRetryable( error, selectorData );
6473

6574
return (
6675
<Fragment>
6776
<ErrorText
68-
message={ error.message }
77+
message={ message }
6978
reconnectURL={ error.data?.reconnectURL }
7079
/>
7180
{ shouldDisplayRetry && (
@@ -81,5 +90,7 @@ ErrorNotice.propTypes = {
8190
error: PropTypes.shape( {
8291
message: PropTypes.string,
8392
} ),
93+
storeName: PropTypes.string,
94+
message: PropTypes.string,
8495
shouldDisplayError: PropTypes.func,
8596
};

‎assets/js/components/ErrorNotice.test.js

Lines changed: 93 additions & 161 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,18 @@
1919
/**
2020
* Internal dependencies
2121
*/
22-
import { createTestRegistry, provideModules } from '../../../tests/js/utils';
22+
import {
23+
createTestRegistry,
24+
provideModules,
25+
untilResolved,
26+
} from '../../../tests/js/utils';
2327
import {
2428
ERROR_CODE_MISSING_REQUIRED_SCOPE,
2529
ERROR_REASON_INSUFFICIENT_PERMISSIONS,
2630
} from '../util/errors';
2731
import { fireEvent, render } from '../../../tests/js/test-utils';
2832
import ErrorNotice from './ErrorNotice';
29-
import { MODULES_ANALYTICS } from '../modules/analytics/datastore/constants';
33+
import { MODULES_TAGMANAGER } from '../modules/tagmanager/datastore/constants';
3034

3135
describe( 'ErrorNotice', () => {
3236
let registry;
@@ -39,7 +43,7 @@ describe( 'ErrorNotice', () => {
3943
{ slug: moduleName, name: 'Test Module' },
4044
] );
4145
invalidateResolutionSpy = jest.spyOn(
42-
registry.dispatch( MODULES_ANALYTICS ),
46+
registry.dispatch( MODULES_TAGMANAGER ),
4347
'invalidateResolution'
4448
);
4549
} );
@@ -48,161 +52,105 @@ describe( 'ErrorNotice', () => {
4852
invalidateResolutionSpy.mockReset();
4953
} );
5054

51-
it( "should not render the `Retry` button if the error's `selectorData.name` is not `getReport`", () => {
52-
const { queryByText } = render(
53-
<ErrorNotice
54-
error={ {
55-
code: 'test-error-code',
56-
message: 'Test error message',
57-
data: {
58-
reason: '',
59-
},
60-
selectorData: {
61-
args: [],
62-
name: 'getAccountID',
63-
storeName: MODULES_ANALYTICS,
64-
},
65-
} }
66-
/>,
55+
async function renderErrorNotice( { error, storeName } ) {
56+
fetchMock.get(
57+
/^\/google-site-kit\/v1\/modules\/tagmanager\/data\/accounts/,
6758
{
68-
registry,
59+
body: error,
60+
status: 403,
6961
}
7062
);
7163

72-
expect( queryByText( /retry/i ) ).not.toBeInTheDocument();
73-
} );
64+
registry.select( MODULES_TAGMANAGER ).getAccounts();
65+
66+
await untilResolved( registry, MODULES_TAGMANAGER ).getAccounts();
67+
68+
expect( console ).toHaveErrored();
69+
70+
const selectorError = registry
71+
.select( MODULES_TAGMANAGER )
72+
.getError( 'getAccounts', [] );
7473

75-
it( 'should not render the `Retry` button if the error reason is `ERROR_REASON_INSUFFICIENT_PERMISSIONS`', () => {
76-
const { queryByText } = render(
77-
<ErrorNotice
78-
error={ {
79-
code: 'test-error-code',
80-
message: 'Test error message',
81-
data: {
82-
reason: ERROR_REASON_INSUFFICIENT_PERMISSIONS,
83-
},
84-
selectorData: {
85-
args: [],
86-
name: 'getReport',
87-
storeName: MODULES_ANALYTICS,
88-
},
89-
} }
90-
/>,
74+
return render(
75+
<ErrorNotice error={ selectorError } storeName={ storeName } />,
9176
{
9277
registry,
9378
}
9479
);
80+
}
81+
82+
it( 'should not render the `Retry` button if the error reason is `ERROR_REASON_INSUFFICIENT_PERMISSIONS`', async () => {
83+
const { queryByText } = await renderErrorNotice( {
84+
error: {
85+
code: 'test-error-code',
86+
message: 'Test error message',
87+
data: {
88+
reason: ERROR_REASON_INSUFFICIENT_PERMISSIONS,
89+
},
90+
},
91+
storeName: MODULES_TAGMANAGER,
92+
} );
9593

9694
expect( queryByText( /retry/i ) ).not.toBeInTheDocument();
9795
} );
9896

99-
it( 'should not render the `Retry` button if the error reason is `ERROR_CODE_MISSING_REQUIRED_SCOPE`', () => {
100-
const { queryByText } = render(
101-
<ErrorNotice
102-
error={ {
103-
code: ERROR_CODE_MISSING_REQUIRED_SCOPE,
104-
message: 'Test error message',
105-
data: {
106-
reason: '',
107-
},
108-
selectorData: {
109-
args: [],
110-
name: 'getReport',
111-
storeName: MODULES_ANALYTICS,
112-
},
113-
} }
114-
/>,
115-
{
116-
registry,
117-
}
118-
);
97+
it( 'should not render the `Retry` button if the error reason is `ERROR_CODE_MISSING_REQUIRED_SCOPE`', async () => {
98+
const { queryByText } = await renderErrorNotice( {
99+
error: {
100+
code: ERROR_CODE_MISSING_REQUIRED_SCOPE,
101+
message: 'Test error message',
102+
data: {
103+
reason: '',
104+
},
105+
},
106+
storeName: MODULES_TAGMANAGER,
107+
} );
119108

120109
expect( queryByText( /retry/i ) ).not.toBeInTheDocument();
121110
} );
122111

123-
it( 'should not render the `Retry` button if the error is an auth error', () => {
124-
const { queryByText } = render(
125-
<ErrorNotice
126-
error={ {
127-
code: 'test-error-code',
128-
message: 'Test error message',
129-
data: {
130-
reason: '',
131-
reconnectURL: 'example.com',
132-
},
133-
selectorData: {
134-
args: [],
135-
name: 'getReport',
136-
storeName: MODULES_ANALYTICS,
137-
},
138-
} }
139-
/>,
140-
{
141-
registry,
142-
}
143-
);
112+
it( 'should not render the `Retry` button if the error is an auth error', async () => {
113+
const { queryByText } = await renderErrorNotice( {
114+
error: {
115+
code: 'test-error-code',
116+
message: 'Test error message',
117+
data: {
118+
reason: '',
119+
reconnectURL: 'example.com',
120+
},
121+
},
122+
storeName: MODULES_TAGMANAGER,
123+
} );
144124

145125
expect( queryByText( /retry/i ) ).not.toBeInTheDocument();
146126
} );
147127

148-
it( 'should render the `Retry` button if the error is retryable', () => {
149-
const { queryByText } = render(
150-
<ErrorNotice
151-
error={ {
152-
code: 'test-error-code',
153-
message: 'Test error message',
154-
data: {
155-
reason: '',
156-
},
157-
selectorData: {
158-
args: [
159-
{
160-
dimensions: [ 'ga:date' ],
161-
metrics: [ { expression: 'ga:users' } ],
162-
startDate: '2020-08-11',
163-
endDate: '2020-09-07',
164-
},
165-
],
166-
name: 'getReport',
167-
storeName: MODULES_ANALYTICS,
168-
},
169-
} }
170-
/>,
171-
{
172-
registry,
173-
}
174-
);
128+
it( 'should render the `Retry` button if the error is retryable', async () => {
129+
const { queryByText } = await renderErrorNotice( {
130+
error: {
131+
code: 'test-error-code',
132+
message: 'Test error message',
133+
data: {
134+
reason: '',
135+
},
136+
},
137+
storeName: MODULES_TAGMANAGER,
138+
} );
175139

176140
expect( queryByText( /retry/i ) ).toBeInTheDocument();
177141
} );
178142

179-
it( 'should dispatch the `invalidateResolution` if the error is retryable', () => {
180-
const { queryByText, getByRole } = render(
181-
<ErrorNotice
182-
error={ {
183-
code: 'test-error-code',
184-
message: 'Test error message',
185-
data: {
186-
reason: '',
187-
},
188-
selectorData: {
189-
args: [
190-
{
191-
dimensions: [ 'ga:date' ],
192-
metrics: [ { expression: 'ga:users' } ],
193-
startDate: '2020-08-11',
194-
endDate: '2020-09-07',
195-
},
196-
],
197-
name: 'getReport',
198-
storeName: MODULES_ANALYTICS,
199-
},
200-
} }
201-
/>,
202-
{
203-
registry,
204-
}
205-
);
143+
it( 'should dispatch the `invalidateResolution` if the error is retryable', async () => {
144+
const { queryByText, getByRole } = await renderErrorNotice( {
145+
error: {
146+
code: 'test-error-code',
147+
message: 'Test error message',
148+
data: {
149+
reason: '',
150+
},
151+
},
152+
storeName: MODULES_TAGMANAGER,
153+
} );
206154

207155
expect( queryByText( /retry/i ) ).toBeInTheDocument();
208156

@@ -211,34 +159,18 @@ describe( 'ErrorNotice', () => {
211159
expect( invalidateResolutionSpy ).toHaveBeenCalledTimes( 1 );
212160
} );
213161

214-
it( 'should not render the retry button if the store name is not available', () => {
215-
const { queryByText } = render(
216-
<ErrorNotice
217-
error={ {
218-
code: 'test-error-code',
219-
message: 'Test error message',
220-
data: {
221-
reason: '',
222-
reconnectURL: 'example.com',
223-
},
224-
selectorData: {
225-
args: [
226-
{
227-
dimensions: [ 'ga:date' ],
228-
metrics: [ { expression: 'ga:users' } ],
229-
startDate: '2020-08-11',
230-
endDate: '2020-09-07',
231-
},
232-
],
233-
name: 'getReport',
234-
storeName: '',
235-
},
236-
} }
237-
/>,
238-
{
239-
registry,
240-
}
241-
);
162+
it( 'should not render the retry button if the store name is not available', async () => {
163+
const { queryByText } = await renderErrorNotice( {
164+
error: {
165+
code: 'test-error-code',
166+
message: 'Test error message',
167+
data: {
168+
reason: '',
169+
reconnectURL: 'example.com',
170+
},
171+
},
172+
storeName: '',
173+
} );
242174

243175
expect( queryByText( /retry/i ) ).not.toBeInTheDocument();
244176
} );

‎assets/js/components/ReportError.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,11 @@ export default function ReportError( { moduleSlug, error } ) {
6262

6363
const errors = Array.isArray( error ) ? error : [ error ];
6464

65-
const retryableErrors = errors.filter( isErrorRetryable );
65+
const retryableErrors = errors.filter(
66+
( err ) =>
67+
isErrorRetryable( err, err.selectorData ) &&
68+
err.selectorData.name === 'getReport'
69+
);
6670

6771
const showRetry = !! retryableErrors.length;
6872

0 commit comments

Comments
 (0)