Support for Aggregate Queries #355
Conversation
src/Parse/ParseQuery.php
Outdated
| public function distinct($key) | ||
| { | ||
| $sessionToken = null; | ||
| if (ParseUser::getCurrentUser()) { |
There was a problem hiding this comment.
You can do something like this to avoid calling ParseUser::getCurrentUser() twice, while still validating there is a current user.
if($user = ParseUser::getCurrentUser()) {
...
}$user would be set in the conditional block then to use.
| 'aggregate/'.$this->className.'?'.$queryString, | ||
| $sessionToken, | ||
| null, | ||
| true |
There was a problem hiding this comment.
I haven't looked over the PR in detail yet over on the server, but does this require the master key to work? If not we should be adding a method parameter to optionally use the master key, same as how the other query methods work.
There was a problem hiding this comment.
The server requires a masterKey for security purposes
src/Parse/ParseQuery.php
Outdated
| } | ||
|
|
||
| /** | ||
| * Execute a aggregate query and returns aggregate results. |
There was a problem hiding this comment.
Just a little textual typo on a, Execute an aggregrate...
src/Parse/ParseQuery.php
Outdated
| public function aggregate($pipeline) | ||
| { | ||
| $sessionToken = null; | ||
| if (ParseUser::getCurrentUser()) { |
There was a problem hiding this comment.
Same thing as above regarding calling this twice.
| 'aggregate/'.$this->className.'?'.$queryString, | ||
| $sessionToken, | ||
| null, | ||
| true |
There was a problem hiding this comment.
Same thing here regarding the use of the master key. If it's not required to work it should be a method parameter with a default of false.
Codecov Report
@@ Coverage Diff @@
## master #355 +/- ##
==========================================
+ Coverage 99% 99.01% +0.01%
==========================================
Files 38 38
Lines 3522 3559 +37
==========================================
+ Hits 3487 3524 +37
Misses 35 35
Continue to review full report at Codecov.
|
|
@montymxb I noticed this issue as well parse-community/parse-server#4351 |
There was a problem hiding this comment.
Awesome! The code is good to go, but can I request that you add a small section in the README for this? You may have noticed that I've restructured it to start with a table of contents, everything is pretty easy to find from that. You can add a part in the Queries section. You can add it above or below the Relative Time sub-section, wherever works so long as it's in that group.
As part of the upcoming 1.4.0 release I'll be going over all changes since 1.3.0 and putting up a complete PR for the docs, so we're good on that end.
| $this->assertEquals(0, count($results)); | ||
| } | ||
|
|
||
| public function testDisinctOnUsers() |
There was a problem hiding this comment.
Whoop almost missed this, typo here on testDisinctOnUsers
|
Great feature to move on |
Support for
distinct()andaggregate()queries.Pending: parse-community/parse-server#4207