Skip to content

fix(polymorphic): skip pre-delete FK nullify when datasource cascades#291

Merged
bexchauveto merged 2 commits intomainfrom
fix/polymorphic-cascade-on-delete-289
Apr 29, 2026
Merged

fix(polymorphic): skip pre-delete FK nullify when datasource cascades#291
bexchauveto merged 2 commits intomainfrom
fix/polymorphic-cascade-on-delete-289

Conversation

@bexchauveto
Copy link
Copy Markdown
Member

@bexchauveto bexchauveto commented Apr 28, 2026

Summary

Fixes #289. The agent's delete_records action pre-emptively UPDATEs polymorphic child rows to NULL both the FK id and type column before issuing the parent delete. For ActiveRecord parents that declare dependent: :destroy, this breaks deletion entirely:

  • Rails 5+ belongs_to :foo, polymorphic: true adds a presence validator → the UPDATE fails with Validation failed: <relation> must exist.
  • A null: false FK column → the UPDATE violates the NOT NULL constraint.
  • Even when the FK is nullable, the manual nullify defeats dependent: :destroy: by the time Rails' destroy callback runs, its WHERE notable_id = ? AND notable_type = ? matches zero rows, silently orphaning the children.

The reporter showed the bug works correctly from the Rails console (Rails handles the cascade), but fails through Forest because of this pre-delete UPDATE.

Fix

Plumb a cascade_on_delete flag through to the polymorphic schemas; when true, the agent skips its manual nullify and lets the datasource handle the cascade itself.

  • Toolkit — added cascade_on_delete (default false) to PolymorphicOneToManySchema and PolymorphicOneToOneSchema. The flag rides through the RPC layer for free (**schema reconstruction + .to_json serializer dumps all instance vars).
  • AR datasource — sets cascade_on_delete: true when the association declares dependent: :destroy, :destroy_async, or :delete_all.
  • Agent routedelete_records skips the polymorphic-cleanup loop body when field_schema.cascade_on_delete is truthy. Guarded with respond_to? for compat with anything the customizer might inject.

Mongoid / Snowflake / RPC keep the existing behavior (cascade_on_delete defaults to false, so the manual cleanup still runs).

Test plan

  • forest_admin_datasource_toolkit rspec — 463 examples, 0 failures (added defaults + explicit-flag specs on both schemas)
  • forest_admin_datasource_active_record rspec — 147 examples, 0 failures (added: cascade_on_delete=true for Project.has_many :members, as: :memberable, dependent: :destroy; =false for User.has_one :address, as: :addressable)
  • forest_admin_agent rspec — 683 examples, 0 failures (new with polymorphic relation that cascades on delete context: foreign collection's update is NOT called, parent delete still is)
  • forest_admin_datasource_rpc rspec — 160 examples, 0 failures (sanity: schema round-trips through RPC unchanged)
  • End-to-end repro against a Rails 8.1 / PG 15 app: a Customer with has_many :notes, as: :notable, dependent: :destroy and notable_id/notable_type null: false — delete now performs the same SQL as customer.destroy! (children deleted via cascade, then parent), instead of erroring on the nullify.

Note

Skip FK nullification before delete for polymorphic relations when datasource cascades

  • Adds a cascade_on_delete flag (default false) to PolymorphicOneToManySchema and PolymorphicOneToOneSchema in the toolkit.
  • The ActiveRecord datasource sets this flag based on the association's dependent option (:destroy, :destroy_async, or :delete_all) via a new cascade_on_delete? helper in Collection.
  • The delete route in Delete#delete_records now skips the pre-delete FK nullify step for polymorphic relations when cascade_on_delete is true, avoiding redundant or conflicting updates before the DB cascade fires.

Macroscope summarized 198e837.

…#289)

Forest's delete route nullifies polymorphic FKs on the foreign collection
before delete. This breaks ActiveRecord parents that declare
`dependent: :destroy`: the UPDATE either violates a NOT NULL constraint
on the FK or fails Rails' default `belongs_to` presence validation,
preventing deletion entirely.

Add a `cascade_on_delete` flag to PolymorphicOneToMany/OneToOne schemas
(default false). The AR datasource sets it to true when the association
declares `dependent: :destroy`, `:destroy_async`, or `:delete_all`, and
`delete_records` skips its manual nullify when the flag is set. Other
datasources (Mongoid, RPC, Snowflake) keep current behavior.
@qltysh
Copy link
Copy Markdown

qltysh Bot commented Apr 28, 2026

6 new issues

Tool Category Rule Count
qlty Duplication Found 46 lines of similar code in 2 locations (mass = 241) 4
qlty Structure Function with many parameters (count = 6): initialize 2

Mirrors the existing exclusion for many_to_many_schema.rb after adding
the cascade_on_delete kwarg pushed param count past Max=5.
Copy link
Copy Markdown
Member

@matthv matthv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bexchauveto bexchauveto merged commit 5e05ad0 into main Apr 29, 2026
36 checks passed
@bexchauveto bexchauveto deleted the fix/polymorphic-cascade-on-delete-289 branch April 29, 2026 12:26
forest-bot added a commit that referenced this pull request Apr 29, 2026
## [1.27.1](v1.27.0...v1.27.1) (2026-04-29)

### Bug Fixes

* **polymorphic:** skip pre-delete FK nullify when datasource cascades ([#291](#291)) ([5e05ad0](5e05ad0)), closes [#289](#289)
@forest-bot
Copy link
Copy Markdown
Member

🎉 This PR is included in version 1.27.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

delete_records nullifies PolymorphicOneToMany FKs before destroy, failing when FK is NOT NULL

3 participants