Don't soft-destroy nested records before save

Gemfile

@@ -12,6 +12,7 @@ else
   gem 'mongoid', version
 end
 
+gem 'ostruct'
 gem 'rake'
 gem 'rspec'
 gem 'rubocop'

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

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