Consistently Persist PD FactoryBot Records in v5 - #51341

In preparation for an update to FactoryBot 5, which fixes some unintuitive behavior when building objects with descendants. Specifically, before, when creating records with build, any associated records created as a result would unexpectedly be persisted with create rather than just initialized with build.

In fact, for the School model specifically, that behavior is required, in a couple of ways: in the :pd_teacher_application factory, we pass the id of the School object upstream to the :pd_teacher_application factory which as part of its own persistence logic expects its school_id attribute to reference a persisted object. And in the School model itself, because it doesn't use a standard Rails auto-incrementing integer id, we manually assign an id based on the latest persisted value; that means that we always want to persist any built School record before building another one, or we'll get an ActiveRecord::RecordNotUnique error.

Starting in FactoryBot 5, the default association strategy will now be inherited from the parent factory, so building a hash will build but not persist the associated School object. In most cases this is probably desirable, but here we are relying on the old strange behavior and so now need to explicitly specify that we always want to persist the associated School object.

In a couple other places, we were implicitly (and I think accidentally) depending on this behavior to allow us to build and then validate records which require their associated records to be persisted in order to pass validation. In those cases, the fix was either to create rather than build the original record in the first place, or to explicitly create the associated records we want to be persisted.

Links

• https://github.com/thoughtbot/factory_bot/blob/main/GETTING_STARTED.md#build-strategies-1

Testing story

Without this change, we get the following error when trying to run tests on FactoryBot 5:

ERROR["test_deleting_user_deletes_dependent_pd_applications", "UserTest", 21.59177164407447]
 test_deleting_user_deletes_dependent_pd_applications#UserTest (21.59s)
Minitest::UnexpectedError:         ActiveRecord::RecordNotFound: Couldn't find School with 'id'=60001411747
            app/models/school_info.rb:98:in `sync_from_schools'
            lib/school_info_deduplicator.rb:20:in `get_duplicate_school_info'
            app/models/pd/application/teacher_application.rb:105:in `update_user_school_info!'
            test/models/user_test.rb:3156:in `block in <class:UserTest>'
            test/testing/setup_all_and_teardown_all.rb:36:in `run'

With this change, the test passes.

We also had two tests that were just validating internal FactoryBot persistence logic. One of them is no longer accurate because we actually now are persisting some of these dependencies consistently, and the other one is just a bit unnecessary. Remove them both.

Follow-up Work

Update FactoryBot to v5