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

Fix: parse options during show instead of after token response #725

Merged
merged 3 commits into from
Mar 27, 2018
Merged

Fix: parse options during show instead of after token response #725

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9086ff0
Fix: parse options during show instead of after token response
DanDeMicco Mar 20, 2018
ccafe60
Merge branch 'master' into preview-options
DanDeMicco Mar 20, 2018
df324a0
Merge branch 'master' into preview-options
DanDeMicco Mar 27, 2018
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
13 changes: 6 additions & 7 deletions src/lib/Preview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,9 @@ class Preview extends EventEmitter {
// if proper valid file objects were passed in.
this.updateCollection(options.collection);

// Parse the preview options
this.parseOptions(this.previewOptions);

// Load the preview
this.load(fileIdOrFile);
}
Expand Down Expand Up @@ -750,8 +753,8 @@ class Preview extends EventEmitter {
return;
}

// Parse the preview options supplied by show()
this.parseOptions(this.previewOptions, tokenMap);
// Set the authorization token
this.options.token = tokenMap[this.file.id];

this.setupUI();

Expand Down Expand Up @@ -795,10 +798,9 @@ class Preview extends EventEmitter {
*
* @private
* @param {Object} previewOptions - Options specified by show()
* @param {Object} tokenMap - Map of file ID to access token
* @return {void}
*/
parseOptions(previewOptions, tokenMap) {
parseOptions(previewOptions) {
const options = Object.assign({}, previewOptions);

// Reset all options
Expand All @@ -807,9 +809,6 @@ class Preview extends EventEmitter {
// Container for preview
this.options.container = options.container;

// Authorization token
this.options.token = tokenMap[this.file.id];

// Shared link URL
this.options.sharedLink = options.sharedLink;

Expand Down
76 changes: 43 additions & 33 deletions src/lib/__tests__/Preview-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ describe('lib/Preview', () => {
beforeEach(() => {
stubs.load = sandbox.stub(preview, 'load');
stubs.updateCollection = sandbox.stub(preview, 'updateCollection');
stubs.parseOptions = sandbox.stub();
});

it('should set the preview options with string token', () => {
Expand Down Expand Up @@ -188,6 +189,20 @@ describe('lib/Preview', () => {
expect(e.message).to.equal('Bad access token!');
}
});

it('should parse the preview options', () => {
preview.retryCount = 0;
preview.parseOptions = stubs.parseOptions;

const token = 'token';
const options = {
foo: 'bar'
};

preview.show(file, token, options);

expect(stubs.parseOptions).to.be.calledWith(Object.assign({}, options, { token }));
});
});

describe('hide()', () => {
Expand Down Expand Up @@ -298,7 +313,7 @@ describe('lib/Preview', () => {
});

it('should set the preview collection to an array of file ids when files passed in', () => {
let files = ['1', { id: '2' }, 3, { id: '4' }, { id: 5 }];
const files = ['1', { id: '2' }, 3, { id: '4' }, { id: 5 }];

preview.updateCollection(files);
expect(stubs.updateFileCache).to.be.calledWith([{ id: '2' }, { id: '4' }, { id: '5' }]);
Expand Down Expand Up @@ -908,7 +923,7 @@ describe('lib/Preview', () => {
it('should fetch file from cache and convert file id to string when file id passed as a number', () => {
const fileId = 123;
preview.load(fileId);
expect(file.getCachedFile).to.be.calledWith(preview.cache, { fileId : fileId.toString() });
expect(file.getCachedFile).to.be.calledWith(preview.cache, { fileId: fileId.toString() });
});

it('should fetch file from cache using file version ID as key if file version ID is in options', () => {
Expand Down Expand Up @@ -1005,7 +1020,6 @@ describe('lib/Preview', () => {
describe('handleTokenResponse()', () => {
beforeEach(() => {
stubs.loadFromServer = sandbox.stub(preview, 'loadFromServer');
stubs.parseOptions = sandbox.stub(preview, 'parseOptions');
stubs.setupUI = sandbox.stub(preview, 'setupUI');
stubs.checkPermission = sandbox.stub(file, 'checkPermission');
stubs.checkFileValid = sandbox.stub(file, 'checkFileValid');
Expand All @@ -1018,14 +1032,20 @@ describe('lib/Preview', () => {

preview.handleTokenResponse({});
expect(stubs.loadFromServer).to.be.called;
expect(stubs.parseOptions).to.not.be.called;
});

it('should parse the preview options', () => {
it('should set the token option', () => {
preview.retryCount = 0;
const TOKEN = 'bar';
const FILE_ID = '123';
preview.file = {
id: FILE_ID
};
preview.handleTokenResponse({
[FILE_ID]: TOKEN
});

preview.handleTokenResponse({});
expect(stubs.parseOptions).to.be.called;
expect(preview.options.token).to.equal(TOKEN);
});

it('should setup UI', () => {
Expand Down Expand Up @@ -1091,93 +1111,85 @@ describe('lib/Preview', () => {
stubs.assign = sandbox.spy(Object, 'assign');
stubs.disableViewers = sandbox.stub(preview, 'disableViewers');
stubs.enableViewers = sandbox.stub(preview, 'enableViewers');
stubs.tokens = {
0: 'file0'
};

preview.file = {
id: 0
};
});

it('should use the saved preview options', () => {
preview.parseOptions(preview.previewOptions, stubs.tokens);
preview.parseOptions(preview.previewOptions);
expect(stubs.assign).to.be.calledWith(preview.previewOptions);
});

it('should set the container', () => {
preview.parseOptions(preview.previewOptions, stubs.tokens);
preview.parseOptions(preview.previewOptions);
expect(preview.options.container).to.equal(containerEl);
});

it('should set the token based on the file id', () => {
preview.parseOptions(preview.previewOptions, stubs.tokens);
expect(preview.options.token).to.equal(stubs.tokens[0]);
});

it('should set shared link and shared link password', () => {
preview.parseOptions(preview.previewOptions, stubs.tokens);
preview.parseOptions(preview.previewOptions);
expect(preview.options.sharedLink).to.equal(stubs.sharedLink);
expect(preview.options.sharedLinkPassword).to.equal(stubs.sharedLinkPassword);
});

it('should save a reference to the api host', () => {
preview.parseOptions(preview.previewOptions, stubs.tokens);
preview.parseOptions(preview.previewOptions);
expect(preview.options.apiHost).to.equal('endpoint');

// Check default
preview.previewOptions.apiHost = undefined;
preview.parseOptions(preview.previewOptions, stubs.tokens);
preview.parseOptions(preview.previewOptions);
expect(preview.options.apiHost).to.equal('https://api.box.com');
});

it('should save a reference to the app host', () => {
preview.parseOptions(preview.previewOptions, stubs.tokens);
preview.parseOptions(preview.previewOptions);
expect(preview.options.appHost).to.equal(stubs.appHost);

// Check default
preview.previewOptions.appHost = undefined;
preview.parseOptions(preview.previewOptions, stubs.tokens);
preview.parseOptions(preview.previewOptions);
expect(preview.options.appHost).to.equal('https://app.box.com');
});

it('should set whether to show the header or a custom logo', () => {
preview.parseOptions(preview.previewOptions, stubs.tokens);
preview.parseOptions(preview.previewOptions);
expect(preview.options.header).to.equal(stubs.header);
expect(preview.options.logoUrl).to.equal(stubs.logoUrl);

preview.previewOptions.header = undefined;
preview.previewOptions.logoUrl = undefined;

preview.parseOptions(preview.previewOptions, stubs.tokens);
preview.parseOptions(preview.previewOptions);
expect(preview.options.header).to.equal('light');
expect(preview.options.logoUrl).to.equal('');
});

it('should set whether to show a download link or annotations', () => {
preview.parseOptions(preview.previewOptions, stubs.tokens);
preview.parseOptions(preview.previewOptions);
expect(preview.options.showDownload).to.be.true;
expect(preview.options.showAnnotations).to.be.true;
});

it('should set whether to skip load from the server and any server updates', () => {
preview.parseOptions(preview.previewOptions, stubs.tokens);
preview.parseOptions(preview.previewOptions);
expect(preview.options.skipServerUpdate).to.be.false;

preview.previewOptions.skipServerUpdate = true;
preview.parseOptions(preview.previewOptions, stubs.tokens);
preview.parseOptions(preview.previewOptions);
expect(preview.options.skipServerUpdate).to.be.true;
});

it('should set whether to fix dependencies', () => {
preview.parseOptions(preview.previewOptions, stubs.tokens);
preview.parseOptions(preview.previewOptions);
expect(preview.options.fixDependencies).to.be.true;
});

it('should add user created loaders before standard loaders', () => {
const expectedLoaders = stubs.loaders.concat(loaders);

preview.parseOptions(preview.previewOptions, stubs.tokens);
preview.parseOptions(preview.previewOptions);
expect(preview.loaders[0]).to.equal(expectedLoaders[0]);
expect(preview.loaders).to.deep.equal(expectedLoaders);
});
Expand All @@ -1192,7 +1204,7 @@ describe('lib/Preview', () => {
}
};

preview.parseOptions(preview.previewOptions, stubs.tokens);
preview.parseOptions(preview.previewOptions);
expect(stubs.disableViewers).to.be.calledWith('Office');
expect(stubs.enableViewers).to.be.calledWith('text');
});
Expand Down Expand Up @@ -1537,9 +1549,7 @@ describe('lib/Preview', () => {
try {
preview.loadViewer();
} catch (e) {
expect(e.message).to.equal(
util.replacePlaceholders(__('error_unsupported'), ['ZIP'])
);
expect(e.message).to.equal(util.replacePlaceholders(__('error_unsupported'), ['ZIP']));
}
});

Expand Down
5 changes: 2 additions & 3 deletions src/lib/file.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,9 +298,8 @@ export function canDownload(file, previewOptions) {

// Can download watermarked representation if base check passes, user has preview permissions, Preview is configured
// to force watermarked downloads, and the file is watermarked
const canDownloadWatermarked = passBaseDownloadCheck &&
hasPreviewPermission &&
shouldDownloadWM(file, previewOptions);
const canDownloadWatermarked =
passBaseDownloadCheck && hasPreviewPermission && shouldDownloadWM(file, previewOptions);

return canDownloadOriginal || canDownloadWatermarked;
}