diff --git a/lib/mongoid/validatable/associated.rb b/lib/mongoid/validatable/associated.rb index b4b1b4fedd..e8559c09fa 100644 --- a/lib/mongoid/validatable/associated.rb +++ b/lib/mongoid/validatable/associated.rb @@ -69,13 +69,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 b1c29fec69..4072af11ea 100644 --- a/spec/mongoid/validatable/associated_spec.rb +++ b/spec/mongoid/validatable/associated_spec.rb @@ -37,12 +37,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 @@ -50,12 +56,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