moiristo / deep_cloneable

This gem gives every ActiveRecord::Base object the possibility to do a deep clone that includes user specified associations.
MIT License
786 stars 88 forks source link

deep_clone mess up with date offset if not saved yet #51

Closed AlexGaspar closed 9 years ago

AlexGaspar commented 9 years ago

Hey,

If the record is not saved yet and you try to create a deep clone of it, it will result in false date offset

foo = Foo.new(param['foo']) # param['foo'] as bar_attributes in it
foo_cloned = foo.deep_clone include: :bar

>> foo_cloned.bar.first.start_at
=> Fri, 31 Jul 2015 14:00:00 CEST +02:00
>> foo.bar.first.start_at
=> Fri, 31 Jul 2015 12:00:00 CEST +02:00

For this to not happend I've to save the record and reload it.

foo = Foo.new(param['foo']) # param['foo'] as bar_attributes in it
foo.save
foo.reload
foo_cloned = foo.deep_clone include: :bar

>> foo_cloned.bar.first.start_at
=> Fri, 31 Jul 2015 12:00:00 CEST +02:00
>> foo.bar.first.start_at
=> Fri, 31 Jul 2015 12:00:00 CEST +02:00

Do you have an idea why this happen? Alex

moiristo commented 9 years ago

It doesn't look like this is an issue that is caused by deep_cloneable as it doesn't modify attribute values. There seems to be a difference in how activerecord parses a user specified timestamp and a timestamp returned by the database in your application. Did you specify your time zone properly in (Rails.application.)config.time_zone?

AlexGaspar commented 9 years ago

Thanks for the quick response!

Yep : config.time_zone = 'Europe/Brussels. And in all cases start_at.class returns ActiveSupport::TimeWithZone.

moiristo commented 9 years ago

The gem uses ActiveRecord#dup for creating a copy of the object. Isn't the result of foo.bar.first.dup.start_at the same?

AlexGaspar commented 9 years ago

Yep, I still have the issue, so I guess, you're doing nothing wrong :)

>> foo.bar.first.dup.start_at
=> Fri, 31 Jul 2015 14:00:00 CEST +02:00
>> foo.bar.first.start_at
=> Fri, 31 Jul 2015 12:00:00 CEST +02:00
moiristo commented 9 years ago

:thumbsup: which AR version is this?

AlexGaspar commented 9 years ago

Also if I don't dup the model but start_at it works correctly.

>> foo.bar.first.dup.start_at
=> Fri, 31 Jul 2015 14:00:00 CEST +02:00
>> foo.bar.first.start_at.dup
=> Fri, 31 Jul 2015 12:00:00 CEST +02:00

AR : activerecord (3.2.21)

moiristo commented 9 years ago

Can reproduce.. seems to be fixed in 4:

3.2.22:

irb(main)> user = User.new
=> #<User id: nil>
irb(main)> user.activated_at = "Fri, 31 Jul 2015 14:00:00 CEST +02:00"
=> "Fri, 31 Jul 2015 14:00:00 CEST +02:00"
irb(main)> user.activated_at
=> Fri, 31 Jul 2015 14:00:00 CEST +02:00
irb(main)> user.dup.activated_at
=> Fri, 31 Jul 2015 16:00:00 CEST +02:00

4.1.12:

irb(main) dummy:development> user = User.new
=> #<User id: nil>
irb(main) dummy:development> user.activated_at = "Fri, 31 Jul 2015 14:00:00 CEST +02:00"
=> "Fri, 31 Jul 2015 14:00:00 CEST +02:00"
irb(main) dummy:development> user.activated_at
=> Fri, 31 Jul 2015 14:00:00 CEST +02:00
irb(main) dummy:development> user.dup.activated_at
=> Fri, 31 Jul 2015 14:00:00 CEST +02:00

Please let me know when you find a solution for 3.2 :)

AlexGaspar commented 9 years ago

Thanks man, really appreciate your help on this :+1:

We're planing to upgrade, so I just quick fixed it, with a for_eachreplacing the cloned dates by the original dates(not pretty at all, but...).