MemberJunction / MemberJunction/MJ
BaseEntity has no delete-validation seam — every app hand-rolls Delete() overrides to explain a refusal
- Dominant language
- TSQL
- Stars
- 29
- Forks
- 6
- Avg merge
- 2d 1h
- Merged PRs (30d)
- 323
Description
## Summary
`BaseEntity` has a save-validation seam — `Validate()` and `ValidateAsync()` — with a well-defined contract: return a `ValidationResult`, let the framework decide, get field-named errors in front of the user. **There is no equivalent for delete.** No `ValidateDelete()`, no `CanDelete()`, no `BeforeDelete` event that can veto.
So the only way to refuse a delete with an explanation is to override `Delete()` itself (`baseEntity.ts:4267`), check by hand, and return `false` before calling `super.Delete()`.
## Why that is worse than it sounds
**1. The message is lost.** `Validate()` returns `ValidationResult` with `ValidationErrorInfo[]`, each carrying a `Source` — which is how MJ's forms mark the offending *field*. A `Delete()` override returns `boolean`. There is nowhere to put "this template is referenced by 5 signed contracts"; the caller gets `false` and has to invent its own explanation, or the user gets whatever the raw FK error says.
**2. Every app reimplements it, differently.** In `bizapps-accounting` alone:
- `JournalEntryTypeEntityServer.Delete()`
- `JournalEntryLineEntityServer.Delete()`
…each with its own probe, its own logging, its own way of surfacing the reason. `bizapps-contracts` is about to add four more for the same reason (a provision referenced by modifications, a template referenced by signed contracts, a contract with lineage children, a type in use). That is six hand-rolled versions of a hook that should exist once.
**3. The guarantees `Validate()` gets, delete does not.** A save runs validation inside the framework's flow, so every caller — form, `SaveEntityGraph`, an agent, a Remote Operation — is covered by one rule. A `Delete()` override only covers callers that go through that subclass's method, and there is no framework guarantee about ordering relative to permissions, Record Changes, or soft-delete handling.
**4. Referential refusal is the common case, and it is always ugly.** Where FKs are `NO_ACTION` (the correct default), the database already refuses the delete — correctly. What is missing is the layer that turns `The DELETE statement conflicted with the REFERENCE constraint "FK_..."` into a sentence a user can act on. Today every app writes that layer by hand or ships the raw error.
## Proposed
A delete-side counterpart to the save seam, symmetrical with what already exists:
```typescript
// Called by BaseEntity.Delete() before any provider work, exactly as Validate()
// is called by Save(). A non-Success result aborts the delete.
public ValidateDelete(): ValidationResult
public async ValidateDeleteAsync(): Promise
```
Ideally with the same conventions the save side already has: `ValidationErrorInfo.Source` naming the relationship or field that blocks it, `ValidationErrorType` so a warning can be distinguished from a refusal, and the async variant opt-in the way `ValidateAsync` is (only run when a subclass overrides it, per the `DefaultSkipAsyncValidation` note in `baseEntity.ts`).
Two smaller alternatives, if a new virtual is unwelcome:
- **A vetoable event.** `Delete()` already raises events; making a `before_delete` event cancellable with a reason would cover most of this without new API surface.
- **Metadata-driven refusal.** MJ knows every FK to a row (`EntityRelationship`). A generic "this row is referenced by N `` records" message, derived from metadata and emitted by `Delete()` when the provider reports an FK violation, would fix the *common* case with no per-app code at all — and would arguably be the higher-value change, since it needs no subclass.
Happy to be told a hook exists and we missed it — we looked for `ValidateDelete`, `CanDelete` and `BeforeDelete` in `packages/MJCore/src/generic/baseEntity.ts` and found none.
## Environment
MJ `6.1.0-edge.2`. Observed while auditing validation coverage in `bizapps-contracts`; the accounting overrides above are the existing prior art.
Contributor guide
Research direction
Read packages/MJCore/src/generic/baseEntity.ts around Delete() at line 4267, then compare the existing Validate()/ValidateAsync() seam. Review the Delete() overrides named in bizapps-accounting and the proposed alternatives; done requires a decided delete-validation approach with its caller coverage and user-facing refusal behavior defined.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- backend-api-design
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100