Skip to content

Admin abstractions, naming, performance and lint guardrails - #46

Closed
theobong wants to merge 33 commits into
chore/admin-campaign-08-cleanup-bfrom
chore/admin-campaign-09-abstraction
Closed

theobong wants to merge 33 commits into
chore/admin-campaign-08-cleanup-bfrom
chore/admin-campaign-09-abstraction

Conversation

@theobong

Copy link
Copy Markdown
Member

What changed

This is the last PR of the admin cleanup train: repo-wide abstractions, naming, performance, and lint and dead-code guardrails.

Abstractions:

  • One infinite-list query factory, one flatPages helper and one invalidation helper.
  • One selection hook replaces ten copied selection effects.
  • Shared list pieces: states, card, search and "Load more".
  • One Leaflet base for tiles, resize and teardown.
  • One implementation per display helper: initials, short IDs, compact counts, dates, relative age, reason prompts.

Naming:

  • Hooks follow one scheme: useXList, useXListInfinite, useX.
  • Display helpers are named in one verb family.
  • The home query hooks live in features/home.

Performance:

  • Memoized list rows.
  • Date formatters built once.
  • No duplicate refetches when a mutation invalidates overlapping keys.

Guardrails:

  • ESLint 9 flat config with typed rules.
  • knip and jscpd checks in CI.

Visible in: admin. A few display formats change; see the copy table.

Before you start

Verify

[Admin]

Changed displays:

  1. Open "Jurisdictions" and look at a large city's row. Expect: population reads like "3.9M pop" (it was "3899k pop"). Smaller places read like "138.7K pop".
  2. Open "Analytics". Expect: bar values of 1,000 or more read like "1.2K" or "1M".
  3. Open "Reports", select a report with chat, and hover a message time. Expect: it reads like "Sep 22, 2026, 10:00:00 AM" (it was "9/22/2026, 10:00:00 AM"). A report that was already sent shows "Already sent · Sep 22, 2026, ...".
  4. Open "Mail" and hover a row's time. Expect: the same "Sep 22, 2026, 10:00:00 AM" style.
  5. On the Dashboard, look at the preview avatars and the report reporter card. Expect: two-letter initials. A name starting with an emoji shows the whole emoji, and a blank name shows "?".
  6. Open "Mail", leave the list idle for two minutes, and watch a recent row. Expect: its age label advances ("5m", then "6m", then "7m"). It also advances in "Jurisdictions" on the waiting age.

Unchanged behavior on the new shared code:
7. In every section, type in the search box, pick a row with the mouse and with Enter, click "Load more", switch filters, and follow a deep link from the Dashboard. Expect: the same selection behavior as #45: a deep link wins, a filtered-out pick stays open (Host messaging clears it), and a pick that leaves the list after an action clears.
8. On "Moderation", "Keep" an item. On "Organizations", "Suspend" an org with a reason. Expect: the same toasts, and the "Reason (required)" prompt.
9. On the Dashboard, check the live map, then open a jurisdiction's boundary map. Expect: the tiles, pins and outline render as before.

Regression

Every list and action: [Admin]

  1. Flag a report, send a mail reply, and mark an inbox message read. Expect: one toast each; the list and detail refresh once (no double refresh).
  2. Open every section once. Expect: no console errors and no layout changes.

Not covered

  • The render-time gains were measured in jsdom with the React Profiler, not in a real browser.

