From f4cade224c9d4664bb4d83f37a433e4c4f762502 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Wed, 28 Oct 2026 03:54:36 +0100 Subject: [PATCH] fix: ask in the app's own dialogs instead of browser confirm boxes in the admin panel --- .../marketplace-install-version.test.tsx | 12 +++++- app/(main)/admin/_pages/branding.tsx | 28 +++++++++---- app/(main)/admin/_pages/extension-detail.tsx | 21 +++++++++- .../admin/_pages/marketplace-detail.tsx | 23 +++++++++-- app/(main)/admin/_pages/marketplace.tsx | 27 ++++++++++--- .../__tests__/no-native-dialogs.test.ts | 40 +++++++++++++++++++ 6 files changed, 131 insertions(+), 20 deletions(-) create mode 100644 components/settings/__tests__/no-native-dialogs.test.ts diff --git a/app/(main)/admin/__tests__/marketplace-install-version.test.tsx b/app/(main)/admin/__tests__/marketplace-install-version.test.tsx index e33885cc5..b8efa0cef 100644 --- a/app/(main)/admin/__tests__/marketplace-install-version.test.tsx +++ b/app/(main)/admin/__tests__/marketplace-install-version.test.tsx @@ -28,8 +28,16 @@ describe('installVersion', () => { it('asks before installing an older version and repeats with force when the admin agrees', async () => { responses = [json({ error: 'Version 2.0.9 is older than the installed 2.1.0.', downgrade: { installedVersion: '2.1.0' } }, 409), json({ warnings: [] })]; const asked: string[] = []; - const result = await installVersion(BODY, (message) => { asked.push(message); return true; }); - expect(asked).toEqual(['Version 2.0.9 is older than the installed 2.1.0.\n\nInstall the older version anyway?']); + const result = await installVersion(BODY, (q) => { asked.push(`${q.title} ${q.message}`); return true; }); + expect(asked).toEqual(['Install the older version anyway? Version 2.0.9 is older than the installed 2.1.0.']); + expect(calls.map((c) => c.body.force)).toEqual([undefined, true]); + expect(result.ok).toBe(true); + }); + + // The question is the page's ConfirmDialog, not window.confirm. + it('waits for the answer of the page dialog', async () => { + responses = [json({ error: 'withdrawn', yanked: { reason: null } }, 409), json({ warnings: [] })]; + const result = await installVersion(BODY, async (q) => q.title === 'Install it anyway?'); expect(calls.map((c) => c.body.force)).toEqual([undefined, true]); expect(result.ok).toBe(true); }); diff --git a/app/(main)/admin/_pages/branding.tsx b/app/(main)/admin/_pages/branding.tsx index a4c1318e0..125dd9ef1 100644 --- a/app/(main)/admin/_pages/branding.tsx +++ b/app/(main)/admin/_pages/branding.tsx @@ -19,6 +19,9 @@ import { Badge } from '@/components/admin/badge'; import { Flash, useFlash } from '@/components/admin/flash-message'; import { inputClass, RevertButton } from '@/components/admin/config-items'; import { cn } from '@/lib/utils'; +import { ConfirmDialog } from '@/components/ui/confirm-dialog'; +import { useConfirmDialog } from '@/hooks/use-confirm-dialog'; +import { useAdminUnsavedStore } from '@/stores/admin-unsaved-store'; interface ConfigEntry { value?: unknown; @@ -326,9 +329,14 @@ export function BrandingPage() { async function handleDeleteDomain() { if (!selectedHost) return; - if (!confirm(`Remove branding entry for ${selectedHost}? Uploaded files for this domain will be left behind on disk.`)) { - return; - } + const ok = await confirm({ + title: `Remove the branding for ${selectedHost}?`, + message: 'The settings for this domain are removed. Files uploaded for it stay on disk.', + confirmText: 'Remove', + cancelText: 'Cancel', + variant: 'destructive', + }); + if (!ok) return; const next = domainEntries.filter(e => e.host !== selectedHost); const res = await apiFetch('/api/admin/config', { method: 'PATCH', @@ -346,17 +354,22 @@ export function BrandingPage() { } } + // Another domain's settings replace the form: the page's own leave prompt + // asks first when this one holds edits. function handleScopeChange(host: string | null) { - if (Object.keys(edits).length > 0 && !confirm('Discard unsaved changes?')) return; - setSelectedHost(host); - setEdits({}); - setMessage(null); + requestLeave(() => { + setSelectedHost(host); + setEdits({}); + setMessage(null); + }, 'branding'); } const hasEdits = Object.keys(edits).length > 0; const wildcardScope = !!selectedHost && !EXACT_HOST_RE.test(selectedHost); useAdminUnsavedChanges('branding', hasEdits); + const requestLeave = useAdminUnsavedStore((s) => s.requestLeave); + const { dialogProps: confirmProps, confirm } = useConfirmDialog(); if (loading) { return ( @@ -419,6 +432,7 @@ export function BrandingPage() { setMessage(null)} /> +
diff --git a/app/(main)/admin/_pages/extension-detail.tsx b/app/(main)/admin/_pages/extension-detail.tsx index 51d9d8e2b..361260db4 100644 --- a/app/(main)/admin/_pages/extension-detail.tsx +++ b/app/(main)/admin/_pages/extension-detail.tsx @@ -20,6 +20,8 @@ import { inputClass } from '@/components/admin/config-items'; import { ExtensionIcon } from '@/components/admin/extension-icon'; import { cn } from '@/lib/utils'; import { useExtensionIcons } from './use-extension-icons'; +import { ConfirmDialog } from '@/components/ui/confirm-dialog'; +import { useConfirmDialog } from '@/hooks/use-confirm-dialog'; export interface ConfigField { type: 'string' | 'secret' | 'boolean' | 'number' | 'select'; @@ -72,6 +74,7 @@ interface Props { * can turn it off, what it may do, and the versions kept for a rollback. */ export function ExtensionDetailPage({ kind, id }: Props) { + const { dialogProps: confirmProps, confirm } = useConfirmDialog(); const words = KIND_LABEL[kind]; const listHref = adminPath(kind === 'plugin' ? 'plugins' : 'themes'); const [entry, setEntry] = useState(null); @@ -216,7 +219,13 @@ export function ExtensionDetailPage({ kind, id }: Props) { async function rollback(version: string) { if (!entry) return; - if (!confirm(`Put v${version} of "${entry.name}" back in place of v${entry.version}? The copy kept on this server is used.`)) return; + const ok = await confirm({ + title: `Roll back to v${version}?`, + message: `"${entry.name}" v${version} replaces v${entry.version}. The copy kept on this server is used.`, + confirmText: 'Roll back', + cancelText: 'Cancel', + }); + if (!ok) return; setBusy(`rollback:${version}`); setMessage(null); try { @@ -239,7 +248,14 @@ export function ExtensionDetailPage({ kind, id }: Props) { async function remove() { if (!entry) return; - if (!confirm(`Remove "${entry.name}" for every user? Its kept versions are removed too; this cannot be undone.`)) return; + const ok = await confirm({ + title: `Remove "${entry.name}"?`, + message: 'It is removed for every user, with the versions kept on this server. This cannot be undone.', + confirmText: 'Remove', + cancelText: 'Cancel', + variant: 'destructive', + }); + if (!ok) return; setBusy('remove'); setMessage(null); const res = await apiFetch(words.endpoint, { @@ -324,6 +340,7 @@ export function ExtensionDetailPage({ kind, id }: Props) { {entry.description &&

{entry.description}

} setMessage(null)} /> +
diff --git a/app/(main)/admin/_pages/marketplace-detail.tsx b/app/(main)/admin/_pages/marketplace-detail.tsx index aae7b42f9..0eaa086e4 100644 --- a/app/(main)/admin/_pages/marketplace-detail.tsx +++ b/app/(main)/admin/_pages/marketplace-detail.tsx @@ -37,6 +37,8 @@ import { AdminButtonLink } from '@/components/admin/button-link'; import { Badge } from '@/components/admin/badge'; import { Flash, useFlash } from '@/components/admin/flash-message'; import { installVersion } from './marketplace'; +import { ConfirmDialog } from '@/components/ui/confirm-dialog'; +import { useConfirmDialog } from '@/hooks/use-confirm-dialog'; const CURRENT_APP_VERSION = process.env.NEXT_PUBLIC_APP_VERSION || '0.0.0'; @@ -129,6 +131,7 @@ const tdClass = 'px-4 py-2 align-top'; const smallButtonClass = 'h-8 gap-1 text-xs'; export function MarketplaceDetailPage({ slug }: { slug: string }) { + const { dialogProps: confirmProps, confirm } = useConfirmDialog(); const [data, setData] = useState(null); const [loading, setLoading] = useState(true); @@ -184,7 +187,7 @@ export function MarketplaceDetailPage({ slug }: { slug: string }) { const isUpdate = data.installed; setBusy(`install:${version}`); setMessage(null); - const result = await installVersion({ slug: data.extension.slug, version, type: data.extension.type }); + const result = await installVersion({ slug: data.extension.slug, version, type: data.extension.type }, confirm); setBusy(null); if (!result.ok) { setMessage({ type: 'error', text: result.error }); @@ -203,7 +206,13 @@ export function MarketplaceDetailPage({ slug }: { slug: string }) { async function handleRollback(version: string) { if (!data) return; - if (!confirm(`Put v${version} of "${data.extension.name}" back in place of v${data.installedVersion}? The copy kept on this server is used; the directory is not asked.`)) return; + const ok = await confirm({ + title: `Roll back to v${version}?`, + message: `"${data.extension.name}" v${version} replaces v${data.installedVersion}. The copy kept on this server is used; the directory is not asked.`, + confirmText: 'Roll back', + cancelText: 'Cancel', + }); + if (!ok) return; setBusy(`rollback:${version}`); setMessage(null); try { @@ -228,7 +237,14 @@ export function MarketplaceDetailPage({ slug }: { slug: string }) { async function handleUninstall() { if (!data) return; - if (!confirm(`Remove "${data.extension.name}"? Its kept versions are removed too; this cannot be undone.`)) return; + const ok = await confirm({ + title: `Remove "${data.extension.name}"?`, + message: 'The versions kept on this server are removed too. This cannot be undone.', + confirmText: 'Remove', + cancelText: 'Cancel', + variant: 'destructive', + }); + if (!ok) return; setUninstalling(true); setMessage(null); @@ -414,6 +430,7 @@ export function MarketplaceDetailPage({ slug }: { slug: string }) { )} setMessage(null)} /> +
{data.requestedVersionMissing && ( diff --git a/app/(main)/admin/_pages/marketplace.tsx b/app/(main)/admin/_pages/marketplace.tsx index a58925975..68e34ce2f 100644 --- a/app/(main)/admin/_pages/marketplace.tsx +++ b/app/(main)/admin/_pages/marketplace.tsx @@ -20,6 +20,8 @@ import { Badge } from '@/components/admin/badge'; import { EmptyState } from '@/components/admin/empty-state'; import { Flash, useFlash } from '@/components/admin/flash-message'; import { ConfigNote, inputClass } from '@/components/admin/config-items'; +import { ConfirmDialog } from '@/components/ui/confirm-dialog'; +import { useConfirmDialog } from '@/hooks/use-confirm-dialog'; const CURRENT_APP_VERSION = process.env.NEXT_PUBLIC_APP_VERSION || '0.0.0'; @@ -78,12 +80,19 @@ const pillClass = (active: boolean) => cn( /** * Installs one version of an extension. A version the publisher withdrew, * or one older than the installed version, comes back as 409; the admin is - * asked, and the install is repeated with `force` if they insist. Returns - * the error text to show, or null. + * asked (in the page's ConfirmDialog), and the install is repeated with + * `force` if they insist. Returns the error text to show, or null. */ +export interface InstallQuestion { + title: string; + message: string; + confirmText: string; + cancelText: string; +} + export async function installVersion( body: { slug: string; version: string; type: 'plugin' | 'theme' }, - confirm: (message: string) => boolean = (m) => window.confirm(m), + confirm: (question: InstallQuestion) => boolean | Promise, ): Promise<{ ok: true; warnings: string[]; signature: string | null; updated: boolean } | { ok: false; error: string }> { const send = (force: boolean) => apiFetch('/api/admin/marketplace', { method: 'POST', @@ -95,10 +104,14 @@ export async function installVersion( if (res.status === 409) { const data = await res.json().catch(() => ({})); if (data.yanked) { - if (!confirm(`${data.error}\n\nInstall it anyway?`)) return { ok: false, error: 'Not installed.' }; + if (!(await confirm({ title: 'Install it anyway?', message: data.error, confirmText: 'Install', cancelText: 'Cancel' }))) { + return { ok: false, error: 'Not installed.' }; + } res = await send(true); } else if (data.downgrade) { - if (!confirm(`${data.error}\n\nInstall the older version anyway?`)) return { ok: false, error: 'Not installed.' }; + if (!(await confirm({ title: 'Install the older version anyway?', message: data.error, confirmText: 'Install', cancelText: 'Cancel' }))) { + return { ok: false, error: 'Not installed.' }; + } res = await send(true); } else { return { ok: false, error: data.error || 'Installation failed' }; @@ -113,6 +126,7 @@ export async function installVersion( } export function MarketplacePage() { + const { dialogProps: confirmProps, confirm } = useConfirmDialog(); const [extensions, setExtensions] = useState([]); const [loading, setLoading] = useState(true); const [query, setQuery] = useState(''); @@ -194,7 +208,7 @@ export function MarketplacePage() { async function install(ext: { slug: string; name: string; type: 'plugin' | 'theme' }, version: string, isUpdate: boolean) { markInstalling(ext.slug, true); setMessage(null); - const result = await installVersion({ slug: ext.slug, version, type: ext.type }); + const result = await installVersion({ slug: ext.slug, version, type: ext.type }, confirm); markInstalling(ext.slug, false); if (!result.ok) { setMessage({ type: 'error', text: result.error }); @@ -257,6 +271,7 @@ export function MarketplacePage() { /> setMessage(null)} /> +
{updates && noticeCount > 0 && ( diff --git a/components/settings/__tests__/no-native-dialogs.test.ts b/components/settings/__tests__/no-native-dialogs.test.ts new file mode 100644 index 000000000..303453740 --- /dev/null +++ b/components/settings/__tests__/no-native-dialogs.test.ts @@ -0,0 +1,40 @@ +import { readdirSync, readFileSync, statSync } from 'node:fs'; +import { join, relative } from 'node:path'; +import { describe, expect, it } from 'vitest'; + +// R4: no window.confirm, alert() or prompt() in Settings and the admin +// panel. Questions go through ConfirmDialog (the useConfirmDialog hook's +// confirm({ ... }) is fine), results through toasts. + +const ROOT = join(__dirname, '..', '..', '..'); +const DIRS = ['components/settings', 'components/admin', 'components/filters', 'components/identity', 'app/(main)/admin']; + +function sources(dir: string): string[] { + return readdirSync(dir).flatMap((name) => { + const path = join(dir, name); + if (statSync(path).isDirectory()) return name === '__tests__' ? [] : sources(path); + return /\.tsx?$/.test(name) ? [path] : []; + }); +} + +const NATIVE = [ + /\bwindow\.(confirm|alert|prompt)\s*\(/, + /(? { + it('are not used', () => { + const found: string[] = []; + for (const dir of DIRS) { + for (const file of sources(join(ROOT, dir))) { + readFileSync(file, 'utf8').split('\n').forEach((line, i) => { + if (line.trim().startsWith('//') || line.trim().startsWith('*')) return; + if (NATIVE.some((re) => re.test(line))) found.push(`${relative(ROOT, file)}:${i + 1}: ${line.trim()}`); + }); + } + } + expect(found).toEqual([]); + }); +});