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.

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

Update: Allow passed in collections to be of file objects #359

Merged
merged 1 commit into from
Sep 12, 2017

Conversation

priyajeet
Copy link
Contributor

@priyajeet priyajeet commented Sep 1, 2017

Allows tokens to be null or undefined. Useful when the passed in file or collection has well formed file objects. This is for offline use case or the use case where the parent project (like ContentExplorer UI Element) has already made the call.

Also changes the token getter to pass in typed ids for files and handle cases when typed ids are returned from the token service to make it consistent with other UI Elements. Since all the UI elements can either be requesting folder or file ids, the token service needs to know what type of id we are sending it, as such either file_ or folder_ is prefixed to all the ids. Likewise the token service can/will return typed ids after fetching tokens. So it needs to be mapped back to what preview wants.

@priyajeet priyajeet force-pushed the master branch 4 times, most recently from 8e6feba to cdc691e Compare September 12, 2017 03:15
Also allows tokens to be null or undefined. Useful when the
passed in file or collection has well formed file objects.

Also changes the token getter to pass in typed ids for files
and handle cases when typed ids are returned from the
token service to make it consistant with other UI Elements.
Since all the UI elements can either be requesting folder
or file ids, the token service needs to know what type of
id we are sending it, as such either file_ or folder_ is
prefixed to all the ids. Likewise the token service will
return typed ids after fetching tokens.
Copy link
Contributor

@JustinHoldstock JustinHoldstock left a comment

Choose a reason for hiding this comment

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

After translating strings and replying to comment on reusing errors, I approve

this.previewOptions = Object.assign({}, options, { token });
} else {
throw new Error('Missing access token!');
throw new Error('Bad access token!');
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be a translated string?

fileIds.push(fileOrId.id);
files.push(fileOrId);
} else {
throw new Error('Bad collection provided!');
Copy link
Contributor

Choose a reason for hiding this comment

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

Translated string?

@@ -1,19 +1,46 @@
// Create an error to throw if needed
const error = new Error('Missing Auth Token!');
const error = new Error('Bad Auth Token!');
Copy link
Contributor

Choose a reason for hiding this comment

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

Another translated string

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, I can't say I've ever seen anyone reuse an Error Object. Should there be any gotchas with that?

@priyajeet priyajeet merged commit cc59009 into box:master Sep 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants