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

Fixed odd behavior of inverse_of with multiple belongs_to to same class #40643

Merged
merged 1 commit into from
Nov 21, 2020

Conversation

ktmouk
Copy link
Contributor

@ktmouk ktmouk commented Nov 18, 2020

Fix: #35204.

This PR added validation to automatic_inverse_of that foreign_keys are the same.

If class has multiple belongs_to to same class, automatic_inverse_of can find the wrong inverse_name.

class Room < ActiveRecord::Base
  belongs_to :user
  belongs_to :owner, class_name: "User"
end

class User < ActiveRecord::Base
  has_one :room
  has_one :owned_room, class_name: "Room", foreign_key: "owner_id"
end

user = User.create!
owned_room = Room.create!(owner: user)

p user.room

The current automatic_inverse_of validates the reflection that found from associations.
However, its validation does not validate that foreign keys are the same.

so this issue can be fixed by adding a validation of foreign keys.

@ktmouk ktmouk marked this pull request as ready for review November 19, 2020 06:13
@ktmouk ktmouk force-pushed the fixed-automatic-inverse-of branch 2 times, most recently from c01e05c to 1f969fc Compare November 19, 2020 07:43
@kamipo
Copy link
Member

kamipo commented Nov 19, 2020

As I said in the Slack, can you elaborate on the context bit more in the commit message?

I know the context that what is the cause and a way to prevent the issue, but it is preferable to share the context for other reviewers and future readers.

@ktmouk ktmouk force-pushed the fixed-automatic-inverse-of branch from 1f969fc to ad90033 Compare November 20, 2020 15:00
@ktmouk ktmouk changed the title Fixed automatic_inverse_of Fixed odd behavior of inverse_of with multiple belongs_to to same class Nov 20, 2020
activerecord/CHANGELOG.md Outdated Show resolved Hide resolved
@kamipo
Copy link
Member

kamipo commented Nov 20, 2020

user = User.create!
owned_room = Room.create!(owner: owned_room)

Typo s/owned_room/user/

user = User.create!
owned_room = Room.create!(owner: user)

Fix: rails#35204.

This PR added validation to `automatic_inverse_of` that foreign_keys are the same.

If class has multiple `belongs_to` to same class, `automatic_inverse_of` can find the wrong `inverse_name`.

```ruby
class Room < ActiveRecord::Base
  belongs_to :user
  belongs_to :owner, class_name: "User"
end

class User < ActiveRecord::Base
  has_one :room
  has_one :owned_room, class_name: "Room", foreign_key: "owner_id"
end

user = User.create!
owned_room = Room.create!(owner: user)

p user.room
```

The current `automatic_inverse_of` validates the `reflection` that found from associations.
However, its validation does not validate that foreign keys are the same.

so this issue can be fixed by adding a validation of foreign keys.

Co-authored-by: alpaca-tc <alpaca-tc@alpaca.tc>
Co-authored-by: cat2koban <taba.noritomo@moneyforward.co.jp>
Co-authored-by: luccafort <konishi.tatsuro@moneyforward.co.jp>
@ktmouk ktmouk force-pushed the fixed-automatic-inverse-of branch from ad90033 to 5b4a516 Compare November 20, 2020 17:08
@kamipo kamipo merged commit 1a45a47 into rails:master Nov 21, 2020
@kamipo
Copy link
Member

kamipo commented Nov 21, 2020

Thank you and congrats for your first contribution 🎉

kamipo added a commit that referenced this pull request Nov 21, 2020
Fixed odd behavior of inverse_of with multiple belongs_to to same class
@ktmouk ktmouk deleted the fixed-automatic-inverse-of branch November 21, 2020 04:39
@ktmouk
Copy link
Contributor Author

ktmouk commented Nov 21, 2020

Thank you, too!

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.

Odd behavior of inverse_of with multiple belongs_to to same class
2 participants