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

Read has_one through association target from memory for new records #42575

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

intrip
Copy link
Contributor

@intrip intrip commented Jun 23, 2021

Summary

When reading an has_one through association for a new record and loading from DB is not possible retrieves the association target from the in memory through association.

Fixes #42387

@intrip intrip force-pushed the 42387-load-has-one-through-from-memory branch 4 times, most recently from 58f9719 to a533a19 Compare June 23, 2021 12:15
@intrip intrip marked this pull request as ready for review June 23, 2021 12:38
@ball-hayden
Copy link

and loading from DB is not possible

Possibly a stupid question, but does it make sense to favour the database over what we have in memory?

Thinking about my test from #42387, if I adapt it to the following:

  def test_nonfiction_book_has_author
    author = Author.new(name: "Author")
    book = Book.new(author: author, title: "Book")
    book.save!
    nonfiction_book = NonfictionBook.new(book: book, subject_area: "Ruby on Rails")

    author.update!(name: "New Author")

    assert_equal book.author.name, author.name
    assert_equal nonfiction_book.author.name, author.name
  end

If we load from the database in the has_one through: case, do we not end up with nonfiction_book.author.name being "New Author" while book.author.name stays as "Author"?

@intrip
Copy link
Contributor Author

intrip commented Jun 23, 2021

and loading from DB is not possible

Possibly a stupid question, but does it make sense to favour the database over what we have in memory?

Thinking about my test from #42387, if I adapt it to the following:

  def test_nonfiction_book_has_author
    author = Author.new(name: "Author")
    book = Book.new(author: author, title: "Book")
    book.save!
    nonfiction_book = NonfictionBook.new(book: book, subject_area: "Ruby on Rails")

    author.update!(name: "New Author")

    assert_equal book.author.name, author.name
    assert_equal nonfiction_book.author.name, author.name
  end

If we load from the database in the has_one through: case, do we not end up with nonfiction_book.author.name being "New Author" while book.author.name stays as "Author"?

Honestly I didn't run the above piece of code but I think it should use the DB already in such situation because we set the FK: https://github.com/rails/rails/blob/main/activerecord/lib/active_record/associations/association.rb#L281

This PR handles the case when the associations were not already persisted without impacting the already existing behaviour.

When reading an `has_one` through association for a new record and loading from DB
is not possible retrieves the association target from the in memory through association.

Fixes rails#42387
@intrip intrip force-pushed the 42387-load-has-one-through-from-memory branch from a533a19 to bef7d67 Compare June 25, 2021 15:36
@rails-bot
Copy link

rails-bot bot commented Sep 23, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

@rails-bot rails-bot bot added the stale label Sep 23, 2021
@intrip
Copy link
Contributor Author

intrip commented Sep 23, 2021

still relevant

@rails-bot rails-bot bot removed the stale label Sep 23, 2021
@rails-bot
Copy link

rails-bot bot commented Dec 22, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

@rails-bot rails-bot bot added the stale label Dec 22, 2021
@intrip
Copy link
Contributor Author

intrip commented Dec 22, 2021

ping

@rails-bot rails-bot bot removed the stale label Dec 22, 2021
@mpmenne
Copy link

mpmenne commented Feb 8, 2022

@intrip nice PR 👍. I think this is causing an issue for me. Using a has_one through a delegated type relationship and the active record object is returning nil.

@intrip
Copy link
Contributor Author

intrip commented Feb 10, 2022

@mpmenne I don't understand how this could cause you issue since is not merged yet: are you using this patch in your codebase?

If so could you please provide me a code example?

@mpmenne
Copy link

mpmenne commented Feb 11, 2022

@intrip sorry. I meant to say #42387 was causing me an issue. Would love to see this PR merged. Sorry for the confusion.

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.

Delegated Type associations do not cascade
3 participants