Add nearsphere convenience methods #582

mtrezza · 2018-06-12T13:37:17Z

Adding convenience methods for queries with geoWithin.nearSphere.

Depending on parse-community/parse-server#4825

flovilmart

Besides the ‘fit’ issue do you think it would be nice / possible to have ‘sorted’ as an additional param instead of new functions altogether?

flovilmart · 2018-06-12T14:41:34Z

src/__tests__/ParseQuery-test.js

@@ -616,6 +616,63 @@ describe('ParseQuery', () => {
    });
  });

+  fit('can generate near-geopoint queries without sorting', () => {


Oops, needs to replace by ‘it’

…ameter

codecov · 2018-06-12T15:27:57Z

Codecov Report

Merging #582 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #582      +/-   ##
==========================================
+ Coverage    84.7%   84.71%   +<.01%     
==========================================
  Files          48       48              
  Lines        4015     4017       +2     
  Branches      905      906       +1     
==========================================
+ Hits         3401     3403       +2     
  Misses        614      614

Impacted Files	Coverage Δ
src/ParseQuery.js	`94.23% <100%> (+0.02%)`	⬆️

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 0fbc604...c594faa. Read the comment docs.

mtrezza · 2018-06-12T15:47:50Z

@flovilmart good idea, introduced sorted parameter. I will make the sorted parameter default to true if not set so that existing code does not break.

flovilmart · 2018-06-12T16:38:18Z

Ok, tests are failing for now as the feature require an unpublished version of parse-server

flovilmart · 2018-06-12T20:08:06Z

I’ll do a release of the server later so we can get going with this one.

flovilmart · 2018-06-12T21:58:41Z

@mtrezza I’m wondering, can we make those tests succeed without the new parse-server? This way, we’ll be able to ship in a single server release with the JS SDK?

mtrezza · 2018-06-12T23:09:49Z

@flovilmart Yes, we could remove the tests against the query results (for now) in integration/test/ParseGeoPointTest.js and only keep the tests against the generated query syntax in src/__tests__/ParseQuery-test.js.

The query results are already tested in parse-community/parse-server#4825 so if the query syntax is correct and passes the tests, it should yield the same query results. The integration tests behave a bit like chicken-and-egg it seems.

However, I think there should also be a test against query results of the convenience methods, so I'd add them ASAP later on. What do you suggest?

flovilmart · 2018-06-12T23:11:10Z

We can do the integrations tests in parse server instead, for this one or mark them xit or pending / optional

mtrezza · 2018-06-12T23:15:43Z

Yes, I already added the tests in parse server with parse-community/parse-server#4827. I was taking guidance from the existing tests, but now that you say it, some tests seem to be redundant because the integration tests are in the SDK and in parse server. Or is it best practice to test integration from both sides?

…n of parse server will fail

mtrezza · 2018-06-12T23:33:28Z

@flovilmart done

dplewis · 2018-06-14T02:39:20Z

I’m wondering, can we make those tests succeed without the new parse-server? This way, we’ll be able to ship in a single server release with the JS SDK?

@flovilmart Can we do something similar to what we did with the PHP SDK and have it always go against the bleeding edge?

dplewis · 2018-06-23T16:29:44Z

@flovilmart Have you seen my last comment #582 (comment)

What do you think about using the latest parse server for testing?

flovilmart · 2018-06-23T16:30:53Z

We can definitely do that :) easier to catch regessions!

mtrezza · 2018-06-23T17:33:15Z

What do I have to change in code?

…

On 23.06.2018, at 18:30, Florent Vilmart ***@***.***> wrote: We can definitely do that :) easier to catch regessions! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

dplewis · 2018-06-23T17:40:59Z

@mtrezza Change the version of parse server used for test to the latest.

https://github.com/parse-community/Parse-SDK-JS/blob/master/integration/package.json#L6

to github:parse-community/parse-server#latest

Then remove your xit from test

Also I'm confused by change to the existing test cases if sorted is optional why are you setting true if true is the default?

