#4538 ActiveRecord.clone does not clear out timestamps - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#4538 ✓resolved

ActiveRecord.clone does not clear out timestamps

Reported by arash | May 5th, 2010 @ 09:09 PM

The docs says that a cloned object is 'treated as a new record', however the updated_at/created_at timestamps seems to be persisted after the save! This is a bit counter intuitive.

I'm using the following monkey patch to work around this:

module ActiveRecord

class Base
  def clone_with_no_timestamps
    record = clone_without_no_timestamps
    record.updated_at = nil if record.respond_to?(:updated_at)
    record.created_at = nil if record.respond_to?(:created_at)
    record
  end

  alias_method_chain :clone, :no_timestamps
end

end

Comments and changes to this ticket

You flagged this item as spam.
Cesario May 8th, 2010 @ 11:48 AM
- Tag set to “activerecord, patch”
Here's a patch against master. It's forcing updated_at, updated_on, created_at and created_on to nil on the clone object.
- clone_clear_timestamps.diff 6.6 KB
Dan Pickett May 9th, 2010 @ 06:42 PM
- Tag changed from “activerecord, patch” to “activerecord, bugmash, patch”
You flagged this item as spam.
Cesario May 14th, 2010 @ 08:50 AM
- Tag changed from “activerecord, bugmash, patch” to “3.0.0.beta3, activerecord, bugmash, patch”
You flagged this item as spam.
Christopher Redinger May 14th, 2010 @ 06:11 PM
+1

This patch applies cleanly. It's well tested. All tests passed.
Neeraj Singh May 14th, 2010 @ 06:20 PM
If clone method is going to wipe out timestamps then clone should accpet an option (:preserve_timestamps => true) that will preserve timestamps incase I need it.

pleax May 15th, 2010 @ 02:06 AM

-1

There is a problem with current patch with models that have no created_at/updated_at fields.
For that kind of models following test will fail:

test "cloned model attributes" do
  token = Token.new # model without created_at/updated_at timestamps
  cloned_token = token.clone

  [:created_at, :updated_at].each do |key|
    assert_equal token.has_attribute?(key), cloned_token.has_attribute?(key)
  end
end

You flagged this item as spam.
Cesario May 15th, 2010 @ 07:54 AM
Here's a more complete patch, that takes into account the comment made by pleax.

Neeraj, from what I've learned so far, AR::Base does not override clone (or dup). It just overrides its specific behavior by defining initialize_copy.

So if one need to preserve the timestamps, this would need to be redefined by another specific initialize_copy method in one's own classes.

Is this an acceptable solution? If not, I'll have to reintroduce clone and I'm almost sure it's a bad idea if I refer to ticket/patch #3164.

Thanks for your feedbacks!
- clone_clear_timestamps_v2.diff 6.9 KB
Jeff Kreeftmeijer May 15th, 2010 @ 10:22 AM
+1

Applies cleanly. All tests passing. :)
You flagged this item as spam.
pleax May 15th, 2010 @ 10:40 AM
+1

With updated patch test-cases and implementation seem full and correct for me.
You flagged this item as spam.
PacoGuzman May 15th, 2010 @ 11:13 AM
+1 All tests passing
Santiago Pastorino May 15th, 2010 @ 04:57 PM
I don't agree with doing that way i prefer the default behavior the way it is and an option (:preserve_timestamps => false)
Jeremy Kemper May 15th, 2010 @ 05:54 PM
- State changed from “new” to “open”
Perhaps dup should keep the timestamps (identical copy) and clone should not (new copy).
You flagged this item as spam.
Cesario May 15th, 2010 @ 06:29 PM
Jeremy: dup is already preserving the timestamps (it's not using initialize_copy).
You flagged this item as spam.
Michael Raidel May 15th, 2010 @ 07:11 PM
+1

All ar-mysql- and -postgresql-tests pass (well, there are 2 failures for postgresql and 2 failures and 1 error for mysql, but they are the same without the path), patch applies cleanly, behaviour makes sense
You flagged this item as spam.
Brian Underwood November 19th, 2010 @ 02:06 PM
- Importance changed from “” to “Low”
What happened to this ticket? I would love for this to change. I think the preserve_timestamps timestamps option is good, but I think it makes more sense for the timestamps not to be preserved by default.
You flagged this item as spam.
Cesario November 24th, 2010 @ 09:26 AM
Brian, here are some follow-ups about this https://github.com/rails/rails/pull/113
Aaron Patterson November 24th, 2010 @ 05:20 PM
- State changed from “open” to “resolved”
I've applied Cesario's patches to master. dup will not preserve timestamps.
You flagged this item as spam.
bingbing March 26th, 2011 @ 02:03 AM
louis vuitton watches