Skip to content

Commit 10558ed

Browse files
JohnMcLearclaude
andauthored
fix(admin/pads): apply filter chip server-side, before pagination (#7798)
* fix(admin/pads): apply filter chip server-side, before pagination Before: PadPage's filter chip (`active`/`recent`/`empty`/`stale`) ran on the client AFTER the 12-row page slice was already on screen. On a deployment with hundreds of pads it produced obviously wrong results — click "empty pads" on page 1 with 100 empties and only the 0–12 empties within the current page passed the filter. thm reported this on a 3.1.0 deployment. Move the filter into `PadSearchQuery` so the `/settings` socket can apply it before slicing: 1. pattern filter on names (cheap) 2. hydrate metadata for the matching pad universe iff a non-`all` filter is set or a non-`padName` sort is requested 3. apply filter chip on the hydrated set 4. sort + slice → `total` reflects the filtered universe so the pagination footer makes sense The original handler also had a 4-way `if/else if` that duplicated the hydrate-and-sort loop per `sortBy`. Folded those into one pipeline with a single comparator switch. Client side, `PadPage.tsx`: - drop the client-side `filteredResults` filter (server already filters) - chip click writes `filter` into searchParams (debounced refetch) and resets `currentPage` to 0 - older clients that don't send `filter` keep working — server defaults to `all` Stats cards (totalUsers/activeCount/emptyCount) still count the visible page only — that's a pre-existing UI limitation tracked separately. Closes the regression thm reported. Test plan - `tsc --noEmit` clean (server + admin) - New backend spec `padLoadFilter.ts` exercises filter:empty with small `limit` to lock in the bug-fix, plus all/active/omitted cases - `5 passing` locally on Node 25 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * address Qodo review on #7798 1. Functional setState updaters for every searchParams mutation (Qodo bug 1). The debounced pattern handler captured a render-time snapshot of searchParams; a faster chip click or sort change in between would be silently reverted when the debounce fired. Now every mutation merges against the latest state. 2. Concurrency-limited hydration (Qodo bug 3). The earlier draft issued Promise.all over the full candidate set, fanning out to thousands of in-flight padManager.getPad() reads on busy deployments. New mapWithConcurrency() caps concurrent loads at 16 — empirically enough to saturate a single ueberDB driver without pushing the event loop into back-pressure. 3. Test cleanup deletes the injected test-admin (Qodo bug 4). The original snapshot/restore pattern saved `settings.users` by reference; reassigning the same reference in after() left the inserted key in place and could leak into later backend specs. 4. Document the new `filter` field on the `padLoad` socket query in admin/README.md (Qodo rule violation 2). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 228b49f commit 10558ed

6 files changed

Lines changed: 398 additions & 152 deletions

File tree

admin/README.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,3 +64,25 @@ const SettingsPanel = () => {
6464
The admin endpoints are not yet present in the OpenAPI spec — this client is
6565
in place to support upcoming work (see issue #7638 follow-up). For now, it is
6666
exercised only by the smoke test.
67+
68+
## Socket.io: `padLoad` query shape
69+
70+
The admin `/settings` namespace's `padLoad` event accepts a `PadSearchQuery`
71+
defined in `src/node/types/PadSearchQuery.ts`:
72+
73+
| field | type | required | notes |
74+
| ------------ | ----------------------------------------------------------------- | -------- | ----- |
75+
| `pattern` | `string` | yes | Substring match on pad name. |
76+
| `offset` | `number` | yes | Pagination start, in items. Clamped server-side. |
77+
| `limit` | `number` | yes | Page size. Capped at 12. |
78+
| `ascending` | `boolean` | yes | Sort direction. |
79+
| `sortBy` | `"padName" \| "lastEdited" \| "userCount" \| "revisionNumber"` | yes | Column to sort by. |
80+
| `filter` | `"all" \| "active" \| "recent" \| "empty" \| "stale"` *(opt.)* | no | Filter chip; defaults to `"all"`. Applied **before** pagination so `total` and the page slice both reflect the filtered universe. Older clients that omit the field get the unchanged `"all"` behaviour. |
81+
82+
Filter semantics — applied after pattern matching, before sort + slice:
83+
84+
- `active`: `userCount > 0`
85+
- `recent`: edited within the last 7 days
86+
- `empty`: `revisionNumber === 0`
87+
- `stale`: not edited in the last 365 days
88+
- `all` / missing: no further filtering

admin/src/pages/PadPage.tsx

Lines changed: 43 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,16 @@
11
import {Trans, useTranslation} from "react-i18next";
22
import {useEffect, useMemo, useState} from "react";
33
import {useStore} from "../store/store.ts";
4-
import {PadSearchQuery, PadSearchResult} from "../utils/PadSearch.ts";
4+
import {PadFilter, PadSearchQuery, PadSearchResult} from "../utils/PadSearch.ts";
55
import {useDebounce} from "../utils/useDebounce.ts";
66
import * as Dialog from "@radix-ui/react-dialog";
77
import {ChevronLeft, ChevronRight, Eye, Trash2, FileStack, PlusIcon, Search, X, RefreshCw, History} from "lucide-react";
88
import {useForm} from "react-hook-form";
99
import type {TFunction} from "i18next";
1010

1111
type PadCreateProps = { padName: string }
12-
type FilterId = 'all' | 'active' | 'recent' | 'empty' | 'stale'
1312

14-
const PAD_FILTER_IDS: FilterId[] = ['all', 'active', 'recent', 'empty', 'stale']
15-
16-
const isRecent = (ts: number) => (Date.now() - ts) < 86_400_000 * 7
17-
const isStale = (ts: number) => (Date.now() - ts) > 86_400_000 * 365
13+
const PAD_FILTER_IDS: PadFilter[] = ['all', 'active', 'recent', 'empty', 'stale']
1814

1915
function relativeTime(t: TFunction, ts: number): string {
2016
const d = (Date.now() - ts) / 1000
@@ -58,12 +54,23 @@ function sanitizeLocale(lng?: string): string {
5854
export const PadPage = () => {
5955
const settingsSocket = useStore(state => state.settingsSocket)
6056
const [searchParams, setSearchParams] = useState<PadSearchQuery>({
61-
offset: 0, limit: 12, pattern: '', sortBy: 'lastEdited', ascending: false,
57+
offset: 0, limit: 12, pattern: '', sortBy: 'lastEdited', ascending: false, filter: 'all',
6258
})
6359
const {t, i18n} = useTranslation()
6460
const locale = sanitizeLocale(i18n.resolvedLanguage ?? i18n.language)
6561
const [searchTerm, setSearchTerm] = useState('')
66-
const [filter, setFilter] = useState<FilterId>('all')
62+
// Read filter off searchParams so chip changes round-trip through
63+
// the server (`filter` is applied before pagination there). Clicking
64+
// a chip used to filter only the current 12-row page slice.
65+
//
66+
// All searchParams mutations go through functional updaters because the
67+
// debounced pattern handler captures a render-time snapshot and would
68+
// otherwise revert a faster chip click / sort change made in between.
69+
const filter: PadFilter = searchParams.filter ?? 'all'
70+
const setFilter = (f: PadFilter) => {
71+
setCurrentPage(0)
72+
setSearchParams((sp) => ({...sp, filter: f, offset: 0}))
73+
}
6774
const [selected, setSelected] = useState<Set<string>>(new Set())
6875
const pads = useStore(state => state.pads)
6976
const [currentPage, setCurrentPage] = useState(0)
@@ -78,28 +85,23 @@ export const PadPage = () => {
7885
[pads, searchParams.limit]
7986
)
8087

81-
const filteredResults = useMemo(() => {
82-
const r = pads?.results ?? []
83-
if (filter === 'active') return r.filter(p => p.userCount > 0)
84-
if (filter === 'recent') return r.filter(p => isRecent(p.lastEdited))
85-
if (filter === 'empty') return r.filter(p => p.revisionNumber === 0)
86-
if (filter === 'stale') return r.filter(p => isStale(p.lastEdited))
87-
return r
88-
}, [pads, filter])
89-
90-
const totalUsers = useMemo(() => (pads?.results ?? []).reduce((s, p) => s + p.userCount, 0), [pads])
91-
const activeCount = useMemo(() => (pads?.results ?? []).filter(p => p.userCount > 0).length, [pads])
92-
const emptyCount = useMemo(() => (pads?.results ?? []).filter(p => p.revisionNumber === 0).length, [pads])
88+
// The server applies `filter` before paginating; the page payload is
89+
// already the filtered slice. The stats cards still reflect the
90+
// current page (pre-existing behaviour) — making them global would
91+
// require a separate aggregate query.
92+
const visibleResults = pads?.results ?? []
93+
const totalUsers = useMemo(() => visibleResults.reduce((s, p) => s + p.userCount, 0), [pads])
94+
const activeCount = useMemo(() => visibleResults.filter(p => p.userCount > 0).length, [pads])
95+
const emptyCount = useMemo(() => visibleResults.filter(p => p.revisionNumber === 0).length, [pads])
9396
const lastActivity = useMemo(() => {
94-
const r = pads?.results ?? []
95-
return r.length ? Math.max(...r.map(p => p.lastEdited)) : null
97+
return visibleResults.length ? Math.max(...visibleResults.map(p => p.lastEdited)) : null
9698
}, [pads])
9799

98-
const allSelected = filteredResults.length > 0 && filteredResults.every(p => selected.has(p.padName))
100+
const allSelected = visibleResults.length > 0 && visibleResults.every(p => selected.has(p.padName))
99101
const toggleAll = () => {
100102
const s = new Set(selected)
101-
if (allSelected) filteredResults.forEach(p => s.delete(p.padName))
102-
else filteredResults.forEach(p => s.add(p.padName))
103+
if (allSelected) visibleResults.forEach(p => s.delete(p.padName))
104+
else visibleResults.forEach(p => s.add(p.padName))
103105
setSelected(s)
104106
}
105107
const toggleOne = (name: string) => {
@@ -109,7 +111,10 @@ export const PadPage = () => {
109111
}
110112

111113
useDebounce(() => {
112-
setSearchParams({...searchParams, pattern: searchTerm})
114+
// Functional updater so this delayed callback can't clobber a faster
115+
// user interaction (e.g. clicking a filter chip mid-typing).
116+
setSearchParams((sp) => ({...sp, pattern: searchTerm, offset: 0}))
117+
setCurrentPage(0)
113118
}, 500, [searchTerm])
114119

115120
useEffect(() => {
@@ -250,7 +255,7 @@ export const PadPage = () => {
250255
<section className="pm-section">
251256
<div className="pm-section-header">
252257
<h2><Trans i18nKey="admin_pads.all_pads"/></h2>
253-
<span className="pm-count-badge">{filteredResults.length}</span>
258+
<span className="pm-count-badge">{visibleResults.length}</span>
254259
<div className="pm-spacer"/>
255260
<div className="pm-toolbar">
256261
<div className="pm-search">
@@ -268,12 +273,12 @@ export const PadPage = () => {
268273
<select
269274
className="pm-select"
270275
value={searchParams.sortBy}
271-
onChange={e => setSearchParams({
272-
...searchParams,
276+
onChange={e => setSearchParams((sp) => ({
277+
...sp,
273278
sortBy: e.target.value,
274279
// Keep current direction when only the column changes; the
275280
// ↑/↓ button below is the sole control for direction.
276-
})}
281+
}))}
277282
>
278283
<option value="lastEdited">{t('ep_admin_pads:ep_adminpads2_last-edited')}</option>
279284
<option value="padName">{t('admin_pads.sort.name')}</option>
@@ -282,10 +287,10 @@ export const PadPage = () => {
282287
</select>
283288
<button
284289
className="pm-sort-dir"
285-
onClick={() => setSearchParams({
286-
...searchParams,
287-
ascending: !searchParams.ascending,
288-
})}
290+
onClick={() => setSearchParams((sp) => ({
291+
...sp,
292+
ascending: !sp.ascending,
293+
}))}
289294
title={t(searchParams.ascending
290295
? 'admin_plugins.sort_ascending'
291296
: 'admin_plugins.sort_descending')}
@@ -330,7 +335,7 @@ export const PadPage = () => {
330335
</div>
331336
)}
332337

333-
{filteredResults.length > 0 ? (
338+
{visibleResults.length > 0 ? (
334339
<div className="pm-table-wrap">
335340
<table>
336341
<thead>
@@ -348,7 +353,7 @@ export const PadPage = () => {
348353
</tr>
349354
</thead>
350355
<tbody>
351-
{filteredResults.map(pad => {
356+
{visibleResults.map(pad => {
352357
const isEmpty = pad.revisionNumber === 0
353358
const isSel = selected.has(pad.padName)
354359
return (
@@ -432,7 +437,7 @@ export const PadPage = () => {
432437
onClick={() => {
433438
const p = currentPage - 1
434439
setCurrentPage(p)
435-
setSearchParams({...searchParams, offset: p * searchParams.limit})
440+
setSearchParams((sp) => ({...sp, offset: p * sp.limit}))
436441
}}
437442
>
438443
<ChevronLeft size={14}/> <Trans i18nKey="admin_pads.pagination.previous"/>
@@ -444,7 +449,7 @@ export const PadPage = () => {
444449
onClick={() => {
445450
const p = currentPage + 1
446451
setCurrentPage(p)
447-
setSearchParams({...searchParams, offset: p * searchParams.limit})
452+
setSearchParams((sp) => ({...sp, offset: p * sp.limit}))
448453
}}
449454
>
450455
<Trans i18nKey="admin_pads.pagination.next"/> <ChevronRight size={14}/>

admin/src/utils/PadSearch.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
1+
export type PadFilter = 'all' | 'active' | 'recent' | 'empty' | 'stale';
2+
13
export type PadSearchQuery = {
24
pattern: string;
35
offset: number;
46
limit: number;
57
ascending: boolean;
68
sortBy: string;
9+
filter?: PadFilter;
710
}
811

912

0 commit comments

Comments
 (0)