From 719563dd2262c24112ff61fde21f2bddf239fcb2 Mon Sep 17 00:00:00 2001 From: 2GT-Rich <107219997+2GT-Rich@users.noreply.github.com> Date: Mon, 7 Sep 2026 21:05:55 -0700 Subject: [PATCH] feat(backups): filter, preview, compare and bulk delete (v0.24.2-beta) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #134. Backups are `/export compact` output — plain RouterOS script, not the binary `.backup` blob — which is what makes the reporter's "it's just a text, after all" correct and most of this cheap. - Filters: device, type, date range and free text over device name, filename and notes. Dates are inclusive at both ends, because a picker showing "to: 3 Sep" that stops at that day's first instant is a trap. - Preview in place, on row click. No round trip through the downloads folder to see what a backup contains. - Compare any two, reusing the lineDiff that Config History already uses rather than adding a second diff implementation. - Bulk delete, with selection and a confirmation that names what goes. Two deliberate choices. Bulk delete had to respect an existing invariant: a backup and its config snapshot are one artifact, joined by ON DELETE CASCADE so they cannot drift apart. Removing fifty in one click can therefore destroy restore points nobody looked at. The confirmation is built from a server-side preview that reports the backups per device *and* how many config snapshots will go with them, and it is capped at 200 per call. The action is also site-scoped, so it cannot reach into another site's history. The Filename column is gone from the table but the filename is not gone: it is the natural handle for a file — device and timestamp, the thing you would quote to a colleague — so it moved to the preview and diff headers where you are actually looking at that file, rather than being dropped. Diff direction is extracted and tested. Selection order in a table is whatever the user happened to click and carries no meaning, so the pair is ordered oldest-first; rendered the other way round every addition reads as a deletion, and nothing about the output looks wrong while it does. Incidentally fixed: 'pre-upgrade' backups were labelled "Manual" in the table because the badge only knew three types. Type wording now has one home. The list query's placeholder counter is replaced by a bind() helper deriving from params.length. The counter's final increment was dead, and whoever added the next filter would have inherited a stale index. Verified against 110 real backups: every filter and combination returns the expected counts and partitions cleanly, preview and diff read real .rsc with file_path withheld, and bulk delete removed three throwaway backups plus their files and left the other 110 untouched. Guards checked: empty, missing, non-numeric and injected ids, unknown ids, and the 200 cap. Co-Authored-By: Claude Opus 5 --- README.md | 4 +- backend/package.json | 2 +- backend/src/routes/backups.ts | 209 +++++++++++- frontend/package.json | 2 +- .../components/backups/BackupViewerModal.tsx | 172 ++++++++++ frontend/src/pages/BackupsPage.tsx | 311 +++++++++++++++++- frontend/src/services/api.ts | 49 ++- frontend/src/utils/backupCompare.test.ts | 44 +++ frontend/src/utils/backupCompare.ts | 40 +++ frontend/src/version.ts | 2 +- 10 files changed, 793 insertions(+), 42 deletions(-) create mode 100644 frontend/src/components/backups/BackupViewerModal.tsx create mode 100644 frontend/src/utils/backupCompare.test.ts create mode 100644 frontend/src/utils/backupCompare.ts diff --git a/README.md b/README.md index 4c1d8dc..a09a23b 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@

