From e885bec0b4b2513bc60b7cb503356d6432fe36bd Mon Sep 17 00:00:00 2001 From: Olivier Bex-Chauvet Date: Tue, 21 Jul 2026 14:37:08 +0200 Subject: [PATCH 1/4] fix(forest_admin_agent): build PolymorphicManyToOne linkage id from foreign key (#332) A polymorphic belongs_to can't be SQL-joined, so the datasource returns a phantom related object without a primary key. The serializer read the id off that phantom, producing an empty id that crashes the frontend list view ("Expected id to be a string or number, received null"). Rebuild the linkage id from the foreign key already loaded on the owner record (no extra query), and omit the data key for unlinked records. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../serializer/forest_serializer.rb | 19 +++++++++++ .../serializer/forest_serializer_spec.rb | 32 +++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/serializer/forest_serializer.rb b/packages/forest_admin_agent/lib/forest_admin_agent/serializer/forest_serializer.rb index 1da0e4c89..56710e88a 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/serializer/forest_serializer.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/serializer/forest_serializer.rb @@ -93,6 +93,25 @@ def add_to_one_association(name, options = {}, &block) @to_one_associations[name] = format_field(name, options) end + # For a PolymorphicManyToOne the related record can't be SQL-joined, so the datasource returns + # a phantom object without a primary key. Rebuild the id from the foreign key already loaded on + # the owner record instead of reading it off that phantom (which yields an empty id and crashes + # the frontend). No extra query is issued. + def has_one_relationship(attribute_name, attr_data) + object = super + field = ForestAdminAgent::Facades::Container.datasource + .get_collection(@options[:class_name].gsub('::', '__')) + .schema[:fields][attribute_name.to_s] + return object unless field&.type == 'PolymorphicManyToOne' + + foreign_key = @object[field.foreign_key] + foreign_type = @object[field.foreign_key_type_field] + return nil if foreign_key.nil? || foreign_type.nil? + + primary_key = (field.foreign_key_targets || {})[foreign_type] || 'id' + (object.is_a?(Hash) ? object : {}).merge(primary_key => foreign_key) + end + def has_one_relationships return {} if @to_one_associations.nil? data = {} diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/serializer/forest_serializer_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/serializer/forest_serializer_spec.rb index 984fd2e2b..ef6f2e730 100644 --- a/packages/forest_admin_agent/spec/lib/forest_admin_agent/serializer/forest_serializer_spec.rb +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/serializer/forest_serializer_spec.rb @@ -77,6 +77,38 @@ module Serializer expect(relationship['data']['id']).to eq('10') end + it 'builds the linkage id from the foreign key when the related record is a phantom (issue #332)' do + # The AR datasource skips polymorphic relations while building the SELECT, so the related + # object comes back without a primary key. The id must still be built from the owner FK. + record = { + 'id' => 1, + 'title' => 'Registration Certificate', + 'documentable_id' => 10, + 'documentable_type' => 'Car', + 'documentable' => { '*' => nil } + } + + result = JSONAPI::Serializer.serialize(record, class_name: 'Document', serializer: described_class) + + relationship = result['data']['relationships']['documentable'] + expect(relationship['data']['type']).to eq('Car') + expect(relationship['data']['id']).to eq('10') + end + + it 'omits the data key for an unlinked polymorphic relation (issue #332)' do + record = { + 'id' => 3, + 'title' => 'Orphan', + 'documentable_id' => nil, + 'documentable_type' => nil, + 'documentable' => { '*' => nil } + } + + result = JSONAPI::Serializer.serialize(record, class_name: 'Document', serializer: described_class) + + expect(result['data']['relationships']['documentable']).not_to have_key('data') + end + it 'serializes a polymorphic belongs_to relation pointing to User' do record = { 'id' => 2, From 45d224c11c3fdc16ad965c5dc6957e4622df9a35 Mon Sep 17 00:00:00 2001 From: Olivier Bex-Chauvet Date: Tue, 21 Jul 2026 14:45:43 +0200 Subject: [PATCH 2/4] fix(forest_admin_agent): only rebuild polymorphic linkage id when relation is loaded The previous commit fabricated a relationship linkage even when the relation was never projected (e.g. update/store responses expose only the FK columns, not the related object). That made the serializer resolve the type-field value as a collection name and raise "Collection not found". Guard on super returning nil: rebuild the id only for the phantom object that list projections return; otherwise keep the prior behavior. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../lib/forest_admin_agent/serializer/forest_serializer.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/serializer/forest_serializer.rb b/packages/forest_admin_agent/lib/forest_admin_agent/serializer/forest_serializer.rb index 56710e88a..0644c3b53 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/serializer/forest_serializer.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/serializer/forest_serializer.rb @@ -99,6 +99,10 @@ def add_to_one_association(name, options = {}, &block) # the frontend). No extra query is issued. def has_one_relationship(attribute_name, attr_data) object = super + # nil means the relation wasn't projected/loaded at all (e.g. update/store responses) — keep + # the previous behavior. Only the phantom object returned by list projections needs rebuilding. + return object if object.nil? + field = ForestAdminAgent::Facades::Container.datasource .get_collection(@options[:class_name].gsub('::', '__')) .schema[:fields][attribute_name.to_s] From 870bad82793bd2bcf7c5af2578e1b24cdf158d3f Mon Sep 17 00:00:00 2001 From: Olivier Bex-Chauvet Date: Tue, 21 Jul 2026 14:51:25 +0200 Subject: [PATCH 3/4] fix(forest_admin_agent): format polymorphic type for foreign_key_targets lookup foreign_key_targets is keyed by formatted collection names (Admin::User => Admin__User), but the type column stores the raw class name. For a namespaced target with a custom primary key the lookup missed and fell back to 'id', storing the FK under the wrong field and still emitting an empty linkage id. Format the type value before the lookup. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../serializer/forest_serializer.rb | 4 ++- .../serializer/forest_serializer_spec.rb | 34 +++++++++++++++++-- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/serializer/forest_serializer.rb b/packages/forest_admin_agent/lib/forest_admin_agent/serializer/forest_serializer.rb index 0644c3b53..0e7740caf 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/serializer/forest_serializer.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/serializer/forest_serializer.rb @@ -112,7 +112,9 @@ def has_one_relationship(attribute_name, attr_data) foreign_type = @object[field.foreign_key_type_field] return nil if foreign_key.nil? || foreign_type.nil? - primary_key = (field.foreign_key_targets || {})[foreign_type] || 'id' + # foreign_key_targets is keyed by formatted collection names (Admin::User => Admin__User), + # but foreign_type is the raw AR type column value, so format it before the lookup. + primary_key = (field.foreign_key_targets || {})[foreign_type.gsub('::', '__')] || 'id' (object.is_a?(Hash) ? object : {}).merge(primary_key => foreign_key) end diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/serializer/forest_serializer_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/serializer/forest_serializer_spec.rb index ef6f2e730..0442996fb 100644 --- a/packages/forest_admin_agent/spec/lib/forest_admin_agent/serializer/forest_serializer_spec.rb +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/serializer/forest_serializer_spec.rb @@ -34,6 +34,17 @@ module Serializer } ) + # Namespaced model (Admin::User => Admin__User) with a custom primary key + admin_user_collection = build_collection( + name: 'Admin__User', + schema: { + fields: { + 'reference' => ColumnSchema.new(column_type: 'String', is_primary_key: true, filter_operators: [Operators::EQUAL]), + 'name' => ColumnSchema.new(column_type: 'String') + } + } + ) + document_collection = build_collection( name: 'Document', schema: { @@ -45,8 +56,8 @@ module Serializer 'documentable' => PolymorphicManyToOneSchema.new( foreign_key: 'documentable_id', foreign_key_type_field: 'documentable_type', - foreign_collections: %w[Car User], - foreign_key_targets: { 'Car' => 'id', 'User' => 'id' } + foreign_collections: %w[Car User Admin__User], + foreign_key_targets: { 'Car' => 'id', 'User' => 'id', 'Admin__User' => 'reference' } ) } } @@ -55,6 +66,7 @@ module Serializer allow(ForestAdminAgent::Builder::AgentFactory.instance).to receive(:send_schema).and_return(nil) datasource.add_collection(car_collection) datasource.add_collection(user_collection) + datasource.add_collection(admin_user_collection) datasource.add_collection(document_collection) ForestAdminAgent::Builder::AgentFactory.instance.add_datasource(datasource) ForestAdminAgent::Builder::AgentFactory.instance.build @@ -95,6 +107,24 @@ module Serializer expect(relationship['data']['id']).to eq('10') end + it 'builds the linkage id for a namespaced target with a custom primary key (issue #332)' do + # foreign_key_targets is keyed by the formatted name (Admin__User), but the type column + # stores the raw class name (Admin::User); the lookup must reconcile the two. + record = { + 'id' => 4, + 'title' => 'Audit Log', + 'documentable_id' => 'ref-42', + 'documentable_type' => 'Admin::User', + 'documentable' => { '*' => nil } + } + + result = JSONAPI::Serializer.serialize(record, class_name: 'Document', serializer: described_class) + + relationship = result['data']['relationships']['documentable'] + expect(relationship['data']['type']).to eq('Admin__User') + expect(relationship['data']['id']).to eq('ref-42') + end + it 'omits the data key for an unlinked polymorphic relation (issue #332)' do record = { 'id' => 3, From 618ae03240a2318afe17afde22d4fa609846f0eb Mon Sep 17 00:00:00 2001 From: Olivier Bex-Chauvet Date: Tue, 21 Jul 2026 15:55:09 +0200 Subject: [PATCH 4/4] test(forest_admin_agent): cover included document; trim comments (PR #333 review) Add a spec asserting the compound `included` document builds its id and self-link from the foreign key (#332's second symptom). Trim the verbose comments per review, keeping only the non-obvious foreign_key_targets note. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../serializer/forest_serializer.rb | 10 +++------- .../serializer/forest_serializer_spec.rb | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/serializer/forest_serializer.rb b/packages/forest_admin_agent/lib/forest_admin_agent/serializer/forest_serializer.rb index 0e7740caf..4b3e6a0af 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/serializer/forest_serializer.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/serializer/forest_serializer.rb @@ -93,15 +93,11 @@ def add_to_one_association(name, options = {}, &block) @to_one_associations[name] = format_field(name, options) end - # For a PolymorphicManyToOne the related record can't be SQL-joined, so the datasource returns - # a phantom object without a primary key. Rebuild the id from the foreign key already loaded on - # the owner record instead of reading it off that phantom (which yields an empty id and crashes - # the frontend). No extra query is issued. + # A PolymorphicManyToOne can't be SQL-joined, so list projections return a phantom object with no + # primary key. Rebuild the id from the foreign key on the owner record. def has_one_relationship(attribute_name, attr_data) object = super - # nil means the relation wasn't projected/loaded at all (e.g. update/store responses) — keep - # the previous behavior. Only the phantom object returned by list projections needs rebuilding. - return object if object.nil? + return object if object.nil? # relation not projected (e.g. update/store responses) field = ForestAdminAgent::Facades::Container.datasource .get_collection(@options[:class_name].gsub('::', '__')) diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/serializer/forest_serializer_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/serializer/forest_serializer_spec.rb index 0442996fb..4ca18912d 100644 --- a/packages/forest_admin_agent/spec/lib/forest_admin_agent/serializer/forest_serializer_spec.rb +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/serializer/forest_serializer_spec.rb @@ -107,6 +107,24 @@ module Serializer expect(relationship['data']['id']).to eq('10') end + it 'builds the included document id and self-link from the foreign key (issue #332)' do + record = { + 'id' => 1, + 'title' => 'Registration Certificate', + 'documentable_id' => 10, + 'documentable_type' => 'Car', + 'documentable' => { '*' => nil } + } + + result = JSONAPI::Serializer.serialize( + record, class_name: 'Document', serializer: described_class, include: 'documentable' + ) + + included = result['included'].find { |r| r['type'] == 'Car' } + expect(included['id']).to eq('10') + expect(included['links']['self']).to eq('/forest/Car/10') + end + it 'builds the linkage id for a namespaced target with a custom primary key (issue #332)' do # foreign_key_targets is keyed by the formatted name (Admin__User), but the type column # stores the raw class name (Admin::User); the lookup must reconcile the two.