Skip to content

Support includeAll Query #632

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

Merged
merged 5 commits into from
Aug 10, 2018
Merged

Conversation

dplewis
Copy link
Member

@dplewis dplewis commented Aug 10, 2018

parse-community/parse-server#4838

provides shorthand for include('*')

@codecov
Copy link

codecov bot commented Aug 10, 2018

Codecov Report

Merging #632 into master will increase coverage by 0.08%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #632      +/-   ##
==========================================
+ Coverage   84.98%   85.07%   +0.08%     
==========================================
  Files          48       48              
  Lines        3850     3853       +3     
  Branches      871      871              
==========================================
+ Hits         3272     3278       +6     
+ Misses        578      575       -3
Impacted Files Coverage Δ
src/ParseQuery.js 94.52% <100%> (+0.72%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6d5112a...7dedf31. Read the comment docs.

@flovilmart
Copy link
Contributor

Can we use the '*' as an include argument?

query.include('*')

and perhaps includeAll being a shorthand for that? This will be easier to handle on the server also instead of a new parameter.

@dplewis dplewis requested a review from flovilmart August 10, 2018 15:18
@dplewis dplewis changed the title Support includeAll Query Support include('*') / includeAll Query Aug 10, 2018
@dplewis
Copy link
Member Author

dplewis commented Aug 10, 2018

@flovilmart This is what you mean right?

@flovilmart
Copy link
Contributor

I mean to treat include all as just using .include('*') which is a change that isn't required in the SDK in the end if we support it server side :)

@dplewis
Copy link
Member Author

dplewis commented Aug 10, 2018

I'll make an update server side. While preserving parse-community/parse-server#4838

@dplewis dplewis changed the title Support include('*') / includeAll Query Support includeAll Query Aug 10, 2018
@@ -1313,6 +1315,10 @@ class ParseQuery {
/**
* Includes nested Parse.Objects for the provided key. You can use dot
* notation to specify which fields in the included object are also fetched.
*
* If you want to include all nested Parse.Objects pass in '*'
* <pre>query.include('*');</pre>
Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps we need to put a version number here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Which version? 2.1.0?

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean server version?

Copy link
Member Author

@dplewis dplewis Aug 10, 2018

Choose a reason for hiding this comment

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

I have no idea mate which version is, I can try to backtrack

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok ok, so basically it already works if you pass include All in the rest SDK.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes that is correct. Basically this won't work until the next release unless I re-add includeAll to this SDK

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, let’s just use the * notation then we’ll perhaps be able to expand on with reg ex etc...

Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

Choose a reason for hiding this comment

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

How best can we update the guides simultaneously with the API docs

@@ -1313,6 +1315,10 @@ class ParseQuery {
/**
* Includes nested Parse.Objects for the provided key. You can use dot
* notation to specify which fields in the included object are also fetched.
*
* If you want to include all nested Parse.Objects pass in '*'
* <pre>query.include('*');</pre>
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

@dplewis
Copy link
Member Author

dplewis commented Aug 10, 2018

@flovilmart can this be merged?

@dplewis dplewis merged commit 000c1fb into parse-community:master Aug 10, 2018
@dplewis dplewis deleted the includeAll branch August 10, 2018 21:00
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.

3 participants