Uh oh!
There was an error while loading. Please reload this page.
fix(soft-delete): support delegate and mixin models - #2726
Conversation
The soft-delete plugin previously broke on models whose `@deletedAt` marker is inherited rather than declared directly on the queried model: - **Mixin models** (`@deletedAt` from a `type` mixin) now soft-delete correctly — the marker is flattened into the model's fields. - **Delegate (polymorphic) models** marked on the base now filter reads and rewrite deletes correctly. The marker physically lives on the base table, so `getDeletedAtField` skips inherited (`originModel`) markers, and the read filter walks the base chain to key the `IS NULL` predicate off the base table in the outer WHERE. - `transformJoin` skips any delegate-hierarchy member: those markers are always handled by `transformSelectQuery` (reconstruction joins via the outer WHERE, relation reads via their subquery FROM). Adding an ON-clause predicate on a LEFT reconstruction join would only null the joined columns and leak the soft-deleted row. Adds runtime tests for the mixin case, the delegate case (delete through both the concrete and base accessors), and a regression test ensuring updates can't touch an already soft-deleted delegate row. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe ChangesSoft-delete delegate hierarchy fix
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Uh oh!
There was an error while loading. Please reload this page.
Summary
The soft-delete plugin previously broke on models whose
@deletedAtmarker is inherited rather than declared directly on the queried model — specifically mixin-composed models and delegate (polymorphic) models. This PR makes both work at runtime.Mixin models
A
@deletedAtdeclared on atypemixin is flattened into the consuming model's fields, so it already resolves correctly — added a runtime test to lock that in.Delegate (polymorphic) models
The marker physically lives on the base table, but it surfaces on every concrete sub-model's field list (tagged with
originModel). Two problems followed:Article.deletedAt IS NULL, but that column only exists on the baseContent. Fixed by havinggetDeletedAtFieldskip inherited (originModel) markers.FROM Article LEFT JOIN Content. Putting the filter in the join'sONclause on a LEFT join only nulls the joined columns instead of excluding the row. Fixed by walking the base chain in the read filter and keying theIS NULLpredicate off the base table in the outerWHERE.transformJoinnow skips any delegate-hierarchy member: those markers are always handled bytransformSelectQuery(reconstruction joins via the outer WHERE, relation reads via their subquery's own FROM). Adding an ON-clause predicate for a hierarchy member is at best redundant and, on a LEFT reconstruction join, leaks the soft-deleted row.Deletes already work once reads are fixed: the ORM rewrites a concrete delegate delete into a delete on the base table, which the plugin converts to an
UPDATE <base> SET deletedAt.Tests
soft-deletes a model whose @deletedAt is inherited from a mixinsoft-deletes a delegate model marked on the base(delete through both the concrete and base accessors)does not update an already soft-deleted delegate rowAll soft-delete tests pass (18 passing, 2 pre-existing dialect-specific skips), lint and typecheck clean.
Known limitation (out of scope)
For a polymorphic hierarchy, the
@deletedAtmarker should live on the root@@delegatebase. A marker on a mid-level model is bypassed by deletes (the ORM cascades the delete up to the root, which has no marker, causing a hard delete). Worth a separate validation/doc follow-up.🤖 Generated with Claude Code
Summary by CodeRabbit
@deletedAtmarkers are inherited from parent models or type mixins.