feat(web): webhook subscriptions CRUD UI + global nav expansion

CHANGELOG.md

@@ -1,3 +1,69 @@
+## [Unreleased]
+
+### Added -- Webhook subscriptions CRUD UI (PR #69) + API hardening (PR #68)
+- **New ``/webhooks`` management page** (``web/src/app/webhooks/page.tsx``)
+  renders a full subscription lifecycle alongside the existing DLQ surface:
+  the ``WebhookSubscriptionsGrid`` (AG Grid, id/url/description/filter/created_at
+  columns + per-row ``Revoke`` action) and a ``CreateWebhookPanel``
+  (3-phase state machine: closed → form → one-shot secret reveal with
+  a mandatory acknowledge-before-Done guard, modelled on Stripe / GitHub).
+- **New ``CreateWebhookPanel`` 3-phase state machine**: closed →
+  form → reveal. The reveal phase surfaces the one-shot plaintext
+  ``secret`` returned by ``POST /api/v1/webhooks`` (Fernet envelope
+  encryption-at-rest for every later fetch), with copy-to-clipboard +
+  a ``I have securely stored this secret.`` acknowledgement gate before
+  ``Done`` (which calls ``router.refresh()`` so the new row appears in
+  the grid).
+- **New ``DEFAULT_WEBHOOK_FILTER`` shared constant** exporting
+  ``{ kind: "upload_completed" }`` from ``web/src/lib/api/webhooks.ts``
+  -- mirrors the Pydantic closed-set on the backend so callers (form
+  + third-party integrations) automatically produce a backend-acceptable
+  payload, closing the prod-bug "leave-empty-filter → 422" gap.
+- **New network-boundary tests** (``web/tests/api/webhooks.test.ts``):
+  6 ``vi.stubGlobal('fetch', ...)`` cases asserting the parsed JSON
+  body of ``POST /api/v1/webhooks`` carries ``filter: { kind:
+  upload_completed }`` when the caller omits or passes an empty filter,
+  that the inverse pattern (caller-supplied filter) is honoured, that
+  ``fetchWebhookSubscriptions`` forwards ``limit`` + ``offset`` as query
+  params, that ``revokeWebhook`` sends ``DELETE`` to the canonicalised
+  URL, and that 4xx upstream bodies propagate via ``ApiError``.
+- **Global header nav expansion** (``web/src/app/layout.tsx``): the
+  pre-PR-69 nav exposed only ``Players`` + ``Compare``. PR-69 adds
+  ``/webhooks`` + ``/account`` + ``/upload`` so every primary
+  analyst surface is reachable from any other page.
+
+### Changed -- API hardening (PR #68)
+- **DB session lifecycle**: ``get_session`` (``apps/api/src/gw2analytics_api/database.py``)
+  now commits on successful yield and rolls back on exception. Route
+  handlers no longer need to call ``db.commit()`` for their writes;
+  the canonical 5xx path rolls back automatically.
+- **N+1 on ``GET /fights``**: ``selectinload(OrmFight.agents, OrmFight.skills)``
+  eagerly fetches the per-fight agents + skills alongside the trimmed
+  page, so a 50-fight list with 5 agents/fight stops issuing 51
+  round-trips.
+- **Pagination on ``GET /webhooks``**: ``limit`` (1-1000) +
+  ``offset`` (>= 0) query parameters, mirroring the existing
+  ``GET /fights`` pattern.
+
+### Fixed
+- **a11y warning on CreateWebhookPanel**: each of the 3 form inputs
+  (URL, description, filter) carries a ``name="..."`` attribute so
+  Chrome devtools no longer surfaces "A form field element should
+  have an id or name attribute". ``aria-describedby`` continues to
+  anchor the URL field's help text; the form's tab order remains
+  unchanged.
+
+### Changed (docs + repo hygiene)
+- **CONTRIBUTING.md**: added a §"Branch cleanup after merge"
+  subsection documenting the GitHub UI delete button, the
+  ``git push origin --delete <branch>`` CLI path, and the
+  one-sweep cleanup snippet for accumulating dep-bumps + feature
+  branches.
+- **.github/dependabot.yml**: header comment now references
+  the GitHub repo setting "Automatically delete head branches"
+  (Settings → General → Pull Requests) which is the canonical
+  way to stop dependabot branches from accumulating per PR.
+
 ## [0.16.0] - 2026-07-24
 
 ### Added — Full-stack refactoring: Phases 1-7 complete

CONTRIBUTING.md

@@ -493,6 +493,36 @@ the formatter for any file in the index. If it auto-fixes something,
 add the fix to the **same commit** (not a follow-up) to keep each commit
 self-contained.
 
