-
Notifications
You must be signed in to change notification settings - Fork 481
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
Backpack client api: add fetch file and get file list #41760
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
fetch: function(childPath, callback) { | ||
this.fetchWithDataType(childPath, callback, 'json'); | ||
}, | ||
|
||
/** | ||
* Retrieve a collection. | ||
* @param {string} childPath The path underneath api_base_url | ||
* @param {AjaxNodeStyleCallback} callback - Expected result is the requested | ||
* collection object. | ||
*/ | ||
fetchWithDataType: function(childPath, callback, dataType) { | ||
$.ajax({ | ||
url: this.api_base_url + '/' + childPath, | ||
type: 'get', | ||
dataType: 'json' | ||
dataType: dataType | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: could keep this as one function, but just have the dataType
param default to 'json'
if not specified? (e.g. fetch(childPath, callback, dataType = 'json')
)
Edit: oh actually I think IE doesn't support default parameters, but we can still do dataType: dataType || 'json'
here to keep it one function if desired
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done!
@@ -21,6 +25,38 @@ export default class BackpackClientApi { | |||
}); | |||
} | |||
|
|||
fetchFile(filename, onError, onSuccess) { | |||
if (!this.channelId) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: potentially just use !this.hasBackpack()
for these checks? So that logic can stay contained to hasBackpack
if it ever becomes more than just checking channel ID?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good call, done!
Add fetch a single file and get the list of all filenames api calls to BackpackClientApi. These will be used for importing files from the backpack and checking for name collisions. We call the error callback if these functions are called before the backpack has been initialized. I also added a
hasBackpack
function to the api which users of the api can use to check if the backpack has been initialized yet. We only initialize the backpack once the user saves at least one file.Links
Testing story
Tested locally and added unit tests.
PR Checklist: