Skip to content
This repository has been archived by the owner on Apr 17, 2018. It is now read-only.

Inconsistent timestamps behaviour when hard setting the :at/:on values #4

Open
solnic opened this issue May 17, 2011 · 10 comments
Open

Comments

@solnic
Copy link
Contributor

solnic commented May 17, 2011

So in a model we have timestamps :at.

It creates created_at and updated_at, that's fine.

However, upon creating a new record, if I hard set both values, created_at respects my provided value, but updated_at does not. I can only assume the :on pair behaves the same too.

Is this a feature or a bug?


Created by Fred Wu - 2010-04-16 05:30:23 UTC

Original Lighthouse ticket: http://datamapper.lighthouseapp.com/projects/20609/tickets/1245

@solnic
Copy link
Contributor Author

solnic commented May 17, 2011

I would think that this is a bug, I added a standalone to reproduce the behavior at: http://github.com/snusnu/dm-snippets/commit/e134b6d9c1ad45e3bf80dd417783a27d4cb49c0c

by Martin Gamsjaeger (snusnu)

@solnic
Copy link
Contributor Author

solnic commented May 17, 2011

If you set these fields on your own, then why do you use dm-timestamps? One of the reasons why one would want to use this plugin is to not care about setting these fields manually. I’m not entirely sure if this is a valid bug.

by Piotr Solnica (solnic)

@solnic
Copy link
Contributor Author

solnic commented May 17, 2011

I’m not entirely sure either but it seems a bit weird that stuff behaves differently for created_at(on) and updates_at(on)

by Martin Gamsjaeger (snusnu)

@solnic
Copy link
Contributor Author

solnic commented May 17, 2011

Well for me, I need it in testing. :)

by Fred Wu

@solnic
Copy link
Contributor Author

solnic commented May 17, 2011

I would accept a patch (with specs) to dm-timestamps that makes it so those values are not set if attribute_dirty?(property_name) returns true.

While I think this is a bit of an edge case, in other parts of DM we’ve made it so that user-supplied data takes precedence over defaults set by the system, so I think it fits in with the approach we’ve taken elsewhere.

by Dan Kubb (dkubb)

@solnic
Copy link
Contributor Author

solnic commented May 17, 2011

I’d like to help because this is blocking me from writing tests for my own project. A little direction? I’m new to working with git & plugins. Bear with me on what’s probably a 101 question, and feel free to point me to resources on contributing

I’m pretty sure I have this fixed (with specs), but can’t get spec to work:

