diff --git a/server/db/migrations-pg.ts b/server/db/migrations-pg.ts index 7e3130d06..d6efc2aba 100644 --- a/server/db/migrations-pg.ts +++ b/server/db/migrations-pg.ts @@ -1190,4 +1190,34 @@ export const pgMigrations: Migration[] = [ on plugin_media_sources (asset_id); `, }, + { + // Existing avatars predate anything writing `media_usage_refs`, so the + // first build that warns before a delete would still have said nothing + // about the avatar already set — the one case the feature exists for. + // + // Idempotent by construction: `not exists` on the same key + // `setMediaUsageRef` writes, so re-running inserts nothing and the row a + // later avatar change moves is the row this created. + // + // `028`, skipping `027`, on purpose: #335 is in review and already claims + // `027_data_tables_created_by_plugin`. Ids are only ever sorted, so a gap + // costs nothing — and whichever of the two lands first, neither has to be + // renumbered. A migration an installation has already recorded can never + // be renamed: the runner keys on the full id, so a new one re-runs SQL + // that is not idempotent and fails the boot. + id: '028_backfill_avatar_usage_refs', + sql: ` + insert into media_usage_refs (asset_id, ref_kind, ref_id, ref_path) + select u.avatar_media_id, 'user.avatar', u.id, '' + from users u + where u.avatar_media_id is not null + and not exists ( + select 1 from media_usage_refs r + where r.asset_id = u.avatar_media_id + and r.ref_kind = 'user.avatar' + and r.ref_id = u.id + and r.ref_path = '' + ); + `, + }, ] diff --git a/server/db/migrations-sqlite.ts b/server/db/migrations-sqlite.ts index db3deb93a..958e24bb5 100644 --- a/server/db/migrations-sqlite.ts +++ b/server/db/migrations-sqlite.ts @@ -1266,4 +1266,34 @@ export const sqliteMigrations: Migration[] = [ on plugin_media_sources (asset_id); `, }, + { + // Existing avatars predate anything writing `media_usage_refs`, so the + // first build that warns before a delete would still have said nothing + // about the avatar already set — the one case the feature exists for. + // + // Idempotent by construction: `not exists` on the same key + // `setMediaUsageRef` writes, so re-running inserts nothing and the row a + // later avatar change moves is the row this created. + // + // `028`, skipping `027`, on purpose: #335 is in review and already claims + // `027_data_tables_created_by_plugin`. Ids are only ever sorted, so a gap + // costs nothing — and whichever of the two lands first, neither has to be + // renumbered. A migration an installation has already recorded can never + // be renamed: the runner keys on the full id, so a new one re-runs SQL + // that is not idempotent and fails the boot. + id: '028_backfill_avatar_usage_refs', + sql: ` + insert into media_usage_refs (asset_id, ref_kind, ref_id, ref_path) + select u.avatar_media_id, 'user.avatar', u.id, '' + from users u + where u.avatar_media_id is not null + and not exists ( + select 1 from media_usage_refs r + where r.asset_id = u.avatar_media_id + and r.ref_kind = 'user.avatar' + and r.ref_id = u.id + and r.ref_path = '' + ); + `, + }, ] diff --git a/server/handlers/cms/me.ts b/server/handlers/cms/me.ts index 46eb9feb2..b40ef4a36 100644 --- a/server/handlers/cms/me.ts +++ b/server/handlers/cms/me.ts @@ -54,6 +54,7 @@ import { acceptUploadedMedia, readUploadForm, } from './mediaUpload' +import { setMediaUsageRef } from '../../repositories/media' import { Type } from '@core/utils/typeboxHelpers' import { isValidEmail } from '@core/utils/email' import { MIN_PASSWORD_LENGTH, PASSWORD_TOO_SHORT_MESSAGE } from '@core/utils/passwordPolicy' @@ -328,6 +329,15 @@ export async function handleMeRoutes( if (asset instanceof Response) return asset const updated = await setUserAvatarMediaId(db, user.id, asset.id) + // Register the dependency so the media library stops treating a profile + // picture as an anonymous upload. Without it the asset is indistinguishable + // from a decorative one, and purging it nulls `avatar_media_id` through + // the column's `on delete set null` — silently, from the operator's side. + await setMediaUsageRef(db, { + assetId: asset.id, + refKind: 'user.avatar', + refId: user.id, + }) if (!updated) { // The user row vanished between auth and the update (e.g. concurrent // soft-delete). The uploaded asset stays in the media library — it's @@ -351,6 +361,10 @@ export async function handleMeRoutes( const updated = await setUserAvatarMediaId(db, user.id, null) if (!updated) return jsonResponse({ error: 'User not found' }, { status: 404 }) + // The asset stays in the library on purpose (see the file header), but it + // is no longer depended on — so the warning has to stop firing for it. + await setMediaUsageRef(db, { assetId: null, refKind: 'user.avatar', refId: user.id }) + await createAuditEvent(db, { actorUserId: user.id, action: 'user.update', diff --git a/server/handlers/cms/media.ts b/server/handlers/cms/media.ts index 00c122be2..51d3bc7ef 100644 --- a/server/handlers/cms/media.ts +++ b/server/handlers/cms/media.ts @@ -14,6 +14,10 @@ * permitted on already-trashed * assets) and removes the file * (`media.delete`) + * POST /admin/api/cms/media/usage — which of these assets are still + * depended on, and by what: + * recorded settings refs plus + * page content, computed live * POST /admin/api/cms/media/:id/restore — restore a soft-deleted asset * (`media.write`) * POST /admin/api/cms/media/:id/replace — overwrite the bytes for an asset @@ -38,11 +42,13 @@ */ import type { DbClient } from '../../db/client' import { requireCapability } from '../../auth/authz' +import { collectContentUsageRefs } from '../../media/contentUsage' import { assignAssetToFolders, deleteMediaAsset, getMediaAsset, listMediaAssets, + listMediaUsageRefs, restoreMediaAsset, softDeleteMediaAsset, updateMediaAssetMetadata, @@ -293,7 +299,41 @@ async function handleDeleteMedia( const ID_PATTERN = '(?[^/]+)' +/** + * Which of the given assets something still depends on. + * + * A POST because the id list is a selection and can be long — a query string + * of a hundred ids is the wrong shape for a read this cheap. + * + * The UI calls this before a destructive action so it can name what is about + * to break instead of warning in the abstract. Requires `media.read`: the + * response reveals nothing beyond what the library already lists. + */ +async function handleMediaUsage(req: Request, db: DbClient): Promise { + const user = await requireCapability(req, db, 'media.read') + if (user instanceof Response) return user + + const body = await readValidatedBody(req, MediaUsageQuerySchema) + if (!body) return badRequest('Invalid request body') + + // Two sources, one answer. Settings (an avatar, later a favicon) are + // recorded in `media_usage_refs` because they have one writer and an + // explicit set/unset. Page content is COMPUTED, because it has neither — + // see `server/media/contentUsage.ts` for why a table would go wrong there. + // Both run in parallel; the caller cannot tell which side a ref came from. + const [stored, content] = await Promise.all([ + listMediaUsageRefs(db, body.assetIds), + collectContentUsageRefs(db, body.assetIds), + ]) + return jsonResponse({ usage: [...stored, ...content] }) +} + +const MediaUsageQuerySchema = Type.Object({ + assetIds: Type.Array(Type.String(), { maxItems: 500 }), +}, { additionalProperties: false }) + const MEDIA_ROUTES: readonly Route<[]>[] = [ + { method: 'POST', pattern: `${MEDIA_PREFIX}/usage`, handler: handleMediaUsage }, { method: 'GET', pattern: MEDIA_PREFIX, handler: handleListMedia }, { method: 'POST', pattern: MEDIA_PREFIX, handler: handleUploadMedia }, { diff --git a/server/media/contentUsage.ts b/server/media/contentUsage.ts new file mode 100644 index 000000000..b97b585e3 --- /dev/null +++ b/server/media/contentUsage.ts @@ -0,0 +1,126 @@ +/** + * Which pages use these files — worked out from the site itself, not from a + * stored index. + * + * The counterpart to `media_usage_refs`, and the split between them is the + * point: a stored reference suits a SETTING, which has one writer and an + * explicit set/unset — an avatar, a favicon, a logo. Page content has + * neither. It is written continuously by the collab relay, and removing an + * image produces no event at all, so a table would fill with references to + * nodes that no longer exist and the warning would start being wrong. + * + * A wrong warning is worse than none: an operator who is misled once stops + * reading it. So this computes the answer at the moment it is asked, from + * `getDraftSiteDocument` — which cannot drift, because there is nothing to + * keep in sync. + * + * The cost lands where it belongs. Walking every page tree is O(site), and it + * happens only when someone asks to permanently delete something — never on a + * page load, never on a trash. For the site sizes this product is built for, + * that is a few milliseconds on an action that is about to be irreversible. + * If a site ever grows past that, the fix is to cache this — with the walk + * still the source of truth, so the cache can be checked against it. + * + * Deliberately reads the DRAFT document, not the published artefacts: an + * image placed on an unpublished page is still in use, and a warning that + * only knew about live pages would let a delete quietly break the next + * publish. + */ + +// Registry population. The walk asks the registry which props are +// image/media-typed, so without the base modules registered it matches +// nothing and reports NO usage — a warning that is silently always empty, +// which is the one failure mode worse than not having it. Same import +// `pageDiff.ts` and the collab relay make, and for the same reason. +import '@modules/base' +import { registry } from '@core/module-engine' +import { collectSiteStyleBackgroundImagePaths } from '@core/publisher' +import { placeholder, type DbClient } from '../db/client' +import { getDraftSiteDocument } from '../repositories/publish' +import { collectPageMediaPaths } from '../publish/mediaPrefetch' +import type { MediaUsageRef } from '../repositories/media' + +/** + * `ref_kind` values this module produces. They share the namespace with the + * stored kinds (`user.avatar`), so a caller merges the two lists without + * caring which side each one came from. + */ +export const PAGE_CONTENT_REF_KIND = 'page.content' +export const SITE_STYLES_REF_KIND = 'site.styles' + +/** + * Map the requested asset ids to the `public_path` each one is stored under. + * + * Content props hold the path, not the id — and `replaceMediaAssetBinary` + * keeps the path stable across a file swap precisely so page references + * survive it. The path is therefore the join key, and this is the one query + * that translates. + */ +async function pathsForAssetIds( + db: DbClient, + assetIds: string[], +): Promise> { + const placeholders = assetIds.map((_, i) => placeholder(db.dialect, i + 1)).join(', ') + const { rows } = await db.unsafe<{ id: string; public_path: string }>( + `select id, public_path from media_assets where id in (${placeholders})`, + assetIds, + ) + const byPath = new Map() + for (const row of rows) byPath.set(row.public_path, row.id) + return byPath +} + +/** + * Which of `assetIds` the site's own content references, and where. + * + * One ref per (asset, page) — a file used by four nodes on one page is one + * page to fix, and repeating its title four times would turn the warning into + * the wall of text it exists to avoid. + */ +export async function collectContentUsageRefs( + db: DbClient, + assetIds: string[], +): Promise { + if (assetIds.length === 0) return [] + + const assetIdByPath = await pathsForAssetIds(db, assetIds) + if (assetIdByPath.size === 0) return [] + + const site = await getDraftSiteDocument(db) + if (!site) return [] + + const refs: MediaUsageRef[] = [] + + for (const page of site.pages) { + // `collectPageMediaPaths` descends into the definition tree of every + // Visual Component the page references, so an image inside a VC body is + // attributed to the page that renders it — which is the page that would + // break, and so the one worth naming. + const used = collectPageMediaPaths(page, site, registry) + for (const path of used) { + const assetId = assetIdByPath.get(path) + if (!assetId) continue + refs.push({ + assetId, + refKind: PAGE_CONTENT_REF_KIND, + refId: page.id, + label: page.title || page.slug, + }) + } + } + + // Site-level style backgrounds belong to no single page — every page that + // matches the rule renders them, so naming one page would be misleading. + for (const path of collectSiteStyleBackgroundImagePaths(site)) { + const assetId = assetIdByPath.get(path) + if (!assetId) continue + refs.push({ + assetId, + refKind: SITE_STYLES_REF_KIND, + refId: 'site', + label: 'site styles', + }) + } + + return refs +} diff --git a/server/publish/mediaPrefetch.ts b/server/publish/mediaPrefetch.ts index 660fd1ad3..28df49a39 100644 --- a/server/publish/mediaPrefetch.ts +++ b/server/publish/mediaPrefetch.ts @@ -45,9 +45,18 @@ interface MediaPrefetchOptions { /** * Collect every `/uploads/...` path referenced by an image/media-typed prop - * across the page tree. + * in ONE page's tree. + * + * Site-level style backgrounds are deliberately NOT included: they belong to + * the site, not to any page that happens to be rendering. The prefetch adds + * them separately, and the media-usage lookup attributes them to the site so + * a delete warning can say where a file is actually used. */ -function collectMediaPaths(page: Page, site: SiteDocument, registry: IModuleRegistry): Set { +export function collectPageMediaPaths( + page: Page, + site: SiteDocument, + registry: IModuleRegistry, +): Set { const paths = new Set() // Descend into referenced VC definition trees so an image/media prop inside a // VC body is resolved too (ISS-022). @@ -64,6 +73,16 @@ function collectMediaPaths(page: Page, site: SiteDocument, registry: IModuleRegi paths.add(value) } }) + return paths +} + +/** The page's own references plus the site-level style backgrounds. */ +function collectMediaPaths( + page: Page, + site: SiteDocument, + registry: IModuleRegistry, +): Set { + const paths = collectPageMediaPaths(page, site, registry) for (const path of collectSiteStyleBackgroundImagePaths(site)) { paths.add(path) } diff --git a/server/repositories/media.ts b/server/repositories/media.ts index 5878f6702..a50477e39 100644 --- a/server/repositories/media.ts +++ b/server/repositories/media.ts @@ -542,3 +542,86 @@ export async function importMediaAsset( externally_hosted = excluded.externally_hosted ` } + +// --------------------------------------------------------------------------- +// Usage references +// +// `media_usage_refs` has existed since the media schema landed but nothing +// wrote to it, so the library could not tell a decorative upload from an +// asset something depends on. That gap is how a profile picture — stored as +// an ordinary library row, with no marker distinguishing it — could be swept +// into the trash during a tidy-up and purged, silently nulling +// `users.avatar_media_id` through its `on delete set null` foreign key. +// +// `ref_kind` namespaces the source so more can be registered without touching +// consumers: `user.avatar` here, page nodes and site settings next. +// --------------------------------------------------------------------------- + +/** A thing that depends on an asset, resolved for display. */ +export interface MediaUsageRef { + assetId: string + refKind: string + refId: string + /** Human-readable, e.g. a person's name for an avatar. Never a raw id. */ + label: string +} + +/** + * Point a `(kind, id)` pair at an asset, replacing whatever it pointed at + * before. + * + * Deleting first is what makes this a MOVE rather than an accumulation: a + * user who changes their avatar four times should leave one row, not four, + * or the fifth deletion would warn about pictures they replaced months ago. + */ +export async function setMediaUsageRef( + db: DbClient, + args: { assetId: string | null; refKind: string; refId: string; refPath?: string }, +): Promise { + const refPath = args.refPath ?? '' + await db` + delete from media_usage_refs + where ref_kind = ${args.refKind} and ref_id = ${args.refId} and ref_path = ${refPath} + ` + if (!args.assetId) return + await db` + insert into media_usage_refs (asset_id, ref_kind, ref_id, ref_path) + values (${args.assetId}, ${args.refKind}, ${args.refId}, ${refPath}) + ` +} + +/** + * Which of these assets are still depended on, with something an operator can + * recognise. + * + * Takes a LIST because the question is always asked about a selection — the + * deletion path needs one round trip, not one per file. + */ +export async function listMediaUsageRefs( + db: DbClient, + assetIds: string[], +): Promise { + if (assetIds.length === 0) return [] + const placeholders = assetIds.map((_, i) => placeholder(db.dialect, i + 1)).join(", ") + const { rows } = await db.unsafe<{ + asset_id: string + ref_kind: string + ref_id: string + label: string | null + }>( + `select r.asset_id, r.ref_kind, r.ref_id, + case when r.ref_kind = 'user.avatar' + then coalesce(nullif(u.display_name, ''), u.email) + else null end as label + from media_usage_refs r + left join users u on u.id = r.ref_id and r.ref_kind = 'user.avatar' + where r.asset_id in (${placeholders})`, + assetIds, + ) + return rows.map((row) => ({ + assetId: row.asset_id, + refKind: row.ref_kind, + refId: row.ref_id, + label: row.label ?? row.ref_kind, + })) +} diff --git a/src/__tests__/media/purgeConfirmation.test.tsx b/src/__tests__/media/purgeConfirmation.test.tsx new file mode 100644 index 000000000..5c3bd7afd --- /dev/null +++ b/src/__tests__/media/purgeConfirmation.test.tsx @@ -0,0 +1,130 @@ +/** + * Permanent deletion of a media asset has to ask first. + * + * "Delete permanently" removes the original binary AND every generated size + * from the storage adapter — not just the row. It is the only media action + * with no undo, and it sat one unguarded click away inside the trash preview, + * beside the Restore button it visually matches. + * + * These tests pin the confirmation itself rather than the wording: that the + * mutation does NOT fire on the first click, that Cancel leaves the asset + * alone, and that the second click is what commits. + * + * The dialog is local to the window on purpose. `useConfirmDelete` falls back + * to running `commit()` immediately when no `ConfirmDeleteProvider` is mounted + * (confirmDeleteHook.ts), and this window also renders from the dashboard + * media widget, which mounts none — so routing through that hook would have + * left one surface silently unguarded while looking covered everywhere. + */ + +import { afterEach, describe, expect, it } from 'bun:test' +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { MediaViewerWindow } from '@admin/pages/media/components/MediaViewerWindow/MediaViewerWindow' +import { AdminSessionContext } from '@admin/sessionContext' + +afterEach(cleanup) + +/** A trashed asset — the only state in which the purge button renders. */ +function trashedAsset() { + return { + id: 'asset_1', + filename: 'logo.png', + mimeType: 'image/png', + sizeBytes: 1200, + publicPath: '/uploads/logo.png', + uploadedByUserId: null, + createdAt: '2026-01-01T00:00:00.000Z', + altText: '', + caption: '', + title: '', + tags: [], + width: null, + height: null, + durationMs: null, + dominantColor: null, + deletedAt: '2026-02-01T00:00:00.000Z', + replacedAt: null, + folderIds: [], + blurHash: null, + variants: [], + posterPath: null, + } +} + +function renderViewer() { + const purged: string[] = [] + const editor = { + asset: trashedAsset(), + tagPalette: [], + folderById: new Map(), + updateAsset: async () => undefined, + renameAsset: async () => undefined, + replaceAssetFile: async () => undefined, + restoreAsset: async () => undefined, + purgeAsset: async (id: string) => { + purged.push(id) + }, + lookupUsage: async () => [], + } + // `media.delete` is what renders the purge button at all. + const session = { + user: { id: 'u1', capabilities: ['media.delete', 'media.write'] }, + setUser: () => {}, + } + render( + + [0]['editor']} + open + onClose={() => {}} + /> + , + ) + return { purged } +} + +/** + * The button in the sidebar, not the one inside the dialog footer. + * + * Awaited because the click looks up what still depends on the asset before + * opening the dialog — the warning has to be part of what the operator reads, + * not something that appears after they have decided. + */ +async function clickPurgeButton() { + const buttons = screen.getAllByRole('button', { name: /delete permanently/i }) + fireEvent.click(buttons[0]!) + await screen.findByRole('alertdialog') +} + +describe('permanent media deletion asks first', () => { + it('does not purge on the first click', async () => { + const { purged } = renderViewer() + await clickPurgeButton() + expect(purged).toEqual([]) + }) + + it('names the asset so the operator can see what they are about to lose', async () => { + renderViewer() + await clickPurgeButton() + // The filename also appears in the window chrome, so match the dialog's + // own heading rather than any occurrence of it. + expect(screen.getByText('Delete "logo.png" permanently?')).toBeTruthy() + }) + + it('cancelling leaves the asset alone', async () => { + const { purged } = renderViewer() + await clickPurgeButton() + fireEvent.click(screen.getByRole('button', { name: /cancel/i })) + expect(purged).toEqual([]) + }) + + it('confirming is what commits the purge', async () => { + const { purged } = renderViewer() + await clickPurgeButton() + // The footer button — the last match, since the sidebar button is still + // mounted behind the dialog. + const buttons = screen.getAllByRole('button', { name: /delete permanently/i }) + fireEvent.click(buttons[buttons.length - 1]!) + expect(purged).toEqual(['asset_1']) + }) +}) diff --git a/src/__tests__/media/usageWarning.test.ts b/src/__tests__/media/usageWarning.test.ts new file mode 100644 index 000000000..2b09c2cb7 --- /dev/null +++ b/src/__tests__/media/usageWarning.test.ts @@ -0,0 +1,103 @@ +/** + * What a destructive confirmation says when part of the selection is in use. + * + * The scenario that produced these rules: eleven files selected, one of them + * a profile picture, and the operator about to purge the lot. A warning that + * says "some of these are in use" tells them nothing actionable; one that + * blocks the delete stops them replacing their own avatar. + */ + +import { describe, expect, it } from 'bun:test' +import { buildUsageWarning } from '@admin/pages/media/utils/usageWarning' + +const avatar = (assetId: string, label: string) => ({ + assetId, refKind: 'user.avatar', refId: 'u1', label, +}) + +describe('the usage warning', () => { + it('says nothing when nothing is depended on', () => { + // The ordinary confirmation stands on its own; an empty warning box + // would train people to ignore the space it occupies. + expect(buildUsageWarning(11, [])).toBeNull() + }) + + it('separates the used from the safe', () => { + // "1 of 11" is the whole point — it tells the operator the other ten + // carry no risk, which a blanket warning never does. + const warning = buildUsageWarning(11, [avatar('a1', 'Ada Lovelace')]) + expect(warning?.heading).toBe('1 of 11 is still in use:') + }) + + it('drops the count when everything selected is in use', () => { + // "2 of 2" reads like arithmetic. Naming the state is clearer. + const warning = buildUsageWarning(2, [avatar('a1', 'Ada'), avatar('a2', 'Grace')]) + expect(warning?.heading).toBe('These files are still in use:') + }) + + it('names what breaks rather than describing the reference', () => { + const warning = buildUsageWarning(3, [avatar('a1', 'Ada Lovelace')]) + expect(warning?.lines).toEqual(['profile picture — Ada Lovelace']) + }) + + it('counts an asset once but names every place it is used', () => { + // Two different things, and the warning has to get both right. + // + // The COUNT is about what disappears: one file is lost, not two, so "2 of + // 4" would overstate the damage. + // + // The LINES are about where to go afterwards. Naming one of the two and + // hiding the other sends the operator to fix half of it and leaves the + // rest broken — which is how the warning would end up blamed for the + // breakage it was meant to prevent. + const warning = buildUsageWarning(4, [ + avatar('a1', 'Ada Lovelace'), + { assetId: 'a1', refKind: 'user.avatar', refId: 'u2', label: 'Grace Hopper' }, + ]) + expect(warning?.heading).toBe('1 of 4 is still in use:') + expect(warning?.lines).toEqual([ + 'profile picture — Ada Lovelace', + 'profile picture — Grace Hopper', + ]) + }) + + it('names each page a file appears on, because each one breaks', () => { + const onPage = (refId: string, title: string) => ({ + assetId: 'a1', refKind: 'page.content', refId, label: title, + }) + const warning = buildUsageWarning(1, [ + onPage('p1', 'Home'), + onPage('p2', 'About us'), + ]) + expect(warning?.heading).toBe('This file is still in use:') + expect(warning?.lines).toEqual([ + 'on the page — Home', + 'on the page — About us', + ]) + }) + + it('describes a site-wide background without pretending it is a page', () => { + const warning = buildUsageWarning(1, [ + { assetId: 'a1', refKind: 'site.styles', refId: 'site', label: 'site styles' }, + ]) + expect(warning?.lines).toEqual(['a site-wide background style']) + }) + + it('summarises past three so the dialog stays readable', () => { + const refs = ['a1', 'a2', 'a3', 'a4', 'a5'].map((id, i) => avatar(id, `User ${i + 1}`)) + const warning = buildUsageWarning(20, refs) + expect(warning?.lines).toHaveLength(4) + expect(warning?.lines.at(-1)).toBe('and 2 more') + }) + + it('does not summarise at exactly three', () => { + const refs = ['a1', 'a2', 'a3'].map((id, i) => avatar(id, `User ${i + 1}`)) + expect(buildUsageWarning(9, refs)?.lines).toHaveLength(3) + }) + + it('uses singular and plural correctly', () => { + expect(buildUsageWarning(5, [avatar('a1', 'Ada')])?.heading).toContain(' is still') + expect( + buildUsageWarning(5, [avatar('a1', 'Ada'), avatar('a2', 'Grace')])?.heading, + ).toContain(' are still') + }) +}) diff --git a/src/__tests__/media/usageWarningInConfirmation.test.tsx b/src/__tests__/media/usageWarningInConfirmation.test.tsx new file mode 100644 index 000000000..3f2705991 --- /dev/null +++ b/src/__tests__/media/usageWarningInConfirmation.test.tsx @@ -0,0 +1,181 @@ +/** + * The confirmation names what it is about to break. + * + * `buildUsageWarning` is tested on its own (usageWarning.test.ts) — these + * cover the part that only shows up once it is wired: that every surface + * offering a permanent delete looks the usage up BEFORE it opens the dialog, + * that the answer reaches the dialog the operator is reading, and that a + * failed lookup degrades to the plain confirmation instead of blocking a + * delete or swallowing one. + * + * Each surface that offers the delete owns its own dialog, so each has to be + * wired separately — and a test per surface is the only thing that catches one + * being left behind. The bulk window is a third such surface; its permanent + * delete arrives with #500, and its wiring belongs in whichever of the two + * lands second. + */ + +import { afterEach, describe, expect, it } from 'bun:test' +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { MediaViewerWindow } from '@admin/pages/media/components/MediaViewerWindow/MediaViewerWindow' +import { AdminSessionContext } from '@admin/sessionContext' +import type { CmsMediaUsageRef } from '@core/persistence/cmsMedia' + +afterEach(cleanup) + +function asset(id: string, filename: string) { + return { + id, + filename, + mimeType: 'image/png', + sizeBytes: 1200, + publicPath: `/uploads/${filename}`, + uploadedByUserId: null, + createdAt: '2026-01-01T00:00:00.000Z', + altText: '', + caption: '', + title: '', + tags: [], + width: null, + height: null, + durationMs: null, + dominantColor: null, + deletedAt: '2026-02-01T00:00:00.000Z', + replacedAt: null, + folderIds: [], + blurHash: null, + variants: [], + posterPath: null, + } +} + +const AVATAR_REF: CmsMediaUsageRef = { + assetId: 'a1', + refKind: 'user.avatar', + refId: 'u1', + label: 'Ada Lovelace', +} + +const session = { + user: { id: 'u1', capabilities: ['media.delete', 'media.write'] }, + setUser: () => {}, +} + +// ── Viewer window ─────────────────────────────────────────────────────────── + +function renderViewer(lookupUsage: (ids: string[]) => Promise) { + const asked: string[][] = [] + const editor = { + asset: asset('a1', 'logo.png'), + tagPalette: [], + folderById: new Map(), + updateAsset: async () => undefined, + renameAsset: async () => undefined, + replaceAssetFile: async () => undefined, + restoreAsset: async () => undefined, + purgeAsset: async () => undefined, + lookupUsage: async (ids: string[]) => { + asked.push(ids) + return await lookupUsage(ids) + }, + } + render( + + [0]['editor']} + open + onClose={() => {}} + /> + , + ) + return { asked } +} + +async function openViewerPurge() { + const buttons = screen.getAllByRole('button', { name: /delete permanently/i }) + fireEvent.click(buttons[0]!) + await screen.findByRole('alertdialog') +} + +describe('the viewer window', () => { + it('names the person whose avatar it is', async () => { + renderViewer(async () => [AVATAR_REF]) + await openViewerPurge() + expect(screen.getByText(/profile picture — Ada Lovelace/)).toBeTruthy() + }) + + it('asks before the dialog opens, not after', async () => { + // If the lookup were fired alongside the dialog, the warning would land + // after the operator has already read it and moved to the button. + const { asked } = renderViewer(async () => [AVATAR_REF]) + await openViewerPurge() + expect(asked).toEqual([['a1']]) + }) + + it('says nothing extra when nothing depends on the file', async () => { + renderViewer(async () => []) + await openViewerPurge() + expect(screen.queryByText(/still in use/)).toBeNull() + expect(screen.getByText('Delete "logo.png" permanently?')).toBeTruthy() + }) + + it('still confirms when the lookup fails', async () => { + // Advisory, not a gate. A network blip must not turn a delete into a + // dead button — nor into an unconfirmed one. + renderViewer(async () => { + throw new Error('network') + }) + await openViewerPurge() + expect(screen.getByText('Delete "logo.png" permanently?')).toBeTruthy() + expect(screen.queryByText(/still in use/)).toBeNull() + }) +}) + +// ── The grid's context menu ───────────────────────────────────────────────── +// +// It routes through the shared `ConfirmDeleteDialog` rather than a local one, +// so what has to hold is the generic `details` slot that carries the warning +// across that boundary — plus the guarantee the surrounding +// `void (async () => …)()` depends on. + +describe('the shared confirm dialog', () => { + it('renders the caller-owned details below the description', async () => { + const { ConfirmDeleteDialog } = await import( + '@admin/shared/dialogs/ConfirmDeleteDialog/ConfirmDeleteDialog' + ) + render( + 1 of 3 is still in use:

} + onCancel={() => {}} + onConfirm={() => {}} + />, + ) + expect(screen.getByText('1 of 3 is still in use:')).toBeTruthy() + }) +}) + +describe('resolveUsageWarning', () => { + it('never rejects, so the dialog it precedes always opens', async () => { + // Every surface opens its dialog from a floating `void (async () => …)()`. + // A rejection there is dropped silently and the dialog never appears — a + // delete button that does nothing at all. + const { resolveUsageWarning } = await import('@admin/pages/media/utils/usageWarning') + const warning = await resolveUsageWarning(async () => { + throw new Error('network') + }, ['a1']) + expect(warning).toBeNull() + }) + + it('does not ask the server about an empty selection', async () => { + const { resolveUsageWarning } = await import('@admin/pages/media/utils/usageWarning') + let called = false + const warning = await resolveUsageWarning(async () => { + called = true + return [] + }, []) + expect(called).toBe(false) + expect(warning).toBeNull() + }) +}) diff --git a/src/__tests__/server/mediaContentUsage.test.ts b/src/__tests__/server/mediaContentUsage.test.ts new file mode 100644 index 000000000..099232a1c --- /dev/null +++ b/src/__tests__/server/mediaContentUsage.test.ts @@ -0,0 +1,185 @@ +/** + * Which pages use a file — worked out from the site, not from a stored index. + * + * The avatar case could be RECORDED because a profile picture has one writer + * and an explicit set/unset. Page content has neither: it is written + * continuously by the collab relay, and deleting an image emits no event at + * all — so a table would slowly fill with references to nodes that no longer + * exist, and the delete warning would start naming pages that are fine. + * + * That is the failure worth testing against, because a warning that is + * sometimes wrong is worse than no warning: it gets ignored. Everything here + * pins the property that makes it impossible — the answer comes from the + * current document, so it cannot describe a state the document is not in. + */ + +import { afterEach, describe, expect, it } from 'bun:test' +import type { SiteShell } from '@core/page-tree' +import { normalizeSiteRuntimeConfig } from '@core/site-runtime' +import { createTestDb } from '../helpers/createTestDb' +import { saveDraftSite } from '../../../server/repositories/site' +import { createDataRow, saveDataRowDraft } from '../../../server/repositories/data' +import { pageToCells } from '../../../src/core/data/pageFromRow' +import { collectContentUsageRefs } from '../../../server/media/contentUsage' + +const cleanups: Array<() => Promise> = [] +afterEach(async () => { + for (const cleanup of cleanups.splice(0)) await cleanup() +}) + +const HERO_PATH = '/uploads/hero.png' + +function siteShell(overrides: Partial = {}): SiteShell { + return { + id: 'project_1', + name: 'Site', + files: [], + visualComponents: [], + breakpoints: [{ id: 'desktop', label: 'Desktop', width: 1440, icon: 'monitor' }], + settings: { shortcuts: {} }, + styleRules: {}, + packageJson: { dependencies: {}, devDependencies: {} }, + runtime: normalizeSiteRuntimeConfig(undefined), + createdAt: 1000, + updatedAt: 2000, + ...overrides, + } +} + +/** A page whose single image node points at `src`, or none when null. */ +function pageWith(id: string, title: string, slug: string, src: string | null) { + const nodes: Record = { + root: { + id: 'root', + moduleId: 'base.body', + props: {}, + breakpointOverrides: {}, + children: src === null ? [] : ['img_1'], + classIds: [], + }, + } + if (src !== null) { + nodes['img_1'] = { + id: 'img_1', + moduleId: 'base.image', + props: { src }, + breakpointOverrides: {}, + children: [], + classIds: [], + } + } + return { id, title, slug, rootNodeId: 'root', nodes } +} + +async function freshDb() { + const { db, cleanup } = await createTestDb() + cleanups.push(cleanup) + // `data_rows.created_by` is a foreign key into `users`. + await db` + insert into users (id, email, email_normalized, display_name, password_hash, status, role_id) + values ('admin_1', 'ada@example.com', 'ada@example.com', 'Ada', 'hash', 'active', 'owner') + ` + await db` + insert into media_assets (id, filename, mime_type, size_bytes, storage_path, public_path) + values ('a1', 'hero.png', 'image/png', 10, '/s/a1', ${HERO_PATH}) + ` + await db` + insert into media_assets (id, filename, mime_type, size_bytes, storage_path, public_path) + values ('a2', 'unused.png', 'image/png', 10, '/s/a2', '/uploads/unused.png') + ` + return db +} + +async function seedPage( + db: Awaited>, + page: ReturnType, +) { + await createDataRow(db, { + id: page.id, + tableId: 'pages', + cells: pageToCells(page as never), + slug: page.slug, + }, 'admin_1') +} + +describe('media used by page content', () => { + it('names the page an image sits on', async () => { + const db = await freshDb() + await saveDraftSite(db, siteShell()) + await seedPage(db, pageWith('page_home', 'Home', 'index', HERO_PATH)) + + const refs = await collectContentUsageRefs(db, ['a1']) + expect(refs).toHaveLength(1) + expect(refs[0]!.refKind).toBe('page.content') + expect(refs[0]!.label).toBe('Home') + expect(refs[0]!.assetId).toBe('a1') + }) + + it('reports nothing for a file no page references', async () => { + const db = await freshDb() + await saveDraftSite(db, siteShell()) + await seedPage(db, pageWith('page_home', 'Home', 'index', HERO_PATH)) + + expect(await collectContentUsageRefs(db, ['a2'])).toEqual([]) + }) + + it('names every page, because every one of them breaks', async () => { + const db = await freshDb() + await saveDraftSite(db, siteShell()) + await seedPage(db, pageWith('page_home', 'Home', 'index', HERO_PATH)) + await seedPage(db, pageWith('page_about', 'About us', 'about', HERO_PATH)) + + const labels = (await collectContentUsageRefs(db, ['a1'])).map((r) => r.label) + expect(labels.sort()).toEqual(['About us', 'Home']) + }) + + it('stops reporting as soon as the image is taken off the page', async () => { + // The whole reason this is computed rather than recorded. Removing an + // image emits no event a table could listen for, so a stored reference + // would still be pointing at this page — and the warning would send the + // operator to fix something that is already fine. + const db = await freshDb() + await saveDraftSite(db, siteShell()) + await seedPage(db, pageWith('page_home', 'Home', 'index', HERO_PATH)) + expect(await collectContentUsageRefs(db, ['a1'])).toHaveLength(1) + + await saveDataRowDraft(db, 'page_home', { + cells: pageToCells(pageWith('page_home', 'Home', 'index', null) as never), + slug: 'index', + }, 'admin_1') + + expect(await collectContentUsageRefs(db, ['a1'])).toEqual([]) + }) + + it('sees a draft page, not just what has been published', async () => { + // An image on an unpublished page is still in use: deleting it would + // break the page the moment it goes live. Reading published artefacts + // instead of the draft document would have missed exactly this. + const db = await freshDb() + await saveDraftSite(db, siteShell()) + await seedPage(db, pageWith('page_draft', 'Not yet live', 'soon', HERO_PATH)) + + const refs = await collectContentUsageRefs(db, ['a1']) + expect(refs).toHaveLength(1) + expect(refs[0]!.label).toBe('Not yet live') + }) + + it('answers for a whole selection in one call', async () => { + const db = await freshDb() + await saveDraftSite(db, siteShell()) + await seedPage(db, pageWith('page_home', 'Home', 'index', HERO_PATH)) + + const refs = await collectContentUsageRefs(db, ['a1', 'a2']) + expect(refs.map((r) => r.assetId)).toEqual(['a1']) + }) + + it('returns nothing for an empty selection without loading the site', async () => { + const db = await freshDb() + expect(await collectContentUsageRefs(db, [])).toEqual([]) + }) + + it('reports nothing when there is no site document yet', async () => { + const db = await freshDb() + expect(await collectContentUsageRefs(db, ['a1'])).toEqual([]) + }) +}) diff --git a/src/__tests__/server/mediaUsageRefs.test.ts b/src/__tests__/server/mediaUsageRefs.test.ts new file mode 100644 index 000000000..9dc1104a3 --- /dev/null +++ b/src/__tests__/server/mediaUsageRefs.test.ts @@ -0,0 +1,188 @@ +/** + * Knowing that something still depends on a media asset. + * + * `media_usage_refs` shipped with the media schema and nothing ever wrote to + * it, so the library could not tell a decorative upload from an asset the + * product depends on. A profile picture is stored as an ordinary library row + * with no marker of any kind — so tidying up the library swept one into the + * trash, purging it hard-deleted the row, and `users.avatar_media_id` went + * quietly to NULL through its `on delete set null` foreign key. The profile + * fell back to a Gravatar identicon with nothing to explain why. + * + * These cover the two behaviours the warning depends on: a reference MOVES + * rather than accumulating, and a cleared one stops reporting. + */ + +import { afterEach, describe, expect, it } from 'bun:test' +import { createTestDb } from '../helpers/createTestDb' +import { pgMigrations } from '../../../server/db/migrations-pg' +import { sqliteMigrations } from '../../../server/db/migrations-sqlite' +import { + listMediaUsageRefs, + setMediaUsageRef, +} from '../../../server/repositories/media' + +const cleanups: Array<() => Promise> = [] + +afterEach(async () => { + for (const cleanup of cleanups.splice(0)) await cleanup() +}) + +async function freshDb() { + const { db, cleanup } = await createTestDb() + cleanups.push(cleanup) + await db` + insert into users (id, email, email_normalized, display_name, password_hash, status, role_id) + values ('u1', 'ada@example.com', 'ada@example.com', 'Ada Lovelace', 'hash', 'active', 'owner') + ` + return db +} + +async function insertAsset(db: Awaited>, id: string) { + await db` + insert into media_assets (id, filename, mime_type, size_bytes, storage_path, public_path) + values (${id}, ${`${id}.png`}, 'image/png', 10, ${`/s/${id}`}, ${`/uploads/${id}.png`}) + ` +} + +describe('media usage references', () => { + it('reports nothing for an asset nobody depends on', async () => { + const db = await freshDb() + await insertAsset(db, 'a1') + expect(await listMediaUsageRefs(db, ['a1'])).toEqual([]) + }) + + it('names the person whose avatar it is, not the raw id', async () => { + // The whole point is a confirmation an operator can act on. "u1" tells + // them nothing; "Ada Lovelace" tells them what breaks. + const db = await freshDb() + await insertAsset(db, 'a1') + await setMediaUsageRef(db, { assetId: 'a1', refKind: 'user.avatar', refId: 'u1' }) + + const refs = await listMediaUsageRefs(db, ['a1']) + expect(refs).toHaveLength(1) + expect(refs[0]!.label).toBe('Ada Lovelace') + expect(refs[0]!.refKind).toBe('user.avatar') + }) + + it('falls back to the email when there is no display name', async () => { + const db = await freshDb() + await db`update users set display_name = '' where id = 'u1'` + await insertAsset(db, 'a1') + await setMediaUsageRef(db, { assetId: 'a1', refKind: 'user.avatar', refId: 'u1' }) + expect((await listMediaUsageRefs(db, ['a1']))[0]!.label).toBe('ada@example.com') + }) + + it('MOVES the reference when the avatar is replaced', async () => { + // Four avatar changes must leave one row, not four — otherwise deleting + // the fifth picture would warn about ones replaced months ago, and the + // warning becomes noise the operator learns to click past. + const db = await freshDb() + await insertAsset(db, 'old') + await insertAsset(db, 'new') + await setMediaUsageRef(db, { assetId: 'old', refKind: 'user.avatar', refId: 'u1' }) + await setMediaUsageRef(db, { assetId: 'new', refKind: 'user.avatar', refId: 'u1' }) + + expect(await listMediaUsageRefs(db, ['old'])).toEqual([]) + expect((await listMediaUsageRefs(db, ['new']))[0]!.label).toBe('Ada Lovelace') + }) + + it('stops reporting once the avatar is cleared', async () => { + // The asset deliberately stays in the library, but nothing depends on it + // any more — so deleting it should no longer warn. + const db = await freshDb() + await insertAsset(db, 'a1') + await setMediaUsageRef(db, { assetId: 'a1', refKind: 'user.avatar', refId: 'u1' }) + await setMediaUsageRef(db, { assetId: null, refKind: 'user.avatar', refId: 'u1' }) + expect(await listMediaUsageRefs(db, ['a1'])).toEqual([]) + }) + + it('answers for a whole selection in one call', async () => { + // The deletion path asks about every selected file at once; asking per + // file would mean one round trip per row of a bulk delete. + const db = await freshDb() + for (const id of ['a1', 'a2', 'a3']) await insertAsset(db, id) + await setMediaUsageRef(db, { assetId: 'a2', refKind: 'user.avatar', refId: 'u1' }) + + const refs = await listMediaUsageRefs(db, ['a1', 'a2', 'a3']) + expect(refs).toHaveLength(1) + expect(refs[0]!.assetId).toBe('a2') + }) + + it('returns nothing for an empty selection without touching the database', async () => { + const db = await freshDb() + expect(await listMediaUsageRefs(db, [])).toEqual([]) + }) +}) + +/** + * Run one shipped migration's own SQL, by id. + * + * `createTestDb` has already applied every migration before the test writes a + * row, so the backfill ran against an empty `users` table and the tracker now + * says it is done. Replaying its SQL directly is what actually exercises it — + * and running it twice is the only honest test of the `not exists` guard. + */ +async function replayMigration(db: Awaited>, id: string) { + const list = db.dialect === 'postgres' ? pgMigrations : sqliteMigrations + const migration = list.find((m) => m.id === id) + if (!migration) throw new Error(`No migration ${id} — was it renamed?`) + await db.unsafe(migration.sql) +} + +const BACKFILL = '028_backfill_avatar_usage_refs' + +describe('the avatar backfill', () => { + it('protects an avatar that was set before anything recorded usage', async () => { + // Every install that already has an avatar is in exactly this state. + // Without the backfill, the first build that warns before a delete would + // still say nothing about the picture already set — the one case the + // whole feature exists for. + const db = await freshDb() + await insertAsset(db, 'a1') + await db`update users set avatar_media_id = 'a1' where id = 'u1'` + + await replayMigration(db, BACKFILL) + + const refs = await listMediaUsageRefs(db, ['a1']) + expect(refs).toHaveLength(1) + expect(refs[0]!.refKind).toBe('user.avatar') + expect(refs[0]!.label).toBe('Ada Lovelace') + }) + + it('adds nothing on top of a reference that is already there', async () => { + const db = await freshDb() + await insertAsset(db, 'a1') + await db`update users set avatar_media_id = 'a1' where id = 'u1'` + await setMediaUsageRef(db, { assetId: 'a1', refKind: 'user.avatar', refId: 'u1' }) + + await replayMigration(db, BACKFILL) + await replayMigration(db, BACKFILL) + + expect(await listMediaUsageRefs(db, ['a1'])).toHaveLength(1) + }) + + it('leaves a user with no avatar alone', async () => { + const db = await freshDb() + await insertAsset(db, 'a1') + await replayMigration(db, BACKFILL) + expect(await listMediaUsageRefs(db, ['a1'])).toEqual([]) + }) + + it('writes the row a later avatar change will MOVE, not a second one', async () => { + // The backfilled row has to be indistinguishable from one the app wrote, + // or changing the avatar afterwards would leave the old picture warning + // forever. `setMediaUsageRef` deletes on (ref_kind, ref_id, ref_path) — + // so the backfill must write the same key. + const db = await freshDb() + await insertAsset(db, 'old') + await insertAsset(db, 'new') + await db`update users set avatar_media_id = 'old' where id = 'u1'` + await replayMigration(db, BACKFILL) + + await setMediaUsageRef(db, { assetId: 'new', refKind: 'user.avatar', refId: 'u1' }) + + expect(await listMediaUsageRefs(db, ['old'])).toEqual([]) + expect(await listMediaUsageRefs(db, ['new'])).toHaveLength(1) + }) +}) diff --git a/src/admin/pages/media/MediaPage.tsx b/src/admin/pages/media/MediaPage.tsx index f81e3aa29..8b2246188 100644 --- a/src/admin/pages/media/MediaPage.tsx +++ b/src/admin/pages/media/MediaPage.tsx @@ -65,6 +65,7 @@ export function MediaPage() { replaceAssetFile: workspace.replaceAssetFile, restoreAsset: workspace.restoreAsset, purgeAsset: workspace.purgeAsset, + lookupUsage: workspace.lookupUsage, } : null diff --git a/src/admin/pages/media/components/MediaCanvas/MediaCanvas.tsx b/src/admin/pages/media/components/MediaCanvas/MediaCanvas.tsx index 3dba08963..fe200e671 100644 --- a/src/admin/pages/media/components/MediaCanvas/MediaCanvas.tsx +++ b/src/admin/pages/media/components/MediaCanvas/MediaCanvas.tsx @@ -19,6 +19,7 @@ import { FilterBar, type FilterBarItem } from '@ui/components/FilterBar' import { Select } from '@ui/components/Select' import { Skeleton } from '@ui/components/Skeleton' import { canDeleteMedia, canWriteMedia } from '@admin/access' +import { useConfirmDelete } from '@admin/shared/dialogs/ConfirmDeleteDialog' import { useCurrentAdminUser } from '@admin/sessionContext' import { ExplorerItemContextMenu, @@ -53,6 +54,8 @@ import { writeMediaFolderDragData, } from '../../utils/mediaDragDrop' import { useMediaDnd } from '../../hooks/useMediaDnd' +import { UsageWarningNotice } from '../UsageWarningNotice' +import { resolveUsageWarning } from '../../utils/usageWarning' import styles from './MediaCanvas.module.css' import { AssetRow, @@ -122,6 +125,7 @@ function folderMatchesQuery(folder: CmsMediaFolder, query: string): boolean { export function MediaCanvas({ workspace, selectionMode = 'standard' }: MediaCanvasProps) { const currentUser = useCurrentAdminUser() + const confirmDelete = useConfirmDelete() const [viewMode, setViewModeState] = useState(readStoredMediaViewMode) const [contextMenu, setContextMenu] = useState(null) const [renameTarget, setRenameTarget] = useState(null) @@ -530,8 +534,30 @@ export function MediaCanvas({ workspace, selectionMode = 'standard' }: MediaCanv onDelete={() => { const target = contextMenu.asset setContextMenu(null) - if (trashView) void workspace.purgeAsset(target.id) - else void workspace.trashAsset(target.id) + if (!trashView) { + void workspace.trashAsset(target.id) + return + } + // Purging is the one media action nothing can undo: it removes the + // original and every generated size from the storage adapter, not + // just the row. `alwaysConfirm` because the `confirmBeforeDelete` + // preference defaults off, and an operator who turned it off was + // opting out of confirming a TRASH, which is reversible. + // + // The usage lookup is awaited BEFORE the dialog opens so the + // warning is on screen when the operator reads it, not after. + void (async () => { + const warning = await resolveUsageWarning(workspace.lookupUsage, [target.id]) + confirmDelete({ + title: `Delete "${target.filename}" permanently?`, + description: + 'This removes the file and every generated size from disk. It cannot be undone.', + details: , + confirmLabel: 'Delete permanently', + alwaysConfirm: true, + commit: () => void workspace.purgeAsset(target.id), + }) + })() }} showRename={canWrite} showDelete={canDelete} diff --git a/src/admin/pages/media/components/MediaViewerWindow/MediaViewerWindow.tsx b/src/admin/pages/media/components/MediaViewerWindow/MediaViewerWindow.tsx index 27b3a6a3b..6bd340ff1 100644 --- a/src/admin/pages/media/components/MediaViewerWindow/MediaViewerWindow.tsx +++ b/src/admin/pages/media/components/MediaViewerWindow/MediaViewerWindow.tsx @@ -27,6 +27,7 @@ import { useState, type ReactNode } from 'react' import { createPortal } from 'react-dom' import { Button } from '@ui/components/Button' +import { Dialog } from '@ui/components/Dialog' import { Input, Textarea } from '@ui/components/Input' import { canDeleteMedia, canReplaceMedia, canWriteMedia } from '@admin/access' import { useCurrentAdminUser } from '@admin/sessionContext' @@ -37,7 +38,14 @@ import { TrashSolidIcon } from 'pixel-art-icons/icons/trash-solid' import { VideoSolidIcon } from 'pixel-art-icons/icons/video-solid' import { PanelHeader } from '@admin/shared/PanelHeader' import { useDraggablePanel } from '@admin/shared/FloatingWindow' -import type { CmsMediaAsset, CmsMediaFolder, UpdateCmsMediaAssetInput } from '@core/persistence/cmsMedia' +import type { + CmsMediaAsset, + CmsMediaFolder, + CmsMediaUsageRef, + UpdateCmsMediaAssetInput, +} from '@core/persistence/cmsMedia' +import { resolveUsageWarning, type UsageWarning } from '../../utils/usageWarning' +import { UsageWarningNotice } from '../UsageWarningNotice' import { bucketForMime } from '../../utils/filters' import { useDebouncedSave } from '../../hooks/useDebouncedSave' import { TagEditor } from '../TagEditor/TagEditor' @@ -63,6 +71,7 @@ export interface MediaAssetEditor { replaceAssetFile: (id: string, file: File) => Promise restoreAsset: (id: string) => Promise purgeAsset: (id: string) => Promise + lookupUsage: (assetIds: string[]) => Promise } interface MediaViewerWindowProps { @@ -101,6 +110,8 @@ function ViewerForAsset({ editor, onClose }: ViewerForAssetProps) { const currentUser = useCurrentAdminUser() const { asset } = editor const [replaceOpen, setReplaceOpen] = useState(false) + const [purgeConfirmOpen, setPurgeConfirmOpen] = useState(false) + const [purgeWarning, setPurgeWarning] = useState(null) const bucket = bucketForMime(asset.mimeType) const canWrite = canWriteMedia(currentUser) const canReplace = canReplaceMedia(currentUser) @@ -328,7 +339,17 @@ function ViewerForAsset({ editor, onClose }: ViewerForAssetProps) { + + + } + > +

+ This removes the file and every generated size from disk. Any page + still referencing it will render a broken image. +

+ + , document.body, ) diff --git a/src/admin/pages/media/components/UsageWarningNotice/UsageWarningNotice.module.css b/src/admin/pages/media/components/UsageWarningNotice/UsageWarningNotice.module.css new file mode 100644 index 000000000..fcb13f650 --- /dev/null +++ b/src/admin/pages/media/components/UsageWarningNotice/UsageWarningNotice.module.css @@ -0,0 +1,46 @@ +/* The warning that appears inside a destructive confirmation when part of the + selection is still depended on. Warning tone, not danger: the dialog around + it is already danger, and repeating that here would say "this is dangerous" + twice while saying "and this part in particular" nowhere. */ + +.notice { + display: flex; + gap: var(--space-l); + padding: var(--space-l); + margin-top: var(--space-l); + border-radius: var(--radius); + background: var(--warning-10); + color: var(--warning-text); + font-size: var(--text-s); +} + +.icon { + flex: none; + color: var(--warning); +} + +.body { + display: flex; + flex-direction: column; + gap: var(--space-xs); + min-width: 0; +} + +.heading { + margin: 0; + font-weight: 600; +} + +.list { + margin: 0; + padding: 0; + list-style: none; + display: flex; + flex-direction: column; + gap: var(--space-2xs); +} + +.item { + /* A long filename or display name must not push the dialog wider. */ + overflow-wrap: anywhere; +} diff --git a/src/admin/pages/media/components/UsageWarningNotice/UsageWarningNotice.tsx b/src/admin/pages/media/components/UsageWarningNotice/UsageWarningNotice.tsx new file mode 100644 index 000000000..b2b156029 --- /dev/null +++ b/src/admin/pages/media/components/UsageWarningNotice/UsageWarningNotice.tsx @@ -0,0 +1,31 @@ +/** + * Names what a destructive media action is about to break. + * + * Rendered inside the confirmation, not instead of it: the operator is still + * allowed to go ahead — replacing an avatar begins by deleting the old one — + * so this informs and gets out of the way. `buildUsageWarning` owns the + * wording; this owns how it looks. + * + * `role="status"` rather than `alert`: the dialog announces itself on open and + * the assertive interruption belongs to that, not to a detail inside it. + */ +import { WarningDiamondSolidIcon } from 'pixel-art-icons/icons/warning-diamond-solid' +import type { UsageWarning } from '../../utils/usageWarning' +import s from './UsageWarningNotice.module.css' + +export function UsageWarningNotice({ warning }: { warning: UsageWarning | null }) { + if (!warning) return null + return ( +
+
+ ) +} diff --git a/src/admin/pages/media/components/UsageWarningNotice/index.ts b/src/admin/pages/media/components/UsageWarningNotice/index.ts new file mode 100644 index 000000000..e72bfe976 --- /dev/null +++ b/src/admin/pages/media/components/UsageWarningNotice/index.ts @@ -0,0 +1 @@ +export { UsageWarningNotice } from './UsageWarningNotice' diff --git a/src/admin/pages/media/hooks/useMediaWorkspace.ts b/src/admin/pages/media/hooks/useMediaWorkspace.ts index 298135168..7ae0da975 100644 --- a/src/admin/pages/media/hooks/useMediaWorkspace.ts +++ b/src/admin/pages/media/hooks/useMediaWorkspace.ts @@ -24,6 +24,7 @@ import { listCmsMediaAssets, listCmsMediaFolders, normalizeCmsMediaAsset, + listCmsMediaUsage, purgeCmsMediaAsset, renameCmsMediaAsset, replaceCmsMediaAssetFile, @@ -33,6 +34,7 @@ import { updateCmsMediaFolder, type CmsMediaAsset, type CmsMediaFolder, + type CmsMediaUsageRef, type UpdateCmsMediaAssetInput, } from '@core/persistence/cmsMedia' import { buildFolderTree, type MediaFolderNode } from '../utils/folderTree' @@ -107,6 +109,11 @@ export interface UseMediaWorkspaceResult extends WorkspaceLoadState { trashAsset: (assetId: string) => Promise restoreAsset: (assetId: string) => Promise purgeAsset: (assetId: string) => Promise + /** + * Which of these assets something still depends on. Asked before a + * destructive action so the confirmation can name what breaks. + */ + lookupUsage: (assetIds: string[]) => Promise setAssetFolders: ( assetId: string, input: { add?: string[]; remove?: string[] }, @@ -417,6 +424,11 @@ export function useMediaWorkspace(): UseMediaWorkspaceResult { return null }) + // Failure is absorbed by `resolveUsageWarning`, the one door every + // confirmation goes through — see the note there. + const lookupUsage = (assetIds: string[]): Promise => + listCmsMediaUsage(assetIds) + const purgeAsset = async (assetId: string): Promise => { await assetMut('Could not delete asset permanently', async () => { await purgeCmsMediaAsset(assetId) @@ -549,6 +561,7 @@ export function useMediaWorkspace(): UseMediaWorkspaceResult { trashAsset, restoreAsset, purgeAsset, + lookupUsage, setAssetFolders, moveAssetsToFolder, createFolder, diff --git a/src/admin/pages/media/hooks/useStandaloneMediaEditor.ts b/src/admin/pages/media/hooks/useStandaloneMediaEditor.ts index 95adfec77..ff58d1805 100644 --- a/src/admin/pages/media/hooks/useStandaloneMediaEditor.ts +++ b/src/admin/pages/media/hooks/useStandaloneMediaEditor.ts @@ -17,6 +17,7 @@ */ import { deleteCmsMediaAsset, + listCmsMediaUsage, purgeCmsMediaAsset, renameCmsMediaAsset, replaceCmsMediaAssetFile, @@ -24,6 +25,7 @@ import { updateCmsMediaAsset, type CmsMediaAsset, type CmsMediaFolder, + type CmsMediaUsageRef, type UpdateCmsMediaAssetInput, } from '@core/persistence/cmsMedia' import type { MediaAssetEditor } from '../components/MediaViewerWindow/MediaViewerWindow' @@ -125,6 +127,11 @@ export function useStandaloneMediaEditor({ } } + // Failure is absorbed by `resolveUsageWarning`, the one door every + // confirmation goes through — see the note there. + const lookupUsage = (assetIds: string[]): Promise => + listCmsMediaUsage(assetIds) + if (!asset) return null return { asset, @@ -135,5 +142,6 @@ export function useStandaloneMediaEditor({ replaceAssetFile, restoreAsset, purgeAsset, + lookupUsage, } } diff --git a/src/admin/pages/media/utils/usageWarning.ts b/src/admin/pages/media/utils/usageWarning.ts new file mode 100644 index 000000000..5a1f33376 --- /dev/null +++ b/src/admin/pages/media/utils/usageWarning.ts @@ -0,0 +1,113 @@ +/** + * The sentence a destructive confirmation shows when part of a selection is + * still depended on. + * + * Three rules, from watching a real deletion go wrong: + * + * SEPARATE. "1 of 11" lets the operator see the other ten are safe. A + * blanket "some of these are in use" is the kind of warning people learn to + * click past, because it never tells them which. + * + * NAME IT. "your profile picture", not "has a reference". The point is that + * they recognise what they are about to lose. + * + * DO NOT BLOCK. Deleting an in-use asset is a legitimate thing to want — + * replacing an avatar starts exactly that way. The confirmation informs; + * the operator still decides. + */ + +import type { CmsMediaUsageRef } from '@core/persistence/cmsMedia' + +/** Beyond this many named items the list becomes a wall rather than a warning. */ +const MAX_NAMED = 3 + +export interface UsageWarning { + /** e.g. `1 of 11 is still in use:` */ + heading: string + /** One line per named dependency, already resolved for display. */ + lines: string[] +} + +function describe(ref: CmsMediaUsageRef): string { + switch (ref.refKind) { + case 'user.avatar': + return `profile picture — ${ref.label}` + case 'page.content': + return `on the page — ${ref.label}` + case 'site.styles': + return 'a site-wide background style' + default: + // A kind this build does not know about — a newer server, or a source + // added since. The label is already display-ready, so showing it plain + // beats inventing a phrasing for something we cannot describe. + return ref.label + } +} + +/** + * `null` when nothing in the selection is depended on — the caller shows its + * ordinary confirmation and says nothing extra. + */ +export function buildUsageWarning( + selectionSize: number, + refs: readonly CmsMediaUsageRef[], +): UsageWarning | null { + if (refs.length === 0) return null + + // The COUNT is per asset: a file used on three pages is still one file to + // lose, and "3 of 11" would overstate what the operator is about to break. + const byAsset = new Map() + for (const ref of refs) { + const list = byAsset.get(ref.assetId) + if (list) list.push(ref) + else byAsset.set(ref.assetId, [ref]) + } + const usedCount = byAsset.size + // The LINES are per place, because each one is somewhere to go and fix. + const used = [...byAsset.values()].flat() + + const heading = selectionSize > usedCount + ? `${usedCount} of ${selectionSize} ${usedCount === 1 ? 'is' : 'are'} still in use:` + : `${usedCount === 1 ? 'This file is' : 'These files are'} still in use:` + + const named = used.slice(0, MAX_NAMED).map(describe) + const rest = used.length - named.length + if (rest > 0) named.push(`and ${rest} more`) + + return { heading, lines: named } +} + +/** + * Ask what depends on these assets, then phrase it. + * + * NEVER REJECTS. This is the single door all three confirmations go through, + * so the guarantee belongs here rather than in each `lookupUsage` — the + * warning is advisory, and a failed request has to degrade to the plain + * confirmation. If this could reject, the `void (async () => …)()` that opens + * each dialog would drop the rejection on the floor and the dialog would + * never appear: a delete button that silently does nothing, which is worse + * than one that deletes without the extra warning. + * + * Deliberately NOT a hook holding state: every caller awaits this immediately + * before opening its confirmation, and a `setState` would not be visible in + * the closure that opens the dialog — the warning would always be one delete + * behind. Callers that need it across renders (a dialog kept open) store the + * returned value themselves. + * + * `selectionSize` is what the operator is being asked about, which is not + * always `assetIds.length`: the bulk window purges only the trashed members of + * a mixed selection, so "1 of 3" has to count the three it will delete. + */ +export async function resolveUsageWarning( + lookupUsage: (assetIds: string[]) => Promise, + assetIds: string[], + selectionSize: number = assetIds.length, +): Promise { + if (assetIds.length === 0) return null + try { + return buildUsageWarning(selectionSize, await lookupUsage(assetIds)) + } catch (err) { + console.error('[usageWarning] usage lookup failed:', err) + return null + } +} diff --git a/src/admin/shared/dialogs/ConfirmDeleteDialog/ConfirmDeleteContext.tsx b/src/admin/shared/dialogs/ConfirmDeleteDialog/ConfirmDeleteContext.tsx index 82ba0edbe..7db4eb7e3 100644 --- a/src/admin/shared/dialogs/ConfirmDeleteDialog/ConfirmDeleteContext.tsx +++ b/src/admin/shared/dialogs/ConfirmDeleteDialog/ConfirmDeleteContext.tsx @@ -58,6 +58,7 @@ export function ConfirmDeleteProvider({ children }: { children: ReactNode }) { title={pending.request.title} description={pending.request.description} confirmLabel={pending.request.confirmLabel} + details={pending.request.details} onCancel={handleCancel} onConfirm={handleConfirm} /> diff --git a/src/admin/shared/dialogs/ConfirmDeleteDialog/ConfirmDeleteDialog.tsx b/src/admin/shared/dialogs/ConfirmDeleteDialog/ConfirmDeleteDialog.tsx index cd3016b23..9519ab3f1 100644 --- a/src/admin/shared/dialogs/ConfirmDeleteDialog/ConfirmDeleteDialog.tsx +++ b/src/admin/shared/dialogs/ConfirmDeleteDialog/ConfirmDeleteDialog.tsx @@ -11,7 +11,7 @@ * only owns the two-button confirmation contract. */ -import { useEffect, useRef } from 'react' +import { useEffect, useRef, type ReactNode } from 'react' import { Button } from '@ui/components/Button' import { Dialog } from '@ui/components/Dialog' @@ -24,6 +24,8 @@ interface ConfirmDeleteDialogProps { confirmLabel?: string /** Cancel button label — defaults to "Cancel". */ cancelLabel?: string + /** Caller-owned content rendered below the description. */ + details?: ReactNode onCancel: () => void onConfirm: () => void } @@ -33,6 +35,7 @@ export function ConfirmDeleteDialog({ description, confirmLabel = 'Delete', cancelLabel = 'Cancel', + details, onCancel, onConfirm, }: ConfirmDeleteDialogProps) { @@ -79,6 +82,7 @@ export function ConfirmDeleteDialog({ } > {description &&

{description}

} + {details} ) } diff --git a/src/admin/shared/dialogs/ConfirmDeleteDialog/confirmDeleteHook.ts b/src/admin/shared/dialogs/ConfirmDeleteDialog/confirmDeleteHook.ts index 711319db3..eecc8eb90 100644 --- a/src/admin/shared/dialogs/ConfirmDeleteDialog/confirmDeleteHook.ts +++ b/src/admin/shared/dialogs/ConfirmDeleteDialog/confirmDeleteHook.ts @@ -6,7 +6,7 @@ * `frameworkChangeConfirmHook.ts`. */ -import { createContext, use } from 'react' +import { createContext, use, type ReactNode } from 'react' export interface ConfirmDeleteRequest { /** Short title shown in the dialog header. e.g. "Delete header layer?" */ @@ -15,6 +15,12 @@ export interface ConfirmDeleteRequest { description?: string /** Confirm button label — defaults to "Delete". */ confirmLabel?: string + /** + * Extra content below the description — a caller-owned detail the generic + * dialog knows nothing about. Media passes the list of things that still + * depend on the files being deleted; most callers pass nothing. + */ + details?: ReactNode /** * Force this request through the dialog even when the user's * `confirmBeforeDelete` preference is off. Use for destructive actions with diff --git a/src/core/persistence/cmsMedia.ts b/src/core/persistence/cmsMedia.ts index 45cce28bc..1be2a5ffb 100644 --- a/src/core/persistence/cmsMedia.ts +++ b/src/core/persistence/cmsMedia.ts @@ -5,7 +5,9 @@ import { CmsMediaFolderEnvelopeSchema, CmsMediaFolderListResponseSchema, CmsMediaListResponseSchema, + CmsMediaUsageEnvelopeSchema, type CmsMediaAssetWire, + type CmsMediaUsageRef, type CmsMediaFolder, } from './responseSchemas' @@ -202,6 +204,33 @@ export async function renameCmsMediaAsset( * — the file stays on disk; restore() un-stamps; `purgeCmsMediaAsset()` * finishes the job. */ +/** + * Which of these assets something still depends on. + * + * Called before a destructive action so the confirmation can name what + * breaks rather than warning in the abstract. A POST because a selection can + * carry a hundred ids, which is the wrong shape for a query string. + */ +export async function listCmsMediaUsage( + assetIds: string[], + options: ClientBase = {}, +): Promise { + if (assetIds.length === 0) return [] + const { fetchImpl, basePath } = resolveClient(options) + const res = await fetchImpl(`${basePath}/media/usage`, { + method: 'POST', + credentials: 'include', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ assetIds }), + }) + const payload = await readEnvelope( + res, + CmsMediaUsageEnvelopeSchema, + `CMS media usage lookup failed with ${res.status}`, + ) + return payload.usage +} + export async function deleteCmsMediaAsset( assetId: string, options: ClientBase = {}, @@ -338,3 +367,7 @@ export async function deleteCmsMediaFolder( }) await assertOk(res, `CMS folder delete failed with ${res.status}`) } + +// Re-exported so consumers import media types from the media module rather +// than reaching into the shared schema file. +export type { CmsMediaUsageRef } diff --git a/src/core/persistence/responseSchemas.ts b/src/core/persistence/responseSchemas.ts index 218855836..b6c483718 100644 --- a/src/core/persistence/responseSchemas.ts +++ b/src/core/persistence/responseSchemas.ts @@ -140,6 +140,23 @@ export const CmsMediaAssetEnvelopeSchema = Type.Object({ asset: CmsMediaAssetSchema, }) +/** + * Something that depends on a media asset. `label` is already resolved for + * display — a person's name for an avatar — because the caller renders it + * into a confirmation and has no way to look an id up. + */ +export const CmsMediaUsageRefSchema = Type.Object({ + assetId: Type.String(), + refKind: Type.String(), + refId: Type.String(), + label: Type.String(), +}) +export type CmsMediaUsageRef = Static + +export const CmsMediaUsageEnvelopeSchema = Type.Object({ + usage: Type.Array(CmsMediaUsageRefSchema), +}) + const CmsMediaFolderSchema = Type.Object({ id: Type.String(), parentId: Type.Union([Type.String(), Type.Null()]),