diff --git a/Gemfile b/Gemfile index 998a9f9..9a0563d 100644 --- a/Gemfile +++ b/Gemfile @@ -12,6 +12,7 @@ else gem 'mongoid', version end +gem 'ostruct' gem 'rake' gem 'rspec' gem 'rubocop' diff --git a/lib/mongoid/paranoia/monkey_patches.rb b/lib/mongoid/paranoia/monkey_patches.rb index 76163c3..0f83e0b 100644 --- a/lib/mongoid/paranoia/monkey_patches.rb +++ b/lib/mongoid/paranoia/monkey_patches.rb @@ -12,41 +12,34 @@ module Document class_attribute :paranoid end end - end -end -Mongoid::Document.include Mongoid::Paranoia::Document + # Skip paranoid docs flagged for destruction when checking whether a + # candidate is already related, so they do not block a new sibling with + # the same `==` key during a destroy-and-re-add nested attributes update. + # Non-paranoid candidates fall through to stock Mongoid behavior via + # `super`, so any future upstream fix is inherited automatically. + module EmbedsManyProxyExtensions + private -module Mongoid - module Association - module Nested - class Many - # Destroy the child document, needs to do some checking for embedded - # relations and delay the destroy in case parent validation fails. - # - # @api private - # - # @example Destroy the child. - # builder.destroy(parent, relation, doc) - # - # @param [ Document ] parent The parent document. - # @param [ Proxy ] relation The relation proxy. - # @param [ Document ] doc The doc to destroy. - # - # @since 3.0.10 - def destroy(parent, relation, doc) - doc.flagged_for_destroy = true - if !doc.embedded? || parent.new_record? || doc.paranoid? - destroy_document(relation, doc) - else - parent.flagged_destroys.push(-> { destroy_document(relation, doc) }) - end - end + # @example Check if a document is already related. + # relation.send(:object_already_related?, document) + # + # @param [ Document ] document The candidate document to check. + # + # @return [ true, false ] If a non-flagged sibling matches. + def object_already_related?(document) + return super unless document.paranoid? + # rubocop:disable Style/CaseEquality -- matches upstream Mongoid's dedup check + _target.any? {|existing| existing._id && !existing.flagged_for_destroy? && existing === document } + # rubocop:enable Style/CaseEquality end end end end +Mongoid::Document.include Mongoid::Paranoia::Document +Mongoid::Association::Embedded::EmbedsMany::Proxy.prepend(Mongoid::Paranoia::EmbedsManyProxyExtensions) + module Mongoid module Association module Embedded diff --git a/spec/mongoid/nested_attributes_spec.rb b/spec/mongoid/nested_attributes_spec.rb index b6c07b6..09989c4 100644 --- a/spec/mongoid/nested_attributes_spec.rb +++ b/spec/mongoid/nested_attributes_spec.rb @@ -68,24 +68,29 @@ } end - it "removes the first document from the relation" do - expect(persisted.paranoid_phones.size).to eq(2) + it "flags the marked document for destruction" do + expect(phone_one.flagged_for_destroy?).to be true end - it "does not delete the unmarked document" do - expect(persisted.paranoid_phones.first.number).to eq("3") + it "does not soft-delete the marked document until save" do + expect(phone_one).not_to be_destroyed + expect(phone_one.reload.deleted_at).to be_nil end - it "adds the new document to the relation" do - expect(persisted.paranoid_phones.last.number).to eq("4") + it "keeps the marked document in the relation pending save" do + expect(persisted.paranoid_phones.size).to eq(3) + end + + it "applies the update to the unmarked document" do + expect(persisted.paranoid_phones.find(phone_two.id).number).to eq("3") end - it "has the proper persisted count" do - expect(persisted.paranoid_phones.count).to eq(1) + it "adds the new document to the relation" do + expect(persisted.paranoid_phones.last.number).to eq("4") end - it "soft deletes the removed document" do - expect(phone_one).to be_destroyed + it "counts only persisted documents" do + expect(persisted.paranoid_phones.count).to eq(2) end context "when saving the parent" do @@ -106,6 +111,101 @@ expect(persisted.reload.paranoid_phones.last.number).to eq("4") end end + + context "when saving the parent fails validation" do + + before do + Person.class_eval { validate { errors.add(:base, "nope") } } + persisted.save + end + + after do + Person._validate_callbacks.clear + end + + it "does not soft-delete the marked document" do + expect(phone_one.reload.deleted_at).to be_nil + end + + it "leaves the persisted collection intact" do + expect(persisted.reload.paranoid_phones.count).to eq(2) + end + + it "does not persist the new document" do + expect(persisted.reload.paranoid_phones.where(number: "4")).to be_empty + end + end + end + end + end + end + + context "when the child overrides equality" do + + before(:all) do + ParanoidPhone.class_eval do + def ==(other) + other.is_a?(self.class) && number == other.number + end + alias_method :eql?, :== + end + end + + after(:all) do + ParanoidPhone.send(:remove_method, :==) + ParanoidPhone.send(:remove_method, :eql?) + end + + context "when the parent is persisted" do + + let!(:persisted) do + Person.create do |p| + p.paranoid_phones << [ phone_one, phone_two ] + end + end + + context "when destroying then re-adding a sibling with the same key" do + + before do + persisted.paranoid_phones_attributes = + { + "bar" => { "id" => phone_one.id, "_destroy" => "1" }, + "baz" => { "number" => "1" } + } + end + + it "keeps the new sibling in the relation" do + fresh = persisted.paranoid_phones.send(:_target).reject(&:flagged_for_destroy?) + expect(fresh.map(&:number)).to include("1") + end + + context "when saving the parent" do + + before do + persisted.save + end + + it "soft-deletes the original sibling" do + expect(phone_one.reload.deleted_at).not_to be_nil + end + + it "persists the new sibling" do + reloaded = persisted.reload.paranoid_phones + expect(reloaded.map(&:number)).to contain_exactly("1", "2") + expect(reloaded.where(number: "1").first.id).not_to eq(phone_one.id) + end + end + end + + context "when pushing a duplicate of a live sibling" do + + before do + persisted.paranoid_phones.push(ParanoidPhone.new(number: "1")) + end + + it "does not add the duplicate to the relation" do + target = persisted.paranoid_phones.send(:_target) + expect(target.count {|p| p.number == "1" }).to eq(1) end end end