From 86a5f33bbee5a20786863ff3a8f62696ffc3cbf1 Mon Sep 17 00:00:00 2001 From: eileencodes Date: Wed, 13 Sep 2023 10:35:19 -0400 Subject: [PATCH] Don't pass active_record to `derive_fk_query_constraints` We already have access to the `active_record` on the reflection here so there's no point in passing it to `derive_fk_query_constraints`. In addition the id, fk check wasn't actually doing anything here, it was holdover from debugging I was doing when implementing this functionality. --- activerecord/lib/active_record/reflection.rb | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 87795a7dcd124..43c96955faec4 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -507,7 +507,7 @@ def foreign_key(infer_from_inverse_of: true) derived_fk = derive_foreign_key(infer_from_inverse_of: infer_from_inverse_of) if active_record.has_query_constraints? - derived_fk = derive_fk_query_constraints(active_record, derived_fk) + derived_fk = derive_fk_query_constraints(derived_fk) end derived_fk @@ -770,13 +770,13 @@ def derive_foreign_key(infer_from_inverse_of: true) end end - def derive_fk_query_constraints(klass, foreign_key) - primary_query_constraints = klass.query_constraints_list - owner_pk = klass.primary_key + def derive_fk_query_constraints(foreign_key) + primary_query_constraints = active_record.query_constraints_list + owner_pk = active_record.primary_key if primary_query_constraints.size != 2 raise ArgumentError, <<~MSG.squish - The query constraints list on the `#{klass}` model has more than 2 + The query constraints list on the `#{active_record}` model has more than 2 attributes. Active Record is unable to derive the query constraints for the association. You need to explicitly define the query constraints for this association. @@ -785,19 +785,13 @@ def derive_fk_query_constraints(klass, foreign_key) if !primary_query_constraints.include?(owner_pk) raise ArgumentError, <<~MSG.squish - The query constraints on the `#{klass}` model does not include the primary + The query constraints on the `#{active_record}` model does not include the primary key so Active Record is unable to derive the foreign key constraints for the association. You need to explicitly define the query constraints for this association. MSG end - # The primary key and foreign key are both already in the query constraints - # so we don't want to derive the key. In this case we want a single key. - if primary_query_constraints.include?(owner_pk) && primary_query_constraints.include?(foreign_key) - return foreign_key - end - first_key, last_key = primary_query_constraints if first_key == owner_pk @@ -807,7 +801,7 @@ def derive_fk_query_constraints(klass, foreign_key) else raise ArgumentError, <<~MSG.squish Active Record couldn't correctly interpret the query constraints - for the `#{klass}` model. The query constraints on `#{klass}` are + for the `#{active_record}` model. The query constraints on `#{active_record}` are `#{primary_query_constraints}` and the foreign key is `#{foreign_key}`. You need to explicitly set the query constraints for this association. MSG