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 geo_scope with :through #8

Closed
wants to merge 8 commits into from
Closed

Conversation

mcmire
Copy link

@mcmire mcmire commented Jan 16, 2011

This is a fix for #3. Basically :include is now included automatically for a model that acts_as_mappable :through => :some_column. You'll be happy to know that all the tests pass as well.

It should be noted that this branch is based off the ensure_tests_pass_under_mysql_and_postgres branch I made for #7 (this branch is actually 3 commits and starts at c9172e1). Once you pull that branch in, I'll rebase this.

…uccessfully under Postgres

Add Postgres as a development dependency
…> :some_column by automatically .include'ing the :through option

Also, ActsAsMappable::ClassMethods#geo_scope now returns an instance of
ActsAsMappable::Relation instead of ActiveRelation. Basically, we override
with the distance formula. In other words, if you say
Model.geo_scope(...).where("distance > 5") or
Model.geo_scope(...).order("distance desc"), that "distance" is automatically
replaced with the big long SQL that's generated (the Haversine formula, or
whatever). The downside is that you have to put the .where after the .geo_scope,
otherwise it won't work.
@dennisreimann
Copy link

I would be great if this could be pulled into the main repo as it fixes a lot of things and has a proper test suite :)

@chriscrown
Copy link

That would make geokit-rails3 really useful!

@tsujigiri
Copy link

I tried your branch and am not able to get it to work properly. This is what I did: https://gist.github.com/986323

@jlecour
Copy link
Contributor

jlecour commented Jan 31, 2012

Hi, I'm very sorry that I didn't take care of this in a loooong time.

According to GitHub, this PR can't be merged automatically, so it needs at least an update.

And since it's a quite important improvement, I'd like to have some other confirmation that everything is good.

@mcmire
Copy link
Author

mcmire commented Feb 1, 2012

Okay, no worries. Let me take another look at this and rebase the branch and I'll let you know.

@kevinelliott
Copy link

What's the status of this merge?

@mcmire
Copy link
Author

mcmire commented Apr 5, 2012

Thanks for reminding me. I made a new branch that is up to date with master (#26). Give it a shot!

@mcmire mcmire closed this Apr 5, 2012
@bjornlinder
Copy link

I've found that I need to include .joins(:location) to get the geokit scopes to work on my 'acts_as_mappable :through' models, else I get the 'missing FROM-clause entry for table "locations"' error. My models are all 'belongs_to :location'. Would it be appropriate to add in a conditional join based on the relation type?

@mnoack
Copy link
Member

mnoack commented Sep 25, 2014

@bjornlinder This issue is in the wrong project (I probably should update the readme).

Please use geokit-rails (which is for rails3 nowdays). Switching projects may even result in some problems being fixed.

Ideally you wouldn't need to do extra joins, but if you have to do something to get it to work I personally think that's fine. It's appropriate if it's the best/only solution.

@bjornlinder
Copy link

Ah - sorry about that. Yes, I am using geokit-rails and rails 4.1.5.

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.

8 participants