Findings addressed

  • ADM-DUP (global):
    • 18 infinite-query bodies now go through lib/infinite.ts (infiniteListOptions, flatPages); query keys and request params are byte-identical.
    • 12 invalidateX helpers now use invalidateKeys, with the same keys, order and returned promise.
    • Ten selection effects become hooks/use-selection.ts, with per-page options.
    • The section list pieces ListStates, LoadMoreButton (inline style replaced by a class), ListCard and SearchBox produce identical DOM.
    • components/map/leaflet-base.ts is shared by both maps, and the tile and boundary layers are reused instead of recreated.
    • One helper each for initials, short IDs (matching the backend's mail-subject id.slice(0, 8)), first names, compact counts, dates, the relative-age ladder (shared relativeAgo), required-reason prompts, and the inbox focus ID.
  • ADM-READ (naming):
    • 14 hooks renamed; for example useOrgsInfinite becomes useOrgListInfinite, useAdminOrg becomes useOrg, and useAdminLogout becomes useOperatorLogout.
    • 15 display helpers renamed into one family; for example getMailPreviewPresentation becomes mailPreviewView, and cancelBlockedFor becomes cancelBlockedMessage.
    • hooks/use-admin-home.ts moves to features/home/use-home.ts (a pure rename commit).
  • ADM-PERF: see Verification.
  • Guardrails:
    • eslint.config.mjs (flat config through the ESLint CLI) keeps next/core-web-vitals and next/typescript.
    • Typed linting through projectService: no-floating-promises, no-misused-promises (JSX attributes allowed), switch-exhaustiveness-check and consistent-type-imports (error). They surfaced no real bugs; 11 mechanical fixes.
    • @next/next/no-img-element is off (meaningless for a static export with unoptimized images). 25 inline disables and 6 inert array-index disables are removed.
    • knip.config.ts: 11 findings go to 0, with one justified ignore.
    • jscpd gates: app code 0.84% (threshold 1%); tests 5.81% (threshold 6%, a ratchet).
    • CI gets knip and jscpd as steps inside the existing "lint / typecheck / build / test" job; no job is renamed or added.

Decisions for the reviewer

  • Helper outputs that change on purpose:
    • Chat and mail timestamps use the admin-wide short-month format.
    • Compact numbers read "3.9M" and "1.2K" instead of "3899k" and "1.2k".
    • Initials are code-point safe, and "?" replaces an empty avatar.
    • The routed date follows the browser locale like every other admin date, instead of hard-coded en-US.
    • Short IDs keep dashes, matching the server mail subjects. Real UUIDs have no dash in their first 8 characters, so they read the same.
  • Link safety: the shared safe-link gate for org and claim links (no IP, punycode, userinfo or overlong URL) was considered and NOT applied. It is hardening that could reject real links, so it is listed in the campaign summary instead.
  • useDiscoveryTask is kept but unused until the discovery-notes decision. knip is told why.
  • Tests jscpd gate: at 6% with 5.81% measured, it is intentionally a ratchet.
  • Date formatters are built once per tab (about 18x faster per row). An operator whose OS time zone changes mid-session needs a reload.

User-visible copy changes

Where Before After
Jurisdictions row population "3899k pop", "139k pop", "1.3k" "3.9M pop", "138.7K pop", "1.3K"
Analytics bar values "1.0k", "1.2k", "1000.0k" "1K", "1.2K", "1M"
Report chat, send card, "Already sent", mail and inbox time tooltips "9/22/2026, 10:00:00 AM" "Sep 22, 2026, 10:00:00 AM"
Mail time tooltip with a missing or unreadable time empty "-" or the raw value
Jurisdictions routed date (non-US locale) "Sep 23" always the browser locale, e.g. "23 Sep"
Avatars, name starting with an emoji broken half-character the full emoji
Avatars, blank name (home previews, report cards) empty circle "?"

Tests changed

  • discovery-page.test.tsx and analytics-page.test.tsx: the compact-number expectations were pinned to the old output ("3899k", "1.2k") and are updated.
  • send-panel.test.ts: the "Already sent" date format is updated.
  • The population and bar-label unit cases moved to the new lib/display.test.ts.
  • Imports and names are updated only for renames.
  • New tests cover useSelection (17), the section-list pieces (10), lib/display, the dates variants, invalidateKeys dedupe (including the object-param safety case), useNow, and the row age ticking.

Verification

  • Node v22.23.2. pnpm lint (ESLint CLI), pnpm typecheck, pnpm build, pnpm test (118 files, 1,210 tests, 3 runs), pnpm --filter admin knip (0 findings) and pnpm --filter admin dup all pass.
  • Render time, measured with the React Profiler over 200 rows while typing 5 characters:
    • Events: 101-116 ms to 12-22 ms
    • Users: 57-67 ms to 7-9 ms
    • Jurisdictions: 152-170 ms to 14-15 ms
    • Host messaging: 77-103 ms to 8-12 ms
    • Signup pages: 75-96 ms to 8-10 ms
    • Organizations: 54-63 ms to 6-7 ms
    • Mail outreach: 61-84 ms to 9-12 ms
    • Mail inbox: 60-71 ms to 8-16 ms
  • Row clicks are 2x to 5x faster.
  • Date formatting: 54 to 2.9 µs per row.
  • Duplicate refetches are removed. For example, flagging an event fetched its detail twice and now fetches it once; marking mail read fetched the thread and the stats twice each.
  • JS bundle: +994 bytes (+0.06%). First Load JS is unchanged.
  • An adversarial review covered abstractions, helpers, performance and guardrails. It confirmed the query factory, selection hook, list pieces, Leaflet base and renames are behavior-neutral. It found and this PR fixed:
    • frozen age labels in memoized rows (rows now take a ticking clock);
    • the safe-link hardening, which is reverted and deferred;
    • an invalidation dedupe that was unsafe for object-param keys;
    • the deletion of a hook that was kept on purpose.

🤖 Generated with Claude Code

…act counts, dates, relative age, safe urls, reason prompts, inbox focus id
@byteful byteful closed this Sep 24, 2026
@byteful
byteful deleted the chore/admin-campaign-09-abstraction branch September 24, 2026 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants