Prevent preloading `last_deployment`/`last_visible_deployment`/`upcoming_deployment` with a legacy approach
Problem
Originally, pointed out by @bala.kumar in !83558 (comment 895662565).
We have Preloaders::Environments::DeploymentPreloader
to preload last_deployment
/last_visible_deployment
correctly and efficiently, however, technically we can still use the legacy preload approach such as resource.preload({last_deployment: deployment_associations})
, which could read data wrongly and inefficiently. We should prevent us from using the legacy approach.
Context discussion thread
Possible solution:
- Introduce argument to the lambda scope of association so direct preloading is not possible as mentioned in https://api.rubyonrails.org/classes/ActiveRecord/Associations/ClassMethods.html
Note: Joining, eager loading and preloading of these associations is not possible. These operations happen before instance creation and the scope will be called with a nil argument.
# Introducing association argument to prevent direct preloading. # Consider using Preloaders::Environments::DeploymentPreloader.new.execute_with_union instead. has_one :last_deployment, -> (environments) { success.ordered }, class_name: 'Deployment', inverse_of: :environment
- Or even better consider moving
last_deployment
to a model class method instead of an association.
Proposal
- Introduce argument to the lambda scope of association so direct preloading is not possible.
- Moving the associations to class method results in
ActiveRecord::AssociationNotFoundError: Association named 'last_deployment' was not found on Environment;
and to avoid it we will have to do further refactor to existing code areas. If required we can create a low priority issue to capture this work in the future and it can be worked on once the serializer fragility epic is closed as some of these associations may not be required then.
POC
Edited by Bala Kumar