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

Skip association validation when the record is persisted, unchanged and invalid #45355

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

muZk
Copy link

@muZk muZk commented Jun 14, 2022

Summary

Fixes #45320

This adds consistency to the behavior described in the issue. Basically, skips validation when the associated record is persisted, unchanged and invalid.

@@ -329,6 +329,7 @@ def validate_collection_association(reflection)
# enabled records if they're marked_for_destruction? or destroyed.
def association_valid?(reflection, record, index = nil)
return true if record.destroyed? || (reflection.options[:autosave] && record.marked_for_destruction?)
return true if !custom_validation_context? && record.persisted? && !record.has_changes_to_save? && !record.valid?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The autosave code is very complex with a lot of conditionals.
I'm wondering if this should be handled instead in

def associated_records_to_validate_or_save(association, new_record, autosave)
if new_record || custom_validation_context?
association && association.target
elsif autosave
association.target.find_all(&:changed_for_autosave?)
else
association.target.find_all(&:new_record?)
end
end

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it has to be done in association_valid? because we need to skip validation but keep it for saving.

I noticed that !record.valid? is not necessary.

Maybe that helps?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] ActiveRecord: Association's validations are ran only on create when assigning with *_ids=
2 participants