Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

fix merging the targer lists in 1.8. #1425

Closed
wants to merge 1 commit into from

3 participants

@dmathieu
Collaborator

For some reason, #delete(record) didn't return the record in memory, but the persisted one.
This could be considered as a bug in ruby, which is fixed in 1.9.

@dmathieu dmathieu fix merging the targer lists in 1.8.
For some reason, #delete(record) didn't return the record in memory, but the persisted one.
This could be considered as a bug, which is fixed in 1.9.
8f81990
@arunagw
Collaborator

Just merged in master.!! from #1417

@dmathieu dmathieu closed this
@jonleighton jonleighton referenced this pull request from a commit
@jonleighton jonleighton Implementing @dmathieu's cleaner fix from #1425. Unfortunately he del…
…eted the branch so I cannot just merge it.
9d17913
@jonleighton jonleighton referenced this pull request from a commit
@jonleighton jonleighton Implementing @dmathieu's cleaner fix from #1425. Unfortunately he del…
…eted the branch so I cannot just merge it.
60cb96a
@jonleighton
Collaborator

I agree with @dmathieu that his fix was cleaner. Unfortunately the branch was deleted so I couldn't merge it and give you the credit, but I have implemented the fix in 60cb96a anyway.

Also merged into 3-1-stable.

@jake3030 jake3030 referenced this pull request from a commit in jake3030/rails
@alloy alloy Allow optional arguments and/or block for Object#try like Object#send…
… does. [#1425 state:resolved]

Original suggestion by Pat Nakajima.

Signed-off-by: Pratik Naik <pratiknaik@gmail.com>
823b623
@ttosch ttosch referenced this pull request from a commit
@jonleighton jonleighton Implementing @dmathieu's cleaner fix from #1425. Unfortunately he del…
…eted the branch so I cannot just merge it.
99742c7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 31, 2011
  1. @dmathieu

    fix merging the targer lists in 1.8.

    dmathieu authored
    For some reason, #delete(record) didn't return the record in memory, but the persisted one.
    This could be considered as a bug, which is fixed in 1.9.
This page is out of date. Refresh to see the latest.
View
5 activerecord/lib/active_record/associations/collection_association.rb
@@ -402,9 +402,8 @@ def merge_target_lists(persisted, memory)
return memory if persisted.empty?
persisted.map! do |record|
- mem_record = memory.delete(record)
-
- if mem_record
+ if mem_id = memory.index(record)
+ mem_record = memory.delete_at(mem_id)
(record.attribute_names - mem_record.changes.keys).each do |name|
mem_record[name] = record[name]
end
Something went wrong with that request. Please try again.