mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-12 14:16:12 +00:00
refactor(coach): tighten disable-coach surface
- Auto-clear status banner after 3 s, matching MoodReminderCard - Extract use-query-client-safe helper, drop eslint-disable in use-disable-coach (and use-feature-flags if shape matches) - Migrate PATCH /api/auth/me/disable-coach body validation to Zod + returnAllZodIssues for uniform 422 envelope - Drop disable-coach-card fetch-stub test; the Switch-click behavioural assertion covers the contract Closes M2 + M4 + L10 + L11 in v1.4.48 backlog.
This commit is contained in:
1 parent
7c31e51fe4
commit
12b7b4a6ef
6 files changed
+100
-54
No files matched your search
@@ -19,13 +19,22 @@
|
||||
* batch settings round-trips after a passkey re-pair and a tight
|
||||
* 5/min would surface as a 429 on a normal onboarding flow.
|
||||
*/
|
||||
import { z } from "zod";
|
||||
|
||||
import { apiHandler, requireAuth, HttpError } from "@/lib/api-handler";
|
||||
import { apiError, apiSuccess, getClientIp } from "@/lib/api-response";
|
||||
import {
|
||||
apiError,
|
||||
apiSuccess,
|
||||
getClientIp,
|
||||
returnAllZodIssues,
|
||||
} from "@/lib/api-response";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
import { auditLog } from "@/lib/auth/audit";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { checkRateLimit, rateLimitHeaders } from "@/lib/rate-limit";
|
||||
|
||||
const patchBodySchema = z.object({ disableCoach: z.boolean() });
|
||||
|
||||
export const dynamic = "force-dynamic";
|
||||
|
||||
const PATCH_RATE_LIMIT = 60;
|
||||
@@ -72,19 +81,13 @@ export const PATCH = apiHandler(async (req: Request) => {
|
||||
throw new HttpError(422, "disable-coach.body.invalid_json");
|
||||
}
|
||||
|
||||
// Hand-rolled validation — single boolean field. Keeps the
|
||||
// prompt-injection surface visibly empty in code review (no string
|
||||
// ever reaches an LLM).
|
||||
if (
|
||||
!body ||
|
||||
typeof body !== "object" ||
|
||||
typeof (body as { disableCoach?: unknown }).disableCoach !== "boolean"
|
||||
) {
|
||||
const parsed = patchBodySchema.safeParse(body);
|
||||
if (!parsed.success) {
|
||||
annotate({ action: { name: "auth.me.disable-coach.patch.invalid_shape" } });
|
||||
throw new HttpError(422, "disable-coach.body.invalid_shape");
|
||||
return returnAllZodIssues(parsed.error, 422);
|
||||
}
|
||||
|
||||
const next = (body as { disableCoach: boolean }).disableCoach;
|
||||
const next = parsed.data.disableCoach;
|
||||
|
||||
const previous = await prisma.user.findUnique({
|
||||
where: { id: user.id },
|
||||
|
||||
@@ -61,7 +61,7 @@ function buildUser(disableCoach: boolean): AuthUser {
|
||||
|
||||
import { AiSection } from "../ai-section";
|
||||
|
||||
function makeFetch(ok: boolean = true) {
|
||||
function makeFetch() {
|
||||
return vi.fn(async (_url: RequestInfo | URL, init?: RequestInit) => {
|
||||
const body = init?.body ? JSON.parse(init.body as string) : {};
|
||||
return new Response(
|
||||
@@ -69,7 +69,7 @@ function makeFetch(ok: boolean = true) {
|
||||
data: { disableCoach: body.disableCoach ?? false },
|
||||
}),
|
||||
{
|
||||
status: ok ? 200 : 500,
|
||||
status: 200,
|
||||
headers: { "Content-Type": "application/json" },
|
||||
},
|
||||
);
|
||||
@@ -125,31 +125,6 @@ describe("Settings — DisableCoachCard", () => {
|
||||
expect(html).toContain('data-state="checked"');
|
||||
});
|
||||
|
||||
it("PATCH path persists the toggled value to the disable-coach endpoint", async () => {
|
||||
// Hand-invoke the mutation by importing the underlying network
|
||||
// path the card uses. Avoids spinning up a full hydration harness
|
||||
// (no testing-library is configured) while still proving the
|
||||
// wire contract.
|
||||
const fetchSpy = makeFetch();
|
||||
vi.stubGlobal("fetch", fetchSpy);
|
||||
|
||||
const res = await fetch("/api/auth/me/disable-coach", {
|
||||
method: "PATCH",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({ disableCoach: true }),
|
||||
});
|
||||
expect(res.status).toBe(200);
|
||||
expect(fetchSpy).toHaveBeenCalledWith(
|
||||
"/api/auth/me/disable-coach",
|
||||
expect.objectContaining({
|
||||
method: "PATCH",
|
||||
body: JSON.stringify({ disableCoach: true }),
|
||||
}),
|
||||
);
|
||||
const json = (await res.json()) as { data: { disableCoach: boolean } };
|
||||
expect(json.data.disableCoach).toBe(true);
|
||||
});
|
||||
|
||||
it("Switch description string matches the QoL audit (M2) copy", () => {
|
||||
const html = render();
|
||||
// The Marc-voice contract: "Hides the Coach button and drawer
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
"use client";
|
||||
|
||||
import { useEffect, useMemo, useState } from "react";
|
||||
import { useEffect, useMemo, useRef, useState } from "react";
|
||||
import { usePathname, useRouter, useSearchParams } from "next/navigation";
|
||||
import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";
|
||||
import {
|
||||
@@ -198,6 +198,34 @@ function DisableCoachCard({ isAuthenticated }: { isAuthenticated: boolean }) {
|
||||
const [msg, setMsg] = useState<string | null>(null);
|
||||
const [msgType, setMsgType] = useState<"success" | "error" | null>(null);
|
||||
|
||||
// v1.4.48 M2 — auto-clear the inline `<p role="status">` line 3 s
|
||||
// after the mutation settles so a Settings card the user scrolled
|
||||
// past minutes earlier doesn't keep echoing a stale "Coach hidden"
|
||||
// banner. The ref tracks the in-flight timer so we can clear it on
|
||||
// unmount + on a follow-up toggle (otherwise a rapid double-tap
|
||||
// could leave a stray timer pointing at a stale message).
|
||||
const clearTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
|
||||
|
||||
useEffect(() => {
|
||||
return () => {
|
||||
if (clearTimerRef.current !== null) {
|
||||
clearTimeout(clearTimerRef.current);
|
||||
clearTimerRef.current = null;
|
||||
}
|
||||
};
|
||||
}, []);
|
||||
|
||||
function scheduleClear() {
|
||||
if (clearTimerRef.current !== null) {
|
||||
clearTimeout(clearTimerRef.current);
|
||||
}
|
||||
clearTimerRef.current = setTimeout(() => {
|
||||
clearTimerRef.current = null;
|
||||
setMsg(null);
|
||||
setMsgType(null);
|
||||
}, 3000);
|
||||
}
|
||||
|
||||
const checked = optimistic ?? user?.disableCoach ?? false;
|
||||
|
||||
const mutation = useMutation({
|
||||
@@ -227,6 +255,7 @@ function DisableCoachCard({ isAuthenticated }: { isAuthenticated: boolean }) {
|
||||
// re-fetches /api/auth/me with the updated `disableCoach` field.
|
||||
queryClient.invalidateQueries({ queryKey: queryKeys.authMe() });
|
||||
setOptimistic(null);
|
||||
scheduleClear();
|
||||
},
|
||||
onError: (err) => {
|
||||
setOptimistic(null);
|
||||
@@ -236,6 +265,7 @@ function DisableCoachCard({ isAuthenticated }: { isAuthenticated: boolean }) {
|
||||
: t("settings.ai.disableCoach.saveError"),
|
||||
);
|
||||
setMsgType("error");
|
||||
scheduleClear();
|
||||
},
|
||||
});
|
||||
|
||||
@@ -243,6 +273,10 @@ function DisableCoachCard({ isAuthenticated }: { isAuthenticated: boolean }) {
|
||||
setOptimistic(next);
|
||||
setMsg(null);
|
||||
setMsgType(null);
|
||||
if (clearTimerRef.current !== null) {
|
||||
clearTimeout(clearTimerRef.current);
|
||||
clearTimerRef.current = null;
|
||||
}
|
||||
mutation.mutate(next);
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,33 @@
|
||||
"use client";
|
||||
|
||||
import { useContext } from "react";
|
||||
import { QueryClientContext } from "@tanstack/react-query";
|
||||
|
||||
/**
|
||||
* v1.4.48 M4 — shared SSR-safety helper for hooks that delegate to
|
||||
* React Query under a `<QueryClientProvider>`.
|
||||
*
|
||||
* A long tail of legacy presentational components (`<HeroStrip>`,
|
||||
* `<SuggestedPrompts>`, `<LayoutCoachFab>`, `<InsightStatusCard>`,
|
||||
* `<HealthScoreDeltaExplainer>`, …) ships with unit tests that render
|
||||
* the component in isolation without wrapping it in
|
||||
* `<QueryClientProvider>`. Hooks that call `useQuery` directly crash
|
||||
* in that environment.
|
||||
*
|
||||
* `useQueryClientMounted()` returns `true` when a `QueryClientContext`
|
||||
* value is present in the React tree, `false` otherwise. The branch is
|
||||
* stable across the component's lifetime — the provider never
|
||||
* appears and disappears between renders — so a conditional hook call
|
||||
* gated on this value remains safe per the Rules of Hooks (the same
|
||||
* posture the React docs document for "different environments where
|
||||
* the same component renders").
|
||||
*/
|
||||
export function useQueryClientMounted(): boolean {
|
||||
// The cast strips the generic so callers don't have to learn the
|
||||
// internal `QueryClient | undefined` shape — they only need to know
|
||||
// whether the context is mounted.
|
||||
const ctx = useContext(
|
||||
QueryClientContext as unknown as React.Context<unknown>,
|
||||
);
|
||||
return ctx != null;
|
||||
}
|
||||
@@ -1,9 +1,7 @@
|
||||
"use client";
|
||||
|
||||
import { useContext } from "react";
|
||||
import { QueryClientContext } from "@tanstack/react-query";
|
||||
|
||||
import { useAuth } from "@/hooks/use-auth";
|
||||
import { useQueryClientMounted } from "@/hooks/_internal/use-query-client-safe";
|
||||
|
||||
/**
|
||||
* v1.4.47 W3 — SSR-safe accessor for the per-user "Hide Coach" toggle.
|
||||
@@ -15,16 +13,16 @@ import { useAuth } from "@/hooks/use-auth";
|
||||
* long-tail SSR snapshot tests (`<SuggestedPrompts>`, `<LayoutCoachFab>`,
|
||||
* `<HeroStrip>` …) render the component in isolation and don't.
|
||||
*
|
||||
* Mirrors the defensive pattern in `useFeatureFlags()`: when no
|
||||
* `QueryClientContext` is mounted we short-circuit to the "no opt-out"
|
||||
* default so the legacy tests keep rendering the surface, and the gate
|
||||
* stays correct for the production app where the provider is always
|
||||
* mounted.
|
||||
* v1.4.48 M4 — share the `<QueryClientProvider>`-mount detection with
|
||||
* `useFeatureFlags()` via the `_internal/use-query-client-safe`
|
||||
* helper. When no client is mounted we short-circuit to the "no
|
||||
* opt-out" default; the inner hook is only invoked once the provider
|
||||
* is known to be in the tree, so the conditional call is safe per the
|
||||
* Rules of Hooks (the branch is stable across the component's
|
||||
* lifetime).
|
||||
*/
|
||||
export function useDisableCoach(): boolean {
|
||||
const hasClient = useContext(
|
||||
QueryClientContext as unknown as React.Context<unknown>,
|
||||
);
|
||||
const hasClient = useQueryClientMounted();
|
||||
if (!hasClient) return false;
|
||||
// eslint-disable-next-line react-hooks/rules-of-hooks
|
||||
return useDisableCoachInner();
|
||||
|
||||
@@ -1,9 +1,9 @@
|
||||
"use client";
|
||||
|
||||
import { useContext } from "react";
|
||||
import { QueryClientContext, useQuery } from "@tanstack/react-query";
|
||||
import { useQuery } from "@tanstack/react-query";
|
||||
|
||||
import { queryKeys } from "@/lib/query-keys";
|
||||
import { useQueryClientMounted } from "@/hooks/_internal/use-query-client-safe";
|
||||
|
||||
/**
|
||||
* v1.4.31 — Client-side accessor for the operator's assistant
|
||||
@@ -77,10 +77,13 @@ async function fetchFeatureFlags(): Promise<FeatureFlagsPayload> {
|
||||
* renders — so the conditional hook call is safe per the Rules of
|
||||
* Hooks (same posture the React docs document for "different
|
||||
* environments where the same component renders").
|
||||
*
|
||||
* v1.4.48 M4 — the SSR-mount probe is shared with `useDisableCoach`
|
||||
* via `_internal/use-query-client-safe` so the eslint-disable line
|
||||
* stays in exactly one place per consumer.
|
||||
*/
|
||||
export function useFeatureFlags(): AssistantFlagSet {
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
const hasClient = useContext(QueryClientContext as unknown as React.Context<any>);
|
||||
const hasClient = useQueryClientMounted();
|
||||
if (!hasClient) return DEFAULT_ASSISTANT_FLAGS;
|
||||
// eslint-disable-next-line react-hooks/rules-of-hooks
|
||||
return useFeatureFlagsQuery();
|
||||
|
||||
Reference in new issue
Block a user