diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index babce8da..69ccd7e1 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -36,7 +36,7 @@ Metrics/BlockLength: # Offense count: 1 # Configuration parameters: CountComments. Metrics/ClassLength: - Max: 120 + Max: 121 # Offense count: 6 Metrics/CyclomaticComplexity: diff --git a/CHANGELOG.md b/CHANGELOG.md index b55441d0..38ce44d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ -### 0.8.6 (Next) +### 0.9.0 (Next) +* [#257](https://github.com/mongoid/mongoid-history/pull/257): Add track_blank_changes option - [@BrianLMatthews](https://github.com/BrianLMatthews). * Your contribution here. ### 0.8.5 (2021/09/18) diff --git a/Gemfile b/Gemfile index 1223ce61..5b89e4db 100644 --- a/Gemfile +++ b/Gemfile @@ -45,5 +45,6 @@ group :test do gem 'request_store' gem 'rspec', '~> 3.1' gem 'rubocop', '~> 0.49.0' + gem 'term-ansicolor', '~> 1.3.0' gem 'yard' end diff --git a/README.md b/README.md index 8fca7257..08892f5f 100644 --- a/README.md +++ b/README.md @@ -75,7 +75,8 @@ class Post :version_field => :version, # adds "field :version, :type => Integer" to track current version, default is :version :track_create => true, # track document creation, default is true :track_update => true, # track document updates, default is true - :track_destroy => true # track document destruction, default is true + :track_destroy => true, # track document destruction, default is true + :track_blank_changes => false # track changes from blank? to blank?, default is false end class Comment @@ -283,6 +284,32 @@ end It will now track only `_id` (Mandatory), `title` and `content` attributes for `pages` relation. +### Track all blank changes + +Normally changes where both the original and modified values respond with `true` to `blank?` (for example `nil` to `false`) aren't tracked. However, there may be cases where it's important to track such changes, for example when a field isn't present (so appears to be `nil`) then is set to `false`. To track such changes, set the `track_blank_changes` option to `true` (it defaults to `false`) when turning on history tracking: + +```ruby +class Book + include Mongoid::Document + ... + field :summary + track_history # Use default of false for track_blank_changes +end + +# summary change not tracked if summary hasn't been set (or has been set to something that responds true to blank?) +Book.find(id).update_attributes(:summary => '') + +class Chapter + include Mongoid::Document + ... + field :title + track_history :track_blank_changes => true +end + +# title change tracked even if title hasn't been set +Chapter.find(id).update_attributes(:title => '') +``` + ### Retrieving the list of tracked static and dynamic fields ```ruby @@ -604,6 +631,6 @@ You're encouraged to contribute to Mongoid History. See [CONTRIBUTING.md](CONTRI ## Copyright -Copyright (c) 2011-2020 Aaron Qian and Contributors. +Copyright (c) 2011-2024 Aaron Qian and Contributors. MIT License. See [LICENSE.txt](LICENSE.txt) for further details. diff --git a/lib/mongoid/history/attributes/update.rb b/lib/mongoid/history/attributes/update.rb index 3276c8d4..4bd41d3e 100644 --- a/lib/mongoid/history/attributes/update.rb +++ b/lib/mongoid/history/attributes/update.rb @@ -18,6 +18,7 @@ def attributes private def changes_from_parent + track_blank_changes = trackable_class.history_trackable_options[:track_blank_changes] parent_changes = {} changes.each do |k, v| change_value = begin @@ -26,7 +27,7 @@ def changes_from_parent elsif trackable_class.tracked_embeds_many?(k) embeds_many_changes_from_parent(k, v) elsif trackable_class.tracked?(k, :update) - { k => format_field(k, v) } unless v.all?(&:blank?) + { k => format_field(k, v) } unless !track_blank_changes && v.all?(&:blank?) end end parent_changes.merge!(change_value) if change_value.present? diff --git a/lib/mongoid/history/options.rb b/lib/mongoid/history/options.rb index cf9b3808..a34cbc52 100644 --- a/lib/mongoid/history/options.rb +++ b/lib/mongoid/history/options.rb @@ -34,6 +34,7 @@ def default_options track_create: true, track_update: true, track_destroy: true, + track_blank_changes: false, format: nil } end diff --git a/lib/mongoid/history/version.rb b/lib/mongoid/history/version.rb index 44856a07..f68147cb 100644 --- a/lib/mongoid/history/version.rb +++ b/lib/mongoid/history/version.rb @@ -1,5 +1,5 @@ module Mongoid module History - VERSION = '0.8.5'.freeze + VERSION = '0.9.0'.freeze end end diff --git a/spec/unit/attributes/update_spec.rb b/spec/unit/attributes/update_spec.rb index 5a88cca3..82021536 100644 --- a/spec/unit/attributes/update_spec.rb +++ b/spec/unit/attributes/update_spec.rb @@ -305,37 +305,69 @@ class EmbOne end end - context 'when original and modified values blank' do - before :each do - class DummyParent - include Mongoid::Document - include Mongoid::History::Trackable - store_in collection: :dummy_parents - has_and_belongs_to_many :other_dummy_parents - track_history on: :fields - end - - class OtherDummyParent - include Mongoid::Document - has_and_belongs_to_many :dummy_parents - end - end - - before :each do - allow(base).to receive(:changes) { changes } - DummyParent.track_history on: :other_dummy_parent_ids - end + [false, true].each do |original_nil| + context "when original value #{original_nil ? 'nil' : 'blank'} and modified value #{original_nil ? 'blank' : 'nil'}" do + [nil, false, true].each do |track_blank_changes| + context "when track_blank_changes #{track_blank_changes.nil? ? 'default' : track_blank_changes}" do + before :each do + class DummyParent + include Mongoid::Document + include Mongoid::History::Trackable + store_in collection: :dummy_parents + has_and_belongs_to_many :other_dummy_parents + field :boolean, type: Boolean + field :string, type: String + field :hash, type: Hash + end + + class OtherDummyParent + include Mongoid::Document + has_and_belongs_to_many :dummy_parents + end + + if track_blank_changes.nil? + DummyParent.track_history on: :fields + else + DummyParent.track_history \ + on: :fields, + track_blank_changes: track_blank_changes + end + + allow(base).to receive(:changes) { changes } + end - let(:base) { described_class.new(DummyParent.new) } - let(:changes) do - { 'other_dummy_parent_ids' => [nil, []] } - end - subject { base.attributes } - it { expect(subject.keys).to_not include 'other_dummy_parent_ids' } + after :each do + Object.send(:remove_const, :DummyParent) + Object.send(:remove_const, :OtherDummyParent) + end - after :each do - Object.send(:remove_const, :DummyParent) - Object.send(:remove_const, :OtherDummyParent) + let(:base) { described_class.new(DummyParent.new) } + subject { base.attributes.keys } + + # These can't be memoizing methods (i.e. lets) because of limits + # on where those can be used. + + cmp = track_blank_changes ? 'should' : 'should_not' + cmp_name = cmp.humanize capitalize: false + + [ + { n: 'many-to-many', f: 'other_dummy_parent_ids', v: [] }, + { n: 'boolean', f: 'boolean', v: false }, + { n: 'empty string', f: 'string', v: '' }, + { n: 'all whitespace string', f: 'string', v: " \t\n\r\f\v" } + # The second character in that string is an actual tab (0x9). + ].each do |d| + context "#{d[:n]} field" do + let(:changes) do + { d[:f] => original_nil ? [nil, d[:v]] : [d[:v], nil] } + end + it "changes #{cmp_name} include #{d[:f]}" do + send(cmp, include(d[:f])) + end + end + end + end + end end end end diff --git a/spec/unit/options_spec.rb b/spec/unit/options_spec.rb index c579f634..170cad9f 100644 --- a/spec/unit/options_spec.rb +++ b/spec/unit/options_spec.rb @@ -103,6 +103,7 @@ class EmbFour track_create: true, track_update: true, track_destroy: true, + track_blank_changes: false, format: nil } end @@ -173,6 +174,7 @@ class EmbFour track_create: true, track_update: true, track_destroy: true, + track_blank_changes: false, fields: %w[foo b], dynamic: [], relations: { embeds_one: {}, embeds_many: {} }, @@ -354,6 +356,11 @@ class EmbFour it { expect(subject[:track_destroy]).to be true } end + describe ':track_blank_changes' do + let(:options) { { track_blank_changes: true } } + it { expect(subject[:track_blank_changes]).to be true } + end + describe '#remove_reserved_fields' do let(:options) { { on: %i[_id _type foo version modifier_id] } } it { expect(subject[:fields]).to eq %w[foo] } diff --git a/spec/unit/trackable_spec.rb b/spec/unit/trackable_spec.rb index 82d09d2c..73161319 100644 --- a/spec/unit/trackable_spec.rb +++ b/spec/unit/trackable_spec.rb @@ -90,6 +90,7 @@ class MyModelWithNoModifier track_create: true, track_update: true, track_destroy: true, + track_blank_changes: false, fields: %w[foo], relations: { embeds_one: {}, embeds_many: {} }, dynamic: [],