From d5f3bd341801ce1380173da9284efba9667be151 Mon Sep 17 00:00:00 2001 From: Doug Knight Date: Wed, 16 Dec 2020 10:07:11 -0600 Subject: [PATCH 1/9] breakup large module The module line limit is preventing extra logic from being added to ensure that conflicting updates are not being created. I've chosen an arbitrary method for reducing the line count in the module. I'm open to suggestions if someone else has a better idea in mind. --- lib/mongoid/history/trackable.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/mongoid/history/trackable.rb b/lib/mongoid/history/trackable.rb index b735ffc..ea2ec4d 100644 --- a/lib/mongoid/history/trackable.rb +++ b/lib/mongoid/history/trackable.rb @@ -19,6 +19,8 @@ def track_history(options = {}) end include MyInstanceMethods + include MyPrivateInstanceMethods + include MyProtectedInstanceMethods extend SingletonMethods delegate :history_trackable_options, to: 'self.class' @@ -136,7 +138,9 @@ def _get_relation(name) def _create_relation(name, value) send("create_#{self.class.relation_alias(name)}!", value) end + end + module MyPrivateInstanceMethods private def ancestor_flagged_for_destroy?(doc) @@ -318,7 +322,9 @@ def next_version def increment_current_version next_version.tap { |version| send("#{history_trackable_options[:version_field]}=", version) } end + end + module MyProtectedInstanceMethods protected def track_history_for_action?(action) From 41c2ff386fdefcc91ad2a6845299248da11fed25 Mon Sep 17 00:00:00 2001 From: Doug Knight Date: Wed, 16 Dec 2020 12:04:30 -0600 Subject: [PATCH 2/9] break up large block in spec The line limit for blocks is prevents additional specs from being added for Mongoid::History::Trackable. I've picked an arbitrary method of reducing the size of the block in order to get past that limit. I'm open to suggestions if someone has a better idea in mind. --- spec/unit/trackable_spec.rb | 371 +++++++++++++++++++----------------- 1 file changed, 192 insertions(+), 179 deletions(-) diff --git a/spec/unit/trackable_spec.rb b/spec/unit/trackable_spec.rb index bcaf4e0..383db72 100644 --- a/spec/unit/trackable_spec.rb +++ b/spec/unit/trackable_spec.rb @@ -1,64 +1,6 @@ require 'spec_helper' -describe Mongoid::History::Trackable do - before :each do - class MyModel - include Mongoid::Document - include Mongoid::History::Trackable - - field :foo - end - - class MyDynamicModel - include Mongoid::Document - include Mongoid::History::Trackable - include Mongoid::Attributes::Dynamic unless Mongoid::Compatibility::Version.mongoid3? - end - - class MyDeeplyNestedModel - include Mongoid::Document - include Mongoid::History::Trackable - - embeds_many :children, class_name: 'MyNestableModel', cascade_callbacks: true # The problem only occurs if callbacks are cascaded - accepts_nested_attributes_for :children, allow_destroy: true - track_history modifier_field: nil - end - - class MyNestableModel - include Mongoid::Document - include Mongoid::History::Trackable - - embedded_in :parent, class_name: 'MyDeeplyNestedModel' - embeds_many :children, class_name: 'MyNestableModel', cascade_callbacks: true - accepts_nested_attributes_for :children, allow_destroy: true - field :name, type: String - track_history modifier_field: nil - end - - class HistoryTracker - include Mongoid::History::Tracker - end - - class User - include Mongoid::Document - end - end - - after :each do - Object.send(:remove_const, :MyModel) - Object.send(:remove_const, :MyDynamicModel) - Object.send(:remove_const, :HistoryTracker) - Object.send(:remove_const, :User) - Object.send(:remove_const, :MyDeeplyNestedModel) - Object.send(:remove_const, :MyNestableModel) - end - - let(:user) { User.create! } - - it 'should have #track_history' do - expect(MyModel).to respond_to :track_history - end - +shared_examples 'track history' do describe '#track_history' do before :each do class MyModelWithNoModifier @@ -590,6 +532,193 @@ def my_changes end end end +end + +shared_examples 'track update' do + describe '#track_update' do + before(:each) { MyModel.track_history(on: :foo, track_update: true) } + + let!(:m) { MyModel.create!(foo: 'bar', modifier: user) } + + it 'should create history' do + expect { m.update_attributes!(foo: 'bar2') }.to change(Tracker, :count).by(1) + end + + it 'should not create history when error raised' do + expect(m).to receive(:update_attributes!).and_raise(StandardError) + expect do + expect { m.update_attributes!(foo: 'bar2') }.to raise_error(StandardError) + end.to change(Tracker, :count).by(0) + end + end +end + +shared_examples 'track destroy' do + describe '#track_destroy' do + before(:each) { MyModel.track_history(on: :foo, track_destroy: true) } + + let!(:m) { MyModel.create!(foo: 'bar', modifier: user) } + + it 'should create history' do + expect { m.destroy }.to change(Tracker, :count).by(1) + end + + it 'should not create history when error raised' do + expect(m).to receive(:destroy).and_raise(StandardError) + expect do + expect { m.destroy }.to raise_error(StandardError) + end.to change(Tracker, :count).by(0) + end + + context 'with a deeply nested model' do + let(:m) do + MyDeeplyNestedModel.create!( + children: [ + MyNestableModel.new( + name: 'grandparent', + children: [ + MyNestableModel.new(name: 'parent 1', children: [MyNestableModel.new(name: 'child 1')]), + MyNestableModel.new(name: 'parent 2', children: [MyNestableModel.new(name: 'child 2')]) + ] + ) + ] + ) + end + let(:attributes) do + { + 'children_attributes' => [ + { + 'id' => m.children[0].id, + 'children_attributes' => [ + { 'id' => m.children[0].children[0].id, '_destroy' => '0' }, + { 'id' => m.children[0].children[1].id, '_destroy' => '1' } + ] + } + ] + } + end + + subject(:updated) do + m.update_attributes attributes + m.reload + end + + let(:names_of_destroyed) do + MyDeeplyNestedModel.tracker_class + .where('association_chain.id' => updated.id, 'action' => 'destroy') + .map { |track| track.original['name'] } + end + + it 'does not corrupt embedded models' do + expect(updated.children[0].children.count).to eq 1 # When the problem occurs, the 2nd child will continue to be present, but will only contain the version attribute + end + + it 'creates a history track for the doc explicitly destroyed' do + expect(names_of_destroyed).to include 'parent 2' + end + + it 'creates a history track for the doc implicitly destroyed' do + expect(names_of_destroyed).to include 'child 2' + end + end + end +end + +shared_examples 'track create' do + describe '#track_create' do + before :each do + class MyModelWithNoModifier + include Mongoid::Document + include Mongoid::History::Trackable + + field :foo + end + end + + after(:each) { Object.send(:remove_const, :MyModelWithNoModifier) } + + before :each do + MyModel.track_history(on: :foo, track_create: true) + MyModelWithNoModifier.track_history modifier_field: nil + end + + it 'should create history' do + expect { MyModel.create!(foo: 'bar', modifier: user) }.to change(Tracker, :count).by(1) + end + + context 'no modifier_field' do + it 'should create history' do + expect { MyModelWithNoModifier.create!(foo: 'bar').to change(Tracker, :count).by(1) } + end + end + + it 'should not create history when error raised' do + expect(MyModel).to receive(:create!).and_raise(StandardError) + expect do + expect { MyModel.create!(foo: 'bar') }.to raise_error(StandardError) + end.to change(Tracker, :count).by(0) + end + end +end + +describe Mongoid::History::Trackable do + before :each do + class MyModel + include Mongoid::Document + include Mongoid::History::Trackable + + field :foo + end + + class MyDynamicModel + include Mongoid::Document + include Mongoid::History::Trackable + include Mongoid::Attributes::Dynamic unless Mongoid::Compatibility::Version.mongoid3? + end + + class MyDeeplyNestedModel + include Mongoid::Document + include Mongoid::History::Trackable + + embeds_many :children, class_name: 'MyNestableModel', cascade_callbacks: true # The problem only occurs if callbacks are cascaded + accepts_nested_attributes_for :children, allow_destroy: true + track_history modifier_field: nil + end + + class MyNestableModel + include Mongoid::Document + include Mongoid::History::Trackable + + embedded_in :parent, class_name: 'MyDeeplyNestedModel' + embeds_many :children, class_name: 'MyNestableModel', cascade_callbacks: true + accepts_nested_attributes_for :children, allow_destroy: true + field :name, type: String + track_history modifier_field: nil + end + + class HistoryTracker + include Mongoid::History::Tracker + end + + class User + include Mongoid::Document + end + end + + after :each do + Object.send(:remove_const, :MyModel) + Object.send(:remove_const, :MyDynamicModel) + Object.send(:remove_const, :HistoryTracker) + Object.send(:remove_const, :User) + Object.send(:remove_const, :MyDeeplyNestedModel) + Object.send(:remove_const, :MyNestableModel) + end + + let(:user) { User.create! } + + it 'should have #track_history' do + expect(MyModel).to respond_to :track_history + end describe '#history_settings' do before(:each) { Mongoid::History.trackable_settings = nil } @@ -818,126 +947,10 @@ class EmbOne end end - describe '#track_update' do - before(:each) { MyModel.track_history(on: :foo, track_update: true) } - - let!(:m) { MyModel.create!(foo: 'bar', modifier: user) } - - it 'should create history' do - expect { m.update_attributes!(foo: 'bar2') }.to change(Tracker, :count).by(1) - end - - it 'should not create history when error raised' do - expect(m).to receive(:update_attributes!).and_raise(StandardError) - expect do - expect { m.update_attributes!(foo: 'bar2') }.to raise_error(StandardError) - end.to change(Tracker, :count).by(0) - end - end - - describe '#track_destroy' do - before(:each) { MyModel.track_history(on: :foo, track_destroy: true) } - - let!(:m) { MyModel.create!(foo: 'bar', modifier: user) } - - it 'should create history' do - expect { m.destroy }.to change(Tracker, :count).by(1) - end - - it 'should not create history when error raised' do - expect(m).to receive(:destroy).and_raise(StandardError) - expect do - expect { m.destroy }.to raise_error(StandardError) - end.to change(Tracker, :count).by(0) - end - - context 'with a deeply nested model' do - let(:m) do - MyDeeplyNestedModel.create!( - children: [ - MyNestableModel.new( - name: 'grandparent', - children: [ - MyNestableModel.new(name: 'parent 1', children: [MyNestableModel.new(name: 'child 1')]), - MyNestableModel.new(name: 'parent 2', children: [MyNestableModel.new(name: 'child 2')]) - ] - ) - ] - ) - end - let(:attributes) do - { - 'children_attributes' => [ - { - 'id' => m.children[0].id, - 'children_attributes' => [ - { 'id' => m.children[0].children[0].id, '_destroy' => '0' }, - { 'id' => m.children[0].children[1].id, '_destroy' => '1' } - ] - } - ] - } - end - - subject(:updated) do - m.update_attributes attributes - m.reload - end - - let(:names_of_destroyed) do - MyDeeplyNestedModel.tracker_class - .where('association_chain.id' => updated.id, 'action' => 'destroy') - .map { |track| track.original['name'] } - end - - it 'does not corrupt embedded models' do - expect(updated.children[0].children.count).to eq 1 # When the problem occurs, the 2nd child will continue to be present, but will only contain the version attribute - end - - it 'creates a history track for the doc explicitly destroyed' do - expect(names_of_destroyed).to include 'parent 2' - end - - it 'creates a history track for the doc implicitly destroyed' do - expect(names_of_destroyed).to include 'child 2' - end - end - end - - describe '#track_create' do - before :each do - class MyModelWithNoModifier - include Mongoid::Document - include Mongoid::History::Trackable - - field :foo - end - end - - after(:each) { Object.send(:remove_const, :MyModelWithNoModifier) } - - before :each do - MyModel.track_history(on: :foo, track_create: true) - MyModelWithNoModifier.track_history modifier_field: nil - end - - it 'should create history' do - expect { MyModel.create!(foo: 'bar', modifier: user) }.to change(Tracker, :count).by(1) - end - - context 'no modifier_field' do - it 'should create history' do - expect { MyModelWithNoModifier.create!(foo: 'bar').to change(Tracker, :count).by(1) } - end - end - - it 'should not create history when error raised' do - expect(MyModel).to receive(:create!).and_raise(StandardError) - expect do - expect { MyModel.create!(foo: 'bar') }.to raise_error(StandardError) - end.to change(Tracker, :count).by(0) - end - end + include_examples 'track history' + include_examples 'track update' + include_examples 'track destroy' + include_examples 'track create' context 'changing collection' do before :each do From 1a3d0ff4995ae643f55587bc615ba0d54f2ed5a2 Mon Sep 17 00:00:00 2001 From: Doug Knight Date: Wed, 16 Dec 2020 11:35:29 -0600 Subject: [PATCH 3/9] fix problem where last seems to cache After making the change to logic for incrementing the version number this spec started failing even though the behavior was actually correct. Though post.history_tracks returned an updated list of records, the call to #last continued to return the same "destroy" track, causing the comment to be created twice, instead of being recreated and then redestroyed. Addressing the last record by index appears to have resolved the problem. --- spec/integration/integration_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/integration/integration_spec.rb b/spec/integration/integration_spec.rb index 5825a00..5ab85f5 100644 --- a/spec/integration/integration_spec.rb +++ b/spec/integration/integration_spec.rb @@ -509,8 +509,8 @@ class Foo < Comment it 'should be possible to destroy after re-create embedded from parent' do comment.destroy - post.history_tracks.last.undo!(user) - post.history_tracks.last.undo!(user) + post.history_tracks[-1].undo!(user) + post.history_tracks[-1].undo!(user) post.reload expect(post.comments.count).to eq(0) end From d76760677ebbab3a044599708698fca5c2e13bad Mon Sep 17 00:00:00 2001 From: Doug Knight Date: Wed, 16 Dec 2020 12:10:32 -0600 Subject: [PATCH 4/9] don't update version on destroy --- lib/mongoid/history/trackable.rb | 9 ++++++- spec/unit/trackable_spec.rb | 41 ++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/lib/mongoid/history/trackable.rb b/lib/mongoid/history/trackable.rb index ea2ec4d..4b20be9 100644 --- a/lib/mongoid/history/trackable.rb +++ b/lib/mongoid/history/trackable.rb @@ -331,9 +331,16 @@ def track_history_for_action?(action) track_history? && !(action.to_sym == :update && modified_attributes_for_update.blank?) end + def increment_current_version?(action) + !( + (action == :destroy) || + ancestor_flagged_for_destroy?(_parent) + ) + end + def track_history_for_action(action) if track_history_for_action?(action) - current_version = ancestor_flagged_for_destroy?(_parent) ? next_version : increment_current_version + current_version = increment_current_version?(action) ? increment_current_version : next_version last_track = self.class.tracker_class.create!( history_tracker_attributes(action.to_sym) .merge(version: current_version, action: action.to_s, trackable: self) diff --git a/spec/unit/trackable_spec.rb b/spec/unit/trackable_spec.rb index 383db72..bdd59df 100644 --- a/spec/unit/trackable_spec.rb +++ b/spec/unit/trackable_spec.rb @@ -621,6 +621,47 @@ def my_changes expect(names_of_destroyed).to include 'child 2' end end + + context 'with multiple embeds_many models' do + let(:m) do + MyDeeplyNestedModel.create!( + children: [ + MyNestableModel.new( + name: 'parent', + children: [ + MyNestableModel.new(name: 'child 1'), + MyNestableModel.new(name: 'child 2'), + MyNestableModel.new(name: 'child 3') + ] + ) + ] + ) + end + + let(:attributes) do + { + 'children_attributes' => [ + { + 'id' => m.children[0].id, + 'children_attributes' => [ + { 'id' => m.children[0].children[0].id, '_destroy' => '0' }, + { 'id' => m.children[0].children[1].id, '_destroy' => '1' }, + { 'id' => m.children[0].children[2].id, '_destroy' => '1' } + ] + } + ] + } + end + + subject(:updated) do + m.update_attributes attributes + m.reload + end + + it 'does not corrupt the document' do + expect(updated.children[0].children.length).to eq(1) + end + end end end From 74f7612d6835efa84484e5c434f7498ebd4cf9d2 Mon Sep 17 00:00:00 2001 From: Doug Knight Date: Wed, 16 Dec 2020 13:26:18 -0600 Subject: [PATCH 5/9] rework increment_current_version? for clarity --- lib/mongoid/history/trackable.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/mongoid/history/trackable.rb b/lib/mongoid/history/trackable.rb index 4b20be9..34d4717 100644 --- a/lib/mongoid/history/trackable.rb +++ b/lib/mongoid/history/trackable.rb @@ -332,10 +332,7 @@ def track_history_for_action?(action) end def increment_current_version?(action) - !( - (action == :destroy) || - ancestor_flagged_for_destroy?(_parent) - ) + action != :destroy && !ancestor_flagged_for_destroy?(_parent) end def track_history_for_action(action) From 88a1be582bb5c98e687d8a36df7c181c202e8766 Mon Sep 17 00:00:00 2001 From: Doug Knight Date: Thu, 17 Dec 2020 10:17:01 -0600 Subject: [PATCH 6/9] Revert "breakup large module" This reverts commit d5f3bd341801ce1380173da9284efba9667be151. --- lib/mongoid/history/trackable.rb | 6 ------ 1 file changed, 6 deletions(-) diff --git a/lib/mongoid/history/trackable.rb b/lib/mongoid/history/trackable.rb index 34d4717..f798523 100644 --- a/lib/mongoid/history/trackable.rb +++ b/lib/mongoid/history/trackable.rb @@ -19,8 +19,6 @@ def track_history(options = {}) end include MyInstanceMethods - include MyPrivateInstanceMethods - include MyProtectedInstanceMethods extend SingletonMethods delegate :history_trackable_options, to: 'self.class' @@ -138,9 +136,7 @@ def _get_relation(name) def _create_relation(name, value) send("create_#{self.class.relation_alias(name)}!", value) end - end - module MyPrivateInstanceMethods private def ancestor_flagged_for_destroy?(doc) @@ -322,9 +318,7 @@ def next_version def increment_current_version next_version.tap { |version| send("#{history_trackable_options[:version_field]}=", version) } end - end - module MyProtectedInstanceMethods protected def track_history_for_action?(action) From 91d6ec52bd7f8b58370080844d2cf3050eec1de1 Mon Sep 17 00:00:00 2001 From: Doug Knight Date: Thu, 17 Dec 2020 10:23:12 -0600 Subject: [PATCH 7/9] relax module length constraint --- .rubocop_todo.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 7bd7def..e5df0be 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -56,7 +56,7 @@ Metrics/MethodLength: # Offense count: 2 # Configuration parameters: CountComments. Metrics/ModuleLength: - Max: 191 + Max: 200 # Offense count: 6 Metrics/PerceivedComplexity: From 8359fd7c8038df6c91aa97c5e3f5e160a8f80a65 Mon Sep 17 00:00:00 2001 From: Doug Knight Date: Thu, 17 Dec 2020 10:32:54 -0600 Subject: [PATCH 8/9] relax block length constraint --- .rubocop_todo.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index e5df0be..babce8d 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -31,7 +31,7 @@ Metrics/AbcSize: # Offense count: 122 # Configuration parameters: CountComments, ExcludedMethods. Metrics/BlockLength: - Max: 837 + Max: 900 # Offense count: 1 # Configuration parameters: CountComments. From 94f8bf3a6e4f00dd062e7de3e3e01dfec5deae9d Mon Sep 17 00:00:00 2001 From: Doug Knight Date: Thu, 17 Dec 2020 10:33:35 -0600 Subject: [PATCH 9/9] revert 41c2ff38 breakup large block --- spec/unit/trackable_spec.rb | 453 +++++++++++++++++------------------- 1 file changed, 220 insertions(+), 233 deletions(-) diff --git a/spec/unit/trackable_spec.rb b/spec/unit/trackable_spec.rb index bdd59df..82d09d2 100644 --- a/spec/unit/trackable_spec.rb +++ b/spec/unit/trackable_spec.rb @@ -1,6 +1,64 @@ require 'spec_helper' -shared_examples 'track history' do +describe Mongoid::History::Trackable do + before :each do + class MyModel + include Mongoid::Document + include Mongoid::History::Trackable + + field :foo + end + + class MyDynamicModel + include Mongoid::Document + include Mongoid::History::Trackable + include Mongoid::Attributes::Dynamic unless Mongoid::Compatibility::Version.mongoid3? + end + + class MyDeeplyNestedModel + include Mongoid::Document + include Mongoid::History::Trackable + + embeds_many :children, class_name: 'MyNestableModel', cascade_callbacks: true # The problem only occurs if callbacks are cascaded + accepts_nested_attributes_for :children, allow_destroy: true + track_history modifier_field: nil + end + + class MyNestableModel + include Mongoid::Document + include Mongoid::History::Trackable + + embedded_in :parent, class_name: 'MyDeeplyNestedModel' + embeds_many :children, class_name: 'MyNestableModel', cascade_callbacks: true + accepts_nested_attributes_for :children, allow_destroy: true + field :name, type: String + track_history modifier_field: nil + end + + class HistoryTracker + include Mongoid::History::Tracker + end + + class User + include Mongoid::Document + end + end + + after :each do + Object.send(:remove_const, :MyModel) + Object.send(:remove_const, :MyDynamicModel) + Object.send(:remove_const, :HistoryTracker) + Object.send(:remove_const, :User) + Object.send(:remove_const, :MyDeeplyNestedModel) + Object.send(:remove_const, :MyNestableModel) + end + + let(:user) { User.create! } + + it 'should have #track_history' do + expect(MyModel).to respond_to :track_history + end + describe '#track_history' do before :each do class MyModelWithNoModifier @@ -532,234 +590,6 @@ def my_changes end end end -end - -shared_examples 'track update' do - describe '#track_update' do - before(:each) { MyModel.track_history(on: :foo, track_update: true) } - - let!(:m) { MyModel.create!(foo: 'bar', modifier: user) } - - it 'should create history' do - expect { m.update_attributes!(foo: 'bar2') }.to change(Tracker, :count).by(1) - end - - it 'should not create history when error raised' do - expect(m).to receive(:update_attributes!).and_raise(StandardError) - expect do - expect { m.update_attributes!(foo: 'bar2') }.to raise_error(StandardError) - end.to change(Tracker, :count).by(0) - end - end -end - -shared_examples 'track destroy' do - describe '#track_destroy' do - before(:each) { MyModel.track_history(on: :foo, track_destroy: true) } - - let!(:m) { MyModel.create!(foo: 'bar', modifier: user) } - - it 'should create history' do - expect { m.destroy }.to change(Tracker, :count).by(1) - end - - it 'should not create history when error raised' do - expect(m).to receive(:destroy).and_raise(StandardError) - expect do - expect { m.destroy }.to raise_error(StandardError) - end.to change(Tracker, :count).by(0) - end - - context 'with a deeply nested model' do - let(:m) do - MyDeeplyNestedModel.create!( - children: [ - MyNestableModel.new( - name: 'grandparent', - children: [ - MyNestableModel.new(name: 'parent 1', children: [MyNestableModel.new(name: 'child 1')]), - MyNestableModel.new(name: 'parent 2', children: [MyNestableModel.new(name: 'child 2')]) - ] - ) - ] - ) - end - let(:attributes) do - { - 'children_attributes' => [ - { - 'id' => m.children[0].id, - 'children_attributes' => [ - { 'id' => m.children[0].children[0].id, '_destroy' => '0' }, - { 'id' => m.children[0].children[1].id, '_destroy' => '1' } - ] - } - ] - } - end - - subject(:updated) do - m.update_attributes attributes - m.reload - end - - let(:names_of_destroyed) do - MyDeeplyNestedModel.tracker_class - .where('association_chain.id' => updated.id, 'action' => 'destroy') - .map { |track| track.original['name'] } - end - - it 'does not corrupt embedded models' do - expect(updated.children[0].children.count).to eq 1 # When the problem occurs, the 2nd child will continue to be present, but will only contain the version attribute - end - - it 'creates a history track for the doc explicitly destroyed' do - expect(names_of_destroyed).to include 'parent 2' - end - - it 'creates a history track for the doc implicitly destroyed' do - expect(names_of_destroyed).to include 'child 2' - end - end - - context 'with multiple embeds_many models' do - let(:m) do - MyDeeplyNestedModel.create!( - children: [ - MyNestableModel.new( - name: 'parent', - children: [ - MyNestableModel.new(name: 'child 1'), - MyNestableModel.new(name: 'child 2'), - MyNestableModel.new(name: 'child 3') - ] - ) - ] - ) - end - - let(:attributes) do - { - 'children_attributes' => [ - { - 'id' => m.children[0].id, - 'children_attributes' => [ - { 'id' => m.children[0].children[0].id, '_destroy' => '0' }, - { 'id' => m.children[0].children[1].id, '_destroy' => '1' }, - { 'id' => m.children[0].children[2].id, '_destroy' => '1' } - ] - } - ] - } - end - - subject(:updated) do - m.update_attributes attributes - m.reload - end - - it 'does not corrupt the document' do - expect(updated.children[0].children.length).to eq(1) - end - end - end -end - -shared_examples 'track create' do - describe '#track_create' do - before :each do - class MyModelWithNoModifier - include Mongoid::Document - include Mongoid::History::Trackable - - field :foo - end - end - - after(:each) { Object.send(:remove_const, :MyModelWithNoModifier) } - - before :each do - MyModel.track_history(on: :foo, track_create: true) - MyModelWithNoModifier.track_history modifier_field: nil - end - - it 'should create history' do - expect { MyModel.create!(foo: 'bar', modifier: user) }.to change(Tracker, :count).by(1) - end - - context 'no modifier_field' do - it 'should create history' do - expect { MyModelWithNoModifier.create!(foo: 'bar').to change(Tracker, :count).by(1) } - end - end - - it 'should not create history when error raised' do - expect(MyModel).to receive(:create!).and_raise(StandardError) - expect do - expect { MyModel.create!(foo: 'bar') }.to raise_error(StandardError) - end.to change(Tracker, :count).by(0) - end - end -end - -describe Mongoid::History::Trackable do - before :each do - class MyModel - include Mongoid::Document - include Mongoid::History::Trackable - - field :foo - end - - class MyDynamicModel - include Mongoid::Document - include Mongoid::History::Trackable - include Mongoid::Attributes::Dynamic unless Mongoid::Compatibility::Version.mongoid3? - end - - class MyDeeplyNestedModel - include Mongoid::Document - include Mongoid::History::Trackable - - embeds_many :children, class_name: 'MyNestableModel', cascade_callbacks: true # The problem only occurs if callbacks are cascaded - accepts_nested_attributes_for :children, allow_destroy: true - track_history modifier_field: nil - end - - class MyNestableModel - include Mongoid::Document - include Mongoid::History::Trackable - - embedded_in :parent, class_name: 'MyDeeplyNestedModel' - embeds_many :children, class_name: 'MyNestableModel', cascade_callbacks: true - accepts_nested_attributes_for :children, allow_destroy: true - field :name, type: String - track_history modifier_field: nil - end - - class HistoryTracker - include Mongoid::History::Tracker - end - - class User - include Mongoid::Document - end - end - - after :each do - Object.send(:remove_const, :MyModel) - Object.send(:remove_const, :MyDynamicModel) - Object.send(:remove_const, :HistoryTracker) - Object.send(:remove_const, :User) - Object.send(:remove_const, :MyDeeplyNestedModel) - Object.send(:remove_const, :MyNestableModel) - end - - let(:user) { User.create! } - - it 'should have #track_history' do - expect(MyModel).to respond_to :track_history - end describe '#history_settings' do before(:each) { Mongoid::History.trackable_settings = nil } @@ -988,10 +818,167 @@ class EmbOne end end - include_examples 'track history' - include_examples 'track update' - include_examples 'track destroy' - include_examples 'track create' + describe '#track_update' do + before(:each) { MyModel.track_history(on: :foo, track_update: true) } + + let!(:m) { MyModel.create!(foo: 'bar', modifier: user) } + + it 'should create history' do + expect { m.update_attributes!(foo: 'bar2') }.to change(Tracker, :count).by(1) + end + + it 'should not create history when error raised' do + expect(m).to receive(:update_attributes!).and_raise(StandardError) + expect do + expect { m.update_attributes!(foo: 'bar2') }.to raise_error(StandardError) + end.to change(Tracker, :count).by(0) + end + end + + describe '#track_destroy' do + before(:each) { MyModel.track_history(on: :foo, track_destroy: true) } + + let!(:m) { MyModel.create!(foo: 'bar', modifier: user) } + + it 'should create history' do + expect { m.destroy }.to change(Tracker, :count).by(1) + end + + it 'should not create history when error raised' do + expect(m).to receive(:destroy).and_raise(StandardError) + expect do + expect { m.destroy }.to raise_error(StandardError) + end.to change(Tracker, :count).by(0) + end + + context 'with a deeply nested model' do + let(:m) do + MyDeeplyNestedModel.create!( + children: [ + MyNestableModel.new( + name: 'grandparent', + children: [ + MyNestableModel.new(name: 'parent 1', children: [MyNestableModel.new(name: 'child 1')]), + MyNestableModel.new(name: 'parent 2', children: [MyNestableModel.new(name: 'child 2')]) + ] + ) + ] + ) + end + let(:attributes) do + { + 'children_attributes' => [ + { + 'id' => m.children[0].id, + 'children_attributes' => [ + { 'id' => m.children[0].children[0].id, '_destroy' => '0' }, + { 'id' => m.children[0].children[1].id, '_destroy' => '1' } + ] + } + ] + } + end + + subject(:updated) do + m.update_attributes attributes + m.reload + end + + let(:names_of_destroyed) do + MyDeeplyNestedModel.tracker_class + .where('association_chain.id' => updated.id, 'action' => 'destroy') + .map { |track| track.original['name'] } + end + + it 'does not corrupt embedded models' do + expect(updated.children[0].children.count).to eq 1 # When the problem occurs, the 2nd child will continue to be present, but will only contain the version attribute + end + + it 'creates a history track for the doc explicitly destroyed' do + expect(names_of_destroyed).to include 'parent 2' + end + + it 'creates a history track for the doc implicitly destroyed' do + expect(names_of_destroyed).to include 'child 2' + end + end + + context 'with multiple embeds_many models' do + let(:m) do + MyDeeplyNestedModel.create!( + children: [ + MyNestableModel.new( + name: 'parent', + children: [ + MyNestableModel.new(name: 'child 1'), + MyNestableModel.new(name: 'child 2'), + MyNestableModel.new(name: 'child 3') + ] + ) + ] + ) + end + + let(:attributes) do + { + 'children_attributes' => [ + { + 'id' => m.children[0].id, + 'children_attributes' => [ + { 'id' => m.children[0].children[0].id, '_destroy' => '0' }, + { 'id' => m.children[0].children[1].id, '_destroy' => '1' }, + { 'id' => m.children[0].children[2].id, '_destroy' => '1' } + ] + } + ] + } + end + + subject(:updated) do + m.update_attributes attributes + m.reload + end + + it 'does not corrupt the document' do + expect(updated.children[0].children.length).to eq(1) + end + end + end + + describe '#track_create' do + before :each do + class MyModelWithNoModifier + include Mongoid::Document + include Mongoid::History::Trackable + + field :foo + end + end + + after(:each) { Object.send(:remove_const, :MyModelWithNoModifier) } + + before :each do + MyModel.track_history(on: :foo, track_create: true) + MyModelWithNoModifier.track_history modifier_field: nil + end + + it 'should create history' do + expect { MyModel.create!(foo: 'bar', modifier: user) }.to change(Tracker, :count).by(1) + end + + context 'no modifier_field' do + it 'should create history' do + expect { MyModelWithNoModifier.create!(foo: 'bar').to change(Tracker, :count).by(1) } + end + end + + it 'should not create history when error raised' do + expect(MyModel).to receive(:create!).and_raise(StandardError) + expect do + expect { MyModel.create!(foo: 'bar') }.to raise_error(StandardError) + end.to change(Tracker, :count).by(0) + end + end context 'changing collection' do before :each do