Skip to content

Commit a6cde28

Browse files
priyajeetmergify[bot]
authored andcommitted
feat(preview): better message when blocked by a policy (#1346)
* feat(preview): better message when blocked by a policy * fix: PR feedback
1 parent 42aaa6e commit a6cde28

File tree

10 files changed

+111
-43
lines changed

10 files changed

+111
-43
lines changed

i18n/en-US.properties

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,8 @@ be.pointAnnotation = Point annotation mode
320320
be.preview = Preview
321321
# Error message when Preview fails due to the files call.
322322
be.previewError = We’re sorry, the preview didn’t load. Please refresh the page.
323+
# Error message when Preview fails due to the files call which is blocked by an access policy.
324+
be.previewErrorBlockedByPolicy = Your access is restricted due to the classification applied to this content.
323325
# Message when new preview is available.
324326
be.previewUpdate = A new version of this file is available.
325327
# Previous file button title

src/constants.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,7 @@ export const ERROR_CODE_UPLOAD_STORAGE_LIMIT_EXCEEDED = 'storage_limit_exceeded'
200200
export const ERROR_CODE_UPLOAD_FILE_SIZE_LIMIT_EXCEEDED = 'file_size_limit_exceeded';
201201
export const ERROR_CODE_UPLOAD_PENDING_APP_FOLDER_SIZE_LIMIT = 'pending_app_folder_size_limit';
202202
export const ERROR_CODE_FETCH_FILE = 'fetch_file_error';
203+
export const ERROR_CODE_FETCH_FILE_DUE_TO_POLICY = 'forbidden_by_policy';
203204
export const ERROR_CODE_FETCH_FOLDER = 'fetch_folder_error';
204205
export const ERROR_CODE_FETCH_CLASSIFICATION = 'fetch_classification_error';
205206
export const ERROR_CODE_FETCH_COMMENTS = 'fetch_comments_error';

src/elements/common/flowTypes.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// @flow
2+
3+
type ErrorType = {
4+
code: string,
5+
details?: Object,
6+
displayMessage?: string,
7+
message?: string,
8+
};
9+
10+
// eslint-disable-next-line import/prefer-default-export
11+
export type { ErrorType };

src/elements/common/messages.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,11 @@ const messages = defineMessages({
3737
description: 'Error message when Preview fails due to the files call.',
3838
defaultMessage: 'We’re sorry, the preview didn’t load. Please refresh the page.',
3939
},
40+
previewErrorBlockedByPolicy: {
41+
id: 'be.previewErrorBlockedByPolicy',
42+
description: 'Error message when Preview fails due to the files call which is blocked by an access policy.',
43+
defaultMessage: 'Your access is restricted due to the classification applied to this content.',
44+
},
4045
previewUpdate: {
4146
id: 'be.previewUpdate',
4247
description: 'Message when new preview is available.',

src/elements/content-preview/ContentPreview.js

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import {
4646
ORIGIN_CONTENT_PREVIEW,
4747
ERROR_CODE_UNKNOWN,
4848
} from '../../constants';
49+
import type { ErrorType } from '../common/flowTypes';
4950
import '../common/fonts.scss';
5051
import '../common/base.scss';
5152
import './ContentPreview.scss';
@@ -92,8 +93,8 @@ type Props = {
9293

9394
type State = {
9495
currentFileId?: string,
96+
error?: ErrorType,
9597
file?: BoxItem,
96-
isFileError: boolean,
9798
isReloadNotificationVisible: boolean,
9899
isThumbnailSidebarOpen: boolean,
99100
prevFileIdProp?: string, // the previous value of the "fileId" prop. Needed to implement getDerivedStateFromProps
@@ -129,13 +130,8 @@ type PreviewMetrics = {
129130
value: number,
130131
};
131132

132-
type PreviewError = {
133-
error: {
134-
code: string,
135-
details: Object,
136-
displayMessage: string,
137-
message: string,
138-
},
133+
type PreviewLibraryError = {
134+
error: ErrorType,
139135
};
140136

141137
const InvalidIdError = new Error('Invalid id for Preview!');
@@ -170,7 +166,7 @@ class ContentPreview extends PureComponent<Props, State> {
170166
updateVersionToCurrent: ?() => void;
171167

172168
initialState: State = {
173-
isFileError: false,
169+
error: undefined,
174170
isReloadNotificationVisible: false,
175171
isThumbnailSidebarOpen: false,
176172
};
@@ -517,16 +513,17 @@ class ContentPreview extends PureComponent<Props, State> {
517513
/**
518514
* Handler for 'preview_error' preview event
519515
*
520-
* @param {PreviewError} previewError - the error data emitted from preview
516+
* @param {PreviewLibraryError} previewError - the error data emitted from preview
521517
* @return {void}
522518
*/
523-
onPreviewError = ({ error, ...rest }: PreviewError): void => {
519+
onPreviewError = ({ error, ...rest }: PreviewLibraryError): void => {
524520
const { code = ERROR_CODE_UNKNOWN } = error;
521+
const { onError } = this.props;
525522

526523
// In case of error, there should be no thumbnail sidebar to account for
527524
this.setState({ isThumbnailSidebarOpen: false });
528525

529-
this.props.onError(
526+
onError(
530527
error,
531528
code,
532529
{
@@ -797,8 +794,14 @@ class ContentPreview extends PureComponent<Props, State> {
797794
* @return {void}
798795
*/
799796
fetchFileErrorCallback = (fileError: ElementsXhrError, code: string): void => {
800-
this.setState({ isFileError: true });
801-
this.props.onError(fileError, code, {
797+
const { onError } = this.props;
798+
const errorCode = fileError.code || code;
799+
const error = {
800+
code: errorCode,
801+
message: fileError.message,
802+
};
803+
this.setState({ error, file: undefined });
804+
onError(fileError, errorCode, {
802805
error: fileError,
803806
});
804807
};
@@ -1102,8 +1105,8 @@ class ContentPreview extends PureComponent<Props, State> {
11021105
}: Props = this.props;
11031106

11041107
const {
1108+
error,
11051109
file,
1106-
isFileError,
11071110
isReloadNotificationVisible,
11081111
currentFileId,
11091112
isThumbnailSidebarOpen,
@@ -1122,6 +1125,7 @@ class ContentPreview extends PureComponent<Props, State> {
11221125
return null;
11231126
}
11241127

1128+
const errorCode = getProp(error, 'code');
11251129
const currentVersionId = getProp(file, 'file_version.id');
11261130
const selectedVersionId = getProp(selectedVersion, 'id', currentVersionId);
11271131
const onHeaderClose = currentVersionId === selectedVersionId ? onClose : this.updateVersionToCurrent;
@@ -1153,7 +1157,8 @@ class ContentPreview extends PureComponent<Props, State> {
11531157
) : (
11541158
<div className="bcpr-loading-wrapper">
11551159
<PreviewLoading
1156-
isLoading={!isFileError}
1160+
errorCode={errorCode}
1161+
isLoading={!errorCode}
11571162
loadingIndicatorProps={{
11581163
size: 'large',
11591164
}}

src/elements/content-preview/PreviewLoading.js

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,25 @@ import { FormattedMessage } from 'react-intl';
99
import IconFileDefault from '../../icons/file/IconFileDefault';
1010
import makeLoadable from '../../components/loading-indicator/makeLoadable';
1111
import messages from '../common/messages';
12+
import { ERROR_CODE_FETCH_FILE_DUE_TO_POLICY } from '../../constants';
1213
import './PreviewLoading.scss';
1314

14-
const PreviewLoading = () => (
15-
<div className="bcpr-loading">
16-
<IconFileDefault height={160} width={160} />
17-
<div className="bcpr-loading-text">
18-
<FormattedMessage {...messages.previewError} />
15+
type Props = {
16+
errorCode?: string,
17+
};
18+
19+
const PreviewLoading = ({ errorCode }: Props) => {
20+
const isBlockedByPolicy = errorCode === ERROR_CODE_FETCH_FILE_DUE_TO_POLICY;
21+
const message = isBlockedByPolicy ? messages.previewErrorBlockedByPolicy : messages.previewError;
22+
return (
23+
<div className="bcpr-PreviewLoading">
24+
<IconFileDefault height={160} width={160} />
25+
<div className="bcpr-PreviewLoading-message">
26+
<FormattedMessage {...message} />
27+
</div>
1928
</div>
20-
</div>
21-
);
29+
);
30+
};
2231

2332
export { PreviewLoading as PreviewLoadingComponent };
2433
export default makeLoadable(PreviewLoading);
Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
@import '../common/variables';
22

3-
.bcpr {
4-
.bcpr-loading {
5-
color: $twos;
6-
position: relative;
7-
text-align: center;
8-
z-index: 1;
3+
.bcpr-PreviewLoading {
4+
align-items: center;
5+
color: $bdl-gray;
6+
display: flex;
7+
flex-direction: column;
8+
text-align: center;
9+
}
910

10-
.bcpr-loading-text {
11-
font-size: 14px;
12-
}
13-
}
11+
.bcpr-PreviewLoading-message {
12+
font-size: 14px;
13+
margin-top: 20px;
14+
max-width: 400px;
1415
}

src/elements/content-preview/__tests__/ContentPreview-test.js

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -379,7 +379,7 @@ describe('elements/content-preview/ContentPreview', () => {
379379
test('should set state to the new file', () => {
380380
instance.fetchFileSuccessCallback(file);
381381
expect(instance.state.file).toEqual(file);
382-
expect(instance.state.isFileError).toEqual(false);
382+
expect(instance.state.error).toBeUndefined();
383383
expect(instance.state.isReloadNotificationVisible).toEqual(false);
384384
});
385385

@@ -390,7 +390,7 @@ describe('elements/content-preview/ContentPreview', () => {
390390
instance.fetchFileSuccessCallback(newFile);
391391

392392
expect(instance.state.file).toEqual(newFile);
393-
expect(instance.state.isFileError).toEqual(false);
393+
expect(instance.state.error).toBeUndefined();
394394
expect(instance.state.isReloadNotificationVisible).toEqual(false);
395395
});
396396

@@ -419,7 +419,7 @@ describe('elements/content-preview/ContentPreview', () => {
419419

420420
expect(instance.stagedFile).toEqual(newFile);
421421
expect(instance.state.file).toEqual(file);
422-
expect(instance.state.isFileError).toBeFalsy();
422+
expect(instance.state.error).toBeUndefined();
423423
expect(instance.state.isReloadNotificationVisible).toBeTruthy();
424424
});
425425
});
@@ -440,10 +440,19 @@ describe('elements/content-preview/ContentPreview', () => {
440440
error = new Error('foo');
441441
});
442442

443-
test('should set the file error state', () => {
444-
instance.fetchFileErrorCallback(error);
445-
expect(instance.state.isFileError).toEqual(true);
443+
test('should set the error state from the error object', () => {
444+
instance.fetchFileErrorCallback(error, 'code');
445+
expect(instance.state.error).toEqual({ code: 'code', message: 'foo' });
446446
expect(instance.fetchFile).not.toBeCalled();
447+
expect(instance.file).toBeUndefined();
448+
expect(onError).toHaveBeenCalled();
449+
});
450+
451+
test('should use the code from response if it exists', () => {
452+
instance.fetchFileErrorCallback({ code: 'specialCode', message: 'specialMessage' }, 'code');
453+
expect(instance.state.error).toEqual({ code: 'specialCode', message: 'specialMessage' });
454+
expect(instance.fetchFile).not.toBeCalled();
455+
expect(instance.file).toBeUndefined();
447456
expect(onError).toHaveBeenCalled();
448457
});
449458
});
@@ -669,7 +678,7 @@ describe('elements/content-preview/ContentPreview', () => {
669678
expect(instance.state.file).toEqual(file);
670679
expect(instance.stagedFile).toBeUndefined();
671680
expect(instance.state.isReloadNotificationVisible).toBeFalsy();
672-
expect(instance.state.isFileError).toBeFalsy();
681+
expect(instance.state.error).toBeUndefined();
673682
});
674683
});
675684

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,20 @@
11
import React from 'react';
22
import { shallow } from 'enzyme';
33
import { PreviewLoadingComponent as PreviewLoading } from '../PreviewLoading';
4+
import { ERROR_CODE_FETCH_FILE_DUE_TO_POLICY } from '../../../constants';
45

5-
const getWrapper = () => shallow(<PreviewLoading />);
6+
const getWrapper = props => shallow(<PreviewLoading {...props} />);
67

78
describe('elements/content-preview/PreviewLoading', () => {
89
describe('render()', () => {
910
test('should render correctly', () => {
1011
const wrapper = getWrapper();
1112
expect(wrapper).toMatchSnapshot();
1213
});
14+
15+
test('should render correctly when blocked by access policy', () => {
16+
const wrapper = getWrapper({ errorCode: ERROR_CODE_FETCH_FILE_DUE_TO_POLICY });
17+
expect(wrapper).toMatchSnapshot();
18+
});
1319
});
1420
});

src/elements/content-preview/__tests__/__snapshots__/PreviewLoading-test.js.snap

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,14 @@
22

33
exports[`elements/content-preview/PreviewLoading render() should render correctly 1`] = `
44
<div
5-
className="bcpr-loading"
5+
className="bcpr-PreviewLoading"
66
>
77
<IconFileDefault
88
height={160}
99
width={160}
1010
/>
1111
<div
12-
className="bcpr-loading-text"
12+
className="bcpr-PreviewLoading-message"
1313
>
1414
<FormattedMessage
1515
defaultMessage="We’re sorry, the preview didn’t load. Please refresh the page."
@@ -18,3 +18,22 @@ exports[`elements/content-preview/PreviewLoading render() should render correctl
1818
</div>
1919
</div>
2020
`;
21+
22+
exports[`elements/content-preview/PreviewLoading render() should render correctly when blocked by access policy 1`] = `
23+
<div
24+
className="bcpr-PreviewLoading"
25+
>
26+
<IconFileDefault
27+
height={160}
28+
width={160}
29+
/>
30+
<div
31+
className="bcpr-PreviewLoading-message"
32+
>
33+
<FormattedMessage
34+
defaultMessage="Your access is restricted due to the classification applied to this content."
35+
id="be.previewErrorBlockedByPolicy"
36+
/>
37+
</div>
38+
</div>
39+
`;

0 commit comments

Comments
 (0)