Skip to content

Commit 560ce66

Browse files
authored
Fix validation checks so that all associated records are validated (#5881) (#5882)
It was previously stopping at the first failed validation.
1 parent a1f1bbc commit 560ce66

File tree

2 files changed

+19
-6
lines changed

2 files changed

+19
-6
lines changed

lib/mongoid/validatable/associated.rb

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,16 @@ def validate_association(document, attribute)
7070
# Now, treating the target as an array, look at each element
7171
# and see if it is valid, but only if it has already been
7272
# persisted, or changed, and hasn't been flagged for destroy.
73-
list.all? do |value|
73+
#
74+
# use map.all? instead of just all?, because all? will do short-circuit
75+
# evaluation and terminate on the first failed validation.
76+
list.map do |value|
7477
if value && !value.flagged_for_destroy? && (!value.persisted? || value.changed?)
7578
value.validated? ? true : value.valid?
7679
else
7780
true
7881
end
79-
end
82+
end.all?
8083
end
8184

8285
document.errors.add(attribute, :invalid) unless valid

spec/mongoid/validatable/associated_spec.rb

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,25 +38,35 @@
3838
User.new(name: "test")
3939
end
4040

41-
let(:description) do
41+
let(:description1) do
42+
Description.new
43+
end
44+
45+
let(:description2) do
4246
Description.new
4347
end
4448

4549
before do
46-
user.descriptions << description
50+
user.descriptions << description1
51+
user.descriptions << description2
52+
user.valid?
4753
end
4854

4955
it "only validates the parent once" do
5056
expect(user).to_not be_valid
5157
end
5258

5359
it "adds the errors from the relation" do
54-
user.valid?
5560
expect(user.errors[:descriptions]).to_not be_nil
5661
end
5762

63+
it 'reports all failed validations' do
64+
errors = user.descriptions.flat_map { |d| d.errors[:details] }
65+
expect(errors.length).to be == 2
66+
end
67+
5868
it "only validates the child once" do
59-
expect(description).to_not be_valid
69+
expect(description1).to_not be_valid
6070
end
6171
end
6272

0 commit comments

Comments
 (0)