- Version + Version License Docker TypeScript @@ -340,7 +340,7 @@ Each network service supports multi-device management with conflict detection: ### Fleet operations - **Firmware orchestration** — fleet version overview, live update checks, and MikroTik's official release notes in-app. Staged rollouts run in **waves** (wave 1 = canary) through a verified pipeline per device: pre-upgrade backup → install → ride out the reboot → confirm it returned healthy on the new version → next. **Halt on failure** stops the rollout if any device fails, and a device that comes back on the *old* version counts as a failure. Rollouts can be scheduled, and cancelling never interrupts an in-flight flash -- **Backups** — on demand or on a daily/weekly/monthly schedule for all online devices, downloadable from the UI. Config snapshots and their restorable `.rsc` are one artifact: delete either and both go, so they never drift apart +- **Backups** — on demand or on a daily/weekly/monthly schedule for all online devices. Filter by device, type and date range; read one in place instead of downloading it; and compare any two as a diff — they are `/export` text, not opaque blobs. Config snapshots and their restorable `.rsc` are one artifact: delete either and both go, so they never drift apart, and a bulk delete says how many snapshots it will take with it - **Configuration templates** — reusable sets (DNS, NTP, syslog) pushed to many devices with per-device result reporting - **Bulk device add** — "Try All" on discovered devices runs as a server-side job that survives a closed browser tab, with live progress and cancel - **Device organisation** — colour-coded tags, rack location, physical address with map support, and per-device notes diff --git a/backend/package.json b/backend/package.json index 6426ae8..acab926 100644 --- a/backend/package.json +++ b/backend/package.json @@ -1,6 +1,6 @@ { "name": "mikrotik-manager-backend", - "version": "0.24.1-beta", + "version": "0.24.2-beta", "description": "Mikrotik Network Management Platform - Backend API", "main": "dist/index.js", "scripts": { diff --git a/backend/src/routes/backups.ts b/backend/src/routes/backups.ts index ad9f58e..fa5878f 100644 --- a/backend/src/routes/backups.ts +++ b/backend/src/routes/backups.ts @@ -12,27 +12,130 @@ router.use(requireAuth); const backupService = new BackupService(); -// GET /api/backups -router.get('/', async (req: Request, res: Response) => { - const { deviceId } = req.query; - let sql = ` - SELECT b.*, d.name as device_name - FROM backups b JOIN devices d ON d.id = b.device_id - WHERE 1=1 - `; - const params: unknown[] = []; - if (deviceId) { - sql += ` AND b.device_id = $1`; - params.push(deviceId); - } - const siteFilter = siteScopeByDevice(activeSite(req), 'b.device_id'); - if (siteFilter) sql += ` AND ${siteFilter}`; - sql += ' ORDER BY b.created_at DESC'; +/** Preview reads the file into memory, so it needs a ceiling. */ +const MAX_PREVIEW_BYTES = 2 * 1024 * 1024; +/** A bulk delete is irreversible; keep one click from reaching an unbounded set. */ +const MAX_BULK_DELETE = 200; - const backups = await query(sql, params); +// GET /api/backups — filter by device, type, date range and free text (#134) +router.get('/', async (req: Request, res: Response) => { + const { deviceId, type, from, to, search } = req.query as Record; + + const filters: string[] = []; + const params: unknown[] = []; + /** + * Bind a value and return its placeholder. + * + * Derived from params.length rather than a hand-kept counter: with a counter, + * the last filter's increment is dead and whoever adds the next one inherits a + * stale index. This cannot get out of step. + */ + const bind = (v: unknown): string => { params.push(v); return `$${params.length}`; }; + + if (deviceId) filters.push(`b.device_id = ${bind(deviceId)}`); + if (type) filters.push(`b.backup_type = ${bind(type)}`); + // Dates are inclusive at both ends: a picker showing "to: 3 Sep" should include + // everything that happened on the 3rd, not stop at its first instant. + if (from) filters.push(`b.created_at >= ${bind(from)}`); + if (to) filters.push(`b.created_at < (${bind(to)}::date + INTERVAL '1 day')`); + if (search) { + const p = bind(`%${search}%`); + filters.push(`(b.filename ILIKE ${p} OR b.notes ILIKE ${p} OR d.name ILIKE ${p})`); + } + + const siteFilter = siteScopeByDevice(activeSite(req), 'b.device_id'); + if (siteFilter) filters.push(siteFilter); + + const where = filters.length ? `WHERE ${filters.join(' AND ')}` : ''; + const backups = await query( + `SELECT b.*, d.name as device_name + FROM backups b JOIN devices d ON d.id = b.device_id + ${where} + ORDER BY b.created_at DESC`, + params + ); res.json(backups); }); +// GET /api/backups/types — the backup types actually present, for the filter +router.get('/types', async (req: Request, res: Response) => { + const siteFilter = siteScopeByDevice(activeSite(req), 'b.device_id'); + const rows = await query<{ backup_type: string; n: string }>( + `SELECT b.backup_type, COUNT(*)::text AS n + FROM backups b + ${siteFilter ? `WHERE ${siteFilter}` : ''} + GROUP BY b.backup_type ORDER BY b.backup_type` + ); + res.json(rows.map((r) => ({ type: r.backup_type, count: parseInt(r.n, 10) }))); +}); + +/** + * POST /api/backups/bulk-delete + * + * Deleting a backup cascades to the config snapshot that shares it — they are + * deliberately one artifact so they cannot drift apart. Doing that fifty times + * from one click deserves to say so, so the count of snapshots that will go with + * them is reported here and shown in the confirmation. + */ +router.post('/bulk-delete', requireWrite, async (req: Request, res: Response) => { + const { ids } = req.body as { ids?: unknown }; + if (!Array.isArray(ids) || ids.length === 0) { + return res.status(400).json({ error: 'ids array is required' }); + } + const clean = ids.map((v) => parseInt(String(v), 10)).filter(Number.isInteger); + if (clean.length === 0) return res.status(400).json({ error: 'ids array is required' }); + if (clean.length > MAX_BULK_DELETE) { + return res.status(400).json({ error: `Cannot delete more than ${MAX_BULK_DELETE} backups at once` }); + } + + // Only ever touch backups in the active site, so a bulk action cannot reach + // across into another site's history. + const siteFilter = siteScopeByDevice(activeSite(req), 'device_id'); + const allowed = await query<{ id: number }>( + `SELECT id FROM backups WHERE id = ANY($1::int[]) ${siteFilter ? `AND ${siteFilter}` : ''}`, + [clean] + ); + const allowedIds = allowed.map((r) => r.id); + if (allowedIds.length === 0) return res.status(404).json({ error: 'No matching backups' }); + + let deleted = 0; + const failures: { id: number; error: string }[] = []; + for (const id of allowedIds) { + try { await backupService.deleteBackup(id); deleted++; } + catch (e) { failures.push({ id, error: (e as Error).message }); } + } + return res.json({ deleted, requested: clean.length, failures }); +}); + +// GET /api/backups/bulk-delete/preview?ids=1,2,3 — what a bulk delete would remove +router.get('/bulk-delete/preview', async (req: Request, res: Response) => { + const ids = String(req.query.ids || '') + .split(',').map((v) => parseInt(v, 10)).filter(Number.isInteger); + if (ids.length === 0) return res.json({ backups: 0, snapshots: 0, devices: [] }); + + const siteFilter = siteScopeByDevice(activeSite(req), 'b.device_id'); + const rows = await query<{ n: string; snapshots: string }>( + `SELECT COUNT(*)::text AS n, + COUNT(dc.id)::text AS snapshots + FROM backups b + LEFT JOIN device_configs dc ON dc.backup_id = b.id + WHERE b.id = ANY($1::int[]) ${siteFilter ? `AND ${siteFilter}` : ''}`, + [ids] + ); + const devices = await query<{ name: string; n: string }>( + `SELECT d.name, COUNT(*)::text AS n + FROM backups b JOIN devices d ON d.id = b.device_id + WHERE b.id = ANY($1::int[]) ${siteFilter ? `AND ${siteFilter}` : ''} + GROUP BY d.name ORDER BY d.name`, + [ids] + ); + return res.json({ + backups: parseInt(rows[0]?.n || '0', 10), + snapshots: parseInt(rows[0]?.snapshots || '0', 10), + devices: devices.map((d) => ({ name: d.name, count: parseInt(d.n, 10) })), + }); +}); + // POST /api/backups router.post('/', requireWrite, async (req: Request, res: Response) => { const { deviceId, notes } = req.body; @@ -93,4 +196,76 @@ router.delete('/:id', requireWrite, async (req: Request, res: Response) => { return res.json({ message: 'Backup deleted' }); }); +/** + * Read a backup's text. + * + * Backups are `/export compact` output, i.e. plain RouterOS script — not the + * binary `.backup` blob — so they can be read in place instead of downloaded + * and opened elsewhere (#134). + */ +async function readBackupText(id: string | number): Promise< + { meta: BackupMeta; text: string } | { error: string; status: number } +> { + const meta = await queryOne( + `SELECT b.id, b.filename, b.size_bytes, b.backup_type, b.notes, b.created_at, + b.file_path, b.device_id, d.name AS device_name + FROM backups b JOIN devices d ON d.id = b.device_id + WHERE b.id = $1`, + [id] + ); + if (!meta) return { error: 'Backup not found', status: 404 }; + if (!fs.existsSync(meta.file_path)) { + return { error: 'Backup file not found on disk', status: 404 }; + } + const stat = fs.statSync(meta.file_path); + if (stat.size > MAX_PREVIEW_BYTES) { + return { + error: `This backup is ${Math.round(stat.size / 1024)} KB, too large to preview. Download it instead.`, + status: 413, + }; + } + return { meta, text: fs.readFileSync(meta.file_path, 'utf8') }; +} + +interface BackupMeta { + id: number; + filename: string; + size_bytes: number | null; + backup_type: string | null; + notes: string | null; + created_at: string; + file_path: string; + device_id: number; + device_name: string; +} + +// GET /api/backups/:id/content — preview in place +router.get('/:id/content', async (req: Request, res: Response) => { + const result = await readBackupText(req.params.id); + if ('error' in result) return res.status(result.status).json({ error: result.error }); + const { file_path: _omit, ...meta } = result.meta; + return res.json({ ...meta, content: result.text }); +}); + +// GET /api/backups/:fromId/diff/:toId — compare two backups +// Mirrors the config-history diff contract: both texts are returned and the +// browser renders them with the same lineDiff used for config snapshots. +router.get('/:fromId/diff/:toId', async (req: Request, res: Response) => { + const [from, to] = await Promise.all([ + readBackupText(req.params.fromId), + readBackupText(req.params.toId), + ]); + for (const r of [from, to]) { + if ('error' in r) return res.status(r.status).json({ error: r.error }); + } + const f = from as { meta: BackupMeta; text: string }; + const t = to as { meta: BackupMeta; text: string }; + return res.json({ + from: { id: f.meta.id, filename: f.meta.filename, device_name: f.meta.device_name, + created_at: f.meta.created_at, text: f.text }, + to: { id: t.meta.id, filename: t.meta.filename, device_name: t.meta.device_name, + created_at: t.meta.created_at, text: t.text }, + }); +}); + export default router; diff --git a/frontend/package.json b/frontend/package.json index 68d2d57..093f279 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "mikrotik-manager-frontend", - "version": "0.24.1-beta", + "version": "0.24.2-beta", "private": true, "type": "module", "scripts": { diff --git a/frontend/src/components/backups/BackupViewerModal.tsx b/frontend/src/components/backups/BackupViewerModal.tsx new file mode 100644 index 0000000..a551e3d --- /dev/null +++ b/frontend/src/components/backups/BackupViewerModal.tsx @@ -0,0 +1,172 @@ +import { useQuery } from '@tanstack/react-query'; +import { X, FileText, GitCompare, Download, Loader2, AlertCircle } from 'lucide-react'; +import { format } from 'date-fns'; +import clsx from 'clsx'; +import { backupsApi } from '../../services/api'; +import { lineDiff } from '../../utils/lineDiff'; + +/** + * Reads a backup in place, or compares two (#134). + * + * Backups are `/export compact` output — plain RouterOS script, not the binary + * `.backup` blob — so both are just text. The diff reuses the same lineDiff + * that Config History already uses, rather than introducing a second one. + */ +interface Props { + /** One id previews; two compare, oldest first. */ + ids: number[]; + onClose: () => void; + onDownload: (id: number, filename: string) => void; +} + +function errorText(e: unknown): string { + const err = e as { response?: { data?: { error?: string } }; message?: string }; + return err.response?.data?.error ?? err.message ?? 'Could not read this backup.'; +} + +export default function BackupViewerModal({ ids, onClose, onDownload }: Props) { + const comparing = ids.length === 2; + + const single = useQuery({ + queryKey: ['backup-content', ids[0]], + queryFn: () => backupsApi.content(ids[0]).then((r) => r.data), + enabled: !comparing && ids.length === 1, + retry: false, + }); + + const pair = useQuery({ + queryKey: ['backup-diff', ids[0], ids[1]], + queryFn: () => backupsApi.diff(ids[0], ids[1]).then((r) => r.data), + enabled: comparing, + retry: false, + }); + + const loading = comparing ? pair.isLoading : single.isLoading; + const error = comparing ? pair.error : single.error; + + const rows = pair.data ? lineDiff(pair.data.from.text, pair.data.to.text) : []; + const added = rows.filter((r) => r.type === 'add').length; + const removed = rows.filter((r) => r.type === 'del').length; + + return ( +

