Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

ActiveRecord compatible association reflection #364

Merged
merged 3 commits into from Dec 14, 2011

Conversation

Projects
None yet
3 participants
Contributor

balexand commented Dec 14, 2011

This pull request provides an ActiveRecord compatible API for reflecting on associations. It allows MongoMapper to be used with association helpers in simple_form and formtastic. It would close #356 and it is an alternative to #362.

I prefer this approach of using an adapter over the approach in #362. I've tested this in simple_form but not formtastic. After @bkeepers decides which, if either, of the pull requests to merge then I'd be happy to test with simple_form. Maybe @jjoe64 and @bjornblomqvist can test with formtastic.

@bkeepers Let me know if you have any feedback. I'd be happy revise things.

Contributor

bkeepers commented Dec 14, 2011

How do you feel about just putting this in the existing Rails module? The list of plugins is already a little overwhelming, and this functionality seems to fit with the Rails module.

This looks much better than my patch.

My changes would not work with simple_form. Why it works with formtastic is becouse formtastic has a code path for mongo.

https://github.com/justinfrench/formtastic/blob/master/lib/formtastic/helpers/reflection.rb

So if this is the code that gets merged some one should do a update over at formtastic to remove the mongo code path.

Contributor

balexand commented Dec 14, 2011

I moved reflect_on_association to the Rails module. Good suggestion; it was silly to create a new module for one method. I left the tests in a separate file. Let me know if there's anything else I can do.

@bkeepers bkeepers added a commit that referenced this pull request Dec 14, 2011

@bkeepers bkeepers Merge pull request #364 from balexand/association_reflection
ActiveRecord compatible association reflection
857970d

@bkeepers bkeepers merged commit 857970d into mongomapper:master Dec 14, 2011

Contributor

bkeepers commented Dec 14, 2011

Pulled. Thanks for your work on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment