Skip to content

Commit

Permalink
Fix: Preview error viewer in platform and IE11 (#696)
Browse files Browse the repository at this point in the history 
- Contact Us redirect to Box support page should only show up in the Box web application
- Fix non-wrapping error text in IE11
- Consolidate download checks into util function
- Move file-specific util method into file.js
  • Loading branch information
@tonyjin
tonyjin authored Mar 7, 2018
1 parent b1a0e03 commit cc09a61
Show file tree
Hide file tree
Showing 12 changed files with 187 additions and 184 deletions.
29 changes: 16 additions & 13 deletions src/lib/Preview.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ import {
appendQueryParams,
replacePlaceholders,
stripAuthFromString,
isValidFileId
isValidFileId,
isBoxWebApp
} from './util';
import {
isDownloadHostBlocked,
Expand All @@ -42,13 +43,13 @@ import {
uncacheFile,
isWatermarked,
getCachedFile,
normalizeFileVersion
normalizeFileVersion,
canDownload
} from './file';
import {
API_HOST,
APP_HOST,
CLASS_NAVIGATION_VISIBILITY,
PERMISSION_DOWNLOAD,
PERMISSION_PREVIEW,
PREVIEW_SCRIPT_NAME,
X_REP_HINT_BASE,
Expand Down Expand Up @@ -475,7 +476,7 @@ class Preview extends EventEmitter {
* @return {void}
*/
print() {
if (checkPermission(this.file, PERMISSION_DOWNLOAD) && checkFeature(this.viewer, 'print')) {
if (canDownload(this.file, this.options) && checkFeature(this.viewer, 'print')) {
this.viewer.print();
}
}
Expand All @@ -489,7 +490,7 @@ class Preview extends EventEmitter {
download() {
const { apiHost, queryParams } = this.options;

if (!checkPermission(this.file, PERMISSION_DOWNLOAD)) {
if (!canDownload(this.file, this.options)) {
return;
}

Expand Down Expand Up @@ -961,10 +962,13 @@ class Preview extends EventEmitter {

// If file is not downloadable, trigger an error
if (file.is_download_available === false) {
const error = new PreviewError(ERROR_CODE.NOT_DOWNLOADABLE, __('error_not_downloadable'), {
linkText: __('link_contact_us'),
linkUrl: SUPPORT_URL
});
const details = isBoxWebApp()
? {
linkText: __('link_contact_us'),
linkUrl: SUPPORT_URL
}
: {};
const error = new PreviewError(ERROR_CODE.NOT_DOWNLOADABLE, __('error_not_downloadable'), details);
throw error;
}

Expand Down Expand Up @@ -1021,7 +1025,7 @@ class Preview extends EventEmitter {
}

// Show download button if download permissions exist, options allow, and browser has ability
if (checkPermission(this.file, PERMISSION_DOWNLOAD) && this.options.showDownload && Browser.canDownload()) {
if (canDownload(this.file, this.options)) {
this.ui.showLoadingDownloadButton(this.download);
}

Expand Down Expand Up @@ -1170,11 +1174,10 @@ class Preview extends EventEmitter {
this.emitLoadMetrics();

// Show or hide print/download buttons
// canDownload is not supported by all of our browsers, so for now we need to check isMobile
if (checkPermission(this.file, PERMISSION_DOWNLOAD) && this.options.showDownload && Browser.canDownload()) {
if (canDownload(this.file, this.options)) {
this.ui.showDownloadButton(this.download);

if (checkFeature(this.viewer, 'print') && !Browser.isMobile()) {
if (checkFeature(this.viewer, 'print')) {
this.ui.showPrintButton(this.print);
}
}
Expand Down
136 changes: 40 additions & 96 deletions src/lib/__tests__/Preview-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import PreviewError from '../PreviewError';
import * as file from '../file';
import * as util from '../util';
import * as dr from '../downloadReachability';
import { API_HOST, CLASS_NAVIGATION_VISIBILITY } from '../constants';
import { API_HOST, CLASS_NAVIGATION_VISIBILITY, PERMISSION_PREVIEW } from '../constants';
import { VIEWER_EVENT, ERROR_CODE, LOAD_METRIC, PREVIEW_METRIC } from '../events';
import Timer from '../Timer';

Expand Down Expand Up @@ -703,36 +703,33 @@ describe('lib/Preview', () => {

describe('print()', () => {
beforeEach(() => {
stubs.checkPermission = sandbox.stub(file, 'checkPermission').returns(false);
stubs.checkFeature = sandbox.stub(file, 'checkFeature').returns(false);
stubs.canDownload = sandbox.stub(file, 'canDownload');
stubs.checkFeature = sandbox.stub(file, 'checkFeature');
preview.viewer = {
print: sandbox.stub()
};
});

it('should print if the feature and permissions exist', () => {
stubs.checkPermission.returns(true);
it('should print if file can be downloaded and feature exists', () => {
stubs.canDownload.returns(true);
stubs.checkFeature.returns(true);

preview.print();
expect(preview.viewer.print).to.be.called;
});

it('should not print if feature does not exists', () => {
stubs.checkFeature.returns(true);
it('should not print if feature does not exist', () => {
stubs.canDownload.returns(true);
stubs.checkFeature.returns(false);

preview.print();
expect(preview.viewer.print).to.not.be.called;
});

it('should not print if permissions do not exist', () => {
stubs.checkPermission.returns(true);

preview.print();
expect(preview.viewer.print).to.not.be.called;
});
it('should not print if file cannot be downloaded', () => {
stubs.canDownload.returns(false);
stubs.checkFeature.returns(false);

it('should not print if permissions or feature do not exist', () => {
preview.print();
expect(preview.viewer.print).to.not.be.called;
});
Expand All @@ -747,8 +744,7 @@ describe('lib/Preview', () => {
});

stubs.reachabilityPromise = Promise.resolve(true);

stubs.checkPermission = sandbox.stub(file, 'checkPermission');
stubs.canDownload = sandbox.stub(file, 'canDownload');
stubs.get = sandbox.stub(util, 'get').returns(stubs.promise);
stubs.get = sandbox.stub(dr, 'setDownloadReachability').returns(stubs.reachabilityPromise);
stubs.openUrlInsideIframe = sandbox.stub(util, 'openUrlInsideIframe');
Expand All @@ -759,15 +755,14 @@ describe('lib/Preview', () => {
stubs.replaceDownloadHostWithDefault = sandbox.stub(dr, 'replaceDownloadHostWithDefault').returns('default');
});

it('should not do anything if there is no download permission', () => {
stubs.checkPermission.returns(false);

it('should not do anything if file cannot be downloaded', () => {
stubs.canDownload.returns(false);
preview.download();
expect(stubs.openUrlInsideIframe).to.not.be.called;
});

it('open the default download URL in an iframe if the custom host is blocked or if we were given the default', () => {
stubs.checkPermission.returns(true);
stubs.canDownload.returns(true);
stubs.isDownloadHostBlocked.returns(true);
stubs.isCustomDownloadHost.returns(true);

Expand All @@ -787,7 +782,7 @@ describe('lib/Preview', () => {


it('should check download reachability and fallback if we do not know the status of our custom host', () => {
stubs.checkPermission.returns(true);
stubs.canDownload.returns(true);
stubs.isCustomDownloadHost.returns(true);

preview.download();
Expand Down Expand Up @@ -1449,7 +1444,7 @@ describe('lib/Preview', () => {

stubs.destroy = sandbox.stub(preview, 'destroy');
stubs.checkPermission = sandbox.stub(file, 'checkPermission').returns(true);
stubs.canDownload = sandbox.stub(Browser, 'canDownload').returns(false);
stubs.canDownload = sandbox.stub(file, 'canDownload').returns(false);
stubs.showLoadingDownloadButton = sandbox.stub(preview.ui, 'showLoadingDownloadButton');
stubs.loadPromiseResolve = Promise.resolve();
stubs.determineRepresentationStatusPromise = Promise.resolve();
Expand Down Expand Up @@ -1484,49 +1479,24 @@ describe('lib/Preview', () => {
expect(stubs.destroy).to.not.be.called;
});

it('should throw an error if there is no preview permission', () => {
stubs.checkPermission.returns(false);
it('should throw an error if user does not have permission to preview', () => {
stubs.checkPermission.withArgs(sinon.match.any, PERMISSION_PREVIEW).returns(false);
expect(() => preview.loadViewer()).to.throw(
PreviewError,
/We're sorry, you don't have permission to preview this file./
);
});

it('should show the loading download button if there are sufficient permissions and support', () => {
stubs.checkPermission.withArgs(sinon.match.any, 'can_download').returns(false);
preview.options.showDownload = false;

preview.loadViewer({});
expect(stubs.showLoadingDownloadButton).to.not.be.called;
preview.destroy();

stubs.checkPermission.withArgs(sinon.match.any, 'can_download').returns(true);

preview.loadViewer({});
expect(stubs.showLoadingDownloadButton).to.not.be.called;
preview.destroy();

stubs.checkPermission.withArgs(sinon.match.any, 'can_download').returns(false);
preview.options.showDownload = true;

it('should show the loading download button if file can be downloaded', () => {
stubs.canDownload.returns(true);
preview.loadViewer({});
expect(stubs.showLoadingDownloadButton).to.not.be.called;
preview.destroy();
expect(stubs.showLoadingDownloadButton).to.be.called;
});

stubs.checkPermission.withArgs(sinon.match.any, 'can_download').returns(true);
preview.options.showDownload = true;
it('should not show the loading download button if file can\'t be downloaded', () => {
stubs.canDownload.returns(false);
preview.destroy();

preview.loadViewer({});
expect(stubs.showLoadingDownloadButton).to.not.be.called;

stubs.checkPermission.withArgs(sinon.match.any, 'can_download').returns(true);
preview.options.showDownload = true;
stubs.canDownload.returns(true);

preview.loadViewer({});
expect(stubs.showLoadingDownloadButton).to.be.called;
});

it('should throw an unsupported error if there is no loader for general file types', () => {
Expand Down Expand Up @@ -1701,10 +1671,9 @@ describe('lib/Preview', () => {

describe('finishLoading()', () => {
beforeEach(() => {
stubs.checkPermission = sandbox.stub(file, 'checkPermission');
stubs.canDownload = sandbox.stub(file, 'canDownload');
stubs.checkFeature = sandbox.stub(file, 'checkFeature');
stubs.isMobile = sandbox.stub(Browser, 'isMobile');
stubs.canDownload = sandbox.stub(Browser, 'canDownload');
stubs.showDownloadButton = sandbox.stub(preview.ui, 'showDownloadButton');
stubs.showPrintButton = sandbox.stub(preview.ui, 'showPrintButton');
stubs.hideLoadingIndicator = sandbox.stub(preview.ui, 'hideLoadingIndicator');
Expand All @@ -1727,64 +1696,39 @@ describe('lib/Preview', () => {
};

preview.logger = stubs.logger;
preview.options.showDownload = true;
stubs.canDownload.returns(true);
stubs.checkPermission.returns(true);
stubs.checkFeature.returns(true);
});

it('should only show download button if there is download permission', () => {
stubs.checkPermission.returns(false);

preview.finishLoading();
expect(stubs.showDownloadButton).to.not.be.called;

stubs.checkPermission.returns(true);

preview.finishLoading();
expect(stubs.showDownloadButton).to.be.calledWith(preview.download);
});

it('should show download button if it is requested in the options', () => {
preview.options.showDownload = false;

preview.finishLoading();
expect(stubs.showDownloadButton).to.not.be.called;

preview.options.showDownload = true;

it('should show download button if file can be downloaded', () => {
stubs.canDownload.returns(true);
preview.finishLoading();
expect(stubs.showDownloadButton).to.be.calledWith(preview.download);
expect(stubs.showDownloadButton).to.be.called;
});

it('should show download button if download is supported by browser', () => {
it('should not show download button if file can\'t be downloaded', () => {
stubs.canDownload.returns(false);

preview.finishLoading();
expect(stubs.showDownloadButton).to.not.be.called;
});

it('should show print button if print is supported', () => {
stubs.checkFeature.withArgs(sinon.match.any, 'print').returns(true);
stubs.canDownload.returns(true);

preview.finishLoading();
expect(stubs.showDownloadButton).to.be.called;
expect(stubs.showPrintButton).to.be.called;
});

it('should not show print button if print is not supported', () => {
stubs.checkFeature.withArgs(sinon.match.any, 'print').returns(false);
stubs.canDownload.returns(true);
stubs.isMobile.returns(false);

preview.finishLoading();
expect(stubs.showDownloadButton).to.be.calledWith(preview.download);
expect(stubs.showPrintButton).to.not.be.called;
});

it('should show print button if print is supported', () => {
stubs.checkFeature.returns(false);

it('should not show print button if file can\'t be downloaded', () => {
stubs.checkFeature.withArgs(sinon.match.any, 'print').returns(true);
stubs.canDownload.returns(false);
preview.finishLoading();
expect(stubs.showPrintButton).to.not.be.called;

stubs.checkFeature.returns(true);

preview.finishLoading();
expect(stubs.showPrintButton).to.be.called;
});

it('should increment the preview count', () => {
Expand Down
2 changes: 1 addition & 1 deletion src/lib/__tests__/RepStatus-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ describe('lib/RepStatus', () => {
})
);

sandbox.mock(window).expects('clearTimeout').withArgs(repStatus.statusTimeout);
sandbox.mock(window).expects('clearTimeout');

return repStatus.updateStatus().then(() => {
expect(repStatus.representation.status.state).to.equal(state);
Expand Down
Loading

0 comments on commit cc09a61

Please sign in to comment.