Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MONGOID-5757 Fix validation checks so that all associated records are validated #5882

Merged
merged 1 commit into from
Oct 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions lib/mongoid/validatable/associated.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 14 additions & 4 deletions spec/mongoid/validatable/associated_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,25 +38,35 @@
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
expect(user).to_not be_valid
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

Expand Down
Loading