Do not "touch" models when not updating the base table content. (#65)

* Do not trigger save when destryoing a draft for an unchanged widget

* Update columns instead of calling save

* Do not touch the model on save when revrting back to original

* Remove unnecesary lines

* Test updated_at in vanilla

* Fix wrong expectation on save_draft with existing create draft

* Fix wrong expectation on save_draft with existing create draft

* Test updated_at in skipper

* Do not “touch” model when saving draft and not changing base model.

* Check if skipped attributes were changed before saving model

* Use changed? as “touch” trigger

* One line expect

* Reload Skipper model before proceding with tests.

Needed to force DB precison on Datetime values.

lib/draftsman/draft.rb

@@ -175,23 +175,11 @@ def publish!
         self.item.attributes = self.reify.attributes if Draftsman.stash_drafted_changes? && self.update?
 
         # Write `published_at` attribute
-        self.item.send("#{self.item.class.published_at_attribute_name}=", Time.now)
+        self.item.send("#{self.item.class.published_at_attribute_name}=", current_time_from_proper_timezone)
 
         # Clear out draft
         self.item.send("#{self.item.class.draft_association_name}_id=", nil)
 
-        # Determine which columns should be updated
-        only   = self.item.class.draftsman_options[:only]
-        ignore = self.item.class.draftsman_options[:ignore]
-        skip   = self.item.class.draftsman_options[:skip]
-        attributes_to_change = only.any? ? only : self.item.attribute_names
-        attributes_to_change = attributes_to_change - ignore + ['published_at', "#{self.item.class.draft_association_name}_id"] - skip
-
-        # Save without validations or callbacks
-        self.item.attributes.slice(*attributes_to_change).each do |key, value|
-          self.item.send("#{key}=", value)
-        end
-
         self.item.save(validate: false)
         self.item.reload
 
@@ -277,7 +265,7 @@ def revert!
         end
         # Then clear out the draft ID.
         self.item.send("#{self.item.class.draft_association_name}_id=", nil)
-        self.item.save!(validate: false)
+        self.item.save!(validate: false, touch: false)
         # Then destroy draft.
         self.destroy
       when :destroy

lib/draftsman/model.rb

@@ -326,17 +326,18 @@ def _draft_update
 
               data = merge_metadata_for_draft(data)
               send(self.class.draft_association_name).update(data)
-              self.save
+              save
             else
               the_changes = changes_for_draftsman(:update)
               save_only_columns_for_draft if Draftsman.stash_drafted_changes?
 
-              # Destroy the draft if this record has changed back to the original
-              # record.
+              # Destroy the draft if this record has changed back to the
+              # original values.
               if self.draft? && the_changes.empty?
                 nilified_draft = send(self.class.draft_association_name)
+                touch = changed?
                 send("#{self.class.draft_association_name}_id=", nil)
-                self.save
+                save(touch: touch)
                 nilified_draft.destroy
               # Save an update draft if record is changed notably.
               elsif !the_changes.empty?
@@ -469,7 +470,8 @@ def trash!
       # Updates skipped attributes' values on this model.
       def update_skipped_attributes
         # Skip over this if nothing's being skipped.
-        return true unless draftsman_options[:skip].present?
+        skipped_changed = changed_attributes.keys & draftsman_options[:skip]
+        return true unless skipped_changed.present?
 
         keys = self.attributes.keys.select { |key| draftsman_options[:skip].include?(key) }
         attrs = {}

spec/models/skipper_spec.rb

@@ -53,6 +53,11 @@
       it 'has a value for `skip_me`' do
         expect(subject.skip_me).to eql 'Skipped 1'
       end
+
+      it 'sets `updated_at`' do
+        time = Time.now
+        expect(subject.updated_at).to be > time
+      end
     end
 
     context 'on update' do
@@ -99,6 +104,11 @@
         it 'creates a new draft' do
           expect { subject }.to change(Draftsman::Draft, :count).by(1)
         end
+
+        it 'has a newer `updated_at`' do
+          time = skipper.updated_at
+          expect(subject.updated_at).to be > time
+        end
       end
 
       describe 'changing back to initial state' do
@@ -135,10 +145,18 @@
         it 'destroys the draft' do
           expect { subject }.to change(Draftsman::Draft.where(:id => skipper.draft_id), :count).by(-1)
         end
+
+        it 'has a newer `updated_at`' do
+          time = skipper.updated_at
+          expect(subject.updated_at).to be > time
+        end
       end
 
       context 'with existing `create` draft' do
-        before { skipper.save_draft }
+        before do
+          skipper.save_draft
+          skipper.reload
+        end
 
         context 'with changes' do
           before do
@@ -181,6 +199,33 @@
           it "updates the draft's `name`" do
             expect(subject.draft.reify.name).to eql 'Sam'
           end
+
+          it 'has a newer `updated_at`' do
+            time = skipper.updated_at
+            expect(subject.updated_at).to be > time
+          end
+        end
+
+        context 'with changes to drafted attribute' do
+          before do
+            skipper.name = 'Sam'
+          end
+
+          it 'has a newer `updated_at`' do
+            time = skipper.updated_at
+            expect(subject.updated_at).to be > time
+          end
+        end
+
+        context 'with changes to skipped attribute' do
+          before do
+            skipper.skip_me = 'Skipped 2'
+          end
+
+          it 'has a newer `updated_at`' do
+            time = skipper.updated_at
+            expect(subject.updated_at).to be > time
+          end
         end
 
         context 'with no changes' do
@@ -215,6 +260,11 @@
           it "doesn't change the number of drafts" do
             expect { subject }.to_not change(Draftsman::Draft.where(:id => skipper.draft_id), :count)
           end
+
+          it 'has the original `updated_at`' do
+            time = skipper.updated_at
+            expect(subject.updated_at).to eq time
+          end
         end
       end
 
@@ -266,14 +316,16 @@
           it "updates the draft's `name`" do
             expect(subject.draft.reify.name).to eql 'Steve'
           end
+
+          it 'has the original `updated_at`' do
+            time = skipper.updated_at
+            expect(subject.updated_at).to eq time
+          end
         end
 
         context 'with changes to skipped attributes' do
           before do
             skipper.skip_me = 'Skip and save'
-            skipper.save_draft
-            skipper.reload
-            skipper.attributes = skipper.draft.reify.attributes
           end
 
           it 'is persisted' do
@@ -315,6 +367,11 @@
           it 'updates skipped attribute on draft' do
             expect(subject.draft.reify.skip_me).to eql 'Skip and save'
           end
+
+          it 'has a newer `updated_at`' do
+            time = skipper.updated_at
+            expect(subject.updated_at).to be > time
+          end
         end
 
         context 'with no changes' do
@@ -353,6 +410,11 @@
           it "does not update the draft's `name`" do
             expect(subject.draft.reify.name).to eql 'Sam'
           end
+
+          it 'has the original `updated_at`' do
+            time = skipper.updated_at
+            expect(subject.updated_at).to eq time
+          end
         end
       end
     end

spec/models/vanilla_spec.rb

@@ -9,6 +9,14 @@
     it 'contains column name' do
       expect(vanilla.object_attrs_for_draft_record).to include 'name'
     end
+
+    it 'contains column updated_at' do
+      expect(vanilla.object_attrs_for_draft_record).to include 'updated_at'
+    end
+
+    it 'contains column created_at' do
+      expect(vanilla.object_attrs_for_draft_record).to include 'created_at'
+    end
   end
 
   describe '#save_draft' do
@@ -42,6 +50,18 @@
         vanilla.save_draft
         expect(vanilla.name).to eql 'Bob'
       end
+
+      it 'sets `updated_at`' do
+        time = Time.now
+        vanilla.save_draft
+        expect(vanilla.updated_at).to be > time
+      end
+
+      it 'sets `created_at`' do
+        time = Time.now
+        vanilla.save_draft
+        expect(vanilla.created_at).to be > time
+      end
     end
 
     context 'on update' do
@@ -90,6 +110,12 @@
           it 'creates a new draft' do
             expect { vanilla.save_draft }.to change(Draftsman::Draft, :count).by(1)
           end
+
+          it 'has the original `updated_at`' do
+            vanilla.save_draft
+            vanilla.reload
+            expect(vanilla.updated_at).to eq vanilla.created_at
+          end
         end
 
         describe 'changing back to initial state' do
@@ -129,6 +155,12 @@
           it 'destroys the draft' do
             expect { vanilla.save_draft }.to change(Draftsman::Draft.where(id: vanilla.draft_id), :count).by(-1)
           end
+
+          it 'has the original `updated_at`' do
+            vanilla.save_draft
+            vanilla.reload
+            expect(vanilla.updated_at).to eq vanilla.created_at
+          end
         end
 
         context 'with existing `create` draft' do
@@ -182,6 +214,13 @@
               vanilla.reload
               expect(vanilla.draft.create?).to eql true
             end
+
+            it 'has a new `updated_at`' do
+              time = vanilla.updated_at
+              vanilla.save_draft
+              vanilla.reload
+              expect(vanilla.updated_at).to be > time
+            end
           end # with changes
 
           context 'with no changes' do
@@ -223,6 +262,12 @@
             it "doesn't change the number of drafts" do
               expect { vanilla.save_draft }.to_not change(Draftsman::Draft.where(id: vanilla.draft_id), :count)
             end
+
+            it 'has the original `updated_at`' do
+              vanilla.save_draft
+              vanilla.reload
+              expect(vanilla.updated_at).to eq vanilla.created_at
+            end
           end
         end # with no changes
 
@@ -281,6 +326,12 @@
               vanilla.save_draft
               expect(vanilla.draft.update?).to eql true
             end
+
+            it 'has the original `updated_at`' do
+              vanilla.save_draft
+              vanilla.reload
+              expect(vanilla.updated_at).to eq vanilla.created_at
+            end
           end # with changes
 
           context 'with no changes' do
@@ -328,6 +379,12 @@
               vanilla.reload
               expect(vanilla.draft.reify.name).to eql 'Sam'
             end
+
+            it 'has the original `updated_at`' do
+              vanilla.save_draft
+              vanilla.reload
+              expect(vanilla.updated_at).to eq vanilla.created_at
+            end
           end # with no changes
         end # with existing `update` draft
       end # with stashed drafted changes
@@ -380,6 +437,13 @@
           it 'creates a new draft' do
             expect { vanilla.save_draft }.to change(Draftsman::Draft, :count).by(1)
           end
+
+          it 'has a new `updated_at`' do
+            time = vanilla.updated_at
+            vanilla.save_draft
+            vanilla.reload
+            expect(vanilla.updated_at).to be > time
+          end
         end
 
         describe 'changing back to initial state' do
@@ -419,6 +483,13 @@
           it 'destroys the draft' do
             expect { vanilla.save_draft }.to change(Draftsman::Draft.where(id: vanilla.draft_id), :count).by(-1)
           end
+
+          it 'has a new `updated_at`' do
+            time = vanilla.updated_at
+            vanilla.save_draft
+            vanilla.reload
+            expect(vanilla.updated_at).to be > time
+          end
         end
 
         context 'with existing `create` draft' do
@@ -465,6 +536,13 @@
               vanilla.save_draft
               expect(vanilla.draft.create?).to eql true
             end
+
+            it 'has a new `updated_at`' do
+              time = vanilla.updated_at
+              vanilla.save_draft
+              vanilla.reload
+              expect(vanilla.updated_at).to be > time
+            end
           end
 
           context 'with no changes' do
@@ -500,6 +578,11 @@
             it "doesn't change the number of drafts" do
               expect { vanilla.save_draft }.to_not change(Draftsman::Draft.where(id: vanilla.draft_id), :count)
             end
+
+            it 'has the original `updated_at`' do
+              vanilla.save_draft
+              expect(vanilla.reload.updated_at).to eq vanilla.created_at
+            end
           end
         end
 
@@ -559,6 +642,13 @@
               vanilla.reload
               expect(vanilla.draft.update?).to eql true
             end
+
+            it 'has a new `updated_at`' do
+              time = vanilla.updated_at
+              vanilla.save_draft
+              vanilla.reload
+              expect(vanilla.updated_at).to be > time
+            end
           end # with changes
 
           context 'with no changes' do
@@ -600,6 +690,13 @@
               vanilla.save_draft
               expect(vanilla.draft.reify.name).to eql 'Sam'
             end
+
+            it 'does not update `updated_at`' do
+              time = vanilla.updated_at
+              vanilla.save_draft
+              vanilla.reload
+              expect(vanilla.updated_at).to eq time
+            end
           end # with no changes
         end # with existing `update` draft
       end # without stashed drafted changes