MemberJunction / MemberJunction/MJ

MJ: Files has no server-side delete semantics — attachments "Delete Completely" half-fails on FK dependents and orphans storage bytes on success

Open
#4,046 0 comments 0 reactions 1 assignee Claimed by @SDesai-BC View on GitHub
Dominant language
TSQL
Stars
29
Forks
6
Avg merge
2d 1h
Merged PRs (30d)
323

Description

## Summary

`MJ: Files` has no server-side delete semantics. The record-attachments panel's **"Delete Completely"** action orchestrates a two-step destructive delete **client-side** (link row, then file row) with no transaction and no confirmation, which produces two distinct failures:

1. **Half-failed delete when the file has FK dependents.** The link is deleted first; the `MJ: Files` row delete is then refused by any FK pointing at it (e.g. an app table with a `FileID` column). Net state: the attachment silently disappears from the panel, while the file row — and every other reference to it — survives. The user sees only a generic "Failed to delete attachment" toast, *after* the panel row is already gone.
2. **Orphaned storage bytes when the delete succeeds.** No delete path removes the object from the storage provider. A fully successful "Delete Completely" leaves the bytes in Box/S3/Azure forever.

## Where the code stands today

- `packages/Angular/Generic/file-storage/src/lib/record-attachments/record-attachments.component.ts` — `DeleteAttachment()` (~line 1007): deletes the `MJ: File Entity Record Links` row, **then** the `MJ: Files` row, sequentially, no transaction. The "Delete Completely" menu item invokes it directly with `hardDelete = true` (~line 231 of the template) — no confirmation dialog.
- `spDeleteFile` is a bare `DELETE FROM [File] WHERE ID = @ID` (v2 baseline, `V202407171600__v2.0.x.sql:12812`). No dependency check, no link cleanup, no storage cleanup.
- **There is no `RegisterClass(BaseEntity, 'MJ: Files')` server subclass anywhere** (MJCoreEntitiesServer / MJServer / MJStorage). Files is the only heavily-shared core object without one.
- `FileResolver.DeleteStorageObject` deletes bytes only (by AccountID + ObjectName) and is not called by any row-delete path; `files-grid`'s `deleteFile()` calls `file.Delete()` only. Rows and bytes are never deleted together by anything.

## Why this matters now

Apps are starting to point real FKs at `MJ: Files` (bizapps-forms `FormUpload.FileID`, bizapps-ats `Applicant.ResumeFileID`) and to write `FileEntityRecordLink` rows so the attachments panel shows uploaded files on multiple records. Every such reference makes failure mode 1 reachable from a single un-confirmed click.

## Proposed fix — apply the pattern MJ already uses twice

Precedents in `packages/MJCoreEntitiesServer/src/custom/`:
- `MJTagEntityServer` — same shape as attachments (shared object + EntityID/RecordID soft-link rows + slide-in panel): its subclass "cleans up FK references … **before delete so delete doesn't fail on FK constraints**."
- `MJListEntityServer.Delete()` — contextual authorization inside the subclass, returning a structured `BaseEntityResult` failure message.

**Add `MJFileEntityServer`** (`@RegisterClass(BaseEntity, 'MJ: Files')`), overriding `Delete()`:

1. **Hard-FK dependents → refuse** with a structured message naming them. Discover via `GetEntityDependencies('MJ: Files')` (every entity field with `RelatedEntity = 'MJ: Files'`) — no hardcoded list, future apps covered automatically.
2. **`FileEntityRecordLink` rows → cascade-delete** (Tag pattern — soft links are the file's own projection metadata).
3. **Delete the storage bytes** via `FileStorageEngine` driver (`ProviderID`/`ProviderKey` are on the row), then `super.Delete()`.

Rationale for the 1/2 split: soft links are the file's own metadata (cascade); hard FKs are other apps' data (refuse and name).

Because ClassFactory substitution is global server-side, this fixes the attachments panel, `files-grid`, raw GraphQL deletes, and every future caller at once. The panel's `DeleteAttachment` then collapses to a single `file.Delete()` call (drop its client-side link delete), making the ordering bug structurally impossible.

## Secondary items (same area, smaller)

- Add a confirmation dialog to "Delete Completely" — currently one un-confirmed click.
- Pass `Configuration.Attachments` through `record-form-container`'s `` mount, and add `AllowDelete` / `AllowUnlink` / `AllowUpload` to `IEntityAttachmentsConfiguration` — the panel's `@Input()`s exist but nothing binds them from the generated forms.

## Possible phase 2 (not required for this issue)

`DeleteType='Soft'` on `MJ: Files` + a background sweep that physically removes storage objects for soft-deleted, reference-free files.

## Acceptance

- Deleting a File with any hard-FK dependent fails **atomically** with a message naming the dependents; no link rows are lost.
- Deleting an unreferenced File removes link rows, the storage object, and the row.
- "Delete Completely" in the panel is one server round-trip and asks for confirmation first.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.