ActiveRecord.clone does not clear out timestamps

+1

This patch applies cleanly. It's well tested. All tests passed.

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.

-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

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!

+1

Applies cleanly. All tests passing. :)

+1

With updated patch test-cases and implementation seem full and correct for me.

+1 All tests passing

I don't agree with doing that way i prefer the default behavior the way it is and an option (:preserve_timestamps => false)

Jeremy: dup is already preserving the timestamps (it's not using initialize_copy).

+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

Brian, here are some follow-ups about this https://github.com/rails/rails/pull/113

louis vuitton watches