Skip to content

Commit 2e2bbce

Browse files
authored
Fix validation checks so that all associated records are validated (#5881) (#5883)
It was previously stopping at the first failed validation.
1 parent 9e2d6ff commit 2e2bbce

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
@@ -69,13 +69,16 @@ def validate_association(document, attribute)
6969
# Now, treating the target as an array, look at each element
7070
# and see if it is valid, but only if it has already been
7171
# persisted, or changed, and hasn't been flagged for destroy.
72-
list.all? do |value|
72+
#
73+
# use map.all? instead of just all?, because all? will do short-circuit
74+
# evaluation and terminate on the first failed validation.
75+
list.map do |value|
7376
if value && !value.flagged_for_destroy? && (!value.persisted? || value.changed?)
7477
value.validated? ? true : value.valid?
7578
else
7679
true
7780
end
78-
end
81+
end.all?
7982
end
8083

8184
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
@@ -37,25 +37,35 @@
3737
User.new(name: "test")
3838
end
3939

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

4448
before do
45-
user.descriptions << description
49+
user.descriptions << description1
50+
user.descriptions << description2
51+
user.valid?
4652
end
4753

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

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

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

0 commit comments

Comments
 (0)