fix(forest_admin_agent): build PolymorphicManyToOne linkage id from foreign key (#332) - #333
Merged
Merged
Conversation
…oreign 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) <noreply@anthropic.com>
…ation 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) <noreply@anthropic.com>
1 new issue
|
…ets 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) <noreply@anthropic.com>
matthv
reviewed
Jul 21, 2026
matthv
left a comment
Member
There was a problem hiding this comment.
Nit: comments a bit verbose. The foreign_key_targets keying one justifies the gsub → keep. But # nil means… mostly restates return object, and no extra query is issued. reads as pre-empting a reviewer. Could trim both, wdyt?
The specs only assert the linkage, not the included doc (#332's 2nd symptom, the empty self-link). Could be good to add one with include: 'documentable' asserting included has id => '10'?
…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) <noreply@anthropic.com>
Member
Author
|
Both done in 618ae03:
Full suite green (724 examples). |
Member
|
🎉 This PR is included in version 1.35.4 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #332
Problem
Opening the list view of a collection with a polymorphic
belongs_tocrashes the frontend:The backend returns 200 OK but the payload is malformed: the relationship linkage
idis an empty string.Root cause
A
PolymorphicManyToOnecan't be SQL-joined, soquery.rb:208skips it while building the SELECT. The record therefore comes back with a phantom related object that has no primary key ({ "*" => nil }).ForestSerializerthen reads the PK off that phantom object, producing an empty id in the JSON:API linkage.The existing serializer specs didn't catch this because they hand-fed a fully-populated related hash, which the real datasource never produces.
Fix
Override
has_one_relationshipinForestSerializer. For aPolymorphicManyToOneit rebuilds the linkage id from the foreign key (*_id+*_type) already loaded on the owner record — no extra query — and returnsnil(omitting thedatakey) when the relation is unlinked. Both the linkage and the compoundincludeddocument go through this method, so a single override covers both.This is the workaround reported in #332, moved into the gem.
Tests
Added two specs reproducing the real datasource output (phantom related object) — both fail without the fix:
datakey for an unlinked polymorphic relation🤖 Generated with Claude Code
Note
Fix
PolymorphicManyToOnelinkage id serialization inForestSerializerWhen a polymorphic many-to-one related record is a phantom (no primary key), the serializer now rebuilds the linkage
idfrom the owner's foreign key fields instead of returning an empty or broken relationship.has_one_relationshipinForestSerializerto post-processPolymorphicManyToOnerelationships, computingidfrom the foreign key value and resolving the type usingforeign_key_targets.Admin::User) are formatted toAdmin__Userto match collection naming conventions, and custom primary keys are respected.datamember entirely.Changes since #333 opened
Macroscope summarized 870bad8.