diff --git a/lib/paper_trail/version_concern.rb b/lib/paper_trail/version_concern.rb index 3582e1701..73a085e5c 100644 --- a/lib/paper_trail/version_concern.rb +++ b/lib/paper_trail/version_concern.rb @@ -154,14 +154,18 @@ def object_changes_col_is_json? # they were "at the time", if they are also being versioned by PaperTrail. # # Options: - # :has_one set to `true` to also reify has_one associations. Default is `false`. - # :has_many set to `true` to also reify has_many and has_many :through associations. - # Default is `false`. - # :mark_for_destruction set to `true` to mark the has_one/has_many associations that did not exist in the - # reified version for destruction, instead of remove them. Default is `false`. - # This option is handy for people who want to persist the reified version. - # :dup `false` default behavior - # `true` it always create a new object instance. It is useful for comparing two versions of the same object + # :has_one set to `true` to also reify has_one associations. Default is `false`. + # :has_many set to `true` to also reify has_many and has_many :through associations. + # Default is `false`. + # :mark_for_destruction set to `true` to mark the has_one/has_many associations that did not exist in the + # reified version for destruction, instead of remove them. Default is `false`. + # This option is handy for people who want to persist the reified version. + # :dup `false` default behavior + # `true` it always create a new object instance. It is useful for comparing two versions of the same object + # :unversioned_attributes `:nil` - (default) attributes undefined in version record + # are set to nil in reified record + # `:preserve` - attributes undefined in version record are + # not modified def reify(options = {}) return nil if object.nil? @@ -170,7 +174,8 @@ def reify(options = {}) :version_at => created_at, :mark_for_destruction => false, :has_one => false, - :has_many => false + :has_many => false, + :unversioned_attributes => :nil ) attrs = self.class.object_col_is_json? ? object : PaperTrail.serializer.load(object) @@ -191,7 +196,9 @@ def reify(options = {}) if options[:dup] != true && item model = item # Look for attributes that exist in the model and not in this version. These attributes should be set to nil. - (model.attribute_names - attrs.keys).each { |k| attrs[k] = nil } + if options[:unversioned_attributes] == :nil + (model.attribute_names - attrs.keys).each { |k| attrs[k] = nil } + end else inheritance_column_name = item_type.constantize.inheritance_column class_name = attrs[inheritance_column_name].blank? ? item_type : attrs[inheritance_column_name] @@ -200,7 +207,7 @@ def reify(options = {}) # to look for the item outside of default scope(s) if options[:dup] || (_item = klass.unscoped.find_by_id(item_id)).nil? model = klass.new - else + elsif options[:unversioned_attributes] == :nil model = _item # Look for attributes that exist in the model and not in this version. These attributes should be set to nil. (model.attribute_names - attrs.keys).each { |k| attrs[k] = nil } diff --git a/spec/models/skipper_spec.rb b/spec/models/skipper_spec.rb index ca85ce278..44e48773c 100644 --- a/spec/models/skipper_spec.rb +++ b/spec/models/skipper_spec.rb @@ -1,16 +1,45 @@ require 'rails_helper' describe Skipper, :type => :model do - describe "#update_attributes!", :versioning => true do - context "updating a skipped attribute" do - let(:t1) { Time.zone.local(2015, 7, 15, 20, 34, 0) } - let(:t2) { Time.zone.local(2015, 7, 15, 20, 34, 30) } + with_versioning do + it { is_expected.to be_versioned } - it "should not create a version" do - skipper = Skipper.create!(:another_timestamp => t1) - expect { - skipper.update_attributes!(:another_timestamp => t2) - }.to_not change { skipper.versions.length } + describe "#update_attributes!", :versioning => true do + context "updating a skipped attribute" do + let(:t1) { Time.zone.local(2015, 7, 15, 20, 34, 0) } + let(:t2) { Time.zone.local(2015, 7, 15, 20, 34, 30) } + + it "should not create a version" do + skipper = Skipper.create!(:another_timestamp => t1) + expect { + skipper.update_attributes!(:another_timestamp => t2) + }.to_not change { skipper.versions.length } + end + end + end + + describe "reify" do + context "reifying a with a skipped attribute" do + let(:t1) { Time.zone.local(2015, 7, 15, 20, 34, 0) } + let(:t2) { Time.zone.local(2015, 7, 15, 20, 34, 30) } + + context "without preserve (default)" do + it "should have no timestamp" do + skipper = Skipper.create!(:another_timestamp => t1) + skipper.update_attributes!(:another_timestamp => t2, :name => "Foobar") + skipper = skipper.versions.last.reify + expect(skipper.another_timestamp).to be(nil) + end + end + + context "with preserve" do + it "should preserve its timestamp" do + skipper = Skipper.create!(:another_timestamp => t1) + skipper.update_attributes!(:another_timestamp => t2, :name => "Foobar") + skipper = skipper.versions.last.reify(:unversioned_attributes => :preserve) + expect(skipper.another_timestamp).to eq(t2) + end + end end end end diff --git a/test/dummy/db/migrate/20110208155312_set_up_test_tables.rb b/test/dummy/db/migrate/20110208155312_set_up_test_tables.rb index e16160fe3..a801c5526 100644 --- a/test/dummy/db/migrate/20110208155312_set_up_test_tables.rb +++ b/test/dummy/db/migrate/20110208155312_set_up_test_tables.rb @@ -1,6 +1,7 @@ class SetUpTestTables < ActiveRecord::Migration def self.up create_table :skippers, :force => true do |t| + t.string :name t.datetime :another_timestamp t.timestamps :null => true end diff --git a/test/dummy/db/schema.rb b/test/dummy/db/schema.rb index 47fc6a8f5..88541e19a 100644 --- a/test/dummy/db/schema.rb +++ b/test/dummy/db/schema.rb @@ -13,77 +13,108 @@ ActiveRecord::Schema.define(version: 20110208155312) do - create_table "animals", force: true do |t| + create_table "animals", force: :cascade do |t| t.string "name" t.string "species" end - create_table "articles", force: true do |t| + create_table "articles", force: :cascade do |t| t.string "title" t.string "content" t.string "abstract" t.string "file_upload" end - create_table "authorships", force: true do |t| + create_table "authorships", force: :cascade do |t| t.integer "book_id" t.integer "person_id" end - create_table "books", force: true do |t| + create_table "banana_versions", force: :cascade do |t| + t.string "item_type", null: false + t.integer "item_id", null: false + t.string "event", null: false + t.string "whodunnit" + t.text "object" + t.datetime "created_at" + end + + add_index "banana_versions", ["item_type", "item_id"], name: "index_banana_versions_on_item_type_and_item_id" + + create_table "bananas", force: :cascade do |t| + t.datetime "created_at" + t.datetime "updated_at" + end + + create_table "books", force: :cascade do |t| t.string "title" end - create_table "customers", force: true do |t| + create_table "boolits", force: :cascade do |t| + t.string "name" + t.boolean "scoped", default: true + end + + create_table "customers", force: :cascade do |t| t.string "name" end - create_table "documents", force: true do |t| + create_table "documents", force: :cascade do |t| t.string "name" end - create_table "editors", force: true do |t| + create_table "editors", force: :cascade do |t| t.string "name" end - create_table "editorships", force: true do |t| + create_table "editorships", force: :cascade do |t| t.integer "book_id" t.integer "editor_id" end - create_table "fluxors", force: true do |t| + create_table "fluxors", force: :cascade do |t| t.integer "widget_id" t.string "name" end - create_table "gadgets", force: true do |t| + create_table "fruits", force: :cascade do |t| + t.string "name" + t.string "color" + end + + create_table "gadgets", force: :cascade do |t| t.string "name" t.string "brand" t.datetime "created_at" t.datetime "updated_at" end - create_table "legacy_widgets", force: true do |t| + create_table "legacy_widgets", force: :cascade do |t| t.string "name" t.integer "version" end - create_table "line_items", force: true do |t| + create_table "line_items", force: :cascade do |t| t.integer "order_id" t.string "product" end - create_table "orders", force: true do |t| + create_table "not_on_updates", force: :cascade do |t| + t.datetime "created_at" + t.datetime "updated_at" + end + + create_table "orders", force: :cascade do |t| t.integer "customer_id" t.string "order_date" end - create_table "people", force: true do |t| + create_table "people", force: :cascade do |t| t.string "name" t.string "time_zone" end - create_table "post_versions", force: true do |t| + create_table "post_versions", force: :cascade do |t| t.string "item_type", null: false t.integer "item_id", null: false t.string "event", null: false @@ -96,31 +127,38 @@ add_index "post_versions", ["item_type", "item_id"], name: "index_post_versions_on_item_type_and_item_id" - create_table "post_with_statuses", force: true do |t| + create_table "post_with_statuses", force: :cascade do |t| t.integer "status" end - create_table "posts", force: true do |t| + create_table "posts", force: :cascade do |t| t.string "title" t.string "content" end - create_table "songs", force: true do |t| + create_table "skippers", force: :cascade do |t| + t.string "name" + t.datetime "another_timestamp" + t.datetime "created_at" + t.datetime "updated_at" + end + + create_table "songs", force: :cascade do |t| t.integer "length" end - create_table "things", force: true do |t| + create_table "things", force: :cascade do |t| t.string "name" end - create_table "translations", force: true do |t| + create_table "translations", force: :cascade do |t| t.string "headline" t.string "content" t.string "language_code" t.string "type" end - create_table "version_associations", force: true do |t| + create_table "version_associations", force: :cascade do |t| t.integer "version_id" t.string "foreign_key_name", null: false t.integer "foreign_key_id" @@ -129,7 +167,7 @@ add_index "version_associations", ["foreign_key_name", "foreign_key_id"], name: "index_version_associations_on_foreign_key" add_index "version_associations", ["version_id"], name: "index_version_associations_on_version_id" - create_table "versions", force: true do |t| + create_table "versions", force: :cascade do |t| t.string "item_type", null: false t.integer "item_id", null: false t.string "event", null: false @@ -149,13 +187,13 @@ add_index "versions", ["item_type", "item_id"], name: "index_versions_on_item_type_and_item_id" - create_table "whatchamajiggers", force: true do |t| + create_table "whatchamajiggers", force: :cascade do |t| t.string "owner_type" t.integer "owner_id" t.string "name" end - create_table "widgets", force: true do |t| + create_table "widgets", force: :cascade do |t| t.string "name" t.text "a_text" t.integer "an_integer" @@ -171,7 +209,7 @@ t.datetime "updated_at" end - create_table "wotsits", force: true do |t| + create_table "wotsits", force: :cascade do |t| t.integer "widget_id" t.string "name" t.datetime "created_at"