Skip to content
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

fix for issue #5 #15

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

fix for issue #5 #15

wants to merge 10 commits into from

Conversation

schnittchen
Copy link

No description provided.

@schnittchen
Copy link
Author

these modifications (up to 32b3a46) make the tests pass (except for one, just as before) on ruby 1.8.7 (tested with ree)

@schnittchen
Copy link
Author

sorry, forgot that I had only run part of the tests while debugging. With 81257b8, all but two tests pass:

  • "Mongoid::Contexts::Mongo :page :paginator :kaminari should have num_pages" fails on 1.8.7 as well as on 1.9.2 (issue Fix failed kaminari spec #6 i guess)
  • "Mongoid::Criterion::Inclusion#where Geo Spacial Complex Where#near returns the documents sorted closest to furthest" fails on 1.8.7 only. But I have never seen this type of query before...

@schnittchen
Copy link
Author

About the second failing test mentioned above: I missed that this was exactly the query syntax introduced by the gem. The test fails with

"geo values have to be numbers: { $maxDistance: 20.0, $near: [ 41.23, 2.9 ] }"

because mongodb geo queries are order sensitive in the hash (swapping $maxDistance and $near would make it work!). Though this could be fixed by using BSON::OrderedHash again, that would break other things... Python has the same issues: https://jira.mongodb.org/browse/PYTHON-292

This effectively means the new syntax introduced by the gem is not usable on ruby 1.8, however geoNear seems to work well, so one can use geoNear (if ordering by distance or obtaining the distance is required) and the old way

Bar.where(:location => {"$within" : {"$center" : [center, radius]}})

and its variants otherwise.

@arthurnn
Copy link
Collaborator

arthurnn commented May 8, 2013

Do you mind squashing your commits together, and making sure everything still works before I merge it in ?

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.

None yet

2 participants