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} +

+ )} +
); }