From 9c9e97b0d9dedd24b5da94411c88716569aac9ef Mon Sep 17 00:00:00 2001 From: Sean Linsley Date: Tue, 24 Jul 2018 10:34:55 -0500 Subject: [PATCH] add the ability to track changes on destroy this involved refactoring the internal serialization code so it'd be possible to run it on non-Rails-provided changes --- CHANGELOG.md | 1 + README.md | 8 ++++++++ lib/paper_trail/config.rb | 1 + lib/paper_trail/events/base.rb | 16 ++++++++++------ lib/paper_trail/events/create.rb | 4 +++- lib/paper_trail/events/destroy.rb | 7 +++++++ lib/paper_trail/events/update.rb | 7 +++++-- spec/paper_trail/model_spec.rb | 15 +++++++++++++++ 8 files changed, 50 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e3f4201c..5fa1c4b17 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ recommendations of [keepachangelog.com](http://keepachangelog.com/). - [#1099](/~https://github.com/paper-trail-gem/paper_trail/issues/1099) - Ability to save ~50% storage space by making the `object` column optional. Note that this disables `reify` and `where_object`. +- Ability to track `object_changes` on destroy, to complement ⬆️. ### Fixed diff --git a/README.md b/README.md index f680d2aad..881e4b9a8 100644 --- a/README.md +++ b/README.md @@ -1291,6 +1291,14 @@ The `object` column ends up storing a lot of duplicate data if you have models t and that are updated many times. You can save ~50% of storage space by removing the column from the versions table. It's important to note that this will disable `reify` and `where_object`. +Removing the `object` column also makes it so that destroy events don't store any attributes, since +the default behavior on destroy is only to populate the `object` column. +That can be fixed with this setting: + +```ruby +PaperTrail.config.record_changes_on_destroy = true +``` + ## 7. Testing You may want to turn PaperTrail off to speed up your tests. See [Turning diff --git a/lib/paper_trail/config.rb b/lib/paper_trail/config.rb index 4f39228c8..4b1b923b1 100644 --- a/lib/paper_trail/config.rb +++ b/lib/paper_trail/config.rb @@ -13,6 +13,7 @@ class Config :classes_warned_about_sti_item_types, :i_have_updated_my_existing_item_types, :object_changes_adapter, + :record_changes_on_destroy, :serializer, :version_limit ) diff --git a/lib/paper_trail/events/base.rb b/lib/paper_trail/events/base.rb index 425f1cad8..33e14830d 100644 --- a/lib/paper_trail/events/base.rb +++ b/lib/paper_trail/events/base.rb @@ -119,14 +119,18 @@ def changed_in_latest_version end # @api private - def changes - notable_changes = changes_in_latest_version.delete_if { |k, _v| - !notably_changed.include?(k) - } + def serialize_changes(changes) AttributeSerializers::ObjectChangesAttribute. new(@record.class). - serialize(notable_changes) - notable_changes.to_hash + serialize(changes) + changes.to_hash + end + + # @api private + def notable_changes + changes_in_latest_version.delete_if { |k, _v| + !notably_changed.include?(k) + } end # Rails 5.1 changed the API of `ActiveRecord::Dirty`. See diff --git a/lib/paper_trail/events/create.rb b/lib/paper_trail/events/create.rb index e9a637a26..0e6ec1a49 100644 --- a/lib/paper_trail/events/create.rb +++ b/lib/paper_trail/events/create.rb @@ -20,7 +20,9 @@ def data data[:created_at] = @record.updated_at end if record_object_changes? && changed_notably? - data[:object_changes] = recordable_object_changes(changes) + changes = serialize_changes(notable_changes) + changes = recordable_object_changes(changes) + data[:object_changes] = changes end merge_metadata_into(data) end diff --git a/lib/paper_trail/events/destroy.rb b/lib/paper_trail/events/destroy.rb index 3703cff43..5d0dea6d3 100644 --- a/lib/paper_trail/events/destroy.rb +++ b/lib/paper_trail/events/destroy.rb @@ -21,6 +21,13 @@ def data if record_object? data[:object] = recordable_object(false) end + if record_object_changes? && PaperTrail.config.record_changes_on_destroy + # Rails' implementation returns nothing on destroy :/ + changes = @record.attributes.map { |attr, value| [attr, [value, nil]] }.to_h + changes = serialize_changes(changes) + changes = recordable_object_changes(changes) + data[:object_changes] = changes + end merge_metadata_into(data) end end diff --git a/lib/paper_trail/events/update.rb b/lib/paper_trail/events/update.rb index 8207c1e23..1a9f2d8a5 100644 --- a/lib/paper_trail/events/update.rb +++ b/lib/paper_trail/events/update.rb @@ -17,7 +17,7 @@ class Update < Base def initialize(record, in_after_callback, is_touch, force_changes) super(record, in_after_callback) @is_touch = is_touch - @changes = force_changes.nil? ? changes : force_changes + @force_changes = force_changes end # Return attributes of nascent `Version` record. @@ -35,7 +35,10 @@ def data data[:object] = recordable_object(@is_touch) end if record_object_changes? - data[:object_changes] = recordable_object_changes(@changes) + changes = @force_changes.nil? ? notable_changes : @force_changes + changes = serialize_changes(changes) + changes = recordable_object_changes(changes) + data[:object_changes] = changes end merge_metadata_into(data) end diff --git a/spec/paper_trail/model_spec.rb b/spec/paper_trail/model_spec.rb index bcd49c635..2a9014831 100644 --- a/spec/paper_trail/model_spec.rb +++ b/spec/paper_trail/model_spec.rb @@ -928,4 +928,19 @@ end.to raise_error "where_object can't be called without an object column" end end + + context "record_changes_on_destroy" do + before { PaperTrail.config.record_changes_on_destroy = true } + after { PaperTrail.config.record_changes_on_destroy = nil } + + it "works" do + song = Song.create(length: 4) + changes = YAML.load song.versions.last.attributes["object_changes"] + expect(changes).to eq "id" => [nil, song.id], "length" => [nil, 240] + + song.destroy + changes = YAML.load song.versions.last.attributes["object_changes"] + expect(changes).to eq "id" => [song.id, nil], "length" => [240, nil], "name" => [nil, nil] + end + end end