-
Notifications
You must be signed in to change notification settings - Fork 1
feat(datasource-active-record): expose has_one :through as a to-one relation #325
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -100,21 +100,32 @@ def fetch_associations | |
| source_polymorphic = association.source_reflection&.polymorphic? && | ||
| association.options[:source_type].present? | ||
|
|
||
| add_field( | ||
| association.name.to_s, | ||
| ForestAdminDatasourceToolkit::Schema::Relations::ManyToManySchema.new( | ||
| foreign_collection: format_model_name(association.klass.name), | ||
| origin_key: through_reflection.foreign_key, | ||
| origin_key_target: through_reflection.join_foreign_key, | ||
| foreign_key: association.join_foreign_key, | ||
| foreign_key_target: association.association_primary_key, | ||
| through_collection: format_model_name(through_reflection.klass.name), | ||
| origin_type_field: is_polymorphic ? through_reflection.type : nil, | ||
| origin_type_value: is_polymorphic ? @model.name : nil, | ||
| foreign_type_field: source_polymorphic ? association.source_reflection.foreign_type : nil, | ||
| foreign_type_value: source_polymorphic ? association.options[:source_type] : nil | ||
| if is_polymorphic || source_polymorphic | ||
| add_field( | ||
| association.name.to_s, | ||
| ForestAdminDatasourceToolkit::Schema::Relations::ManyToManySchema.new( | ||
| foreign_collection: format_model_name(association.klass.name), | ||
| origin_key: through_reflection.foreign_key, | ||
| origin_key_target: through_reflection.join_foreign_key, | ||
| foreign_key: association.join_foreign_key, | ||
| foreign_key_target: association.association_primary_key, | ||
| through_collection: format_model_name(through_reflection.klass.name), | ||
| origin_type_field: is_polymorphic ? through_reflection.type : nil, | ||
| origin_type_value: is_polymorphic ? @model.name : nil, | ||
| foreign_type_field: source_polymorphic ? association.source_reflection.foreign_type : nil, | ||
| foreign_type_value: source_polymorphic ? association.options[:source_type] : nil | ||
| ) | ||
| ) | ||
| ) | ||
| else | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 High When a non-polymorphic Also found in 1 other location(s)
🚀 Reply "fix it for me" or copy this AI Prompt for your agent: |
||
| add_field( | ||
| association.name.to_s, | ||
| ForestAdminDatasourceToolkit::Schema::Relations::OneToOneSchema.new( | ||
| foreign_collection: format_model_name(association.klass.name), | ||
| origin_key: association.klass.primary_key, | ||
| origin_key_target: @model.primary_key | ||
| ) | ||
| ) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| end | ||
| elsif association.inverse_of&.polymorphic? | ||
| add_field( | ||
| association.name.to_s, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -196,6 +196,10 @@ def build_select(collection, projection) | |
| if collection.model.table_name == @collection.model.table_name | ||
| if one_to_one_relations.include?(relation_schema.type) | ||
| @select << "#{collection.model.table_name}.#{relation_schema.origin_key_target}" | ||
| # a has_one :through preloads via the intermediate FK on this table; select it so a | ||
| # relation that falls back to preload (e.g. it is also used by a filter) still loads. | ||
| through_fk = through_foreign_key(collection, relation_name) | ||
| @select << "#{collection.model.table_name}.#{through_fk}" if through_fk | ||
|
Comment on lines
+201
to
+202
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 High
- through_fk = through_foreign_key(collection, relation_name)
- @select << "#{collection.model.table_name}.#{through_fk}" if through_fk
+ through_fk = root_through_foreign_key(collection, relation_name)
+ @select << "#{collection.model.table_name}.#{through_fk}" if through_fk🚀 Reply "fix it for me" or copy this AI Prompt for your agent: |
||
| elsif many_to_one_relations.include?(relation_schema.type) | ||
| @select << "#{collection.model.table_name}.#{relation_schema.foreign_key}" | ||
| end | ||
|
|
@@ -285,18 +289,26 @@ def joinable_tables(collection, relation_name, sub_projection, used_tables) | |
| tables | ||
| end | ||
|
|
||
| # The target collection when this hop is safe to collapse into a JOIN, else nil (-> preload). | ||
| # The target collection when this single hop is safe to collapse into a LEFT OUTER JOIN, | ||
| # else nil. A belongs_to qualifies: it matches on the target's primary key, so the JOIN | ||
| # cannot duplicate the parent row (a plain has_one child may not be unique). A has_one :through | ||
| # also qualifies when every hop is a belongs_to (see belongs_to_chain_through?): AR resolves the | ||
| # intermediate join, saving the extra per-hop preload queries. | ||
| def joinable_target(collection, relation_name, used_tables) | ||
| relation_schema = collection.schema[:fields][relation_name] | ||
| # belongs_to only: it joins on the target's primary key, so it can't duplicate the parent | ||
| # (a has_one child may not be unique) | ||
| return unless relation_schema.type == 'ManyToOne' && relation_schema.respond_to?(:foreign_collection) | ||
| return unless relation_schema.respond_to?(:foreign_collection) | ||
|
|
||
| # a scoped association applies its scope to the JOIN and may inject raw/unqualified SQL or | ||
| # extra joins (e.g. `belongs_to :x, -> { where('id > ?', 1) }`) | ||
| reflection = collection.model.reflect_on_association(relation_name.to_sym) | ||
| return if reflection.nil? || reflection.scope | ||
|
|
||
| case relation_schema.type | ||
| when 'ManyToOne' then nil | ||
| when 'OneToOne' then return unless belongs_to_chain_through?(reflection) | ||
| else return | ||
| end | ||
|
|
||
| target = local_ar_collection(collection.datasource, relation_schema.foreign_collection) | ||
| return if target.nil? || !target.model.default_scopes.empty? # same risk as a scoped association | ||
| return unless same_database?(collection.model, target.model) | ||
|
|
@@ -305,6 +317,21 @@ def joinable_target(collection, relation_name, used_tables) | |
| target | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| end | ||
|
|
||
| def belongs_to_chain_through?(reflection) | ||
| return false unless reflection.through_reflection? | ||
|
|
||
| through = reflection.through_reflection | ||
| source = reflection.source_reflection | ||
| return false unless through && source | ||
| return false unless through.belongs_to? && source.belongs_to? | ||
| return false if through.scope || source.scope | ||
| return false unless through.klass.default_scopes.empty? | ||
|
|
||
| same_database?(reflection.active_record, through.klass) | ||
| rescue StandardError | ||
| false | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| end | ||
|
|
||
| def same_database?(model_a, model_b) | ||
| # compare the pools, not connection_specification_name (only an owner class name, shared across shards) | ||
| model_a.connection_pool == model_b.connection_pool | ||
|
|
@@ -330,6 +357,16 @@ def add_join_relation(relation_name) | |
| @query | ||
| end | ||
|
|
||
| # The intermediate foreign key a has_one :through uses on this table, or nil for a plain to-one. | ||
| def through_foreign_key(collection, relation_name) | ||
| reflection = collection.model.reflect_on_association(relation_name.to_sym) | ||
| return unless reflection&.through_reflection? | ||
|
|
||
| reflection.through_reflection.foreign_key | ||
| rescue StandardError | ||
| nil | ||
| end | ||
|
|
||
| def resolve_field(original_field) | ||
| if original_field.include?(':') | ||
| relation_name, column_name = original_field.split(':') | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| class Account < ApplicationRecord | ||
| belongs_to :supplier | ||
| belongs_to :account_history | ||
| has_one :order, through: :account_history | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| class AccountHistory < ApplicationRecord | ||
| has_one :account | ||
| belongs_to :order, optional: true | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| class AddOrderToAccountHistories < ActiveRecord::Migration[7.1] | ||
| def change | ||
| add_reference :account_histories, :order, null: true, foreign_key: true | ||
| end | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found 15 lines of similar code in 2 locations (mass = 94) [qlty:similar-code]