From 0dcc91aae83a44574a4bfd43a5f16d0439faaf13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 10 May 2026 13:04:27 +0200 Subject: [PATCH] refactor(integrations): single status pill, drop redundant containers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Withings card had three status displays (top-right badge, mid-card banner with the same state + two timestamps, and an inner badge). Mood Log was even worse — four, including a "letzter Sync" line at the very bottom. The pill in the card header is now the canonical surface for both. Withings keeps its actionable error message under the pill (the pill can't fit "Withings refresh error: 503 - upstream"), the duplicate {n}/{threshold} badge is gone. Mood Log gains the visual divider Withings always had — Marc called the asymmetry out. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../__tests__/integrations-section.test.tsx | 201 +++++-- .../settings/integrations-section.tsx | 564 ++++++++---------- 2 files changed, 415 insertions(+), 350 deletions(-) diff --git a/src/components/settings/__tests__/integrations-section.test.tsx b/src/components/settings/__tests__/integrations-section.test.tsx index c22fdb704..e890c18ac 100644 --- a/src/components/settings/__tests__/integrations-section.test.tsx +++ b/src/components/settings/__tests__/integrations-section.test.tsx @@ -1,18 +1,28 @@ /** - * v1.4.15 Phase B2 — Settings → Integrations status surface tests. + * v1.4.19 Phase A5 — Settings → Integrations status consolidation. * - * The four states this file locks in (one assertion each): - * 1. connected → no banner, "Connected" badge only - * 2. error_transient → banner with "{count}/{threshold}" - * counter, last-error message visible - * 3. error_reauth → "Reconnect required" badge surfaces - * 4. disconnected → "Disconnected" badge surfaces (used - * after the user clicks "Disconnect") + * Marc's verbatim concern (paraphrased): + * - Withings card had 3 status displays — top-right "connected" + * badge, mid-card "connected / last successful / last attempt" + * trio container, and the badge inside that container. + * - Mood Log card had 4 — same as Withings PLUS a "letzter Sync" + * line at the very bottom. * - * We test by rendering only the IntegrationsSection with a TanStack - * Query mock that returns the desired status. The other queries - * (withings status, moodlog status, global services) are stubbed to - * stable defaults — the test isn't about those surfaces. + * After A5: each card has EXACTLY ONE status display (the pill, top + * right). Error context (the failure message itself, when actionable) + * is allowed to surface below the pill since it carries information + * the pill can't fit. Timestamp repetition is gone. + * + * The tests pin: + * 1. Healthy Withings + Mood Log: ONE pill per card, no banner. + * 2. Withings transient error: ONE pill per card + error message, + * NO redundant "{n}/{threshold} consecutive failures" badge, + * NO "Last successful sync / Last attempt" trio. + * 3. Withings reauth: pill carries the "Error — reconnect" label. + * 4. Mood Log disconnected: pill says "Not connected", no other + * status display surfaces. + * 5. Mood Log card has the visual divider that Withings always had + * — consistency Marc explicitly called out. */ import { describe, expect, it, vi, beforeEach } from "vitest"; import { renderToStaticMarkup } from "react-dom/server"; @@ -31,14 +41,24 @@ vi.mock("@/hooks/use-auth", () => ({ }), })); -// Per-test mock — we'll rewrite the integration-status payload between -// cases via `setIntegrationStatus()`. Withings + moodlog status return -// stable defaults so the cards render their connect-CTA branch (the -// banner is what we actually assert on). let integrationStatusPayload: unknown = null; +let withingsStatusPayload: unknown = { connected: false, configured: false }; +let moodLogStatusPayload: unknown = { + configured: false, + enabled: false, + lastSyncedAt: null, + entryCount: 0, + webhookSecret: null, +}; function setIntegrationStatus(payload: unknown) { integrationStatusPayload = payload; } +function setWithingsStatus(payload: unknown) { + withingsStatusPayload = payload; +} +function setMoodLogStatus(payload: unknown) { + moodLogStatusPayload = payload; +} vi.mock("@tanstack/react-query", () => ({ useQuery: ({ queryKey }: { queryKey: unknown[] }) => { @@ -59,20 +79,11 @@ vi.mock("@tanstack/react-query", () => ({ }; } if (key === "withings/status") { - return { - data: { connected: false, configured: false }, - isLoading: false, - }; + return { data: withingsStatusPayload, isLoading: false }; } if (key === "moodlog-status") { return { - data: { - configured: false, - enabled: false, - lastSyncedAt: null, - entryCount: 0, - webhookSecret: null, - }, + data: moodLogStatusPayload, isLoading: false, refetch: vi.fn(), }; @@ -94,12 +105,31 @@ function render() { ); } -describe("IntegrationsSection — status surface", () => { +/** Count occurrences of a substring in the rendered HTML. */ +function count(html: string, needle: string): number { + let n = 0; + let i = 0; + while ((i = html.indexOf(needle, i)) !== -1) { + n++; + i += needle.length; + } + return n; +} + +describe("IntegrationsSection — single-status-display contract (A5)", () => { beforeEach(() => { integrationStatusPayload = null; + withingsStatusPayload = { connected: false, configured: false }; + moodLogStatusPayload = { + configured: false, + enabled: false, + lastSyncedAt: null, + entryCount: 0, + webhookSecret: null, + }; }); - it("renders no error banner when both integrations are healthy", () => { + it("renders exactly ONE status pill per card when both integrations are healthy", () => { setIntegrationStatus({ threshold: 3, integrations: [ @@ -114,19 +144,40 @@ describe("IntegrationsSection — status surface", () => { { integration: "moodlog", state: "connected", - lastSuccessAt: null, - lastAttemptAt: null, + lastSuccessAt: "2026-05-09T17:00:00.000Z", + lastAttemptAt: "2026-05-09T17:00:00.000Z", lastError: null, consecutiveFailures: 0, }, ], }); + setWithingsStatus({ + connected: true, + configured: true, + lastSyncedAt: "2026-05-09T18:00:00.000Z", + }); + setMoodLogStatus({ + configured: true, + enabled: true, + lastSyncedAt: "2026-05-09T17:00:00.000Z", + entryCount: 42, + webhookSecret: "secret123", + }); + const html = render(); - expect(html).not.toContain("Reconnect required"); - expect(html).not.toContain("integration-status-error"); + // Exactly one pill per card → 2 pills total. + expect(count(html, 'data-testid="integration-status-pill"')).toBe(2); + // The redundant banner from v1.4.15 is gone. + expect(html).not.toContain('data-testid="integration-status-banner"'); + // Card-body "letzter Sync" repetition is gone — no + // "Last sync:" label outside the pill. + expect(html).not.toContain("Last sync:"); + // No "Last successful sync" / "Last attempt" trio. + expect(html).not.toContain("Last successful sync"); + expect(html).not.toContain("Last attempt"); }); - it("surfaces an error banner with last-error and {n}/{threshold} when transient failure", () => { + it("Withings transient error keeps the pill + error text but drops the {n}/{threshold} duplicate badge", () => { setIntegrationStatus({ threshold: 3, integrations: [ @@ -148,13 +199,27 @@ describe("IntegrationsSection — status surface", () => { }, ], }); + setWithingsStatus({ + connected: true, + configured: true, + lastSyncedAt: "2026-05-09T08:00:00.000Z", + }); + const html = render(); - expect(html).toContain("2/3 consecutive failures"); + // Pill carries the "Error — reconnect" copy via the pill's + // `data-state="error"` marker. + expect(html).toContain('data-state="error"'); + // Actionable error message still surfaces (we kept it because + // the pill can't fit "Withings refresh error: 503 - upstream"). expect(html).toContain("Withings refresh error: 503 - upstream"); - expect(html).toContain("integration-status-banner"); + // …but the consecutive-failure badge that doubled the pill's + // signal is gone. + expect(html).not.toContain("2/3 consecutive failures"); + expect(html).not.toContain("Last successful sync"); + expect(html).not.toContain("Last attempt"); }); - it("surfaces 'Reconnect required' when a refresh-token grant has revoked", () => { + it("Withings reauth state surfaces 'Error — reconnect' on the pill (Marc's wording)", () => { setIntegrationStatus({ threshold: 3, integrations: [ @@ -176,11 +241,21 @@ describe("IntegrationsSection — status surface", () => { }, ], }); + setWithingsStatus({ + connected: true, + configured: true, + lastSyncedAt: "2026-05-08T12:00:00.000Z", + tokenExpired: true, + }); + const html = render(); - expect(html).toContain("Reconnect required"); + expect(html).toContain('data-state="error"'); + // Old "Reconnect required" banner is gone — the pill is now + // the canonical surface. + expect(html).not.toContain('data-testid="integration-status-banner"'); }); - it("renders the disconnected tombstone state without error styling", () => { + it("Mood Log disconnected state shows 'Not connected' on the pill, no extra status surfaces", () => { setIntegrationStatus({ threshold: 3, integrations: [ @@ -202,8 +277,52 @@ describe("IntegrationsSection — status surface", () => { }, ], }); + const html = render(); - expect(html).toContain("Disconnected"); - expect(html).not.toContain("Reconnect required"); + expect(html).toContain("Not connected"); + expect(html).not.toContain('data-testid="integration-status-banner"'); + // Mood Log card no longer has a trailing "letzter Sync" line. + expect(html).not.toContain("Last sync:"); + }); + + it("Mood Log card carries the visual divider Withings has (consistency)", () => { + setIntegrationStatus({ + threshold: 3, + integrations: [ + { + integration: "withings", + state: "connected", + lastSuccessAt: "2026-05-09T18:00:00.000Z", + lastAttemptAt: "2026-05-09T18:00:00.000Z", + lastError: null, + consecutiveFailures: 0, + }, + { + integration: "moodlog", + state: "connected", + lastSuccessAt: "2026-05-09T17:00:00.000Z", + lastAttemptAt: "2026-05-09T17:00:00.000Z", + lastError: null, + consecutiveFailures: 0, + }, + ], + }); + setWithingsStatus({ + connected: true, + configured: true, + lastSyncedAt: "2026-05-09T18:00:00.000Z", + }); + setMoodLogStatus({ + configured: true, + enabled: true, + lastSyncedAt: "2026-05-09T17:00:00.000Z", + entryCount: 42, + webhookSecret: "secret123", + }); + + const html = render(); + // Both cards include the section divider data-testid so the + // header → body separation is visually consistent. + expect(count(html, 'data-testid="integration-card-divider"')).toBe(2); }); }); diff --git a/src/components/settings/integrations-section.tsx b/src/components/settings/integrations-section.tsx index b597e7230..b52f13842 100644 --- a/src/components/settings/integrations-section.tsx +++ b/src/components/settings/integrations-section.tsx @@ -24,14 +24,14 @@ import { AlertDialogTitle, AlertDialogTrigger, } from "@/components/ui/alert-dialog"; -import { Badge } from "@/components/ui/badge"; import { Button } from "@/components/ui/button"; import { Input } from "@/components/ui/input"; import { Label } from "@/components/ui/label"; import { PasswordInput } from "@/components/settings/password-input"; import { TestConnectionButton } from "@/components/settings/test-connection-button"; +import { IntegrationStatusPill } from "@/components/settings/integration-status-pill"; +import type { IntegrationPillState } from "@/components/settings/integration-status-pill"; import { useAuth } from "@/hooks/use-auth"; -import { formatDateTime } from "@/lib/format"; import { useTranslations } from "@/lib/i18n/context"; import { invalidateKeys, measurementDependentKeys } from "@/lib/query-keys"; @@ -52,8 +52,9 @@ interface GlobalServiceAvailability { } // v1.4.15 Phase B2: shared status payload for both integration cards. -// Mirrors the `IntegrationViewModel` shape returned by -// `GET /api/integrations/status` (single roundtrip, two integrations). +// v1.4.19 Phase A5: the redundant in-card status banner is gone — the +// IntegrationStatusPill now owns state + last-sync presentation, and +// the actionable error message is shown inline above the action row. type IntegrationKey = "withings" | "moodlog"; type IntegrationState = | "connected" @@ -97,9 +98,6 @@ function useIntegrationStatuses(enabled: boolean) { return (await res.json()).data as IntegrationStatusEnvelope; }, enabled, - // The status endpoint is light (one DB read per integration) and - // the user typically lands here right after a sync — refetching - // on focus avoids a stale "last sync 5 min ago" reading. refetchOnWindowFocus: true, }); } @@ -112,104 +110,44 @@ function pickStatus( } /** - * Connection-state badge + "X consecutive failures" + last-error - * surface. Renders nothing when the integration has no history yet - * AND is connected — there's nothing useful to show in that case. + * Collapse the API's four-state machine into the three states the + * pill UI cares about: `error_transient` and `error_reauth` both + * surface as the same "Error — reconnect" pill, the actionable + * difference (whether the user must reconnect vs wait for the next + * retry) is conveyed via the inline error text underneath. */ -function IntegrationStatusBanner({ - status, - threshold, -}: { - status: IntegrationStatusViewModel | undefined; - threshold: number | undefined; -}) { - const { t } = useTranslations(); - if (!status) return null; - - const showBanner = - status.state !== "connected" || - status.consecutiveFailures > 0 || - !!status.lastError || - !!status.lastAttemptAt; - if (!showBanner) return null; - - return ( -
-
- - {status.state !== "connected" && ( - - {t("settings.integrationStatus.consecutiveFailures", { - count: status.consecutiveFailures, - threshold: threshold ?? 3, - })} - - )} -
-
- {status.lastSuccessAt && ( -
-
- {t("settings.integrationStatus.lastSuccess")} -
-
{formatDateTime(status.lastSuccessAt)}
-
- )} - {status.lastAttemptAt && ( -
-
- {t("settings.integrationStatus.lastAttempt")} -
-
{formatDateTime(status.lastAttemptAt)}
-
- )} -
- {status.lastError && ( -
- - - {status.lastError} - -
- )} -
- ); +function pillStateFor( + status: IntegrationStatusViewModel | undefined, +): IntegrationPillState { + if (!status) return "disconnected"; + switch (status.state) { + case "connected": + return "connected"; + case "error_transient": + case "error_reauth": + return "error"; + case "disconnected": + return "disconnected"; + } } -function StatusBadge({ state }: { state: IntegrationState }) { - const { t } = useTranslations(); - switch (state) { - case "connected": - return ( - - {t("settings.integrationStatus.stateConnected")} - - ); - case "error_reauth": - return ( - - {t("settings.integrationStatus.stateReauthRequired")} - - ); - case "error_transient": - return ( - - {t("settings.integrationStatus.stateError")} - - ); - case "disconnected": - return ( - - {t("settings.integrationStatus.stateDisconnected")} - - ); - } +/** + * Inline actionable error message that surfaces under the pill when a + * sync attempt failed. The pill conveys "something is wrong"; this + * line tells the user *what* is wrong so they can act on it. Keeping + * it deliberately small (one icon + one line) so it doesn't recreate + * the v1.4.18 redundant banner Marc removed. + */ +function IntegrationErrorMessage({ message }: { message: string }) { + return ( +

+ + {message} +

+ ); } export function IntegrationsSection() { @@ -229,6 +167,8 @@ export function IntegrationsSection() { const { data: integrationStatus } = useIntegrationStatuses(isAuthenticated); const moodLogEnabled = globalServices?.moodLogGlobal ?? true; + const withingsViewModel = pickStatus(integrationStatus, "withings"); + const moodLogViewModel = pickStatus(integrationStatus, "moodlog"); return (
- } + viewModel={withingsViewModel} /> - {moodLogEnabled && ( - - } - /> - )} + {moodLogEnabled && }
); } function WithingsCard({ isAuthenticated, - statusBanner, + viewModel, }: { isAuthenticated: boolean; - statusBanner: React.ReactNode; + viewModel: IntegrationStatusViewModel | undefined; }) { const { t } = useTranslations(); const [syncing, setSyncing] = useState(false); @@ -392,6 +318,20 @@ function WithingsCard({ setCredsSaving(false); } + // The pill state derives from the cross-integration status + // envelope, but the per-card `lastSyncedAt` is sourced from the + // Withings-specific endpoint so it stays accurate immediately + // after a manual "Sync now" (which only invalidates the per-card + // query). When neither endpoint has answered yet we fall back to + // "disconnected" so the card never renders a status-less header. + const pillState: IntegrationPillState = status?.connected + ? pillStateFor(viewModel) + : "disconnected"; + const pillLastSyncAt = + status?.lastSyncedAt ?? viewModel?.lastSuccessAt ?? null; + const errorMessage = + pillState === "error" && viewModel?.lastError ? viewModel.lastError : null; + return (
@@ -399,36 +339,22 @@ function WithingsCard({

{t("settings.withings")}

-
- {status?.configured && !status?.connected && ( - - {t("settings.configured")} - - )} - {status?.connected && ( - - {t("settings.withingsConnected")} - - )} - {status?.lastSyncedAt && ( - - {t("settings.withingsLastSync")}:{" "} - {formatDateTime(status.lastSyncedAt)} - - )} - {status?.tokenExpired && ( - - {t("settings.withingsTokenExpired")} - - )} -
+

{t("settings.withingsDescription")}

+
+
- {statusBanner} + {errorMessage && }

{t("settings.withingsCredentials")} @@ -607,7 +533,11 @@ function WithingsCard({ ); } -function MoodLogCard({ statusBanner }: { statusBanner: React.ReactNode }) { +function MoodLogCard({ + viewModel, +}: { + viewModel: IntegrationStatusViewModel | undefined; +}) { const { t } = useTranslations(); const queryClient = useQueryClient(); const [url, setUrl] = useState(""); @@ -691,8 +621,20 @@ function MoodLogCard({ statusBanner }: { statusBanner: React.ReactNode }) { } } + // Mirror Withings logic: the pill state comes from the cross- + // integration envelope; per-card `lastSyncedAt` from the Mood Log + // endpoint so a manual sync paints fresh "X min ago" without + // waiting for the envelope to refetch. + const pillState: IntegrationPillState = status?.configured + ? pillStateFor(viewModel) + : "disconnected"; + const pillLastSyncAt = + status?.lastSyncedAt ?? viewModel?.lastSuccessAt ?? null; + const errorMessage = + pillState === "error" && viewModel?.lastError ? viewModel.lastError : null; + return ( -
+
@@ -707,182 +649,186 @@ function MoodLogCard({ statusBanner }: { statusBanner: React.ReactNode }) {

- {status?.configured && ( - - {status.enabled - ? t("settings.withingsConnected") - : t("settings.configured")} - - )} +
-

+

{t("settings.moodLogDescription")}

- {statusBanner} + {/* v1.4.19 A5 — visual divider matches Withings for consistency + (Marc explicitly called the asymmetry out). */} +
-
-
- - setUrl(e.target.value)} - /> -
-
- - setApiKey(e.target.value)} - /> -
-
- - -
-
+
+ {errorMessage && } - {status?.configured && ( -
- {status.webhookSecret && ( -
- -
- - -
-

- {t("settings.moodLogWebhookSecretHelp")} -

-
- )} - -
- {status.lastSyncedAt && ( - - {t("settings.moodLogLastSync")}:{" "} - {formatDateTime(status.lastSyncedAt)} - - )} - - {t("settings.moodLogEntries")}: {status.entryCount} - +
+
+ + setUrl(e.target.value)} + />
- -
+
+ + setApiKey(e.target.value)} + /> +
+
- - - - - - - - - {t("settings.moodLogFullSyncTitle")} - - - {t("settings.moodLogFullSyncDescription")} - - - - {t("common.cancel")} - handleSync(true)}> - {t("settings.moodLogFullSync")} - - - - - - - - {/* v1.4.3: aligned with the Withings disconnect button — - outline + text-destructive instead of solid destructive - so the trigger reads as a reversible action; the actual - "yes, disconnect" confirmation inside the dialog keeps - its solid-red treatment. */} - - - - - - {t("settings.moodLogDisconnectTitle")} - - - {t("settings.moodLogDisconnectDescription")} - - - - {t("common.cancel")} - - {t("settings.moodLogDisconnect")} - - - - +
-
- )} +
- {msg && ( -

- {msg} -

- )} + {status?.configured && ( +
+ {status.webhookSecret && ( +
+ +
+ + +
+

+ {t("settings.moodLogWebhookSecretHelp")} +

+
+ )} + + {/* v1.4.19 A5 — The "letzter Sync" line that used to + live here is gone. The pill in the card header + already carries that information; repeating it + inside the body is the redundancy Marc flagged. The + entry-count is the only number we still surface + because it's an integration-specific datapoint the + pill cannot convey. */} +
+ + {t("settings.moodLogEntries")}: {status.entryCount} + +
+ +
+ + + + + + + + + + {t("settings.moodLogFullSyncTitle")} + + + {t("settings.moodLogFullSyncDescription")} + + + + {t("common.cancel")} + handleSync(true)}> + {t("settings.moodLogFullSync")} + + + + + + + + {/* v1.4.3: aligned with the Withings disconnect button — + outline + text-destructive instead of solid destructive + so the trigger reads as a reversible action; the actual + "yes, disconnect" confirmation inside the dialog keeps + its solid-red treatment. */} + + + + + + {t("settings.moodLogDisconnectTitle")} + + + {t("settings.moodLogDisconnectDescription")} + + + + {t("common.cancel")} + + {t("settings.moodLogDisconnect")} + + + + +
+
+ )} + + {msg && ( +

+ {msg} +

+ )} +
); }