MONGOID-5945 don't try to valid destroyed children (#6154)

Author: jamis

lib/mongoid/validatable/associated.rb

@@ -67,12 +67,13 @@ def validate_association(document, attribute)
         valid = document.validating do
           # Now, treating the target as an array, look at each element
           # and see if it is valid, but only if it has already been
-          # persisted, or changed, and hasn't been flagged for destroy.
+          # persisted, or changed, and hasn't been flagged for destroy
+          # or already destroyed.
           #
           # use map.all? instead of just all?, because all? will do short-circuit
           # evaluation and terminate on the first failed validation.
           list.map do |value|
-            if value && !value.flagged_for_destroy? && (!value.persisted? || value.changed?)
+            if needs_validation?(value)
               value.validated? || value.valid?
             else
               true
@@ -120,6 +121,21 @@ def get_target_documents_for_has_many(target)
       def get_target_documents_for_other(target)
         Array.wrap(target)
       end
+
+      # Returns true if the given value should be validated as part of
+      # an associated validation. Destroyed and flagged-for-destroy
+      # documents are skipped, as are persisted documents that haven't
+      # changed.
+      #
+      # @param [ Mongoid::Document | nil ] value The value to check.
+      #
+      # @return [ true | false ] Whether the value needs validation.
+      def needs_validation?(value)
+        value &&
+          !value.flagged_for_destroy? &&
+          !value.destroyed? &&
+          (!value.persisted? || value.changed?)
+      end
     end
   end
 end

spec/mongoid/validatable/associated_spec.rb

@@ -82,6 +82,25 @@
           expect(user).to be_valid
         end
       end
+
+      context 'when a child document has been destroyed' do
+        let(:user) do
+          User.new(name: 'test')
+        end
+
+        let(:description) do
+          Description.new
+        end
+
+        before do
+          user.descriptions << description
+          description.destroyed = true
+        end
+
+        it 'does not run validation on the destroyed child' do
+          expect(user).to be_valid
+        end
+      end
     end
   end