mtrezza · 2018-06-23T18:17:22Z

Change to the existing test cases was to make it apparent that there is a sorted parameter and to explicitly test with sorted=true. But it would surely be enough to have one case with sorted=true. So I’ll revert the existing test cases and add one for sorted=true.

…

On 23.06.2018, at 19:41, Diamond Lewis ***@***.***> wrote: @mtrezza You can start by changing the version of parse server used for test to the latest. https://github.com/parse-community/Parse-SDK-JS/blob/master/integration/package.json#L6 to github:parse-community/parse-server#latest Then remove your xit from test Also I'm confused by change to the existing test cases if sorted is optional why are you forcing true? I feel like I'll have to change all my calls to sorted = true. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

dplewis · 2018-06-23T18:59:02Z

So I’ll revert the existing test cases and add one for sorted=true.

That’s what I was going for. It looked like it was required

…ivated integration tests for near-sphere

…case with sorted=true parameter

mtrezza · 2018-06-23T22:02:18Z

@dplewis Can you please have a look at the failing integration test, I think it is because parse-server#latest failed to build?

Non-registry package missing package.json: parse-server@github:parse-community/parse-server#latest.
npm ERR! package.json npm can't find a package.json file in your current directory.

dplewis · 2018-06-24T01:13:47Z

@flovilmart According to the docs
https://github.com/parse-community/parse-server#want-to-ride-the-bleeding-edge

I have to do npm install parseplatform/parse-server.git#latest

It currently fails for me

npm ERR! prepareGitDep > babel src/ -d lib/ --copy-files
npm ERR! prepareGitDep 
npm ERR! prepareGitDep 
npm ERR! prepareGitDep 2> npm WARN install Usage of the `--dev` option is deprecated. Use `--only=dev` instead.
npm ERR! prepareGitDep src/ doesn't exist

using npm install parseplatform/parse-server.git#master works

dplewis · 2018-06-24T02:06:58Z

@flovilmart How does this look?

On a side note should we pin the node packages? Don't want a repeat of parse-community/parse-server#2040

flovilmart · 2018-06-25T02:59:17Z

@dplewis see my comment. I’ll have a look tomorrow too

flovilmart · 2018-06-25T18:29:29Z

@dplewis that sounds good! Thanks to this new NPM feature, one can use any branch and test against it. npm will fetch the dev deps and do it's thing! Wow :)

dplewis · 2018-06-25T19:01:39Z

@flovilmart thats pretty awesome now that I think about it.

flovilmart

Excellent everything passes so let's merge it in!

flovilmart · 2018-06-25T19:35:36Z

@mtrezza Thanks again for the PR!

mtrezza · 2018-06-25T20:11:07Z

@flovilmart Sure thing, thanks to you and dplewis for the support.

flovilmart · 2018-06-26T00:03:01Z

Any time!

mtrezza added 2 commits June 12, 2018 14:53

added convenience methods and tests

3b688c2

added more tests

439af5d

flovilmart suggested changes Jun 12, 2018

View reviewed changes

combined methos for sorted and unsorted queries by using sorted par…

837efa8

…ameter

sorted parameter defaults to true if not set

344f1fb

mtrezza mentioned this pull request Jun 12, 2018

add tests for JS SDK nearsphere convenience methods parse-community/parse-server#4827

Closed

made integration tests pending because tests again unpublished versio…

0cd0bd1

…n of parse server will fail

removed duplicate of withinGeoBox that was mistakenly added earlier

937ecf4

mtrezza mentioned this pull request Jun 13, 2018

Add js geoquery sorted parameter parse-community/docs#529

Merged

mtrezza added 2 commits June 23, 2018 23:38

integration tests running against latest parse-server version; re-act…

3fa11b8

…ivated integration tests for near-sphere

reverted existing test cases to without sorted parameter; added test …

a1bb372

…case with sorted=true parameter

test against latest

c594faa

flovilmart approved these changes Jun 25, 2018

View reviewed changes

