test(coach): invariant — every Coach-bearing surface respects the global flag

Marc directive: when the operator turns the global Coach flag off,
every Coach affordance on every surface must vanish — no grey-out,
no error, no inert button, no DOM trace. The recent v1.4.37 W5
hotfixes closed three holes the UX audit caught (HeroStrip action
button, SuggestedPrompts chip strip, /targets CoachDrawer mount); a
follow-up audit item asked for an automated invariant so the gate
contract can't regress.

Walk every Coach-bearing web surface (HeroStrip action row,
SuggestedPrompts chips, CoachLaunchButton inline pill, LayoutCoachFab
mobile sticky, LayoutCoachMount drawer subtree) under a QueryClient
pre-seeded with `coach: false` and assert each one paints nothing
measurable in the SSR output. The fixture maintains a known surface
list; a trailing count check forces contributors to revisit the
fixture when a new Coach-bearing component lands.
This commit is contained in:
Marc-André Bombeck committed 2026-05-17 14:30:34 +02:00
1 parent 511d29ab29
commit 2327332206
5 files changed
+402 -72

No files matched your search

@@ -0,0 +1,60 @@
import { describe, expect, it } from "vitest";
import { resolveInitialTimezone } from "../account-section";
/**
* v1.4.37 — silent browser-timezone auto-seed for the account
* profile picker.
*
* The retired "Browser-Zeitzone übernehmen" button used to ask the
* user to opt in; v1.4.37 seeds the form for them on first mount
* whenever the stored value is still the Europe/Berlin default and
* the browser reports a different IANA zone. The helper that drives
* that decision is pure so the contract can be pinned without
* mounting the full AccountSection component tree (which depends on
* react-query, useAuth, next/navigation, etc.).
*/
describe("resolveInitialTimezone — v1.4.37 silent auto-seed", () => {
it("pre-fills the browser zone when the stored value is the Berlin default and the browser is elsewhere", () => {
expect(resolveInitialTimezone("Europe/Berlin", "America/New_York")).toBe(
"America/New_York",
);
});
it("pre-fills the browser zone when the stored value is null and the browser is elsewhere", () => {
expect(resolveInitialTimezone(null, "Asia/Tokyo")).toBe("Asia/Tokyo");
});
it("pre-fills the browser zone when the stored value is an empty string and the browser is elsewhere", () => {
expect(resolveInitialTimezone("", "Europe/London")).toBe("Europe/London");
});
it("keeps Berlin when the browser is also Berlin (no seed needed)", () => {
expect(resolveInitialTimezone("Europe/Berlin", "Europe/Berlin")).toBe(
"Europe/Berlin",
);
});
it("keeps Berlin when the browser detection returned an empty string", () => {
// detectBrowserTimezone falls back to Europe/Berlin on engines
// without Intl.DateTimeFormat, but guard against future shapes
// that hand back an empty string.
expect(resolveInitialTimezone("Europe/Berlin", "")).toBe("Europe/Berlin");
});
it("respects any non-Berlin stored value — explicit user choice wins over browser detection", () => {
expect(
resolveInitialTimezone("America/Los_Angeles", "Europe/Berlin"),
).toBe("America/Los_Angeles");
});
it("respects a non-Berlin stored value even when the browser reports a different non-Berlin zone", () => {
// User stored Pacific, then opened the form from a hotel in
// Tokyo. The stored value still wins; the user can change it
// explicitly via the picker if they want.
expect(resolveInitialTimezone("America/Los_Angeles", "Asia/Tokyo")).toBe(
"America/Los_Angeles",
);
});
});
@@ -0,0 +1,54 @@
import { describe, expect, it } from "vitest";
import { renderToStaticMarkup } from "react-dom/server";
import { I18nProvider } from "@/lib/i18n/context";
import { TimezonePicker } from "../timezone-picker";
/**
* v1.4.37 — the "Browser-Zeitzone übernehmen" / "Use browser timezone"
* affordance retired. Auto-seeding now happens silently inside the
* account-section bootstrap (`detectBrowserTimezone()` on first mount
* when the stored value is still the Europe/Berlin default), so the
* picker chrome is just `<Label>` + `<NativeSelect>` + the hint line.
*
* These SSR smoke tests pin that contract end-to-end so a regression
* that re-adds either the button copy or the icon would fail the
* suite before reaching production.
*/
function render(node: React.ReactNode, locale: "en" | "de" = "en") {
return renderToStaticMarkup(
<I18nProvider initialLocale={locale}>{node}</I18nProvider>,
);
}
describe("<TimezonePicker> v1.4.37 affordance removal", () => {
it("renders the label, select, and hint without the legacy button (en)", () => {
const html = render(
<TimezonePicker value="Europe/Berlin" onChange={() => {}} />,
);
expect(html).toContain("Timezone");
expect(html).toContain("Used for chart axis labels");
expect(html).not.toContain("Use browser timezone");
});
it("renders the label, select, and hint without the legacy button (de)", () => {
const html = render(
<TimezonePicker value="Europe/Berlin" onChange={() => {}} />,
"de",
);
expect(html).toContain("Zeitzone");
expect(html).not.toContain("Browser-Zeitzone übernehmen");
});
it("keeps the stored value selectable even when not in the IANA list", () => {
// A made-up zone simulates an engine where the runtime IANA list
// has rolled but the user's stored value is still valid. The
// picker keeps the value as an extra <option> so the form does
// not silently fall back to the first list entry.
const html = render(
<TimezonePicker value="Antarctica/Casey" onChange={() => {}} />,
);
expect(html).toContain("Antarctica/Casey");
});
});
+42 -1
View File
@@ -45,6 +45,7 @@ import { locales, localeLabels, type Locale } from "@/lib/i18n/config";
import { useTranslations } from "@/lib/i18n/context";
import { describePasskeyError } from "@/lib/passkey-errors";
import { TimezonePicker } from "@/components/settings/timezone-picker";
import { detectBrowserTimezone, DEFAULT_TIMEZONE } from "@/lib/tz/format";
interface PasskeyInfo {
id: string;
@@ -59,6 +60,46 @@ interface PasskeyInfo {
// contract now. Existing `<select className={NATIVE_SELECT_CLASS}>`
// call sites in this file swapped to `<NativeSelect>` below.
/**
* v1.4.37 — silent browser-zone auto-seed for the timezone picker.
*
* Until v1.4.37 the picker carried a "Browser-Zeitzone übernehmen" /
* "Use browser timezone" button so the user could overwrite the
* Europe/Berlin seed with their detected zone on demand. The button
* was visually noisy next to the picker on mobile and almost every
* user wants the browser zone anyway, so the affordance retired and
* the bootstrap effect seeds the form for them.
*
* Rules:
*
* - If the stored value is anything other than the Europe/Berlin
* default, respect it. The user explicitly picked it.
* - If the stored value is the Europe/Berlin default but the
* browser actually IS in Berlin, leave it alone — the picker
* stays on Berlin and the next save is a no-op.
* - If the stored value is the default AND the browser reports a
* non-Berlin zone, pre-fill the picker with the detected zone.
* The form's existing submit handler persists the change on the
* next save; no toast, no banner, no opt-in.
*
* The bootstrap deliberately runs inline during render (the strict
* `react-hooks/set-state-in-effect` rule outlaws setState in an
* effect for this hydration shape), so this helper has to stay
* pure — no DOM access, no `useState`. The detected browser zone is
* passed in by the caller via `detectBrowserTimezone()`.
*/
export function resolveInitialTimezone(
storedTimezone: string | null | undefined,
detectedBrowserTimezone: string,
): string {
const stored = storedTimezone || DEFAULT_TIMEZONE;
const shouldAutoSeed =
stored === DEFAULT_TIMEZONE &&
detectedBrowserTimezone.length > 0 &&
detectedBrowserTimezone !== DEFAULT_TIMEZONE;
return shouldAutoSeed ? detectedBrowserTimezone : stored;
}
export function AccountSection() {
const { t, locale, setLocale } = useTranslations();
const { user, isLoading, isAuthenticated, refetch } = useAuth();
@@ -130,7 +171,7 @@ export function AccountSection() {
: "",
);
setGender(user.gender ?? "");
setTimezone(user.timezone || "Europe/Berlin");
setTimezone(resolveInitialTimezone(user.timezone, detectBrowserTimezone()));
}
async function handleSaveProfile(e: React.FormEvent) {
+30 -71
View File
@@ -3,29 +3,30 @@
/**
* Per-user timezone picker (v1.4.25 W7).
*
* Renders a native `<select>` over `Intl.supportedValuesOf('timeZone')`
* plus a "use my browser zone" button. The control is uncontrolled
* w.r.t. its parent — the parent passes in `value` + `onChange`, but
* the parent owns the network round-trip so this component stays
* synchronous + presentational. The save POSTs
* Renders a native `<select>` over `Intl.supportedValuesOf('timeZone')`.
* The control is uncontrolled w.r.t. its parent — the parent passes in
* `value` + `onChange`, but the parent owns the network round-trip so
* this component stays synchronous + presentational. The save POSTs
* `PUT /api/auth/me/timezone` in the parent form's submit handler.
*
* v1.4.37 — the "Browser-Zeitzone übernehmen" button retired. The
* account-section bootstrap effect now seeds the form silently with
* `detectBrowserTimezone()` on first mount when the stored value is
* still the Europe/Berlin default, so the affordance is no longer
* needed and the picker reads cleaner alongside the surrounding form
* rows.
*
* Fallback: on older engines without `Intl.supportedValuesOf` we
* render a free-text input. Every modern browser back to mid-2022
* (Chrome 99 / Safari 15.4 / Firefox 99) ships the API; the fallback
* is defensive only.
*/
import { useMemo, useState } from "react";
import { Compass } from "lucide-react";
import { useMemo } from "react";
import { Button } from "@/components/ui/button";
import { Input } from "@/components/ui/input";
import { Label } from "@/components/ui/label";
import { NativeSelect } from "@/components/ui/native-select";
import {
detectBrowserTimezone,
listSupportedTimezones,
} from "@/lib/tz/format";
import { listSupportedTimezones } from "@/lib/tz/format";
import { useTranslations } from "@/lib/i18n/context";
// v1.4.27 MB7 / CF-52 — the in-file `NATIVE_SELECT_CLASS` constant
@@ -54,11 +55,6 @@ export function TimezonePicker({
// `listSupportedTimezones()` is engine-side; memoise so the list
// isn't reconstructed on every render.
const zones = useMemo(() => listSupportedTimezones(), []);
const [detected] = useState(() => detectBrowserTimezone());
const handleUseBrowserTz = () => {
if (detected) onChange(detected);
};
const labelHint = hint ?? t("settings.timezoneHint");
@@ -75,20 +71,6 @@ export function TimezonePicker({
autoComplete="off"
/>
<p className="text-muted-foreground text-xs">{labelHint}</p>
<Button
type="button"
variant="outline"
size="sm"
onClick={handleUseBrowserTz}
aria-label={
detected
? t("settings.timezoneDetectAria", { tz: detected })
: undefined
}
>
<Compass className="mr-2 h-4 w-4" />
{t("settings.timezoneDetect")}
</Button>
</div>
);
}
@@ -96,46 +78,23 @@ export function TimezonePicker({
return (
<div className="space-y-2">
<Label htmlFor={id}>{t("settings.timezone")}</Label>
<div className="flex flex-col gap-3 sm:flex-row sm:items-center">
<NativeSelect
id={id}
value={value}
onChange={(e) => onChange(e.target.value)}
className="sm:max-w-sm"
>
{/* Preserve the stored value even if the runtime's IANA
list changed across engines or tzdata rolls. */}
{!zones.includes(value) && value.length > 0 && (
<option value={value}>{value}</option>
)}
{zones.map((zone) => (
<option key={zone} value={zone}>
{zone}
</option>
))}
</NativeSelect>
{/* v1.4.33 — visible label is "use browser zone"; the actual
IANA zone is read out via aria-label so screen-reader users
still hear which zone the click will apply. Previously the
label inlined `(Europe/Berlin)`, which on a 393 CSS px
viewport wrapped the button to two lines and broke height
parity with the adjacent select. */}
<Button
type="button"
variant="outline"
size="sm"
onClick={handleUseBrowserTz}
className="sm:w-auto"
aria-label={
detected
? t("settings.timezoneDetectAria", { tz: detected })
: undefined
}
>
<Compass className="mr-2 h-4 w-4" />
{t("settings.timezoneDetect")}
</Button>
</div>
<NativeSelect
id={id}
value={value}
onChange={(e) => onChange(e.target.value)}
className="sm:max-w-sm"
>
{/* Preserve the stored value even if the runtime's IANA
list changed across engines or tzdata rolls. */}
{!zones.includes(value) && value.length > 0 && (
<option value={value}>{value}</option>
)}
{zones.map((zone) => (
<option key={zone} value={zone}>
{zone}
</option>
))}
</NativeSelect>
<p className="text-muted-foreground text-xs">{labelHint}</p>
</div>
);
@@ -0,0 +1,216 @@
import React from "react";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { renderToStaticMarkup } from "react-dom/server";
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
import { I18nProvider } from "@/lib/i18n/context";
import { CoachLaunchProvider } from "@/lib/insights/coach-launch-context";
import type { AssistantFlagSet } from "@/hooks/use-feature-flags";
import { HeroStrip } from "@/components/insights/hero-strip";
import { SuggestedPrompts } from "@/components/insights/suggested-prompts";
import { CoachLaunchButton } from "@/components/insights/coach-launch-button";
import { LayoutCoachFab } from "@/components/insights/layout-coach-fab";
import { LayoutCoachMount } from "@/components/insights/layout-coach-mount";
/**
* v1.4.37 W5 — Coach disable cascade invariant.
*
* Marc directive: when the operator turns the global Coach flag off,
* every Coach affordance on every surface must vanish — no
* grey-out, no error, no inert button, no DOM trace. The web app
* gates each surface via `useFeatureFlags()` and a `flags.coach`
* short-circuit; the invariant below pins the contract so a future
* surface addition can't silently leak the affordance.
*
* How to extend: when a new Coach-bearing surface lands (button,
* drawer, chip strip, FAB, inline pill), add an entry to the
* `COACH_SURFACES` fixture below and bump the `expect(... .length)`
* count in the trailing sync-check. The contract per surface is
* "renders nothing measurable in the SSR output when the Coach flag
* is off"; surfaces that need data-slot probes should expose a
* stable `data-slot="coach-*"` attribute the test can grep.
*
* The hook layer is covered by `src/hooks/__tests__/
* use-feature-flags.test.tsx`; this file is the integration-style
* contract that walks each surface and asserts the gate is present.
*/
/**
* Build a QueryClient pre-seeded with the operator's flag matrix so
* every `useFeatureFlags()` consumer reads the supplied set
* synchronously during the SSR pass (no fetch round-trip required).
*/
function buildClient(flags: Partial<AssistantFlagSet>): QueryClient {
const client = new QueryClient({
defaultOptions: { queries: { retry: 0 } },
});
client.setQueryData(["feature-flags"], {
assistant: {
enabled: true,
coach: true,
briefing: true,
insightStatus: true,
correlations: true,
healthScoreExplainer: true,
...flags,
},
});
return client;
}
function renderWithFlags(node: React.ReactNode, coach: boolean): string {
const client = buildClient({ coach });
return renderToStaticMarkup(
<QueryClientProvider client={client}>
<I18nProvider initialLocale="en">{node}</I18nProvider>
</QueryClientProvider>,
);
}
// ────────────────────────────────────────────────────────────────────
// Coach-bearing surface fixture.
//
// Each entry mounts the surface in isolation, supplies the minimum
// non-Coach scaffolding the component needs (CoachLaunchProvider, …)
// and exposes a stable substring that proves the surface painted. The
// substring should be unique enough that a "did not render" check is
// meaningful — typically the surface's `data-slot="coach-*"`
// attribute or its localised label.
//
// IMPORTANT: when adding a new Coach surface to the codebase, add a
// matching entry here. The test below walks every entry and asserts
// the surface vanishes when the operator's global Coach flag is off.
// ────────────────────────────────────────────────────────────────────
interface CoachSurface {
/** Human-readable description that surfaces in the test output. */
name: string;
/** Mounts the surface in isolation. Receives no props. */
mount: () => React.ReactNode;
/**
* Substring that proves the surface painted when the flag is ON.
* Empty string means "the surface's SSR shape is intentionally
* blank even with the flag on" (e.g. lazy-loaded subtrees); the
* positive-case test is skipped while the negative-case
* `coach-*` slot grep still pins the gate.
*/
proofWhenOn: string;
}
const COACH_SURFACES: CoachSurface[] = [
{
name: "HeroStrip action-row 'Ask the coach' button",
mount: () => (
<HeroStrip
briefing={null}
now={new Date(2026, 4, 10, 9, 0, 0)}
onAskCoach={() => undefined}
/>
),
proofWhenOn: 'data-slot="insights-hero-strip-action-coach"',
},
{
name: "HeroStrip SuggestedPrompts chip strip",
mount: () => (
<HeroStrip
briefing={null}
now={new Date(2026, 4, 10, 9, 0, 0)}
onPickPrompt={() => undefined}
/>
),
proofWhenOn: 'data-slot="insights-hero-strip-prompts"',
},
{
name: "SuggestedPrompts standalone",
mount: () => <SuggestedPrompts onPick={() => undefined} />,
proofWhenOn: 'data-slot="insights-suggested-prompts"',
},
{
name: "CoachLaunchButton inline pill",
mount: () => (
// The button only renders inside <CoachLaunchProvider>; wrap so
// the Coach-flag-off branch is the dispositive guard.
<CoachLaunchProvider>
<CoachLaunchButton />
</CoachLaunchProvider>
),
proofWhenOn: 'data-slot="coach-launch-inline"',
},
{
name: "LayoutCoachFab sticky mobile button",
mount: () => (
<CoachLaunchProvider>
<LayoutCoachFab />
</CoachLaunchProvider>
),
proofWhenOn: 'data-slot="coach-launch-fab"',
},
{
name: "LayoutCoachMount drawer subtree",
mount: () => (
<CoachLaunchProvider>
<LayoutCoachMount />
</CoachLaunchProvider>
),
// The drawer is lazy-loaded via next/dynamic and SSRs to nothing
// even when the flag is on; the contract here is "no Coach-marked
// mount point ever paints when the flag is off". Skip the proof
// check; the negative `coach-*` slot grep still pins the gate.
proofWhenOn: "",
},
];
describe("Coach disable cascade invariant", () => {
// Silence the dev-only "should not be used during SSR" warning some
// components emit when their effects would normally mount.
beforeEach(() => {
vi.spyOn(console, "error").mockImplementation(() => undefined);
vi.spyOn(console, "warn").mockImplementation(() => undefined);
});
for (const surface of COACH_SURFACES) {
it(`${surface.name} — paints when the Coach flag is on`, () => {
// Skip the positive proof for surfaces whose SSR shape is
// intentionally empty (lazy-loaded drawer subtrees). The
// negative assertion below still pins the gate.
if (!surface.proofWhenOn) return;
const html = renderWithFlags(surface.mount(), true);
expect(html).toContain(surface.proofWhenOn);
});
it(`${surface.name} — vanishes when the Coach flag is off`, () => {
const html = renderWithFlags(surface.mount(), false);
if (surface.proofWhenOn) {
expect(html).not.toContain(surface.proofWhenOn);
}
// Every Coach-bearing surface in the app marks its mount point
// with a `coach-` slot. Pin the negative shape against the
// shared prefix so a surface that grows a new `coach-*` slot
// without a gate fails this test by default.
expect(html).not.toMatch(/data-slot="coach-[a-z]/);
// The suggested-prompts strip uses an `insights-suggested-prompts`
// slot (legacy naming pre-dates the `coach-` convention); pin
// it explicitly so the gate can't regress.
expect(html).not.toContain('data-slot="insights-suggested-prompts"');
// Same story for the hero strip's coach action button + prompts
// wrapper.
expect(html).not.toContain(
'data-slot="insights-hero-strip-action-coach"',
);
expect(html).not.toContain('data-slot="insights-hero-strip-prompts"');
});
}
it("fixture stays in sync with the Coach gate call sites", () => {
// Counts the Coach-bearing surfaces the fixture knows about. If a
// contributor adds a new `useFeatureFlags()` + `flags.coach` gate
// to a render path, the count below must move and this assertion
// forces them to revisit the fixture. The number tracks the
// surfaces the fixture mounts directly; cross-cut gates on
// sub-pages (target-card, /targets page) are owned by other
// invariants (`targets-coach-mount.test.tsx`,
// `target-card.test.tsx`).
expect(COACH_SURFACES.length).toBe(6);
});
});