Skip to content

Commit

Permalink
Fix: Fail gracefully when previewing a doc with deleted reps (#962)
Browse files Browse the repository at this point in the history
  • Loading branch information
Jeremy Press committed Mar 25, 2019
1 parent 58f7220 commit d8cd848
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 6 deletions.
7 changes: 4 additions & 3 deletions src/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@

<div class='preview-container' data-testid="preview-container"> </div>
<script>
// Create preview first so events can be bound before 'show'
/* global Box */
preview = new Box.Preview();

function setProperty(selector) {
// Get new value, fallback to local storage
var inputValue = document.querySelector('#' + selector + '-set')
Expand All @@ -91,9 +95,6 @@
return;
}

/* global Box */
preview = new Box.Preview();

var previewOptions = options || {
enableThumbnailsSidebar: true,
showAnnotations: true,
Expand Down
1 change: 1 addition & 0 deletions src/lib/events.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export const ERROR_CODE = {
UNSUPPORTED_FILE_TYPE: 'error_unsupported_file_type',
PERMISSIONS_PREVIEW: 'error_permissions_preview',
BAD_INPUT: 'error_bad_input',
DELETED_REPS: 'error_deleted_reps',
LOAD_ANNOTATIONS: 'error_load_annotations',
LOAD_ASSET: 'error_load_asset',
LOAD_CSV: 'error_load_csv',
Expand Down
4 changes: 3 additions & 1 deletion src/lib/viewers/BaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,9 @@ class BaseViewer extends EventEmitter {
* @return {void}
*/
handleDownloadError(err, downloadURL) {
if (this.hasRetriedContentDownload) {
const isRepDeleted = getProp(err, 'details.isRepDeleted', false);

if (this.hasRetriedContentDownload || isRepDeleted) {
this.triggerError(err);
return;
}
Expand Down
17 changes: 16 additions & 1 deletion src/lib/viewers/__tests__/BaseViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,13 +241,28 @@ describe('lib/viewers/BaseViewer', () => {
sandbox.stub(base, 'emitMetric');
});

it('should trigger an error if we have already retried', () => {
it('should trigger an error if we have already retried', () => {
base.hasRetriedContentDownload = true;
base.handleDownloadError('error', 'https://dl.boxcloud.com');
expect(base.triggerError).to.be.called;
expect(base.load).to.not.be.called;
});

it('should trigger an error if the rep was deleted', () => {
base.hasRetriedContentDownload = false;
base.handleDownloadError(
{
details: {
isRepDeleted: true
}
},
'https://dl.boxcloud.com'
);

expect(base.triggerError).to.be.called;
expect(base.load).to.not.be.called;
});

it('should retry load, and check download reachability if we are on a custom host', () => {
base.hasRetriedContentDownload = false;
DownloadReachability.isCustomDownloadHost.returns(false);
Expand Down
13 changes: 12 additions & 1 deletion src/lib/viewers/doc/DocBaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -662,8 +662,19 @@ class DocBaseViewer extends BaseViewer {
// eslint-disable-next-line
console.error(err);

// pdf.js gives us the status code in their error message
const { status, message } = err;

// Display a generic error message but log the real one
const error = new PreviewError(ERROR_CODE.CONTENT_DOWNLOAD, __('error_document'), {}, err.message);
const error =
status === 202
? new PreviewError(
ERROR_CODE.DELETED_REPS,
__('error_refresh'),
{ isRepDeleted: true },
message
)
: new PreviewError(ERROR_CODE.CONTENT_DOWNLOAD, __('error_document'), message);
this.handleDownloadError(error, pdfUrl);
});
}
Expand Down
65 changes: 65 additions & 0 deletions test/integration/sanity/DeletedReps.e2e.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
// <reference types="Cypress" />
describe('Previewing a file with deleted representations', () => {
const token = Cypress.env('ACCESS_TOKEN');

const fileIdDoc = Cypress.env('FILE_ID_DOC');
const fileIdPresentation = Cypress.env('FILE_ID_PRESENTATION');

const REPS_ERROR = 'error_deleted_reps';

const helpers = {
checkDeletedRepError: () => {
cy.window().then((win) => {
win.preview.addListener('preview_error', (data) => {
cy.expect(data.error.code).to.equal(REPS_ERROR);
})
});
}
}

beforeEach(() => {
cy.server();
// Make the representation content call to always return a 202 with no body

// Mocking requests for non original reps
cy.route({
method: 'GET',
url: '**/internal_files/**',
status: 202,
response: {}
});

// Mocking requests for original reps
cy.route({
method: 'GET',
url: '**/content?**',
status: 202,
response: {}
});

cy.visit('/', {
onBeforeLoad (win) {
// Workaround for fetch detection in cypress mocking. https://github.com/cypress-io/cypress/issues/95
delete win.fetch; // eslint-disable-line no-param-reassign
}
})
});

[
{
viewer: 'Document',
fileId: fileIdDoc
},
{
viewer: 'Presentation',
fileId: fileIdPresentation
}
].forEach((test) => {
it(test.viewer, () => {
helpers.checkDeletedRepError();

// Temporarily disabling annotations due to a bug in 2.3
cy.showPreview(token, test.fileId, { showAnnotations: false });
})
})
});

0 comments on commit d8cd848

Please sign in to comment.