flovilmart merged commit 4a6e619 into parse-community:master Jun 25, 2018

mtrezza deleted the add_nearsphere_convenience_methods branch July 7, 2018 15:53

dplewis mentioned this pull request Sep 5, 2019

Fix nearSphere queries when sorting by another key on Postgres parse-community/parse-server#6026

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add nearsphere convenience methods #582

Add nearsphere convenience methods #582

mtrezza commented Jun 12, 2018

flovilmart left a comment

flovilmart Jun 12, 2018

mtrezza Jun 12, 2018

codecov bot commented Jun 12, 2018 •

edited

Loading

mtrezza commented Jun 12, 2018 •

edited

Loading

flovilmart commented Jun 12, 2018 •

edited

Loading

flovilmart commented Jun 12, 2018

flovilmart commented Jun 12, 2018

mtrezza commented Jun 12, 2018

flovilmart commented Jun 12, 2018

mtrezza commented Jun 12, 2018 •

edited

Loading

mtrezza commented Jun 12, 2018

dplewis commented Jun 14, 2018 •

edited

Loading

dplewis commented Jun 23, 2018

flovilmart commented Jun 23, 2018

mtrezza commented Jun 23, 2018 via email

dplewis commented Jun 23, 2018 •

edited

Loading

mtrezza commented Jun 23, 2018 via email

dplewis commented Jun 23, 2018

mtrezza commented Jun 23, 2018 •

edited

Loading

dplewis commented Jun 24, 2018 •

edited

Loading

dplewis commented Jun 24, 2018

flovilmart commented Jun 25, 2018

flovilmart commented Jun 25, 2018

dplewis commented Jun 25, 2018

flovilmart left a comment

flovilmart commented Jun 25, 2018

mtrezza commented Jun 25, 2018

flovilmart commented Jun 26, 2018

Add nearsphere convenience methods #582

Add nearsphere convenience methods #582

Conversation

mtrezza commented Jun 12, 2018

flovilmart left a comment

Choose a reason for hiding this comment

flovilmart Jun 12, 2018

Choose a reason for hiding this comment

mtrezza Jun 12, 2018

Choose a reason for hiding this comment

codecov bot commented Jun 12, 2018 • edited Loading

Codecov Report

mtrezza commented Jun 12, 2018 • edited Loading

flovilmart commented Jun 12, 2018 • edited Loading

flovilmart commented Jun 12, 2018

flovilmart commented Jun 12, 2018

mtrezza commented Jun 12, 2018

flovilmart commented Jun 12, 2018

mtrezza commented Jun 12, 2018 • edited Loading

mtrezza commented Jun 12, 2018

dplewis commented Jun 14, 2018 • edited Loading

dplewis commented Jun 23, 2018

flovilmart commented Jun 23, 2018

mtrezza commented Jun 23, 2018 via email

dplewis commented Jun 23, 2018 • edited Loading

mtrezza commented Jun 23, 2018 via email

dplewis commented Jun 23, 2018

mtrezza commented Jun 23, 2018 • edited Loading

dplewis commented Jun 24, 2018 • edited Loading

dplewis commented Jun 24, 2018

flovilmart commented Jun 25, 2018

flovilmart commented Jun 25, 2018

dplewis commented Jun 25, 2018

flovilmart left a comment

Choose a reason for hiding this comment

flovilmart commented Jun 25, 2018

mtrezza commented Jun 25, 2018

flovilmart commented Jun 26, 2018

codecov bot commented Jun 12, 2018 •

edited

Loading

mtrezza commented Jun 12, 2018 •

edited

Loading

flovilmart commented Jun 12, 2018 •

edited

Loading

mtrezza commented Jun 12, 2018 •

edited

Loading

dplewis commented Jun 14, 2018 •

edited

Loading

dplewis commented Jun 23, 2018 •

edited

Loading

mtrezza commented Jun 23, 2018 •

edited

Loading

dplewis commented Jun 24, 2018 •

edited

Loading