Skip to content

Code review β€” admin UI batch ​

  • Branch: feature/empty-state-component β†’ master
  • Date: 2026-06-23
  • Scope: 25 commits, 63 files (+1517 / βˆ’247)
  • Reviewer: code-reviewer agent over git diff origin/master...HEAD, synthesized.

Deterministic gate ​

CheckResult
nx lint webβœ“ 0 errors (13 warnings, baseline)
nx test webβœ“ 174 passed
coverage gateβœ“ statements 77.5% Β· functions 69.0% Β· lines 82.6% (all β‰₯ threshold)
nx build webβœ“
nx lint/test/build apiβœ“

What's in the batch ​

<ds-empty-state> (adopted everywhere) · ds-table reduced to a bare grid + new <ds-list-header> ([schema]+[store], two-way [(search)], (create)) · list-page header/empty-state create pattern · proposals/pulse form rework + .adm-mform + validation (border + reserved left-aligned message on blur/submit) on ngModel forms and the manual entity modal · ai-config tidy-up + .adm-toggle-list · emptyListInterceptor (null→[]) · notifications bell dropdown · husky pre-push hook · workflow paths-ignore + advanced codeql.yml.

Findings ​

πŸ”΄ Blocking ​

None.

🟑 Should-fix ​

  1. nodes-page.html empty-state overlaps the add form β€” the @empty block wasn't gated on !adding(), so an empty list + open form rendered both. FIXED β€” wrapped in @if (!adding()), matching pulse/proposals.
  2. paths-ignore vs required status checks (conditional) β€” if ci / Analyze (go) / Analyze (javascript-typescript) are required in branch protection, a docs-only PR skips them and the required check stays pending forever (can't merge). OPEN β€” verify branch-protection config; mitigations: keep docs bundled with code (current practice), make the checks non-required, or add a same-named always-green stub for the ignored case.

🟒 Nits ​

  1. "New" vs "Add" verb β€” the "Add" relabel hit the submit buttons; the header create button and grid empty-state button still say "New {singular}". These are entry points (open the form/modal) vs the submit action, so arguably correct β€” OPEN decision: unify to "Add" if desired.
  2. Inline-edit asterisk β€” the modal dropped the required *; the ds-table inline-edit row still renders it (ds-table.component.html). Inline edit has no border+message validation, so the * is its only required indicator β€” left as-is pending a decision.
  3. Spec whitespace β€” double blank line in ds-table.component.spec.ts where buttonByText was removed. FIXED.

Areas verified clean ​

Interceptor (GET + Dispatch-API only, literal-null only, single-resource GETs untouched, all 4 branches tested) Β· two-way [(search)] model↔signal wiring on all 6 pages Β· resetPageOnSearch / modal resetValidation effects (untracked writes, reseed on reopen) Β· canCreate/canDelete/editExternal honored in header + grid empty state (no capability regression) Β· EmptyState imports present everywhere Β· notif dropdown stacking (scrim z300 / panel z320) Β· CSS mirrored to v2 bespoke.css (only the notif change intentionally diverges, with a NOTE) Β· husky hook mirrors CI Β· Go router reorder is a pure goimports sort.

Verdict ​

Safe to push β€” 0 blocking. Two should-fixes resolved in-branch (nodes overlap, spec whitespace); the paths-ignore/required-checks interaction is the one open item to confirm before relying on docs-only skips. Nits 3–4 are open style decisions.