Skip to content

Allow coordinate pairs in $geoWithin/$polygon query #4064

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

Closed
wants to merge 4 commits into from
Closed

Allow coordinate pairs in $geoWithin/$polygon query #4064

wants to merge 4 commits into from

Conversation

madsb
Copy link
Contributor

@madsb madsb commented Jul 30, 2017

This pull requests allows for a polygon that consists of an array of basic coordinate pairs besides an array of Parse.GeoPoints.

@codecov
Copy link

codecov bot commented Jul 30, 2017

Codecov Report

Merging #4064 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4064      +/-   ##
==========================================
+ Coverage    90.7%   90.71%   +0.01%     
==========================================
  Files         116      116              
  Lines        7916     7920       +4     
==========================================
+ Hits         7180     7185       +5     
+ Misses        736      735       -1
Impacted Files Coverage Δ
src/vendor/mongodbUrl.js 100% <ø> (ø) ⬆️
...dapters/Storage/Postgres/PostgresStorageAdapter.js 96.38% <100%> (ø) ⬆️
src/Adapters/Storage/Mongo/MongoTransform.js 84.32% <100%> (+0.05%) ⬆️
src/RestWrite.js 93.16% <0%> (+0.18%) ⬆️

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 92d51de...5aabc80. Read the comment docs.

@dplewis
Copy link
Member

dplewis commented Jul 30, 2017

Can you add support for PFPolygon?

@madsb
Copy link
Contributor Author

madsb commented Jul 31, 2017

@dplewis, I’m not sure I understand. Looks like the iOS SDK already supports withinPolygon queries?

@dplewis
Copy link
Member

dplewis commented Jul 31, 2017

@madsb never mind as it would require change to all sdks

@madsb
Copy link
Contributor Author

madsb commented Jul 31, 2017

Ah, I see - you mean adding something like Parse.Polygon to the JS SDK? I briefly considered that but seemed like it would take a little more time than I had.

@dplewis
Copy link
Member

dplewis commented Jul 31, 2017

Parse.Polygon is already in parse-server

@madsb
Copy link
Contributor Author

madsb commented Jul 31, 2017

@dplewis That's correct, but I was talking about the JS SDK.

Anyway, I think I've found a bug. When saving a Polygon object to the database, the coordinates are saved as [lat, lon] pairs. But mongo expects [lon, lat] which makes $geoIntersect queries fail.

@dplewis
Copy link
Member

dplewis commented Jul 31, 2017

@madsb For that I'm just waiting for parse-server 2.6.0 to be released then I'll rerun the tests. It works locally.

Edited: I meant adding to this PR.

$geoWithin: {
  $polygon:  { __type: 'Polygon', coordinates:[...] },
}

@madsb
Copy link
Contributor Author

madsb commented Jul 31, 2017

The tests are succeeding with the test coordinates, but try running the polygon save open path with these real-life coordinates:

const coords = [[33.5746,-111.9250],[33.4769,-112.0002],[33.4761,-111.8391]];
const closed = [[33.5746,-111.9250],[33.4769,-112.0002],[33.4761,-111.8391],[33.5746,-111.9250]];

@madsb
Copy link
Contributor Author

madsb commented Jul 31, 2017

Alright, never mind... I was confused about the whole lat/lon lon/lat thing. GeoJSON is lon/lat, and thus I must also define lon/lat when saving my Polygon object. Pretty confusing when everything else in Parse is lat/lon 😬

@dplewis
Copy link
Member

dplewis commented Jul 31, 2017

Yeah WGS84 is annoying. What about __type: Polygon to this PR?

$geoWithin: {
  $polygon:  { __type: 'Polygon', coordinates:[...] },
}

@madsb
Copy link
Contributor Author

madsb commented Jul 31, 2017

Yeah, my epiphany above has kind of invalidated this pull request, so I'll close this and open a new one with __type === 'Polygon'.

@madsb madsb closed this Jul 31, 2017
@madsb madsb deleted the polygon-allow-coord-array branch July 31, 2017 15:05
@madsb
Copy link
Contributor Author

madsb commented Jul 31, 2017

@dplewis #4067

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.

2 participants