Skip to content
This repository has been archived by the owner on May 13, 2022. It is now read-only.

Check for inverse_of in manually defined associations #193

Closed
lowjoel opened this issue Dec 25, 2014 · 8 comments
Closed

Check for inverse_of in manually defined associations #193

lowjoel opened this issue Dec 25, 2014 · 8 comments

Comments

@lowjoel
Copy link
Member

lowjoel commented Dec 25, 2014

Rails 4 added the inverse_of option when defining has_one and has_many relationships. It then does heuristics to determine the inverse of the relation, but does not warn when the inverse cannot be found.

inverse_of is necessary for proper behaviour in the following scenario: Assuming two models A and B, with instances a and b, and code like a.b.a.b.a would to be identical to a. We tend to expect this but if inverses are not properly set we will get 3 different A instances created.

In my opinion, adding a warning when ActiveRecord fails to find an association would be sufficient, but more adventurous contributors might want to add a rake task or some sort of validation that can be run in a CI build.

Of course, @ronen and I think that using the schema_associations gem is the best since that defines the inverses automatically, but for the rest of us who do not...

@lowjoel
Copy link
Member Author

lowjoel commented Jan 19, 2015

As a matter of reporting, we can see what the bullet gem does, and do something similar.

@lowjoel
Copy link
Member Author

lowjoel commented Jan 31, 2015

@ronen I've started some work with schema_association_inverses (name suggestions welcome...). That requires a PR from schema_monkey, but is essentially just raising an error when an inverse cannot be detected for now.

I absolutely cannot write specs for it because my development machines are all running Windows -- any way to get the build matrix working on it?

@ronen
Copy link
Member

ronen commented Jan 31, 2015

@lowjoel cool

schema_association_inverses (name suggestions welcome...)

maybe something like...?

  • schema_plus_association_inverses? make it be part of "the family"?
  • schema_plus_assoicaions -- be a general holder for basic association cleanups?

just raising an error when an inverse cannot be detected for now.

Is that likely to break things for core rails or 3rd-party gems that rely on lack of inverse failing quietly? Should this add support for something like inverse: :none to suppress the error if you know that there isn't one?

That requires a PR from schema_monkey

Looks good, go ahead and issue the PR! BTW see also SchemaPlus/schema_monkey#1

I absolutely cannot write specs for it because my development machines are all running Windows -- any way to get the build matrix working on it?

Hmm, I've never done any ruby development on Windows. I have no idea what it would take to get schema_dev rspec working on Windows. Is that something you can try to do? Unfortunately, AFAIK travis-ci doesn't yet have support for Windows, which will make it difficult for me to maintain it.

...or can you run linux in a VirtualBox on Windows and do your development in that...?

@lowjoel
Copy link
Member Author

lowjoel commented Jan 31, 2015

I think schema_plus_association_inverses should be good. schema_plus_associations sounds like the schema_associations gem.

Is that likely to break things for core rails or 3rd-party gems that rely on lack of inverse failing quietly? Should this add support for something like inverse: :none to suppress the error if you know that there isn't one?

I think we can preliminarily implement throwing an error first, once we have all the boilerplate up, we can start implementing nicer error displays (like how Bullet does it)

I have no idea what it would take to get schema_dev rspec working on Windows. Is that something you can try to do?

I could, but I'm afraid I'll have to push the stuff out without specs for now. RVM on Windows is a royal pain. I have a VM for things like these, but it's annoying to maintain separate environments... I'll try to see if I can get the test infra working on Windows then.

Unfortunately, AFAIK travis-ci doesn't yet have support for Windows, which will make it difficult for me to maintain it.

Yup. As a whole Ruby on Windows is fine. What works on Linux tends to work just fine on Windows, unless a native extension is involved...

@ronen
Copy link
Member

ronen commented Jan 31, 2015

I have no idea what it would take to get schema_dev rspec working on Windows. Is that something you can try to do?

I could, but I'm afraid I'll have to push the stuff out without specs for now. RVM on Windows is a royal pain.

Actually you can run tests without schema_dev and without RVM (or rbenv or chruby). You just need to do

$ BUNDLE_GEMFILE=gemfiles/rails-4.2/Gemfile.postgresql bundle exec rspec

...or whatever the equivalent is in windows?

I've been thinking of adding an option to schema_dev so that if the current ruby version is the same as the matrix version, it'll skip the RVM/rbenv/chruby stuff and just issue the above for each item in the matrix.

@lowjoel
Copy link
Member Author

lowjoel commented Mar 15, 2015

@ronen sorry I've been too busy lately. I just upgraded my gem to use the schema_plus_core gem (since that's where all the hooks are defined).

I'm going to give a shot at making the tests run.

@lowjoel
Copy link
Member Author

lowjoel commented Mar 15, 2015

@ronen So to this end, I've submitted SchemaPlus/schema_plus_core#1 to implement the hook needed for this gem. I've manually set up schema_dev in lowjoel/schema_plus_association_inverses@d5fdff3b2c and got the first test up and running.

Let me know what you think.

@lowjoel
Copy link
Member Author

lowjoel commented May 23, 2015

Guys, I've released lowjoel/schema_plus_association_inverses@5574f33

@lowjoel lowjoel closed this as completed May 23, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants