fix(agent): stop the update route from writing foreign keys from relationships

[Tonours]

What

PUT /forest/:collection/:id no longer writes foreign keys from the JSON:API relationships block.

drop_relationships!(args)

And PUT /forest/:collection/:id/relationships/:name can now clear a PolymorphicManyToOne — it could not before, on any payload shape. store untouched.

Why

Fixes#361. Editing one field from a partial Summary panel wiped every belongs_to the panel did not show.

The frontend sends all to-one relations on every update, with data: null for the ones it is not editing. format_attributes turned those nulls into foreign_key => nil. HTTP 200, no error, silent data loss.

Measured, editing name only:

attributes: ["name"]
relationships: ["category", "creator", "portal"]
category -> data=nil # not in the panel
creator -> data=nil # not in the panel
portal -> data={"id": "8"}
→ creator_id 12 → nil, category_id 7 → nil, HTTP 200

Every relation outside the panel, not just one.

The hole this had to plug first

Dropping the block alone would have killed the only working way to clear a polymorphic foreign key. The dedicated route was broken for that case:

• abstract_related_route.rb:9 did args[:params]['data']['type'] unguarded, so data: null raised NoMethodError (500).
• update_polymorphic_many_to_one built foreign_key_type_field from child_collection.name unconditionally, so even data: {id: nil, type: "user"} wrote addressable_type => "user".

Probed on address.addressable:

Payload	before	after
generic PUT, data: null	addressable_id=nil, addressable_type=nil	nothing written
PUT /relationships/addressable, data: null	NoMethodError	204, writes both nil

Two lines, plus a regression spec that raises without them. No spec covered data: null on that path, hence nobody saw it.

Why dropping the block is safe

The frontend fires PUT /forest/:collection/:id/relationships/:name before the record PUT, and that route handles set and clear for all four to-one types it dispatches. The generic PUT arrives second, sometimes with attributes: [].

The Node agent drops the same block (packages/agent/src/routes/modification/update.ts:56-58), same rationale in a comment.

This path was never a coherent relation-writing API: 7 relation types exist, format_attributes branches on 2, the dedicated route dispatches 4 including OneToOne.

Not claimed: that no frontend flow ever reached the generic PUT for relations. e2d20167 (#217) added the PolymorphicManyToOne branch to format_attributes after the dedicated route already handled polymorphic relations (80566a5b, #63), with no issue link explaining why. Either #217 was really about POST and the update half came for free from the shared method, or some flow did go through here. This PR does not need to settle it: the capability survives either way, on the route the frontend calls first.

Removed spec

The with polymorphic many to one relation example from #217 asserted the generic PUT writes memberable_id + memberable_type — the behavior removed here. Inverted rather than deleted: the new examples check no foreign key is written, ManyToOne and PolymorphicManyToOne.

Scope

Does not change:

• store / create. Shared format_attributes, and store_spec.rb covers data: null writing nil at creation on purpose.
• The relationships/:name routes, beyond making the polymorphic clear work.
• The frontend. Root cause of the noise is basic-serializer.js:92 (pruning only runs on create), but fixing that leaves every deployed Ruby agent broken.

Create and update now disagree: POST with memberable: {data: null} writes nil, PUT writes nothing. Deliberate — create has no prior value to destroy.

Release notes

CHANGELOG.md is generated by @semantic-release/changelog from commit messages, so it is not edited here. Two commits carry the change:

• fix(agent): let the relationships route clear a polymorphic many to one
• refactor(agent): name the relationships drop and document why

The 1.16.2 entry announcing "create and update operations" for #217 stays accurate on create. On update the capability moves to the dedicated route rather than disappearing.

How to test

cd packages/forest_admin_agent
bundle exec rspec # 1094 examples, 0 failures
bundle exec rubocop # 170 files, no offenses

On a collection with 3+ belongs_to:

1. Summary panel with one field + one relation, others left out.
2. Edit the field, save, reload → omitted relations survive.
3. Clear the panel's relation, save → it clears, the others survive.

Verified in the browser against a local Rails 8 app (both ManyToOne and PolymorphicManyToOne), with a Rack logger dumping every /forest write. What the wire shows:

edit name only, panel = name + portal
PUT /forest/Experience/51 attributes: ["name"]
relationships: category -> data=nil
creator -> data=nil
portal -> data={id:8}
-> 200, changes = {name, updated_at} creator_id 12 kept, category_id 5 kept
clear portal from the panel
PUT /forest/Experience/51/relationships/portal -> 204 portal_id 8 -> nil
PUT /forest/Experience/51 -> 200 attributes: []
-> creator_id 12 kept, category_id 5 kept
clear a polymorphic attachable
without this PR PUT /forest/Attachment/1/relationships/attachable -> 500, nothing written
with this PR same request -> 204, attachable_id 51 -> nil, attachable_type -> nil

12 edge cases through the API: relationships absent, {}, all-null, explicit ids (ignored, as intended), polymorphic set to another type, data absent, unknown relation name. All as expected.

Two pre-existing issues found, neither introduced here, both left out of scope:

• PUT /relationships/attachable with data: {id: null, type: "X"} returns 204 and writes type="X", id=NULL — an incoherent row. Not reachable from the UI: the frontend only sends data: null.
• PUT /forest/:collection/:id with no data key is a 500 on main as well.

Definition of Done

General

• Write an explicit title for the Pull Request, following Conventional Commits specification
• Test manually the implemented changes
• Validate the code quality (indentation, syntax, style, simplicity, readability)

Security

• Consider the security impact of the changes made

[qltysh]

Coverage Impact

This PR will not change total coverage.

Modified Files with Diff Coverage (3)

Rating	File	% Diff	Uncovered Line #s
	..._agent/lib/forest_admin_agent/routes/abstract_related_route.rb	100.0%
	.../forest_admin_agent/routes/resources/related/update_related.rb	100.0%
	..._admin_agent/lib/forest_admin_agent/routes/resources/update.rb	100.0%
	Total	100.0%

🚦 See full report on Qlty Cloud »

🛟 Help

• Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.

• Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.

• File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)

  • Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

[PMerlet]

Adversarial review. I checked out the branch, replayed the suite, and probed both code paths in throwaway worktrees (origin/main vs this branch) to verify each claim rather than take it on trust.

Claims that hold up

Claim	Verified
1094 examples, 0 failures	✅ replayed
rubocop clean	✅ 170 files, no offense
Node parity (update.ts:56-58)	✅ identical, in-place delete included — the Node comment even says "the frontend is making a second request to update relationships (not sure why) so we purposely ignore"
store untouched	✅ store_spec.rb still fences it with call create with polymorphic foreign key and type and clears both columns when the relationship carries no data
The is_a?(Hash) guard works on the real payload	✅ params.to_unsafe_h returns a HashWithIndifferentAccess, which subclasses Hash, and delete(:relationships) removes the string key
Nothing else reads data[:relationships] on this path	✅ only store.rb:57 and format_attributes, so the in-place mutation breaks nothing today

One blocking finding (inline on update.rb): this removes the only working way to clear a PolymorphicManyToOne, because the dedicated route raises on data: null. Verified with probes, and a 2-line fix is in that comment.

Two smaller asks: the safety argument is contradicted by the history of #217 (inline on the spec), and the code deserves the why comment that Node has.

Out of diff: CHANGELOG.md:631 still announces "create and update operations" for #217 and becomes wrong with this PR. A fix: patch that removes a communicated capability deserves a line in the release notes.

Alternative I considered and rejected: ignoring only the relationships whose data is null (keeping explicit writes working) is strictly less breaking and preserves the update half of #217 — but it diverges from agent-nodejs, keeps alive a relation-writing path that covers 2 of the 7 relation types, and has the exact same polymorphic-clear hole. Dropping the whole block is the better call, provided that hole gets plugged.

[PMerlet]

LGTM

[forest-bot]

🎉 This PR is included in version 1.39.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