The associated model became not validated

[hkdnet]

After we upgraded Rails from 5.2.3 to 5.2.4, our test fails.
I guess AR's behavior was changed through #36671.

Steps to reproduce

https://gist.github.com/hkdnet/9889e283c28f5994bd38e42c12ec3364

This test passes with 5.2.3, 5.1.7 and 4.2.11.1. But it fails with 5.2.4.

Expected behavior

This test passes.

Actual behavior

Fails.

F

Finished in 0.013553s, 73.7844 runs/s, 73.7844 assertions/s.

  1) Failure:
T#test_case [repro.rb:69]:
Expected: false
  Actual: true

1 runs, 1 assertions, 1 failures, 0 errors, 0 skips

System configuration

Rails version: 5.2.4

Ruby version: ruby 2.6.3p62 (2019-04-16 revision 67580) [x86_64-darwin18]

[avit]

Related: #38036

[rafaelfranca]

Hi @wjessop. Can you take a look on this given it is related to the PR you opened some time ago? If you can't it is fine. I'm just trying to give a contributor opportunity to contribute more.

[hkdnet]

Is this behavior expected in Rails 6?

If so, I'm going to fix my code with a custom validator.

[wjessop]

@hkdnet Check out #37192 to see if that fixes your issue /cc @rafaelfranca

[wjessop]

@hkdnet Also see #37295

[hkdnet]

@wjessop
Unfortunately, #37295 does not solve my issue. My issue is not related to validation context.
I confirmed my test fails on v6.0.2 too, which includes #37295's fix.

[pdobb]

@rafaelfranca I'm having this problem too. For any object (A) that has a validates_associated B I can no longer load A (only) and call A#validate and have it validate B records. It skips over validating B records because they are not changed_for_autosave? since they've never even been loaded, let alone changed.

[davidrunger]

We ran into this problem, as well (upgrading from 5.2.3 to 5.2.4.2).

As we can see in the relevant PR description (#36671), the PR aimed to fix this inconsistency:

squeak.valid? # => true (squeak.mouse is not loaded/validated)
squeak.mouse.present? # invalid mouse association is loaded
squeak.valid?  # => false (squeak.mouse is validated)

The problem is that squeak.valid? changes (from true to false) simply by calling squeak.mouse. I agree that this is undesirable / a bug.

The PR solves the problem by changing to this behavior:

squeak.valid? # => true (squeak.mouse is not loaded/validated)
squeak.mouse.present? # invalid mouse association is loaded
squeak.valid?  # => true (squeak.mouse is still not validated)

The inconsistency could have instead been resolved by changing to this behavior:

squeak.valid? # => false (load and validate squeak.mouse)
squeak.mouse.present? # (doesn't really do anything, since squeak.mouse is loaded above)
squeak.valid?  # => false (continue to validate squeak.mouse)

This latter behavior is probably a little bit less performant in some cases (since it will require squeak.mouse to be loaded, if it hasn't already been loaded, when executing the first call to squeak.valid?).

However, the latter behavior has the advantage of checking validations more robustly (which some applications might prefer).

Our application was basically expecting this latter behavior rather than the former behavior, so #36671 broke a few of our tests and somewhat changed the behavior of our application.

It's not a big problem, and we were able to make appropriate adjustments to our application to accommodate the change, but I wanted to flag that this was a bit of a breaking change for us as well and that the change isn't quite what we would have preferred.

[intrip]

I argue that even though this is a regression we should not change the current behaviour:

• Don't validate non dirty association targets #36671 changes the previous behaviour but for good reasons
• achieving what is proposed in The associated model became not validated #38045 (comment) is still possible by using validates_associated :mouse the only difference is that it's no longer the default behaviour; moreover as pointed out doing this by default introduces an hidden global slowdown of performance (imagine having many associations linked to the same model...)