Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fix nested attribute for memory record. #1417

Merged
merged 3 commits into from

3 participants

@arunagw
Collaborator

Fixed nested_attributes tests

I found that

mem_record = memory.delete(record) is returning record itself. with ruby-1.8.7

/cc @jonleighton

/cc @tenderlove

@tenderlove tenderlove merged commit 021e4f1 into rails:master
@dmathieu
Collaborator

I'm sorry, but that's not very dry and not a nice piece of code either.
You're having two conditions when you could have only one, making it look far more complicated than what it really is.

I did the same thing this morning (in the train, therefor I didn't see this pull request) in a way which I believe to be nicer. See #1425

@dmathieu
Collaborator

I've refactored it in #1427

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
9 activerecord/lib/active_record/associations/collection_association.rb
@@ -402,7 +402,14 @@ def merge_target_lists(persisted, memory)
return memory if persisted.empty?
persisted.map! do |record|
- mem_record = memory.delete(record)
+
+ # To work with ruby 1.8.7
+ # > 1.9 #=> mem_record = memory.delete(record)
+ mem_record_index = memory.index(record)
+ if mem_record_index
+ mem_record = memory.at(mem_record_index)
+ memory.delete_at(mem_record_index)
+ end
if mem_record
(record.attribute_names - mem_record.changes.keys).each do |name|
View
10 activerecord/test/cases/associations/cascaded_eager_loading_test.rb
@@ -8,10 +8,12 @@
require 'models/topic'
require 'models/reply'
require 'models/person'
+require 'models/vertex'
+require 'models/edge'
class CascadedEagerLoadingTest < ActiveRecord::TestCase
fixtures :authors, :mixins, :companies, :posts, :topics, :accounts, :comments,
- :categorizations, :people, :categories
+ :categorizations, :people, :categories, :edges, :vertices
def test_eager_association_loading_with_cascaded_two_levels
authors = Author.find(:all, :include=>{:posts=>:comments}, :order=>"authors.id")
@@ -164,12 +166,6 @@ def test_eager_association_loading_where_first_level_returns_nil
authors[2].post_about_thinking.comments.first
end
end
-end
-
-require 'models/vertex'
-require 'models/edge'
-class CascadedEagerLoadingTest < ActiveRecord::TestCase
- fixtures :edges, :vertices
def test_eager_association_loading_with_recursive_cascading_four_levels_has_many_through
source = Vertex.find(:first, :include=>{:sinks=>{:sinks=>{:sinks=>:sinks}}}, :order => 'vertices.id')
Something went wrong with that request. Please try again.