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.

Sign up for GitHub

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

New: Specific error messages for unsupported/tariff restricted files #593

Merged
merged 1 commit into from
Jan 20, 2018
Merged

New: Specific error messages for unsupported/tariff restricted files #593

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 5 additions & 3 deletions src/i18n/en-US.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@ text_truncated=This file has been truncated due to size limits. Please download

# Error messages
# Default preview error message
error_default=We're sorry, the preview didn't load. This file type may not be supported.
error_generic=We're sorry, the preview didn't load.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on language for the new messages?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my limited experience refreshing the page seems to fix the issue when this error occurs. If that is usually the case maybe append something about refreshing?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My opinion is to leave it as is. If we put the expectation that refreshing could potentially fix the issue, it's another thing that can muddy external bug reports.

# Default preview error message
error_unsupported=We're sorry, the preview didn't load. {1} files are not currently supported.
# Account doesn't have a sufficient tariff to preview the requested file type
error_account=We're sorry, your account is unable to preview this file type.
# No permissions preview error message
error_permissions=We're sorry, you don't have permission to preview this file.
# Preview refresh error message suggesting refreshing the page as a possible fix
Expand All @@ -48,8 +52,6 @@ error_rate_limit=We're sorry, the preview didn't load because your request was r
error_reupload=We're sorry, the preview didn't load. Please re-upload the file or contact Box support.
# Preview error message shown when the user's browser doesn't support previews of this file type
error_browser_unsupported=We're sorry, your browser doesn't support preview for {1}.
# Preview error message shown when an iWork file is previewed
error_iwork=We're sorry, iWork files are not currently supported.
# Preview error message shown when document loading fails (most likely due to password or watermark)
error_document=We're sorry, the preview didn't load. This document may be protected.
# Preview error message shown when the document is password protected
Expand Down
18 changes: 13 additions & 5 deletions src/lib/Preview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ import {
openUrlInsideIframe,
getHeaders,
findScriptLocation,
appendQueryParams
appendQueryParams,
replacePlaceholders
} from './util';
import {
getURL,
Expand All @@ -34,7 +35,6 @@ import {
API_HOST,
APP_HOST,
CLASS_NAVIGATION_VISIBILITY,
FILE_EXT_ERROR_MAP,
PERMISSION_DOWNLOAD,
PERMISSION_PREVIEW,
PREVIEW_SCRIPT_NAME,
Expand Down Expand Up @@ -898,10 +898,18 @@ class Preview extends EventEmitter {
// Determine the asset loader to use
const loader = this.getLoader(this.file);

// If no loader then throw an unsupported error
// If file type specific error message, throw the generic one
// If no loader, then check to see if any of our viewers support this file type.
// If they do, we know the account can't preview this file type. If they can't we know this file type is unsupported.
if (!loader) {
throw new Error(FILE_EXT_ERROR_MAP[this.file.extension] || __('error_default'));
const isFileTypeSupported = this.getViewers().find((viewer) => {
return viewer.EXT.indexOf(this.file.extension) > -1;
});

throw new Error(
isFileTypeSupported
? __('error_account')
: replacePlaceholders(__('error_unsupported'), [`.${this.file.extension}`])
);
}

// Determine the viewer to use
Expand Down
14 changes: 6 additions & 8 deletions src/lib/__tests__/Preview-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1325,7 +1325,7 @@ describe('lib/Preview', () => {
try {
preview.loadViewer();
} catch (e) {
expect(spy.threw('Error'));
expect(e.message).to.equal(__('error_permissions'));
}
});

Expand Down Expand Up @@ -1366,27 +1366,25 @@ describe('lib/Preview', () => {
expect(stubs.showLoadingDownloadButton).to.be.called;
});

it('should throw a generic error if there is no loader for general file types', () => {
it('should throw an unsupported error if there is no loader for general file types', () => {
preview.file.extension = 'zip';
stubs.getLoader.returns(undefined);
const spy = sandbox.spy(preview, 'loadViewer');

try {
preview.loadViewer();
} catch (e) {
expect(spy.threw('Error', __('error_default')));
expect(e.message).to.equal(util.replacePlaceholders(__('error_unsupported'), [`.${preview.file.extension}`]));
}
});

it('should throw a specific error if there is no loader for a specific file type', () => {
preview.file.extension = 'key';
it('should throw an account upgrade error if there is no loader but the file type is supported', () => {
preview.file.extension = 'mp4';
stubs.getLoader.returns(undefined);
const spy = sandbox.spy(preview, 'loadViewer');

try {
preview.loadViewer();
} catch (e) {
expect(spy.threw('Error', __('error_iwork')));
expect(e.message).to.equal(__('error_account'));
}
});

Expand Down
7 changes: 0 additions & 7 deletions src/lib/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,4 @@ export const MODEL3D_STATIC_ASSETS_VERSION = '1.12.0';
export const SWF_STATIC_ASSETS_VERSION = '0.112.0';
export const TEXT_STATIC_ASSETS_VERSION = '0.114.0';

// Maps file extension to error message.
export const FILE_EXT_ERROR_MAP = {
numbers: __('error_iwork'),
pages: __('error_iwork'),
key: __('error_iwork')
};

export const PREVIEW_SCRIPT_NAME = 'preview.js';
2 changes: 1 addition & 1 deletion src/lib/util.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,7 @@ export function findScriptLocation(name, script) {
*
* @public
* @param {string} string - String to be interpolated
* @param {string[]} placeholderValues - Custom values to replace into string
* @param {Array} placeholderValues - Ordered array of replacement strings
* @return {string} Properly translated string with replaced custom variable
*/
export function replacePlaceholders(string, placeholderValues) {
Expand Down
2 changes: 1 addition & 1 deletion src/lib/viewers/box3d/model3d/Model3DRenderer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ class Model3DRenderer extends Box3DRenderer {
* @return {void}
*/
onUnsupportedRepresentation() {
this.emit('error', new Error(__('error_default')));
this.emit('error', new Error(__('error_unsupported')));
}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/lib/viewers/error/PreviewErrorViewer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,12 @@ class PreviewErrorViewer extends BaseViewer {
}

/* eslint-disable no-param-reassign */
err = err instanceof Error ? err : new Error(__('error_default'));
err = err instanceof Error ? err : new Error(__('error_generic'));
/* eslint-enable no-param-reassign */

// If there is no display message fallback to the message from above
let displayMessage = err.displayMessage || err.message;
displayMessage = typeof displayMessage === 'string' ? displayMessage : __('error_default');
displayMessage = typeof displayMessage === 'string' ? displayMessage : __('error_generic');

this.iconEl.innerHTML = this.icon;
this.messageEl.textContent = displayMessage;
Expand Down
2 changes: 1 addition & 1 deletion src/lib/viewers/error/__tests__/PreviewErrorViewer-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ describe('lib/viewers/error/PreviewErrorViewer', () => {

error.load(err);

expect(error.messageEl.textContent).to.equal(__('error_default'));
expect(error.messageEl.textContent).to.equal(__('error_generic'));
});

it('should not add download button if the browser cannot download', () => {
Expand Down