Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Add inverse of add target #12443

Merged
merged 2 commits into from

2 participants

@arthurnn
Collaborator

Follow up with fix for #12413.

Add back set_inverse_instance on .add_to_target
We must have it in there too, so when an existent record is being concat to another, we will have the inverse relation.

This line is there on master. So we should add it back.
https://github.com/rails/rails/blob/master/activerecord/lib/active_record/associations/collection_association.rb#L378

review @rafaelfranca

@arthurnn arthurnn Add back set_inverse_instance on .add_to_target
We must have it in there too, so when an existent record is being concat to another,
we will have the inverse relation.
655396c
@rafaelfranca
Owner

Can't we add a regression test?

@arthurnn
Collaborator

:+1: yes we can ! Will look into it.

@arthurnn
Collaborator

It would be nice to add the same regression test into master, as if we remove the same line in there, all tests pass.
So should I commit the regression test in a separate commit, than you cherry-pick to there? Or should I create another PR only with the regression test.

@rafaelfranca
Owner

both options are fine to me. Your call

@arthurnn
Collaborator

Done... if you could then cherry-pick fc59e99 into master, and maybe 4.0-stable, would be nice.

@rafaelfranca rafaelfranca merged commit 7ed5bdc into from
@arthurnn arthurnn deleted the branch
@rafaelfranca
Owner

Done

@rafaelfranca rafaelfranca referenced this pull request from a commit
@rafaelfranca rafaelfranca Revert "Merge pull request #12443 from arthurnn/add_inverse_of_add_ta…
…rget"

This reverts commit 7ed5bdc, reversing
changes made to 31c79e2.

Reason: this caused a regression when the associated record is creted in
a before_create callback.

See #12413 (comment)
9639f65
@rafaelfranca rafaelfranca referenced this pull request from a commit
@rafaelfranca rafaelfranca Revert "Merge pull request #12443 from arthurnn/add_inverse_of_add_ta…
…rget"

This reverts commit 7ed5bdc, reversing
changes made to 31c79e2.

Reason: this caused a regression when the associated record is creted in
a before_create callback.

See #12413 (comment)
fbc69ac
@tenderlove tenderlove referenced this pull request from a commit
@tenderlove tenderlove Merge branch '3-2-15' into 3-2-sec
* 3-2-15:
  bumping to rc3
  Revert "Merge pull request #12413 from arthurnn/inverse_of_on_build"
  Revert "Merge pull request #12443 from arthurnn/add_inverse_of_add_target"
  bumping to rc2
  Merge pull request #12443 from arthurnn/add_inverse_of_add_target
  bumping version to 3.2.15.rc1
  Fix STI scopes using benolee's suggestion. Fixes #11939
eb8807e
@tenderlove tenderlove referenced this pull request from a commit
@tenderlove tenderlove Merge branch '3-2-sec' into 3-2-stable
* 3-2-sec:
  updating changelogs
  bumping to 3.2.15
  bumping to rc3
  Revert "Merge pull request #12413 from arthurnn/inverse_of_on_build"
  Revert "Merge pull request #12443 from arthurnn/add_inverse_of_add_target"
  bumping to rc2
  Merge pull request #12443 from arthurnn/add_inverse_of_add_target
  bumping version to 3.2.15.rc1
  Remove the use of String#% when formatting durations in log messages

Conflicts:
	activerecord/CHANGELOG.md
5f844d6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 4, 2013
  1. @arthurnn

    Add back set_inverse_instance on .add_to_target

    arthurnn authored
    We must have it in there too, so when an existent record is being concat to another,
    we will have the inverse relation.
  2. @arthurnn
This page is out of date. Refresh to see the latest.
View
1  activerecord/lib/active_record/associations/collection_association.rb
@@ -350,6 +350,7 @@ def add_to_target(record)
end
callback(:after_add, record)
+ set_inverse_instance(record)
record
end
View
13 activerecord/test/cases/associations/inverse_associations_test.rb
@@ -290,6 +290,19 @@ def test_parent_instance_should_be_shared_with_first_n_and_last_n_children
def test_trying_to_use_inverses_that_dont_exist_should_raise_an_error
assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Man.find(:first).secret_interests }
end
+
+ def test_child_instance_should_point_to_parent_without_saving
+ man = Man.new
+ i = Interest.create(:topic => 'Industrial Revolution Re-enactment')
+
+ man.interests << i
+ assert_not_nil i.man
+
+ i.man.name = "Charles"
+ assert_equal i.man.name, man.name
+
+ assert !man.persisted?
+ end
end
class InverseBelongsToTests < ActiveRecord::TestCase
Something went wrong with that request. Please try again.