From 030504740dcb257677794e53fb6ade1bd77633e6 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 26 Mar 2026 19:19:35 +0000 Subject: [PATCH 1/3] Fix "Filter required" error when deleting all unfiltered error logs When "Select all" was active with no level/source filter, the batch delete was blocked with a confusing toast. Now falls back to sending the visible entry IDs directly instead of requiring a filter. https://claude.ai/code/session_012v1E6pJFJK1xhWH43SrACu --- client/src/pages/AdminErrors.tsx | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/client/src/pages/AdminErrors.tsx b/client/src/pages/AdminErrors.tsx index e0bdb93..0013fda 100644 --- a/client/src/pages/AdminErrors.tsx +++ b/client/src/pages/AdminErrors.tsx @@ -269,21 +269,25 @@ export default function AdminErrors() { const filters: { level?: string; source?: string } = {}; if (levelFilter !== "all") filters.level = levelFilter; if (sourceFilter !== "all") filters.source = sourceFilter; - if (!filters.level && !filters.source) { - toast({ title: "Filter required", description: "Apply a level or source filter before deleting all entries", variant: "destructive" }); - return; - } - const excluded = Array.from(excludedIds); finalizePrevious(); - batchDeleteMutation.mutate({ - filters, - ...(excluded.length > 0 ? { excludeIds: excluded } : {}), - }); + if (filters.level || filters.source) { + const excluded = Array.from(excludedIds); + batchDeleteMutation.mutate({ + filters, + ...(excluded.length > 0 ? { excludeIds: excluded } : {}), + }); + } else { + const ids = logs + .filter(l => !excludedIds.has(l.id)) + .map(l => l.id); + if (ids.length === 0) return; + batchDeleteMutation.mutate({ ids }); + } } else { finalizePrevious(); batchDeleteMutation.mutate({ ids: Array.from(selectedIds) }); } - }, [finalizePrevious, batchDeleteMutation, selectAll, selectedIds, excludedIds, levelFilter, sourceFilter, toast]); + }, [finalizePrevious, batchDeleteMutation, selectAll, selectedIds, excludedIds, levelFilter, sourceFilter, logs]); const selectionCount = selectAll ? logs.length - excludedIds.size : selectedIds.size; const hasSelection = selectionCount > 0; From 311e60dffba6416896befa69cd0191f1406b52a6 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 26 Mar 2026 19:29:49 +0000 Subject: [PATCH 2/3] Extract batch-delete payload logic into testable utility MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the branching logic from handleBatchDelete into a pure function buildBatchDeletePayload with 11 unit tests covering all code paths. No behavior change — pure refactor of the prior commit's fix. https://claude.ai/code/session_012v1E6pJFJK1xhWH43SrACu --- client/src/pages/AdminErrors.tsx | 29 ++---- client/src/pages/adminErrorsUtils.test.ts | 116 ++++++++++++++++++++++ client/src/pages/adminErrorsUtils.ts | 46 +++++++++ 3 files changed, 169 insertions(+), 22 deletions(-) create mode 100644 client/src/pages/adminErrorsUtils.test.ts create mode 100644 client/src/pages/adminErrorsUtils.ts diff --git a/client/src/pages/AdminErrors.tsx b/client/src/pages/AdminErrors.tsx index 0013fda..c5ba13d 100644 --- a/client/src/pages/AdminErrors.tsx +++ b/client/src/pages/AdminErrors.tsx @@ -1,6 +1,7 @@ import { useState, useRef, useCallback, useEffect } from "react"; import { formatDate, formatDateTime } from "@/lib/date-format"; import { useQuery, useMutation } from "@tanstack/react-query"; +import { buildBatchDeletePayload } from "./adminErrorsUtils"; import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; import { Badge } from "@/components/ui/badge"; import { Button } from "@/components/ui/button"; @@ -265,28 +266,12 @@ export default function AdminErrors() { }, [finalizePrevious, batchDeleteMutation]); const handleBatchDelete = useCallback(() => { - if (selectAll) { - const filters: { level?: string; source?: string } = {}; - if (levelFilter !== "all") filters.level = levelFilter; - if (sourceFilter !== "all") filters.source = sourceFilter; - finalizePrevious(); - if (filters.level || filters.source) { - const excluded = Array.from(excludedIds); - batchDeleteMutation.mutate({ - filters, - ...(excluded.length > 0 ? { excludeIds: excluded } : {}), - }); - } else { - const ids = logs - .filter(l => !excludedIds.has(l.id)) - .map(l => l.id); - if (ids.length === 0) return; - batchDeleteMutation.mutate({ ids }); - } - } else { - finalizePrevious(); - batchDeleteMutation.mutate({ ids: Array.from(selectedIds) }); - } + const payload = buildBatchDeletePayload({ + selectAll, selectedIds, excludedIds, levelFilter, sourceFilter, logs, + }); + if (!payload) return; + finalizePrevious(); + batchDeleteMutation.mutate(payload); }, [finalizePrevious, batchDeleteMutation, selectAll, selectedIds, excludedIds, levelFilter, sourceFilter, logs]); const selectionCount = selectAll ? logs.length - excludedIds.size : selectedIds.size; diff --git a/client/src/pages/adminErrorsUtils.test.ts b/client/src/pages/adminErrorsUtils.test.ts new file mode 100644 index 0000000..5f398c5 --- /dev/null +++ b/client/src/pages/adminErrorsUtils.test.ts @@ -0,0 +1,116 @@ +import { describe, it, expect } from "vitest"; +import { buildBatchDeletePayload } from "./adminErrorsUtils"; + +const defaults = { + selectAll: false, + selectedIds: new Set(), + excludedIds: new Set(), + levelFilter: "all", + sourceFilter: "all", + logs: [{ id: 1 }, { id: 2 }, { id: 3 }], +}; + +describe("buildBatchDeletePayload", () => { + // --- individual selection (selectAll = false) --- + + it("returns ids when individual entries are selected", () => { + const result = buildBatchDeletePayload({ + ...defaults, + selectedIds: new Set([1, 3]), + }); + expect(result).toEqual({ ids: [1, 3] }); + }); + + it("returns null when no individual entries are selected", () => { + const result = buildBatchDeletePayload(defaults); + expect(result).toBeNull(); + }); + + // --- selectAll with filters --- + + it("returns filter-based payload when level filter is active", () => { + const result = buildBatchDeletePayload({ + ...defaults, + selectAll: true, + levelFilter: "error", + }); + expect(result).toEqual({ filters: { level: "error" } }); + }); + + it("returns filter-based payload when source filter is active", () => { + const result = buildBatchDeletePayload({ + ...defaults, + selectAll: true, + sourceFilter: "scheduler", + }); + expect(result).toEqual({ filters: { source: "scheduler" } }); + }); + + it("returns filter-based payload with both filters", () => { + const result = buildBatchDeletePayload({ + ...defaults, + selectAll: true, + levelFilter: "warning", + sourceFilter: "api", + }); + expect(result).toEqual({ filters: { level: "warning", source: "api" } }); + }); + + it("includes excludeIds when filters are active and entries are excluded", () => { + const result = buildBatchDeletePayload({ + ...defaults, + selectAll: true, + levelFilter: "error", + excludedIds: new Set([2]), + }); + expect(result).toEqual({ filters: { level: "error" }, excludeIds: [2] }); + }); + + it("omits excludeIds when the set is empty", () => { + const result = buildBatchDeletePayload({ + ...defaults, + selectAll: true, + levelFilter: "error", + excludedIds: new Set(), + }); + expect(result).toEqual({ filters: { level: "error" } }); + expect(result).not.toHaveProperty("excludeIds"); + }); + + // --- selectAll without filters (the bug fix) --- + + it("falls back to visible log ids when no filters are active", () => { + const result = buildBatchDeletePayload({ + ...defaults, + selectAll: true, + }); + expect(result).toEqual({ ids: [1, 2, 3] }); + }); + + it("excludes entries from the ids fallback when excludedIds is set", () => { + const result = buildBatchDeletePayload({ + ...defaults, + selectAll: true, + excludedIds: new Set([2]), + }); + expect(result).toEqual({ ids: [1, 3] }); + }); + + it("returns null when selectAll is true but all entries are excluded", () => { + const result = buildBatchDeletePayload({ + ...defaults, + selectAll: true, + excludedIds: new Set([1, 2, 3]), + }); + expect(result).toBeNull(); + }); + + it("returns null when selectAll is true with no filters and logs are empty", () => { + const result = buildBatchDeletePayload({ + ...defaults, + selectAll: true, + logs: [], + }); + expect(result).toBeNull(); + }); +}); diff --git a/client/src/pages/adminErrorsUtils.ts b/client/src/pages/adminErrorsUtils.ts new file mode 100644 index 0000000..af4dfa7 --- /dev/null +++ b/client/src/pages/adminErrorsUtils.ts @@ -0,0 +1,46 @@ +/** + * Pure utility for computing the batch-delete payload for admin error logs. + * Extracted so the branching logic can be unit-tested without React. + */ + +export type BatchDeletePayload = + | { ids: number[] } + | { filters: { level?: string; source?: string }; excludeIds?: number[] }; + +/** + * Decides which payload shape to send to POST /api/admin/error-logs/batch-delete. + * + * @returns the payload, or `null` when there is nothing to delete. + */ +export function buildBatchDeletePayload(opts: { + selectAll: boolean; + selectedIds: Set; + excludedIds: Set; + levelFilter: string; + sourceFilter: string; + logs: { id: number }[]; +}): BatchDeletePayload | null { + const { selectAll, selectedIds, excludedIds, levelFilter, sourceFilter, logs } = opts; + + if (!selectAll) { + const ids = Array.from(selectedIds); + return ids.length > 0 ? { ids } : null; + } + + // selectAll path — prefer filter-based delete when a filter is active + const filters: { level?: string; source?: string } = {}; + if (levelFilter !== "all") filters.level = levelFilter; + if (sourceFilter !== "all") filters.source = sourceFilter; + + if (filters.level || filters.source) { + const excluded = Array.from(excludedIds); + return { + filters, + ...(excluded.length > 0 ? { excludeIds: excluded } : {}), + }; + } + + // No filters — fall back to sending visible IDs directly + const ids = logs.filter(l => !excludedIds.has(l.id)).map(l => l.id); + return ids.length > 0 ? { ids } : null; +} From e2aa8b07205f8e1d6a3b609ecb8c37d98c27eccd Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 26 Mar 2026 19:32:51 +0000 Subject: [PATCH 3/3] Clarify selection count in floating action bar Show "N visible entries" instead of "All matching entries" to avoid implying all database entries are selected when only the visible page (up to 100) is being deleted. https://claude.ai/code/session_012v1E6pJFJK1xhWH43SrACu --- client/src/pages/AdminErrors.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/src/pages/AdminErrors.tsx b/client/src/pages/AdminErrors.tsx index c5ba13d..e591b9b 100644 --- a/client/src/pages/AdminErrors.tsx +++ b/client/src/pages/AdminErrors.tsx @@ -659,7 +659,7 @@ export default function AdminErrors() { aria-label="Select all entries" /> - {selectAll ? "All matching entries selected" : "Select all"} + {selectAll ? `All ${logs.length} visible entries selected` : "Select all"}
@@ -761,7 +761,7 @@ export default function AdminErrors() {
- {selectAll ? `All matching entries` : `${selectionCount} selected`} + {selectAll ? `${selectionCount} visible entries` : `${selectionCount} selected`}