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

Rails 5.0/5.1 compatibility: Define through association before has many through #18080

Conversation

jrafanie
Copy link
Member

Rails 5.1+ complains about has_many through associations if the through
association is not yet defined.

Extracted from #18076

According to 85c0224, we want the
cascading delete to avoid orphaned resources, so we'll remove the one
that doesn't explicitly say dependent => destroy.

Rails 5.1+ complains about has_many through associations if the through
association is not yet defined, and in this case, the second
host_storages association occurs after the through association, so rails
yells:

```
     ActiveRecord::HasManyThroughOrderError:
            Cannot have a has_many :through association 'Storage#hosts'
            which goes through 'Storage#host_storages' before the
            through association is defined.
```

Extracted from ManageIQ#18076
Rails 5.1+ complains about has_many through associations if the through
association is not yet defined.

Extracted from ManageIQ#18076
Copy link
Member

@NickLaMuro NickLaMuro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything else seems fine, but confused about the first change.

@@ -18,7 +18,6 @@ class Storage < ApplicationRecord
has_many :storage_files, :dependent => :destroy
has_many :storage_files_files, -> { where("rsc_type = 'file'") }, :class_name => "StorageFile", :foreign_key => "storage_id"
has_many :files, -> { where("rsc_type = 'file'") }, :class_name => "StorageFile", :foreign_key => "storage_id"
has_many :host_storages
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this doing here? Seems unrelated to the original PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, let me explain. There's two host_storages has_manys. One on line line 5 and one here, at line 21. Because this one on line 21 is "last", it wins. This causes a problem because line 6, we have:

has_many :hosts, :through => :host_storages

This causes complaints on rails 5.1 because the through association is needed before it's defined.

@miq-bot
Copy link
Member

miq-bot commented Oct 12, 2018

Checked commits jrafanie/manageiq@6ce9a5c~...6423d74 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 👍

@kbrock kbrock self-assigned this Oct 13, 2018
@kbrock kbrock added this to the Sprint 97 Ending Oct 22, 2018 milestone Oct 13, 2018
@kbrock kbrock merged commit 1244781 into ManageIQ:master Oct 13, 2018
@jrafanie jrafanie deleted the rails_5_1_define_through_association_before_has_many_through branch December 14, 2018 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants