mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-14 22:50:51 +00:00
fix(nutrients): stop the /insights/nutrients overview swallowing a failed read as absence
The page's overview query shares queryKeys.nutrientIntake(30) with MicronutrientsCard, which already renders its own QueryErrorCard on a failed read. But the page's empty-state check ran first and treated undefined data (the failed-read state) the same as a genuinely empty window, returning "no nutrient data yet" and short-circuiting before any of the three cards - including the already-correct one - ever mounted. A self-hoster with HealthKit permissions granted and a read that actually failed saw the same blank invitation as someone who never logged anything. Add the isError branch above the empty-state check so a failed read surfaces its own QueryErrorCard with a retry, and cover it with a test that renders the actual page (not just the card in isolation) so a regression here can't hide behind a passing unit test again.
This commit is contained in:
1 parent
37496f0083
commit
a1c5d78f72
8 files changed
+81
-7
No files matched your search
@@ -6,6 +6,7 @@ import { toast } from "sonner";
|
||||
|
||||
import { Button } from "@/components/ui/button";
|
||||
import { EmptyState } from "@/components/ui/empty-state";
|
||||
import { QueryErrorCard } from "@/components/ui/query-error-card";
|
||||
import { SubPageShell } from "@/components/insights/sub-page-shell";
|
||||
import { HydrationCard } from "@/components/insights/nutrients/hydration-card";
|
||||
import { CaffeineCard } from "@/components/insights/nutrients/caffeine-card";
|
||||
@@ -33,6 +34,16 @@ const WINDOW_DAYS = 30;
|
||||
* on the device is not visible consent to a server / Coach holding
|
||||
* a supplement pattern); this page just makes the toggle
|
||||
* discoverable in context instead of buried in Settings.
|
||||
* - module on, the overview read failed → a `QueryErrorCard` with a
|
||||
* retry. This branch has to sit ABOVE the empty-state check below:
|
||||
* `overview.data` is `undefined` on a failed read exactly like it is
|
||||
* before the first response lands, so without this the page told a
|
||||
* self-hoster who granted every HealthKit permission "no nutrient
|
||||
* data yet" when the read had actually failed — a guess dressed up
|
||||
* as an honest absence. `<MicronutrientsCard>` reads this SAME
|
||||
* `queryKeys.nutrientIntake(WINDOW_DAYS)` query and already renders
|
||||
* its own error card, but this page's old empty-state branch
|
||||
* short-circuited before that card ever mounted.
|
||||
* - module on, zero rows anywhere in the window → one EmptyState
|
||||
* explaining where data comes from (phone health-app sync or
|
||||
* manual water entry) instead of three near-empty cards.
|
||||
@@ -83,6 +94,20 @@ export default function InsightsNutrientsPage() {
|
||||
);
|
||||
}
|
||||
|
||||
if (overview.isError) {
|
||||
return (
|
||||
<SubPageShell
|
||||
title={t("nutrients.page.title")}
|
||||
description={t("nutrients.page.description")}
|
||||
>
|
||||
<QueryErrorCard
|
||||
title={t("nutrients.page.loadError")}
|
||||
onRetry={() => void overview.refetch()}
|
||||
/>
|
||||
</SubPageShell>
|
||||
);
|
||||
}
|
||||
|
||||
const hasAnyData = (overview.data?.nutrients.length ?? 0) > 0;
|
||||
if (!overview.isLoading && !hasAnyData) {
|
||||
return (
|
||||
|
||||
@@ -32,8 +32,11 @@ vi.mock("@/lib/api/api-fetch", () => ({
|
||||
}));
|
||||
|
||||
vi.mock("@/hooks/use-auth", () => ({
|
||||
// `modules.nutrients: true` so the `/insights/nutrients` page tests below
|
||||
// reach their query instead of short-circuiting on the module-off branch;
|
||||
// no other surface in this file reads `user.modules`.
|
||||
useAuth: () => ({
|
||||
user: { gender: "FEMALE", modules: {} },
|
||||
user: { gender: "FEMALE", modules: { nutrients: true } },
|
||||
isAuthenticated: true,
|
||||
isLoading: false,
|
||||
}),
|
||||
@@ -41,6 +44,7 @@ vi.mock("@/hooks/use-auth", () => ({
|
||||
|
||||
import { MentalWellbeing } from "@/components/mental-health/mental-wellbeing";
|
||||
import { MicronutrientsCard } from "@/components/insights/nutrients/micronutrients-card";
|
||||
import InsightsNutrientsPage from "@/app/insights/nutrients/page";
|
||||
|
||||
function makeClient() {
|
||||
return new QueryClient({
|
||||
@@ -140,6 +144,40 @@ describe("micronutrients card", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("/insights/nutrients page — overview read shares the micronutrients queryKey", () => {
|
||||
const np = en.nutrients.page;
|
||||
|
||||
// The page's overview query and `<MicronutrientsCard>` read the exact same
|
||||
// `queryKeys.nutrientIntake(30)` cache entry. Before this branch existed, a
|
||||
// failed read left `overview.data` undefined — indistinguishable from the
|
||||
// pre-response state — so the page's empty-state check fired first and
|
||||
// returned "no nutrient data yet" without ever mounting the three cards
|
||||
// below (including `<MicronutrientsCard>`'s own, already-correct error
|
||||
// card).
|
||||
it("renders the page-level error card, not the empty state, when the overview read fails", async () => {
|
||||
const client = makeClient();
|
||||
await seedFailure(client, queryKeys.nutrientIntake(30));
|
||||
const html = render(<InsightsNutrientsPage />, client);
|
||||
|
||||
expect(html).toContain('data-slot="query-error-card"');
|
||||
expect(html).toContain(np.loadError);
|
||||
expect(html).toContain(en.common.retry);
|
||||
expect(html).not.toContain(np.emptyTitle);
|
||||
// The three-card spine must not mount alongside the error notice.
|
||||
expect(html).not.toContain('data-slot="nutrients-hydration-card"');
|
||||
expect(html).not.toContain('data-slot="nutrients-micronutrients-card"');
|
||||
});
|
||||
|
||||
it("still renders the honest empty state when the window genuinely has no rows", () => {
|
||||
const client = makeClient();
|
||||
client.setQueryData(queryKeys.nutrientIntake(30), { nutrients: [] });
|
||||
const html = render(<InsightsNutrientsPage />, client);
|
||||
|
||||
expect(html).not.toContain('data-slot="query-error-card"');
|
||||
expect(html).toContain(np.emptyTitle);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* The remaining surfaces in the sweep are pages whose render depends on the
|
||||
* router / module gate, which the SSR-only convention cannot drive. Pin their
|
||||
@@ -155,6 +193,11 @@ describe("read-failure branch is wired on every swept surface", () => {
|
||||
"src/components/custom-metrics/custom-metric-values-list.tsx",
|
||||
"src/app/achievements/page.tsx",
|
||||
"src/components/insights/nutrients/micronutrients-card.tsx",
|
||||
// The page shares `MicronutrientsCard`'s exact queryKey and used to
|
||||
// short-circuit to the "no nutrient data yet" EmptyState on a failed
|
||||
// read, before that card's own (already-correct) error branch ever
|
||||
// got a chance to mount.
|
||||
"src/app/insights/nutrients/page.tsx",
|
||||
// A failed comprehensive read used to fall through to "you haven't added
|
||||
// any medications yet" — with a CTA inviting someone on a full daily
|
||||
// regimen to re-enter what the app already holds.
|
||||
|
||||
Reference in new issue
Block a user