-
-
Notifications
You must be signed in to change notification settings - Fork 596
Support Fetch With Include #631
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #631 +/- ##
==========================================
+ Coverage 85.12% 85.53% +0.41%
==========================================
Files 48 48
Lines 3865 3899 +34
Branches 871 882 +11
==========================================
+ Hits 3290 3335 +45
+ Misses 575 564 -11
Continue to review full report at Codecov.
|
src/ParseObject.js
Outdated
* @param {Object} options | ||
* @static | ||
*/ | ||
static fetchAllWithInclude(list: Array<ParseObject>, keys: String|Array<string|Array<string>>, options: RequestOptions) { |
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.
why do we add more things like those warppers? semms un necessary if it's properly documented in the RequestOptions.
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.
I'm sorry I'm not following. How should it be?
I like shorthand but we could leave it like
obj.fetch({ include: ['key1', 'key2',...] })
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.
Yeah, that was my point, but we can keep the nice interfaces also, i don’t mind
@flovilmart I added documentation for the |
There was one small nit :) |
Nit? Where? |
src/ParseObject.js
Outdated
return CoreManager.getObjectController().fetch( | ||
list, | ||
true, | ||
queryOptions | ||
); | ||
} | ||
|
||
/** |
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.
Small nit: remove one space
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.
Here. Sorry, forgot to hit ‘comment’ at the end of the review
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.
Can we add eslint to this project at some point? It looks like it would be a lot of work tho :/
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.
Nobody will ever hate you for adding it.
Is this change documented? |
https://parseplatform.org/Parse-SDK-JS/api/2.1.0/Parse.Object.html#fetchWithInclude If you want to update the guide feel free to open a PR |
Hello @dplewis is there any way by using |
You should be able to pass in ‘*’ |
Hey @dplewis thanks for the help! That worked! |
Added
fetchWithInclude
andfetchAllWithInclude
to Parse Object.Added
fetchWithInclude
to UserAs discussed parse-community/parse-server#4845 (comment)