From 0dcff9b4e51bf3c4d6eac79a2925b512b4b08b1e Mon Sep 17 00:00:00 2001 From: Jamis Buck Date: Fri, 11 Oct 2024 10:38:24 -0600 Subject: [PATCH] Fix validation checks so that all associated records are validated It was previously stopping at the first failed validation. --- lib/mongoid/validatable/associated.rb | 7 +++++-- spec/mongoid/validatable/associated_spec.rb | 18 ++++++++++++++---- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/lib/mongoid/validatable/associated.rb b/lib/mongoid/validatable/associated.rb index b811cf1324..4ecbd4c3ef 100644 --- a/lib/mongoid/validatable/associated.rb +++ b/lib/mongoid/validatable/associated.rb @@ -70,13 +70,16 @@ def validate_association(document, attribute) # 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. - list.all? do |value| + # + # 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?) value.validated? ? true : value.valid? else true end - end + end.all? end document.errors.add(attribute, :invalid) unless valid diff --git a/spec/mongoid/validatable/associated_spec.rb b/spec/mongoid/validatable/associated_spec.rb index b9b72b7dc1..153577dab8 100644 --- a/spec/mongoid/validatable/associated_spec.rb +++ b/spec/mongoid/validatable/associated_spec.rb @@ -38,12 +38,18 @@ User.new(name: "test") end - let(:description) do + let(:description1) do + Description.new + end + + let(:description2) do Description.new end before do - user.descriptions << description + user.descriptions << description1 + user.descriptions << description2 + user.valid? end it "only validates the parent once" do @@ -51,12 +57,16 @@ end it "adds the errors from the relation" do - user.valid? expect(user.errors[:descriptions]).to_not be_nil end + it 'reports all failed validations' do + errors = user.descriptions.flat_map { |d| d.errors[:details] } + expect(errors.length).to be == 2 + end + it "only validates the child once" do - expect(description).to_not be_valid + expect(description1).to_not be_valid end end