+### Branch cleanup after merge
+
+Once a PR is squash-merged into `main`, **delete the feature branch**
+to keep the branch list scannable. Two paths:
+
+1. **GitHub UI** (one click): PR page → scroll to the bottom →
+   "Delete branch" button (appears immediately after the merge).
+2. **CLI**: `git push origin --delete <branch-name>` from any local repo.
+
+Dependabot-created branches (the ``dependabot/npm_and_yarn/...`` and
+``dependabot/uv/...`` ones) are auto-deleted after merge **only** if
+the repository setting **Settings → General → Pull Requests →
+"Automatically delete head branches"** is enabled. The setting lives at
+the repo level, not in ``.github/dependabot.yml`` -- verify it is on
+after opening the repo (otherwise dependabot branches accumulate one
+per dep-update PR). Stale feature branches from past merges can be
+pruned in a single sweep with:
+
+```bash
+# Lists + deletes every merged-into-main remote branch (read-only first):
+git branch -r --merged main | grep -vE '^\s*origin/(main|HEAD)' | tee /tmp/merged-branches.txt
+# Sanity-check the list, then delete (uncomment to actually run):
+git push origin --delete $(awk '{$1=$1;print}' /tmp/merged-branches.txt | sed 's|^origin/||')
+```
+
+The ``grep -vE`` excludes ``main`` + ``HEAD`` so a typo doesn't nuke
+the canonical branch. The same script works for cleaning up
+``refactor/*`` / ``fix/*`` / ``feat/*`` branches whose PRs were
+merged in past sprints.
+
 ## Tagging
 
 We use **semver with a scope suffix**:

apps/api/pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "gw2analytics_api"
-version = "0.10.25"
+version = "0.10.26"
 description = "FastAPI app for GW2Analytics."
 requires-python = ">=3.12"
 # v0.15.2: this nested workspace member has NO PEP 639 ``license`` /

web/package.json

@@ -1,6 +1,6 @@
 {
   "name": "web",
-  "version": "0.10.28",
+  "version": "0.10.29",
   "private": true,
   "// license": "v0.15.2: marks the package as proprietary / unlicensed to match LICENSE + NOTICE.md. The package is also private (line above) which prevents npm publish by default; this field is the explicit publisher-facing posture for parity with PEP 639 / SPDX markers.",
   "license": "UNLICENSED",

web/src/app/layout.tsx

@@ -163,18 +163,22 @@ export default function RootLayout({
               GW2<span style={{ color: "var(--accent)" }}>Analytics</span>
             </span>
           </Link>
-          {/* v0.10.0 plan 032: secondary nav links between
-              the brand and the search bar. ``/players`` and
-              ``/players/compare`` are the 2 most common
-              cross-fight destinations; the analyst can pivot
-              from any page to either view without typing a
-              URL. The link styles mirror the brand link so
-              the nav reads as one consistent strip. */}
+          {/* v0.10.0 plan 032 + PR2 hardening: secondary nav
+              links between the brand and the search bar. The 5
+              links cover the analyst's primary surfaces:
+              - /players : cross-fight roll-up of every account.
+              - /players/compare : side-by-side comparison.
+              - /account : GW2 API key resolver (BFF proxy).
+              - /webhooks : webhook subscription CRUD + DLQ.
+              - /upload : full 3-step upload wizard.
+              The link styles mirror the brand link so the nav
+              reads as one consistent strip. */}
           <nav
             style={{
               display: "flex",
               alignItems: "center",
               gap: 16,
+              flexWrap: "wrap",
             }}
             aria-label="Primary"
           >
