From 5a6f92192890fffa436d29c8073499da2df7462e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 17 May 2026 14:25:57 +0200 Subject: [PATCH] fix(targets): gate CoachDrawer on /targets behind the Coach flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /targets carries two Coach affordances — the per-card 'Ask Coach' CTA on every TargetCard and the page-level CoachDrawer mount. The CTAs were gated through `aiEnabled` (provider chain status), but the drawer mount always rendered regardless of the operator's global Coach flag. When the operator turned Coach off in Settings the drawer still loaded its SSE machinery + Sheet portal subtree even though no on-page CTA could open it. Read flags.coach via useFeatureFlags and fold it into the existing `aiEnabled` gate so the per-card CTAs vanish when either layer is off. Wrap the page-level mount in the same gate as defence-in-depth, since the CTAs already hide when the flag is off. Tests: add two new cases to targets-coach-mount.test.tsx covering the Coach-flag-off branch (CTAs hide + drawer mount disappears). The sibling targets-* test files mock @tanstack/react-query without exposing QueryClientContext; add a use-feature-flags mock that short-circuits to the all-on default so the existing assertions keep running against the visible page shape. --- .../__tests__/targets-coach-mount.test.tsx | 54 ++++++++++++++++--- src/app/__tests__/targets-i18n.test.tsx | 16 ++++++ src/app/__tests__/targets-responsive.test.tsx | 16 ++++++ src/app/__tests__/targets-spacing.test.tsx | 16 ++++++ src/app/targets/page.tsx | 32 ++++++++--- 5 files changed, 120 insertions(+), 14 deletions(-) diff --git a/src/app/__tests__/targets-coach-mount.test.tsx b/src/app/__tests__/targets-coach-mount.test.tsx index 68fd321ef..5700b1695 100644 --- a/src/app/__tests__/targets-coach-mount.test.tsx +++ b/src/app/__tests__/targets-coach-mount.test.tsx @@ -1,17 +1,18 @@ -import { describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import { renderToStaticMarkup } from "react-dom/server"; /** * v1.4.25 W3e — Coach drawer mount + per-card CTA gate. * - * Two scenarios: + * Three scenarios: * 1. AI provider configured → per-card "Ask Coach" CTAs render * alongside each card; the drawer is mounted at the page level. * 2. AI provider NOT configured → CTAs are completely absent (no - * broken-button state) but the drawer mount stays present (it's - * controlled by the parent regardless; no harm in mounting it - * collapsed, and it costs ~0 since closed Sheets render only the - * portal root). + * broken-button state). + * 3. v1.4.37 W5 — operator's global Coach flag off → both the CTAs + * and the page-level drawer mount vanish. The flag default is + * all-on for fresh installs and on any fetch error, so the path + * stays open by default (mocked via `useFeatureFlags` below). */ vi.mock("next/navigation", () => ({ @@ -19,6 +20,20 @@ vi.mock("next/navigation", () => ({ usePathname: () => "/targets", })); +let coachFlag = true; +vi.mock("@/hooks/use-feature-flags", async () => { + const actual = await vi.importActual< + typeof import("@/hooks/use-feature-flags") + >("@/hooks/use-feature-flags"); + return { + ...actual, + useFeatureFlags: () => ({ + ...actual.DEFAULT_ASSISTANT_FLAGS, + coach: coachFlag, + }), + }; +}); + const sampleData = { targets: [ { @@ -110,6 +125,10 @@ function render() { } describe("/targets page — Coach drawer mount + CTA gate", () => { + beforeEach(() => { + coachFlag = true; + }); + it("renders the per-card Coach CTA when an AI provider is configured", () => { providerChainResponse = { activeProvider: "openai" }; const html = render(); @@ -130,4 +149,27 @@ describe("/targets page — Coach drawer mount + CTA gate", () => { // configured we don't render a CTA whose click would 404. expect(html).not.toContain('data-slot="target-coach-cta"'); }); + + // v1.4.37 W5 — Marc directive: when the operator turns off the + // global Coach flag, every Coach affordance on /targets must vanish. + // That is BOTH the per-card CTAs (currently gated through `aiEnabled`) + // AND the page-level mount — so the SSE machinery, + // Sheet portal and source-chip thread never load. + it("hides every per-card Coach CTA when the global Coach flag is off", () => { + coachFlag = false; + providerChainResponse = { activeProvider: "openai" }; + const html = render(); + expect(html).not.toContain('data-slot="target-coach-cta"'); + }); + + it("does not mount the CoachDrawer when the global Coach flag is off", () => { + coachFlag = false; + providerChainResponse = { activeProvider: "openai" }; + const html = render(); + // The drawer renders a Sheet that the body markup exposes via a + // role + the i18n key it carries. We grep on the localised + // composer label that the drawer always paints. + expect(html).not.toContain('data-slot="coach-drawer"'); + expect(html).not.toContain('data-slot="coach-drawer-composer"'); + }); }); diff --git a/src/app/__tests__/targets-i18n.test.tsx b/src/app/__tests__/targets-i18n.test.tsx index 911d85108..f24de103c 100644 --- a/src/app/__tests__/targets-i18n.test.tsx +++ b/src/app/__tests__/targets-i18n.test.tsx @@ -135,6 +135,22 @@ vi.mock("@/hooks/use-auth", () => ({ }), })); +// v1.4.37 W5 — `/targets/page.tsx` now reads `useFeatureFlags` to gate +// the Coach drawer + per-card CTAs. The hook resolves QueryClientContext +// from `@tanstack/react-query`, and this test mocks the library +// without exposing that context. Mock the hook directly with the +// all-on default so the i18n assertions below run against the visible +// Coach surfaces. +vi.mock("@/hooks/use-feature-flags", async () => { + const actual = await vi.importActual< + typeof import("@/hooks/use-feature-flags") + >("@/hooks/use-feature-flags"); + return { + ...actual, + useFeatureFlags: () => actual.DEFAULT_ASSISTANT_FLAGS, + }; +}); + import { I18nProvider } from "@/lib/i18n/context"; import TargetsPage from "../targets/page"; diff --git a/src/app/__tests__/targets-responsive.test.tsx b/src/app/__tests__/targets-responsive.test.tsx index 5495485e5..6d016d1bb 100644 --- a/src/app/__tests__/targets-responsive.test.tsx +++ b/src/app/__tests__/targets-responsive.test.tsx @@ -98,6 +98,22 @@ vi.mock("@/hooks/use-auth", () => ({ }), })); +// v1.4.37 W5 — `/targets/page.tsx` now reads `useFeatureFlags` to gate +// the Coach drawer + per-card CTAs. The hook resolves QueryClientContext +// from `@tanstack/react-query`, and this test mocks the library +// without exposing that context. Mock the hook directly so the page +// runs with the all-on default (CTAs + drawer mount visible) the +// responsive assertions below pin. +vi.mock("@/hooks/use-feature-flags", async () => { + const actual = await vi.importActual< + typeof import("@/hooks/use-feature-flags") + >("@/hooks/use-feature-flags"); + return { + ...actual, + useFeatureFlags: () => actual.DEFAULT_ASSISTANT_FLAGS, + }; +}); + import { I18nProvider } from "@/lib/i18n/context"; import TargetsPage from "../targets/page"; diff --git a/src/app/__tests__/targets-spacing.test.tsx b/src/app/__tests__/targets-spacing.test.tsx index 62c720520..e0f097e0e 100644 --- a/src/app/__tests__/targets-spacing.test.tsx +++ b/src/app/__tests__/targets-spacing.test.tsx @@ -71,6 +71,22 @@ vi.mock("@/hooks/use-auth", () => ({ }), })); +// v1.4.37 W5 — `/targets/page.tsx` now reads `useFeatureFlags` to gate +// the Coach drawer + per-card CTAs. The hook resolves QueryClientContext +// from `@tanstack/react-query`, and this test mocks the library +// without exposing that context. Mock the hook directly with the +// all-on default so the spacing assertions below run against the +// fully-painted page shape. +vi.mock("@/hooks/use-feature-flags", async () => { + const actual = await vi.importActual< + typeof import("@/hooks/use-feature-flags") + >("@/hooks/use-feature-flags"); + return { + ...actual, + useFeatureFlags: () => actual.DEFAULT_ASSISTANT_FLAGS, + }; +}); + import { I18nProvider } from "@/lib/i18n/context"; import TargetsPage from "../targets/page"; diff --git a/src/app/targets/page.tsx b/src/app/targets/page.tsx index 10333214e..599ffa235 100644 --- a/src/app/targets/page.tsx +++ b/src/app/targets/page.tsx @@ -15,6 +15,7 @@ import { import { TargetsSummaryHeader } from "@/components/targets/targets-summary-header"; import { CoachDrawer } from "@/components/insights/coach-panel/coach-drawer"; import { useCoachHandoff } from "@/hooks/use-coach-handoff"; +import { useFeatureFlags } from "@/hooks/use-feature-flags"; type TargetData = TargetCardData; @@ -154,7 +155,15 @@ export default function TargetsPage() { }, enabled: isAuthenticated, }); - const aiEnabled = chainStatus?.activeProvider != null; + // v1.4.37 W5 — operator's global Coach flag layered on top of the + // per-user provider chain. Either gate disables every Coach + // affordance on this page: the per-card CTAs (via `aiEnabled` + // threaded into ) and the page-level mount + // below. The flag default is all-on for fresh installs and on any + // fetch error, so the path stays open by default. + const flags = useFeatureFlags(); + const coachEnabled = flags.coach; + const aiEnabled = coachEnabled && chainStatus?.activeProvider != null; // v1.4.25 W3e — Coach drawer state owned by the page. The per-card // CTA + summary header both feed the same drawer so the user only @@ -281,13 +290,20 @@ export default function TargetsPage() { {/* v1.4.25 W3e — Coach drawer mounted at the page level. The drawer is fully-controlled; per-card CTAs flip `coachOpen` - and seed `coachPrefill` via the `askCoach()` hook callback. */} - + and seed `coachPrefill` via the `askCoach()` hook callback. + v1.4.37 W5 — short-circuit when the operator turned the + global Coach flag off so the SSE machinery + Sheet portal + never mount. The per-card CTAs already hide via `aiEnabled` + above, so the drawer can never be triggered with the flag + off; the gate is defence-in-depth. */} + {coachEnabled && ( + + )} ); }