~/Documents/workspace/dm-timestamps$ spec spec/integration/timestamps_spec.rb 
/opt/local/lib/ruby/vendor_ruby/1.8/rubygems/custom_require.rb:31:in `gem_original_require’: no such file to load -- dm-core/spec/setup (LoadError)

I’ve tried cloning the dm-core project, but those files do not exist.

FWIW, here’s what I have so far:

~/Documents/workspace/dm-timestamps$ git diff
diff --git a/lib/dm-timestamps.rb b/lib/dm-timestamps.rb
index 23ae865..054a2a5 100644
--- a/lib/dm-timestamps.rb
+++ b/lib/dm-timestamps.rb
 @@ -3,8 +3,8 @@ require ’dm-core’
 module DataMapper
   module Timestamps
     TIMESTAMP_PROPERTIES = {
-      :updated_at => [ DateTime, lambda { |r| DateTime.now                             } ],
-      :updated_on => [ Date,     lambda { |r| Date.today                               } ],
+      :updated_at => [ DateTime, lambda { |r| r.updated_at || DateTime.now             } ],
+      :updated_on => [ Date,     lambda { |r| r.updated_at || Date.today               } ],
       :created_at => [ DateTime, lambda { |r| r.created_at || (DateTime.now if r.new?) } ],
       :created_on => [ Date,     lambda { |r| r.created_on || (Date.today   if r.new?) } ],
     }.freeze
diff --git a/spec/integration/timestamps_spec.rb b/spec/integration/timestamps_spec.rb
index 393c668..7f8abff 100644
--- a/spec/integration/timestamps_spec.rb
+++ b/spec/integration/timestamps_spec.rb
 @@ -16,6 +16,18 @@ describe ’DataMapper::Timestamp’ do
         green_smoothie.created_at.should be_a_kind_of(DateTime)
         green_smoothie.created_on.should be_a_kind_of(Date)
       end
+      
+      it "should not set the updated_at/on fields if they’re already set" do
+        green_smoothie = GreenSmoothie.new(:name => ’Banana’)
+        time = (DateTime.now - 100)
+        green_smoothie.updated_at = time
+        green_smoothie.updated_on = time
+        green_smoothie.save
+        green_smoothie.updated_at.should == time
+        green_smoothie.updated_on.should == time
+        green_smoothie.updated_at.should be_a_kind_of(DateTime)
+        green_smoothie.updated_on.should be_a_kind_of(Date)
+      end

       it "should set the created_at/on fields on creation" do
         green_smoothie = GreenSmoothie.new(:name => ’Banana’)
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb

by orbiteleven

@solnic
Copy link
Contributor Author

solnic commented May 17, 2011

Er, scratch that last diff for this one :)

diff --git a/lib/dm-timestamps.rb b/lib/dm-timestamps.rb
index 23ae865..ba2b0a8 100644
--- a/lib/dm-timestamps.rb
+++ b/lib/dm-timestamps.rb
@@ -3,8 +3,8 @@ require ’dm-core’
module DataMapper
  module Timestamps
    TIMESTAMP_PROPERTIES = {
-      :updated_at => [ DateTime, lambda { |r| DateTime.now                             } ],
-      :updated_on => [ Date,     lambda { |r| Date.today                               } ],
-      :updated_at => [ DateTime, lambda { |r| r.updated_at || DateTime.now             } ],
-      :updated_on => [ Date,     lambda { |r| r.updated_on || Date.today               } ],
    :created_at => [ DateTime, lambda { |r| r.created_at || (DateTime.now if r.new?) } ],
    :created_on => [ Date,     lambda { |r| r.created_on || (Date.today   if r.new?) } ],
  }.freeze
 diff --git a/spec/integration/timestamps_spec.rb b/spec/integration/timestamps_spec.rb
 index 393c668..7f8abff 100644
 --- a/spec/integration/timestamps_spec.rb
 +++ b/spec/integration/timestamps_spec.rb
 @@ -16,6 +16,18 @@ describe ’DataMapper::Timestamp’ do
        green_smoothie.created_at.should be_a_kind_of(DateTime)
        green_smoothie.created_on.should be_a_kind_of(Date)
      end
-   
-      it "should not set the updated_at/on fields if they’re already set" do
-        green_smoothie = GreenSmoothie.new(:name => ’Banana’)
-        time = (DateTime.now - 100)
-        green_smoothie.updated_at = time
-        green_smoothie.updated_on = time
-        green_smoothie.save
-        green_smoothie.updated_at.should == time
-        green_smoothie.updated_on.should == time
-        green_smoothie.updated_at.should be_a_kind_of(DateTime)
-        green_smoothie.updated_on.should be_a_kind_of(Date)
- ```
  end
 it "should set the created_at/on fields on creation" do
   green_smoothie = GreenSmoothie.new(:name => ’Banana’)

diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb


by orbiteleven

@solnic
Copy link
Contributor Author

solnic commented May 17, 2011

Hey orbiteleven,

To get up and running just clone dm-timestamps and do:

cd dm-timestamps
export ADAPTER=sqlite
bundle install
bundle exec spec spec/integration

This needs bundler to be installed, if you don’t have just do gem i bundler.

Regarding your patch - you need to check if attribute_dirty?(:updated_at/:updated_on) returns true and only if it does you don’t set the values. Thanks for you help :)

by Piotr Solnica (solnic)

@solnic
Copy link
Contributor Author

solnic commented May 17, 2011

Much better, thanks for the help.

I’ve attached my patch for this ticket. I hope it helps! If not, constructive criticism welcome.

by orbiteleven

@ghost
Copy link

ghost commented Aug 27, 2011

I'm seeing this issue to. I have created_at and updated_at properties. updated_at sets properly but in order to get created_at to work I have to declare timestamps :created_at, :updated_at in the model.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

1 participant