+
e.stopPropagation()} + > +
+ {comparing + ? + : } +
+ {comparing ? ( + <> +

+ Comparing backups +

+ {pair.data && ( + // The filename lives here rather than in the table: it is how you + // would refer to this file to someone else, so it belongs where + // you are actually looking at the file. +

+ {pair.data.from.filename} → {pair.data.to.filename} +

+ )} + + ) : ( + <> +

+ {single.data?.device_name ?? 'Backup'} +

+ {single.data && ( +

+ {single.data.filename} +

+ )} + + )} + {!comparing && single.data && ( +

+ {format(new Date(single.data.created_at), 'MMM d, yyyy HH:mm')} + {single.data.notes ? ` · ${single.data.notes}` : ''} +

+ )} + {comparing && pair.data && ( +

+ +{added}{' '} + −{removed} + + {' '}· {format(new Date(pair.data.from.created_at), 'MMM d HH:mm')} + {' → '}{format(new Date(pair.data.to.created_at), 'MMM d HH:mm')} + +

+ )} +
+ {!comparing && single.data && ( + + )} + +
+ +
+ {loading && ( +
+ Reading… +
+ )} + + {!loading && error && ( +
+ +

{errorText(error)}

+
+ )} + + {!loading && !error && comparing && ( + added + removed === 0 ? ( +

+ These two backups are identical. +

+ ) : ( + 
+                {rows.map((row, i) => (
+                  

+                    {row.type === 'add' ? '+' : row.type === 'del' ? '−' : ' '} {row.text}
+                  

+                ))}
+
+ ) + )} + + {!loading && !error && !comparing && single.data && ( + 
+              {single.data.content}
+
+ )} +
+
+
+ ); +} diff --git a/frontend/src/pages/BackupsPage.tsx b/frontend/src/pages/BackupsPage.tsx index 6e97f0e..4f201e6 100644 --- a/frontend/src/pages/BackupsPage.tsx +++ b/frontend/src/pages/BackupsPage.tsx @@ -1,12 +1,25 @@ import { useState } from 'react'; import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'; -import { HardDrive, Plus, Download, RotateCcw, Trash2, AlertCircle, AlertTriangle, Loader2, X } from 'lucide-react'; +import { + HardDrive, Plus, Download, RotateCcw, Trash2, AlertCircle, AlertTriangle, + Loader2, X, Search, GitCompare, FileText, FilterX, +} from 'lucide-react'; import { backupsApi, devicesApi } from '../services/api'; import { useCanWrite } from '../hooks/useCanWrite'; import type { Backup } from '../types'; +import BackupViewerModal from '../components/backups/BackupViewerModal'; +import { orderForDiff } from '../utils/backupCompare'; import { format } from 'date-fns'; import clsx from 'clsx'; +/** One home for backup-type wording, used by the filter and the table badge. */ +const TYPE_LABEL: Record = { + 'config-snapshot': 'Config Snapshot', + scheduled: 'Scheduled', + manual: 'Manual', + 'pre-upgrade': 'Pre-upgrade', +}; + function formatBytes(bytes?: number): string { if (!bytes) return '—'; if (bytes >= 1024) return `${(bytes / 1024).toFixed(1)} KB`; @@ -25,11 +38,63 @@ export default function BackupsPage() { const [snapshotDeleteTarget, setSnapshotDeleteTarget] = useState(null); const [error, setError] = useState(''); + // Filters (#134). Held together so the query key is a single object. + const [filters, setFilters] = useState({ deviceId: '', type: '', from: '', to: '', search: '' }); + const activeFilterCount = Object.values(filters).filter(Boolean).length; + const clearFilters = () => setFilters({ deviceId: '', type: '', from: '', to: '', search: '' }); + + const [selected, setSelected] = useState([]); + const [viewing, setViewing] = useState(null); + const [bulkConfirm, setBulkConfirm] = useState(false); + const { data: backups = [], isLoading } = useQuery({ - queryKey: ['backups'], - queryFn: () => backupsApi.list().then((r) => r.data), + queryKey: ['backups', filters], + queryFn: () => backupsApi.list(filters).then((r) => r.data), }); + const { data: types = [] } = useQuery({ + queryKey: ['backup-types'], + queryFn: () => backupsApi.types().then((r) => r.data), + }); + + // What a bulk delete would actually remove, including the config snapshots + // that go with it. Fetched before asking, so the confirmation can be specific + // rather than "are you sure?". + const { data: bulkPreview } = useQuery({ + queryKey: ['backup-bulk-preview', selected], + queryFn: () => backupsApi.bulkDeletePreview(selected).then((r) => r.data), + enabled: bulkConfirm && selected.length > 0, + }); + + const bulkDeleteMutation = useMutation({ + mutationFn: () => backupsApi.bulkDelete(selected).then((r) => r.data), + onSuccess: (res) => { + queryClient.invalidateQueries({ queryKey: ['backups'] }); + queryClient.invalidateQueries({ queryKey: ['backup-types'] }); + setBulkConfirm(false); + setSelected([]); + if (res.failures.length > 0) { + setError(`${res.deleted} deleted, ${res.failures.length} failed.`); + } + }, + onError: (err: unknown) => { + const msg = (err as { response?: { data?: { error?: string } } })?.response?.data?.error; + setError(msg || 'Bulk delete failed'); + setBulkConfirm(false); + }, + }); + + const toggle = (id: number) => + setSelected((cur) => (cur.includes(id) ? cur.filter((x) => x !== id) : [...cur, id])); + const allShownSelected = backups.length > 0 && backups.every((b) => selected.includes(b.id)); + const toggleAll = () => + setSelected(allShownSelected ? [] : backups.map((b) => b.id)); + + // Exactly two, oldest first, so the diff reads forwards in time (tested in + // utils/backupCompare — a reversed diff shows additions as deletions and + // looks perfectly normal while doing it). + const comparePair = orderForDiff(selected, backups); + const { data: devices = [] } = useQuery({ queryKey: ['devices'], queryFn: () => devicesApi.list().then((r) => r.data), @@ -169,16 +234,110 @@ export default function BackupsPage() { )} + {/* Filters (#134) */} +
+
+ +
+ + setFilters((f) => ({ ...f, search: e.target.value }))} + /> +
+
+
+ + +
+
+ + +
+
+ + setFilters((f) => ({ ...f, from: e.target.value }))} /> +
+
+ + setFilters((f) => ({ ...f, to: e.target.value }))} /> +
+ {activeFilterCount > 0 && ( + + )} +
+ + {/* Selection actions */} + {selected.length > 0 && ( +
+ + {selected.length} selected + + +
+ + {canWrite && ( + + )} +
+
+ )} + {isLoading ? (
Loading...
) : backups.length === 0 ? (
-

No backups yet

-

- Create a backup to save your device configurations +

+ {activeFilterCount > 0 ? 'No backups match these filters' : 'No backups yet'}

+

+ {activeFilterCount > 0 + ? 'Try widening the date range or clearing a filter.' + : 'Create a backup to save your device configurations'} +

+ {activeFilterCount > 0 && ( + + )}
) : ( @@ -187,24 +346,46 @@ export default function BackupsPage() { + - - + {backups.map((backup) => ( - + setViewing([backup.id])} + title="Click to preview" + className={clsx( + 'cursor-pointer hover:bg-gray-50 dark:hover:bg-slate-700/30', + selected.includes(backup.id) && 'bg-blue-50/60 dark:bg-blue-900/10' + )} + > + - -
+ + DeviceFilename Type Size Created NotesActionsActions
e.stopPropagation()}> + toggle(backup.id)} + className="cursor-pointer" + /> + {backup.device_name || '—'} - {backup.filename} - - {backup.backup_type === 'config-snapshot' - ? 'Config Snapshot' - : backup.backup_type === 'scheduled' - ? 'Scheduled' - : 'Manual'} + {TYPE_LABEL[backup.backup_type ?? ''] ?? backup.backup_type ?? 'Manual'} @@ -238,8 +415,15 @@ export default function BackupsPage() { {backup.notes || ''} + e.stopPropagation()}>
+
)} + {viewing && ( + setViewing(null)} + onDownload={downloadBackup} + /> + )} + + {/* Bulk delete confirmation. + Backups and their config snapshots are deliberately one artifact, so a + bulk delete can remove restore points the operator never looked at. + The confirmation therefore names exactly what goes, per device, and + says plainly that there is no undo. */} + {bulkConfirm && ( +
+
+
+

+ + Delete {selected.length} backup{selected.length === 1 ? '' : 's'}? +

+ +
+
+ {!bulkPreview ? ( +
+ Checking what this removes… +
+ ) : ( + <> +
+
    + {bulkPreview.devices.map((d) => ( +
  • + {d.name} + {d.count} backup{d.count === 1 ? '' : 's'} +
    • + ))} +
+
+ + {bulkPreview.snapshots > 0 && ( +
+

+ {bulkPreview.snapshots} of these + {bulkPreview.snapshots === 1 ? ' is' : ' are'} tied to a{' '} + Config History snapshot. Deleting + {bulkPreview.snapshots === 1 ? ' it' : ' them'} also removes + {bulkPreview.snapshots === 1 ? ' that snapshot' : ' those snapshots'} and the + ability to roll back to {bulkPreview.snapshots === 1 ? 'it' : 'them'}. +

+
+ )} + +

+ This cannot be undone. +

+ + )} + +
+ + +
+
+
+
+ )} + {/* Confirm deletion of a snapshot-linked backup (removes the config snapshot too) */} {snapshotDeleteTarget && (
diff --git a/frontend/src/services/api.ts b/frontend/src/services/api.ts index 114af6a..4b93351 100644 --- a/frontend/src/services/api.ts +++ b/frontend/src/services/api.ts @@ -965,15 +965,60 @@ export const operationsApi = { syncAll: () => api.post('/operations/sync-all', undefined, { timeout: 180_000 }), }; +export interface BackupFilters { + deviceId?: number | string; + type?: string; + from?: string; + to?: string; + search?: string; +} + +export interface BackupContent { + id: number; + filename: string; + device_name: string; + device_id: number; + backup_type: string | null; + notes: string | null; + size_bytes: number | null; + created_at: string; + content: string; +} + +export interface BackupDiffSide { + id: number; + filename: string; + device_name: string; + created_at: string; + text: string; +} + export const backupsApi = { - list: (deviceId?: number) => - api.get('/backups', { params: deviceId ? { deviceId } : {} }), + list: (filters: BackupFilters = {}) => { + // Drop empty values so an unset filter doesn't become `?type=`. + const params = Object.fromEntries( + Object.entries(filters).filter(([, v]) => v !== undefined && v !== '') + ); + return api.get('/backups', { params }); + }, + types: () => api.get<{ type: string; count: number }[]>('/backups/types'), create: (deviceId: number, notes?: string) => api.post('/backups', { deviceId, notes }), download: (id: number) => api.get(`/backups/${id}/download`, { responseType: 'blob' }), + content: (id: number) => api.get(`/backups/${id}/content`), + diff: (fromId: number, toId: number) => + api.get<{ from: BackupDiffSide; to: BackupDiffSide }>(`/backups/${fromId}/diff/${toId}`), restore: (id: number) => api.post(`/backups/${id}/restore`), delete: (id: number) => api.delete(`/backups/${id}`), + bulkDeletePreview: (ids: number[]) => + api.get<{ backups: number; snapshots: number; devices: { name: string; count: number }[] }>( + '/backups/bulk-delete/preview', { params: { ids: ids.join(',') } } + ), + bulkDelete: (ids: number[]) => + api.post<{ deleted: number; requested: number; failures: { id: number; error: string }[] }>( + '/backups/bulk-delete', { ids } + ), }; // ─── Config History ──────────────────────────────────────────────────────────── diff --git a/frontend/src/utils/backupCompare.test.ts b/frontend/src/utils/backupCompare.test.ts new file mode 100644 index 0000000..abb145b --- /dev/null +++ b/frontend/src/utils/backupCompare.test.ts @@ -0,0 +1,44 @@ +import { describe, it, expect } from 'vitest'; +import { orderForDiff, type ComparableBackup } from './backupCompare'; + +const backups: ComparableBackup[] = [ + { id: 10, created_at: '2026-09-01T10:00:00Z' }, + { id: 20, created_at: '2026-09-05T10:00:00Z' }, + { id: 30, created_at: '2026-09-08T10:00:00Z' }, +]; + +describe('orderForDiff', () => { + // The property that matters: whichever order the user clicked in, the diff + // reads forwards in time. Selecting newest-then-oldest must not invert it. + it('always returns [older, newer]', () => { + expect(orderForDiff([10, 30], backups)).toEqual([10, 30]); + expect(orderForDiff([30, 10], backups)).toEqual([10, 30]); + expect(orderForDiff([30, 20], backups)).toEqual([20, 30]); + }); + + it('refuses anything other than a pair', () => { + expect(orderForDiff([], backups)).toBeNull(); + expect(orderForDiff([10], backups)).toBeNull(); + expect(orderForDiff([10, 20, 30], backups)).toBeNull(); + }); + + // Two backups taken in the same second is unlikely but possible; the result + // must still be stable rather than depending on click order. + it('is stable when timestamps tie', () => { + const tied: ComparableBackup[] = [ + { id: 7, created_at: '2026-09-01T10:00:00Z' }, + { id: 9, created_at: '2026-09-01T10:00:00Z' }, + ]; + expect(orderForDiff([9, 7], tied)).toEqual([7, 9]); + expect(orderForDiff([7, 9], tied)).toEqual([7, 9]); + }); + + it('falls back to id when a backup is missing or undated', () => { + expect(orderForDiff([99, 98], [])).toEqual([98, 99]); + const bad: ComparableBackup[] = [ + { id: 1, created_at: 'not-a-date' }, + { id: 2, created_at: 'not-a-date' }, + ]; + expect(orderForDiff([2, 1], bad)).toEqual([1, 2]); + }); +}); diff --git a/frontend/src/utils/backupCompare.ts b/frontend/src/utils/backupCompare.ts new file mode 100644 index 0000000..e2013dc --- /dev/null +++ b/frontend/src/utils/backupCompare.ts @@ -0,0 +1,40 @@ +/** + * Ordering two selected backups for comparison (#134). + * + * A diff is directional: rendered the wrong way round, every addition reads as a + * deletion and vice versa. Nothing about the output looks broken when this is + * backwards, which is exactly why it is worth pinning down — selection order in + * the table is whatever the user happened to click, and carries no meaning. + */ + +export interface ComparableBackup { + id: number; + created_at: string; +} + +/** + * Returns [older, newer] so the diff reads forwards in time. + * + * Returns null unless exactly two are selected: comparing one thing, or three, + * is not a diff, and the caller should disable the action rather than guess. + */ +export function orderForDiff( + selectedIds: number[], + backups: ComparableBackup[] +): [number, number] | null { + if (selectedIds.length !== 2) return null; + + const at = (id: number): number => { + const b = backups.find((x) => x.id === id); + const t = b ? new Date(b.created_at).getTime() : NaN; + return Number.isNaN(t) ? 0 : t; + }; + + const [a, b] = selectedIds; + if (at(a) === at(b)) { + // Same timestamp, or both unknown: fall back to id, which still increases + // with time, so the direction stays meaningful rather than arbitrary. + return a <= b ? [a, b] : [b, a]; + } + return at(a) < at(b) ? [a, b] : [b, a]; +} diff --git a/frontend/src/version.ts b/frontend/src/version.ts index c2458a5..ca409e8 100644 --- a/frontend/src/version.ts +++ b/frontend/src/version.ts @@ -1 +1 @@ -export const APP_VERSION = 'v0.24.1 Beta'; +export const APP_VERSION = 'v0.24.2 Beta';