fix(media): confirm before permanently deleting an asset - #497
Open
mostafasadeghidev wants to merge 1 commit into
Open
fix(media): confirm before permanently deleting an asset#497mostafasadeghidev wants to merge 1 commit into
mostafasadeghidev wants to merge 1 commit into
Conversation
Purging is the only media action with no undo. It removes the original binary AND every generated size from the storage adapter — `handleMediaItem` sweeps `existing.variants` alongside the original — and it fired on a single unguarded click. The button sits in the trash preview beside Restore, same row, same size, one colour apart, so the two read as a pair of equally reversible choices. The same applies to the trash view's right-click Delete. Both paths now confirm, naming the file and saying what goes with it. Two details worth flagging for review: `alwaysConfirm` on the context-menu path. The `confirmBeforeDelete` preference defaults off, and someone who turned it off was opting out of confirming a TRASH — a reversible move — not a purge. Without the flag this change would be a no-op for exactly the operators most likely to hit it. A local `Dialog` rather than `useConfirmDelete` in the viewer window. That hook falls back to running `commit()` immediately when no provider is mounted (confirmDeleteHook.ts:53-56), and this window also renders from the dashboard media widget, which mounts none. Routing it through the hook would have left one surface unguarded while reading as covered on all five. Also worth a maintainer's eye: `capabilityMeta.ts:87-90` tells operators that hard-purge "also requires step-up", but the route only calls `requireCapability(req, db, 'media.delete')` — no `requireStepUp`, unlike `data/tables.ts:169`. Left alone here because a server-side gate is a behaviour change that deserves its own PR, but the docs and the code currently disagree. Four tests pin the behaviour: no purge on first click, the dialog names the asset, Cancel is inert, and the second click is what commits. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mostafasadeghidev
added a commit
to mostafasadeghidev/Instatic
that referenced
this pull request
Sep 6, 2026
…at had no gate The fork gate stopped at CoreBunch#359. Everything merged onto the stack since — CoreBunch#497 through CoreBunch#501, and now CoreBunch#505 — was carried with nothing to catch it being dropped by a merge resolution, which is the exact failure the gate exists for. Pinning them turned one up immediately: CoreBunch#498's guard around `removePluginVersionAssets(…, fromVersion)` is gone, because CoreBunch#359 removed that call outright — an upgrade now leaves the old version's assets on disk so published pages keep resolving. What survives of CoreBunch#498 is the rollback guard, so that is what the row pins. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purging is the only media action with no undo. It removes the original binary and every generated size from the storage adapter —
handleMediaItemsweepsexisting.variantsalongside the original — and it fired on a single unguarded click.The button sits in the trash preview beside Restore: same row, same size, one colour apart, so the two read as a pair of equally reversible choices. The trash view's right-click Delete had the same problem.
Both paths now confirm, naming the file and saying what goes with it.
Two details worth a look
alwaysConfirmon the context-menu path. TheconfirmBeforeDeletepreference defaults off, and someone who turned it off was opting out of confirming a trash — a reversible move — not a purge. Without the flag this change would be a no-op for exactly the operators most likely to hit it.A local
Dialograther thanuseConfirmDeletein the viewer window. That hook falls back to runningcommit()immediately when no provider is mounted (confirmDeleteHook.ts:53-56), and this window also renders from the dashboard media widget, which mounts none. Routing it through the hook would have left one surface unguarded while reading as covered on all five.Separately, docs and code disagree
capabilityMeta.ts:87-90tells operators that hard-purge "also requires step-up", but the route only callsrequireCapability(req, db, 'media.delete')— norequireStepUp, unlikedata/tables.ts:169. Left alone here because a server-side gate is a behaviour change deserving its own PR, but you may want to decide which side is wrong.Tests
Four, pinning behaviour rather than wording: no purge on the first click, the dialog names the asset, Cancel is inert, and the second click is what commits.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com