fix: ask in the app's own dialogs instead of browser confirm boxes in the admin panel

This commit is contained in:
Linus Rath
2026-10-28 03:54:36 +01:00
parent 5ab27a996d
commit f4cade224c
6 changed files with 131 additions and 20 deletions
@@ -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);
});
+21 -7
View File
@@ -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() {
<AdminPageHeader title={PAGE.label} description={PAGE.description} />
<Flash message={message} onDismiss={() => setMessage(null)} />
<ConfirmDialog {...confirmProps} />
<div className="space-y-10">
<SectionNav />
+19 -2
View File
@@ -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<ExtensionEntry | null>(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 && <p className="-mt-2 mb-6 text-sm text-foreground">{entry.description}</p>}
<Flash message={message} onDismiss={() => setMessage(null)} />
<ConfirmDialog {...confirmProps} />
<div className="space-y-10">
<SettingsSection id="availability" title="Availability" description={`Whether users get this ${words.one} and whether they can turn it off.`}>
+20 -3
View File
@@ -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<PreviewData | null>(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 }) {
)}
<Flash message={message} onDismiss={() => setMessage(null)} />
<ConfirmDialog {...confirmProps} />
<div className="mb-6 space-y-3 empty:hidden">
{data.requestedVersionMissing && (
+21 -6
View File
@@ -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<boolean>,
): 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<MarketplaceListing[]>([]);
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() {
/>
<Flash message={message} onDismiss={() => setMessage(null)} />
<ConfirmDialog {...confirmProps} />
<div className="space-y-10">
{updates && noticeCount > 0 && (
@@ -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*\(/,
/(?<![.\w])(alert|prompt)\s*\(/,
// A bare confirm(...) with a message, not the hook's confirm({ ... }).
/(?<![.\w])confirm\s*\((?!\s*\{)/,
];
describe('native browser dialogs in Settings and Admin', () => {
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([]);
});
});