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

fix: actual_value_for(validate) returns false instead of true when there is no validate option #1378

Merged
merged 1 commit into from Nov 19, 2020

Conversation

vsppedro
Copy link
Collaborator

Closes #1357

It seems to me that the problem here is that when there is no validate option on the relationship the value being returned by actual_value_for(name) is nil instead of true - the default value.

In case you want to check the default value for this macro: documantation and active_record/associations/has_many_association.rb#L56.

To avoid this I added a check to validate the value and if it is null the default value will be returned instead, otherwise the code will follow its normal flow. I added some tests just to be sure about the behaviour. What do you think?

@vsppedro vsppedro self-assigned this Nov 17, 2020
@vsppedro vsppedro changed the title fix: has_many without validate option returns false instead of true fix: actual_value_for(validate) returns false instead of true when there is no validate option Nov 17, 2020
Copy link
Collaborator

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

Okay! This looks like a good fix to me. 👍

@vsppedro vsppedro added this to the 4.5.0 milestone Nov 18, 2020
@vsppedro vsppedro merged commit 8772cd4 into master Nov 19, 2020
@vsppedro vsppedro deleted the fix-has-many-without-validate-option branch November 19, 2020 10:44
@vsppedro
Copy link
Collaborator Author

Thank you! Merging.

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.

have_many(:model).validate(false) causes a false positive test
2 participants