MemberJunction / MemberJunction/MJ
IS-A: soft-delete parents and record merge are unhandled (undefined semantics, data corruption risk)
- Dominant language
- TSQL
- Stars
- 29
- Forks
- 6
- Avg merge
- 2d 1h
- Merged PRs (30d)
- 323
Description
## Context
bizapps-common PR [#35](https://github.com/MemberJunction/bizapps-common/pull/35) flips `DeleteType='Soft'` and `AllowRecordMerge=1` on the BAC **Person**/**Organization** entities, which are IS-A parents: `SalesContact`/`SalesAccount` today, BCSaaS `BC: People` (bizapps-common [#36](https://github.com/MemberJunction/bizapps-common/issues/36) decoupling), and AIDP-next `crm.Contact`/`crm.Account` next. Reviewing that PR against MJ's IS-A implementation surfaced three gaps in MJ core. Until they're resolved, soft delete + record merge on IS-A parents produce silently-wrong data.
## Gap 1 — Soft-delete × IS-A has undefined semantics
`isa-relationships.md` has no mention of `DeleteType='Soft'` anywhere in the chain. Observable behavior when a parent is Soft and children are Hard:
- **Last-child cascade**: the overlapping-subtype delete safety (child delete → sibling check → parent cascade) will *stamp* the parent's `__mj_DeletedAt` instead of removing the row. Mechanically it works, but it's untested and undocumented.
- **Zombie children**: nothing prevents (or defines) hard child rows continuing to exist under a soft-deleted parent — e.g. a parent soft-deleted directly (not via child cascade) leaves all subtype rows live. Combined with Gap 3 below, those children remain fully visible/queryable through their base views while the parent has vanished from its own views.
- **Chain semantics**: no defined behavior for Load/Save through a chain containing a soft-deleted ancestor (InnerLoad joins the parent *table*, so loads succeed against a "deleted" parent).
## Gap 2 — `MergeRecords` is not IS-A aware
`DatabaseProviderBase.MergeRecords` (databaseProviderBase.ts:2084) does: remap FK dependencies → `Delete()` the losing record.
- **IS-A children are not remappable dependencies**: a child's PK *is* the parent ID. If `GetRecordDependencies` surfaces the child's PK-FK, the remap would attempt `child.Set('ID', survivorID)` — a primary-key rewrite; if it doesn't, the child is silently ignored.
- **With a Soft parent, the loser's `Delete()` succeeds as an UPDATE** even though subtype rows still exist → the losing record's children survive as live rows under a hidden parent.
- **Unique constraints on children make naive remap impossible**: e.g. BCSaaS `__BCSaaS.Person.UserID` is unique (one person ↔ one MJ User). Merging two Persons that both carry a `BC: People` row cannot produce a single valid survivor without domain logic.
- Concrete failure (the CDP dedup use case that motivated bizapps-common #35): merge Person A into Person B where A is bound to an MJ User. A's `BC: People` row survives under soft-deleted A; A's user still logs in and resolves person A (Gap 3); B never gains the binding. The "merged" duplicate remains a live platform identity.
Suggested direction: `MergeRecords` should either (a) refuse to merge records that carry IS-A subtype rows until handled, or (b) gain explicit IS-A handling (per-subtype survivor resolution, honoring unique constraints, deleting/re-parenting loser child rows inside the merge transaction).
## Gap 3 — CodeGen: child base views ignore ancestor soft-delete (analysis)
Hypothesis checked: *"CodeGen joins a dependency entity's view (not table) when it has calculated virtual fields, so `__mj_DeletedAt` filtering should flow through."* **Refuted** — verified in CodeGenLib `sql_codegen.ts`:
- `generateParentEntityJoins` (~:1641) always joins each IS-A parent's **base table**, by design: *"Each parent is joined via its base table (not view) to avoid view dependency ordering issues."* There is no virtual-field-triggered switch to the parent's view.
- `generateParentEntityFieldSelects` (~:1616) **skips** parent virtual fields (`field.IsVirtual → continue`) and all `__mj_*` fields — virtual parent fields are omitted from the child view, not resolved via a view join.
- The soft-delete filter is only emitted for an entity's **own** `DeleteType` (`SQLServerCodeGenProvider.generateBaseView` ~:127: `entity.DeleteType === 'Soft' → WHERE alias.__mj_DeletedAt IS NULL`). A Hard child of a Soft parent gets no ancestor filter.
Net effect: child base views (e.g. `__BCSaaS.vwPeople`, `vwSalesContacts`) fully expose children of soft-deleted parents.
Suggested fix — keep the table joins (the dependency-ordering rationale is sound) and instead have `generateBaseView` append `AND __mj_isa_pN.__mj_DeletedAt IS NULL` predicates for each ancestor in `ParentChain` with `DeleteType='Soft'`. That inherits the parent's filter without any view-on-view dependency. Whether a child view *should* hide such rows (vs. expose them for admin/merge tooling) is part of the Gap 1 semantics decision.
## Impact / priority
Blocks merging bizapps-common #35 as-is (it's being held until after the next BizAppsCommon build). BCSaaS ships `BC: People` (IS-A child of Person, carries the MJ User binding) in v1.8 — with #35 applied, soft-deleted/merged-away people would retain working logins.
Related, pre-existing IS-A ask: attach-subtype-to-existing-parent API (BCSaaS plans/PERSON_USER_DECOUPLING.md §4.2 G1).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Contributor guide
Assessment
This issue has not been assessed yet.