mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-11 22:02:16 +00:00
fix(onboarding): stop the Withings connect dead-end and disclose BYO setup
The onboarding "Connect Withings" tap opened /api/withings/connect in a new tab, which returned a raw JSON 400 for every brand-new account (Withings credentials are per-user BYO with no env fallback, and the wizard never collects them). WHOOP, Fitbit, Polar, Oura, and Strava shared the identical raw-JSON-on-no-creds pattern on their own /connect routes, while every other failure branch already redirected with a toast reason. All six now redirect to /settings/integrations?<provider>=error&reason=nocreds instead, and the wizard's Withings card checks the integrations-status envelope first: unless credentials are already configured, the CTA reads "Set up in Settings" and links straight to the credentials form rather than launching a doomed OAuth handshake. Also surfaces the BYO-developer-app requirement, which was invisible until a user was deep inside a Settings card. The source-picker step now says up front that device sources need a free developer app on the user's own account; each of the six provider cards gets a compact 3-step guide with the exact callback URL to register, shown until the user has saved their own credentials.
This commit is contained in:
1 parent
4f7420905a
commit
fb73c97125
27 files changed
+744
-73
No files matched your search
@@ -0,0 +1,91 @@
|
||||
import { describe, it, expect, vi } from "vitest";
|
||||
import { renderToStaticMarkup } from "react-dom/server";
|
||||
|
||||
import { I18nProvider } from "@/lib/i18n/context";
|
||||
|
||||
/**
|
||||
* v1.29.x — UX audit H1: the wizard's Withings card must not launch an
|
||||
* OAuth handshake that can only 400 for a brand-new account (Withings
|
||||
* credentials are per-user BYO with no env fallback). The card reads the
|
||||
* consolidated integrations-status envelope and swaps its CTA + href
|
||||
* depending on whether the account already has its own credentials saved.
|
||||
*/
|
||||
|
||||
const integrationStatusState = vi.hoisted(() => ({
|
||||
data: undefined as
|
||||
| {
|
||||
threshold: number;
|
||||
integrations: Array<{ integration: string; configured?: boolean }>;
|
||||
}
|
||||
| undefined,
|
||||
}));
|
||||
|
||||
vi.mock("@tanstack/react-query", () => ({
|
||||
useQuery: () => ({ data: integrationStatusState.data }),
|
||||
useQueryClient: () => ({ invalidateQueries: vi.fn() }),
|
||||
}));
|
||||
|
||||
vi.mock("@/hooks/use-auth", () => ({
|
||||
useAuth: () => ({ isAuthenticated: true }),
|
||||
}));
|
||||
|
||||
vi.mock("next/navigation", () => ({
|
||||
useRouter: () => ({ push: vi.fn(), replace: vi.fn() }),
|
||||
}));
|
||||
|
||||
vi.mock("next/link", () => ({
|
||||
default: ({
|
||||
href,
|
||||
children,
|
||||
...rest
|
||||
}: {
|
||||
href: string;
|
||||
children: React.ReactNode;
|
||||
}) => (
|
||||
<a href={href} {...rest}>
|
||||
{children}
|
||||
</a>
|
||||
),
|
||||
}));
|
||||
|
||||
import { SourceCardGrid } from "../source-card-grid";
|
||||
|
||||
function render() {
|
||||
return renderToStaticMarkup(
|
||||
<I18nProvider initialLocale="en">
|
||||
<SourceCardGrid />
|
||||
</I18nProvider>,
|
||||
);
|
||||
}
|
||||
|
||||
describe("<SourceCardGrid> Withings card", () => {
|
||||
it("points at Settings instead of launching OAuth when unconfigured", () => {
|
||||
integrationStatusState.data = {
|
||||
threshold: 3,
|
||||
integrations: [{ integration: "withings", configured: false }],
|
||||
};
|
||||
const html = render();
|
||||
expect(html).toContain('data-testid="source-card-withings-setup"');
|
||||
expect(html).toContain('href="/settings/integrations#withings"');
|
||||
expect(html).not.toContain('href="/api/withings/connect"');
|
||||
expect(html).toContain("Set up in Settings");
|
||||
});
|
||||
|
||||
it("also redirects to Settings when the status envelope hasn't loaded yet", () => {
|
||||
integrationStatusState.data = undefined;
|
||||
const html = render();
|
||||
expect(html).toContain('href="/settings/integrations#withings"');
|
||||
expect(html).not.toContain('href="/api/withings/connect"');
|
||||
});
|
||||
|
||||
it("opens the real OAuth connect flow once credentials are configured", () => {
|
||||
integrationStatusState.data = {
|
||||
threshold: 3,
|
||||
integrations: [{ integration: "withings", configured: true }],
|
||||
};
|
||||
const html = render();
|
||||
expect(html).toContain('href="/api/withings/connect"');
|
||||
expect(html).not.toContain('data-testid="source-card-withings-setup"');
|
||||
expect(html).toContain("Connect Withings");
|
||||
});
|
||||
});
|
||||
@@ -15,6 +15,11 @@ import {
|
||||
} from "lucide-react";
|
||||
|
||||
import { Button } from "@/components/ui/button";
|
||||
import {
|
||||
pickStatus,
|
||||
useIntegrationStatuses,
|
||||
} from "@/components/settings/integrations/shared";
|
||||
import { useAuth } from "@/hooks/use-auth";
|
||||
import { useTranslations } from "@/lib/i18n/context";
|
||||
import { cn } from "@/lib/utils";
|
||||
import { apiPost } from "@/lib/api/api-fetch";
|
||||
@@ -26,8 +31,12 @@ import { queryKeys } from "@/lib/query-keys";
|
||||
* Presents the data sources HealthLog ships today. The headline cards
|
||||
* cover the two paths most people start on:
|
||||
* 1. Manual entry (enabled, recommended — works on any device)
|
||||
* 2. Withings (enabled — opens OAuth `/api/withings/connect` in a new
|
||||
* tab; the callback flips the connection server-side)
|
||||
* 2. Withings — opens OAuth `/api/withings/connect` in a new tab once
|
||||
* the account has its own Withings developer app configured; the
|
||||
* callback flips the connection server-side. Before that, the card
|
||||
* is honest about the prerequisite (v1.29.x, UX audit H1/H2): the
|
||||
* CTA reads "Set up in Settings" and points at the credentials form
|
||||
* instead of launching an OAuth handshake that can only 400.
|
||||
*
|
||||
* Below them a calm "more sources" row lists the remaining shipped
|
||||
* integrations (Apple Health, WHOOP, Oura, Polar, Nightscout, Fitbit),
|
||||
@@ -97,10 +106,20 @@ export function SourceCardGrid() {
|
||||
const { t } = useTranslations();
|
||||
const router = useRouter();
|
||||
const queryClient = useQueryClient();
|
||||
const { isAuthenticated } = useAuth();
|
||||
|
||||
const [selected, setSelected] = useState<SourceCard["slug"]>("manual");
|
||||
const [advancing, setAdvancing] = useState(false);
|
||||
|
||||
// v1.29.x — the wizard's "Connect Withings" card must not launch an OAuth
|
||||
// handshake that can only fail: Withings credentials are per-user BYO with
|
||||
// no env fallback, and a brand-new account never has them yet. Read the
|
||||
// same consolidated envelope Settings → Integrations uses so the card
|
||||
// knows whether the handshake can actually succeed.
|
||||
const { data: integrationStatus } = useIntegrationStatuses(isAuthenticated);
|
||||
const withingsConfigured =
|
||||
pickStatus(integrationStatus, "withings")?.configured ?? false;
|
||||
|
||||
async function advance() {
|
||||
if (advancing) return;
|
||||
setAdvancing(true);
|
||||
@@ -144,6 +163,9 @@ export function SourceCardGrid() {
|
||||
card.recommendedKey ? t(card.recommendedKey) : null
|
||||
}
|
||||
withingsCtaLabel={t("onboarding.source.withings.cta")}
|
||||
withingsConfigured={withingsConfigured}
|
||||
withingsSetupCtaLabel={t("onboarding.source.withings.setupCta")}
|
||||
withingsSetupHint={t("onboarding.source.withings.setupHint")}
|
||||
/>
|
||||
))}
|
||||
</div>
|
||||
@@ -234,6 +256,14 @@ interface SourceCardItemProps {
|
||||
bodyLabel: string;
|
||||
recommendedLabel: string | null;
|
||||
withingsCtaLabel: string;
|
||||
/** Whether the account already has its own Withings developer-app
|
||||
* credentials saved — i.e. whether `/api/withings/connect` can
|
||||
* actually succeed right now. */
|
||||
withingsConfigured: boolean;
|
||||
/** CTA label shown instead of "Connect Withings" when unconfigured. */
|
||||
withingsSetupCtaLabel: string;
|
||||
/** One-sentence explanation of the BYO credential prerequisite. */
|
||||
withingsSetupHint: string;
|
||||
}
|
||||
|
||||
function SourceCardItem({
|
||||
@@ -244,6 +274,9 @@ function SourceCardItem({
|
||||
bodyLabel,
|
||||
recommendedLabel,
|
||||
withingsCtaLabel,
|
||||
withingsConfigured,
|
||||
withingsSetupCtaLabel,
|
||||
withingsSetupHint,
|
||||
}: SourceCardItemProps) {
|
||||
const Icon = card.Icon;
|
||||
|
||||
@@ -292,20 +325,40 @@ function SourceCardItem({
|
||||
>
|
||||
{headerRow}
|
||||
{body}
|
||||
<div className="flex items-center gap-2 pt-1">
|
||||
<Button asChild variant="outline" size="sm">
|
||||
<a
|
||||
href={card.href}
|
||||
target="_blank"
|
||||
rel="noopener"
|
||||
onClick={onSelect}
|
||||
className="inline-flex items-center gap-1.5"
|
||||
>
|
||||
<Wifi className="size-3.5" />
|
||||
{withingsCtaLabel}
|
||||
</a>
|
||||
</Button>
|
||||
</div>
|
||||
{withingsConfigured ? (
|
||||
<div className="flex items-center gap-2 pt-1">
|
||||
<Button asChild variant="outline" size="sm">
|
||||
<a
|
||||
href={card.href}
|
||||
target="_blank"
|
||||
rel="noopener"
|
||||
onClick={onSelect}
|
||||
className="inline-flex items-center gap-1.5"
|
||||
>
|
||||
<Wifi className="size-3.5" />
|
||||
{withingsCtaLabel}
|
||||
</a>
|
||||
</Button>
|
||||
</div>
|
||||
) : (
|
||||
// No per-user Withings credentials yet — an OAuth handshake here
|
||||
// can only 400 (Withings has no env fallback). Point at the
|
||||
// credentials form instead of launching a doomed connect flow.
|
||||
<div className="space-y-2 pt-1">
|
||||
<p className="text-muted-foreground text-xs">{withingsSetupHint}</p>
|
||||
<Button asChild variant="outline" size="sm">
|
||||
<Link
|
||||
href="/settings/integrations#withings"
|
||||
onClick={onSelect}
|
||||
className="inline-flex items-center gap-1.5"
|
||||
data-testid="source-card-withings-setup"
|
||||
>
|
||||
<Wifi className="size-3.5" />
|
||||
{withingsSetupCtaLabel}
|
||||
</Link>
|
||||
</Button>
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -148,3 +148,33 @@ describe("OAuthProviderCard — per-user BYO credentials form (v1.17.1)", () =>
|
||||
expect(html).toContain("Saved — enter new to replace");
|
||||
});
|
||||
});
|
||||
|
||||
describe("OAuthProviderCard — redirect-URI mini-guide (v1.29.x, UX audit H2)", () => {
|
||||
it("shows the callback URL guide before the user has BYO credentials", () => {
|
||||
process.env.NEXT_PUBLIC_APP_URL = "https://app.example";
|
||||
statusPayload = {
|
||||
connected: false,
|
||||
configured: false,
|
||||
available: true,
|
||||
hasOwnCredentials: false,
|
||||
};
|
||||
const html = render({ credentials: true });
|
||||
expect(html).toContain('data-testid="polar-redirect-guide"');
|
||||
expect(html).toContain('data-testid="polar-redirect-uri"');
|
||||
expect(html).toContain("https://app.example/api/polar/callback");
|
||||
});
|
||||
|
||||
it("hides the guide once the user has stored their own credentials", () => {
|
||||
statusPayload = {
|
||||
connected: true,
|
||||
configured: true,
|
||||
available: true,
|
||||
hasOwnCredentials: true,
|
||||
state: "connected",
|
||||
lastSuccessAt: null,
|
||||
lastError: null,
|
||||
};
|
||||
const html = render({ credentials: true });
|
||||
expect(html).not.toContain('data-testid="polar-redirect-guide"');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,41 @@
|
||||
/**
|
||||
* v1.29.x — UX audit H2: the WHOOP card shows a compact callback-URL guide
|
||||
* while the account has no BYO credentials saved, and steps out of the way
|
||||
* once configured. Mirrors the OAuthProviderCard coverage for Polar/Oura.
|
||||
*/
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { renderToStaticMarkup } from "react-dom/server";
|
||||
|
||||
vi.mock("@tanstack/react-query", () => ({
|
||||
useMutation: () => ({ mutate: vi.fn(), isPending: false }),
|
||||
useQueryClient: () => ({ invalidateQueries: vi.fn() }),
|
||||
}));
|
||||
|
||||
import { I18nProvider } from "@/lib/i18n/context";
|
||||
import { WhoopCard } from "../whoop-card";
|
||||
import type { IntegrationStatusViewModel } from "../shared";
|
||||
|
||||
function render(viewModel?: Partial<IntegrationStatusViewModel>) {
|
||||
return renderToStaticMarkup(
|
||||
<I18nProvider initialLocale="en">
|
||||
<WhoopCard
|
||||
viewModel={viewModel as IntegrationStatusViewModel | undefined}
|
||||
/>
|
||||
</I18nProvider>,
|
||||
);
|
||||
}
|
||||
|
||||
describe("<WhoopCard> redirect-URI mini-guide", () => {
|
||||
it("shows the callback URL guide before credentials are configured", () => {
|
||||
process.env.NEXT_PUBLIC_APP_URL = "https://app.example";
|
||||
const html = render({ connected: false, configured: false });
|
||||
expect(html).toContain('data-testid="whoop-redirect-guide"');
|
||||
expect(html).toContain('data-testid="whoop-redirect-uri"');
|
||||
expect(html).toContain("https://app.example/api/whoop/callback");
|
||||
});
|
||||
|
||||
it("hides the guide once credentials are configured", () => {
|
||||
const html = render({ connected: false, configured: true });
|
||||
expect(html).not.toContain('data-testid="whoop-redirect-guide"');
|
||||
});
|
||||
});
|
||||
@@ -52,7 +52,10 @@ import {
|
||||
pillStateFor,
|
||||
type IntegrationStatusViewModel,
|
||||
} from "./shared";
|
||||
import { IntegrationCardDescription } from "./setup-guide-link";
|
||||
import {
|
||||
IntegrationCardDescription,
|
||||
IntegrationRedirectGuide,
|
||||
} from "./setup-guide-link";
|
||||
|
||||
export function FitbitCard({
|
||||
viewModel,
|
||||
@@ -283,6 +286,12 @@ export function FitbitCard({
|
||||
<h3 className="text-sm font-semibold">
|
||||
{t("settings.fitbitCredentials")}
|
||||
</h3>
|
||||
{!status?.configured && (
|
||||
<IntegrationRedirectGuide
|
||||
provider="fitbit"
|
||||
providerLabel={t("settings.fitbit")}
|
||||
/>
|
||||
)}
|
||||
<form onSubmit={handleSaveCredentials} className="space-y-3">
|
||||
<div className="grid gap-3 sm:grid-cols-2">
|
||||
<div className="space-y-1.5">
|
||||
|
||||
@@ -51,6 +51,7 @@ import {
|
||||
} from "@/lib/query-keys";
|
||||
import {
|
||||
IntegrationCardDescription,
|
||||
IntegrationRedirectGuide,
|
||||
type IntegrationDocsProvider,
|
||||
} from "./setup-guide-link";
|
||||
|
||||
@@ -277,6 +278,12 @@ export function OAuthProviderCard({
|
||||
<h3 className="text-sm font-semibold">
|
||||
{t(`${i18nPrefix}Credentials`)}
|
||||
</h3>
|
||||
{!status?.hasOwnCredentials && (
|
||||
<IntegrationRedirectGuide
|
||||
provider={provider}
|
||||
providerLabel={t(i18nPrefix)}
|
||||
/>
|
||||
)}
|
||||
<form onSubmit={handleSaveCredentials} className="space-y-3">
|
||||
<div className="grid gap-3 sm:grid-cols-2">
|
||||
<div className="space-y-1.5">
|
||||
|
||||
@@ -38,6 +38,69 @@ export function integrationDocsHref(provider: IntegrationDocsProvider): string {
|
||||
return `${INTEGRATION_DOCS_BASE}/${provider}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* The OAuth callback path every BYO-key provider registers against. Every
|
||||
* provider's `getRedirectUri()` (`src/lib/{provider}/client.ts`) derives the
|
||||
* same shape from `NEXT_PUBLIC_APP_URL` unless an operator overrides it with
|
||||
* an explicit `*_REDIRECT_URI` env var — the common case this card targets.
|
||||
*/
|
||||
export function integrationCallbackUrl(
|
||||
provider: IntegrationDocsProvider,
|
||||
): string {
|
||||
const base = process.env.NEXT_PUBLIC_APP_URL ?? "";
|
||||
return `${base}/api/${provider}/callback`;
|
||||
}
|
||||
|
||||
/**
|
||||
* v1.29.x (UX audit H2) — compact 3-step BYO-developer-app guide for the
|
||||
* six providers that require a self-hoster to register their own vendor
|
||||
* app before an OAuth connect can ever succeed (Withings / WHOOP / Fitbit /
|
||||
* Polar / Oura / Strava). The external "Setup guide" link explains the
|
||||
* *why*; this inline callout gives the one value every vendor form asks
|
||||
* for and the app already knows — the callback URL — so it doesn't have to
|
||||
* be reconstructed by hand from the docs. Shown only while the user hasn't
|
||||
* saved their own credentials yet; it steps out of the way once configured.
|
||||
*/
|
||||
export function IntegrationRedirectGuide({
|
||||
provider,
|
||||
providerLabel,
|
||||
}: {
|
||||
provider: IntegrationDocsProvider;
|
||||
providerLabel: string;
|
||||
}) {
|
||||
const { t } = useTranslations();
|
||||
const callbackUrl = integrationCallbackUrl(provider);
|
||||
return (
|
||||
<div
|
||||
className="bg-muted/40 border-border/60 space-y-1.5 rounded-md border p-3 text-xs"
|
||||
data-testid={`${provider}-redirect-guide`}
|
||||
>
|
||||
<p className="text-muted-foreground">
|
||||
{t("settings.integrationRedirectGuide.intro", {
|
||||
provider: providerLabel,
|
||||
})}
|
||||
</p>
|
||||
<ol className="text-muted-foreground list-decimal space-y-1 pl-4">
|
||||
<li>
|
||||
{t("settings.integrationRedirectGuide.step1", {
|
||||
provider: providerLabel,
|
||||
})}
|
||||
</li>
|
||||
<li>
|
||||
{t("settings.integrationRedirectGuide.step2")}{" "}
|
||||
<code
|
||||
className="bg-background text-foreground rounded px-1 py-0.5 font-mono break-all"
|
||||
data-testid={`${provider}-redirect-uri`}
|
||||
>
|
||||
{callbackUrl}
|
||||
</code>
|
||||
</li>
|
||||
<li>{t("settings.integrationRedirectGuide.step3")}</li>
|
||||
</ol>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* v1.18.7 (Wave F) — the one description skeleton every integration card paints:
|
||||
* a WHITE primary sentence (`${i18nPrefix}Description`), then a GREY secondary
|
||||
|
||||
@@ -44,7 +44,10 @@ import {
|
||||
pillStateFor,
|
||||
type IntegrationStatusViewModel,
|
||||
} from "./shared";
|
||||
import { IntegrationCardDescription } from "./setup-guide-link";
|
||||
import {
|
||||
IntegrationCardDescription,
|
||||
IntegrationRedirectGuide,
|
||||
} from "./setup-guide-link";
|
||||
|
||||
export function WhoopCard({
|
||||
viewModel,
|
||||
@@ -264,6 +267,12 @@ export function WhoopCard({
|
||||
<h3 className="text-sm font-semibold">
|
||||
{t("settings.whoopCredentials")}
|
||||
</h3>
|
||||
{!status?.configured && (
|
||||
<IntegrationRedirectGuide
|
||||
provider="whoop"
|
||||
providerLabel={t("settings.whoop")}
|
||||
/>
|
||||
)}
|
||||
<form onSubmit={handleSaveCredentials} className="space-y-3">
|
||||
<div className="grid gap-3 sm:grid-cols-2">
|
||||
<div className="space-y-1.5">
|
||||
|
||||
@@ -37,7 +37,10 @@ import {
|
||||
pillStateFor,
|
||||
type IntegrationStatusViewModel,
|
||||
} from "./shared";
|
||||
import { IntegrationCardDescription } from "./setup-guide-link";
|
||||
import {
|
||||
IntegrationCardDescription,
|
||||
IntegrationRedirectGuide,
|
||||
} from "./setup-guide-link";
|
||||
|
||||
export function WithingsCard({
|
||||
viewModel,
|
||||
@@ -299,6 +302,12 @@ export function WithingsCard({
|
||||
<h3 className="text-sm font-semibold">
|
||||
{t("settings.withingsCredentials")}
|
||||
</h3>
|
||||
{!status?.configured && (
|
||||
<IntegrationRedirectGuide
|
||||
provider="withings"
|
||||
providerLabel={t("settings.withings")}
|
||||
/>
|
||||
)}
|
||||
<form onSubmit={handleSaveCredentials} className="space-y-3">
|
||||
{/* v1.4.27 MB7 / CF-53 — the credentials grid drops from
|
||||
a 3-column row (client-id / secret / save) to a
|
||||
|
||||
Reference in new issue
Block a user