@@ -200,6 +204,39 @@ export default function RootLayout({
             >
               Compare
             </Link>
+            <Link
+              href="/webhooks"
+              data-testid="nav-webhooks"
+              style={{
+                fontSize: 13,
+                color: "var(--link)",
+                textDecoration: "none",
+              }}
+            >
+              Webhooks
+            </Link>
+            <Link
+              href="/account"
+              data-testid="nav-account"
+              style={{
+                fontSize: 13,
+                color: "var(--link)",
+                textDecoration: "none",
+              }}
+            >
+              Account
+            </Link>
+            <Link
+              href="/upload"
+              data-testid="nav-upload"
+              style={{
+                fontSize: 13,
+                color: "var(--link)",
+                textDecoration: "none",
+              }}
+            >
+              Upload
+            </Link>
           </nav>
           <PlayerSearchBar />
         </header>

web/src/app/webhooks/page.tsx

@@ -1,28 +1,122 @@
-import { fetchWebhookDeliveries, type WebhookDlqRow } from "@/lib/api";
+/**
+ * v0.10.25 PR2 webhook management page — extends the pre-PR2
+ * DLQ-only surface with the full subscription lifecycle:
+ *
+ *  - \`CreateWebhookPanel\` (Client Component) renders an inline
+ *    3-phase state machine (closed / form / reveal) so the
+ *    analyst can register a new subscription without leaving
+ *    the page. The :class:\`CreateWebhookPanel\` docstring
+ *    documents the rationale for the inline reveal flow
+ *    (one-shot plaintext secret, Fernet envelope at rest).
+ *
+ *  - \`WebhookSubscriptionsGrid\` (Client Component, AG Grid)
+ *    renders the active subscriptions list with a per-row
+ *    \`Revoke\` action that delegates to
+ *    :func:\`revokeWebhook\` + \`router.refresh()\`. The
+ *    subscriptions list is the operator's primary surface; the
+ *    DLQ below it is the second-order view (failed deliveries
+ *    belonging to a subscription).
+ *
+ * Why parallel \`Promise.all\` for the two fetches
+ * =================================================
+ * The subscriptions list and the DLQ are independent reads
+ * from two unrelated tables. A sequential second fetch would
+ * double the round-trip latency; \`Promise.all\` shortens the
+ * critical path from \`2 \u00d7 ttfb\` to \`max(ttfb_sub,
+ * ttfb_dlq)\`. Each catch is isolated so a single failed fetch
+ * still renders the other surface (a DLQ outage is operational
+ * noise, not a full-page error).
+ *
+ * Why no auth gate
+ * ================
+ * The whole app is unauthenticated; any visitor can hit
+ * \`/api/v1/webhooks\`. A future auth cycle can wrap this
+ * server component in \`if (!session) return <RedirectToLogin
+ * />\` without touching the client child components
+ * (\`CreateWebhookPanel\` + \`WebhookSubscriptionsGrid\` + DLQ
+ * grid are all self-contained).
+ *
+ * Why \`force-dynamic\`
+ * ====================
+ * Bypasses Next.js's static caching so newly registered
+ * subscriptions + newly delivered/replayed rows surface on the
+ * next render without a 60s revalidate sweep. Standard for
+ * read-write ops surfaces.
+ */
+
+import {
+  fetchWebhookDeliveries,
+  fetchWebhookSubscriptions,
+  formatApiError,
+  type WebhookDlqRow,
+  type WebhookSubscriptionRow,
+} from "@/lib/api";
 import { WebhookDlqGrid } from "@/components/WebhookDlqGrid";
+import { WebhookSubscriptionsGrid } from "@/components/WebhookSubscriptionsGrid";
+import { CreateWebhookPanel } from "@/components/CreateWebhookPanel";
 
 import styles from "./page.module.css";
 
 export const dynamic = "force-dynamic";
 
 export default async function WebhooksPage() {
-  let rows: WebhookDlqRow[] = [];
-  let error: string | null = null;
+  const [subsResult, dlqResult] = await Promise.allSettled([
+    fetchWebhookSubscriptions(),
+    fetchWebhookDeliveries(),
+  ]);
+
+  let subscriptions: WebhookSubscriptionRow[] = [];
+  let subscriptionsError: string | null = null;
+  if (subsResult.status === "fulfilled") {
+    subscriptions = subsResult.value;
+  } else {
+    subscriptionsError = formatApiError(subsResult.reason);
+  }
 
-  try {
-    rows = await fetchWebhookDeliveries();
-  } catch (err) {
-    error = err instanceof Error ? err.message : String(err);
+  let dlq: WebhookDlqRow[] = [];
+  let dlqError: string | null = null;
+  if (dlqResult.status === "fulfilled") {
+    dlq = dlqResult.value;
+  } else {
+    dlqError = formatApiError(dlqResult.reason);
   }
 
   return (
     <main className={styles.main}>
-      <h1 className={styles.title}>Webhook DLQ</h1>
-      {error ? (
-        <p className={styles.errorText}>Error: {error}</p>
-      ) : (
-        <WebhookDlqGrid rows={rows} />
-      )}
+      <h1 className={styles.title}>Webhooks</h1>
+
+      <section aria-label="Subscriptions" className={styles.section}>
+        <h2 className={styles.sectionTitle}>Subscriptions</h2>
+        <p className={styles.sectionLede}>
+          Registered webhook endpoints. The plaintext secret is
+          shown only when a subscription is first created; copy it
+          then. Click on any subscription below to inspect or
+          revoke it.
+        </p>
+        <CreateWebhookPanel />
+        {subscriptionsError ? (
+          <p className={styles.errorText} role="alert">
+            Error: {subscriptionsError}
+          </p>
+        ) : (
+          <WebhookSubscriptionsGrid rows={subscriptions} />
+        )}
+      </section>
+
+      <section aria-label="Webhook DLQ" className={styles.section}>
+        <h2 className={styles.sectionTitle}>DLQ (failed deliveries)</h2>
+        <p className={styles.sectionLede}>
+          Webhook deliveries that exhausted retries. Each row
+          offers a one-shot replay.
+        </p>
+        {dlqError ? (
+          <p className={styles.errorText} role="alert">
+            Error: {dlqError}
+          </p>
+        ) : (
+          <WebhookDlqGrid rows={dlq} />
+        )}
+      </section>
     </main>
   );
 }