merge(reconcile): b5-achievements audit findings

This commit is contained in:
Marc-André Bombeck committed 2026-06-16 02:20:45 +02:00
commit f73bc32a72
8 files changed
+262 -33

No files matched your search

+1 -1
View File
@@ -3687,7 +3687,7 @@
},
"integrations": {
"title": "Connect your devices",
"body": "Open Settings to link Withings (auto-sync weight, BP and pulse), moodLog, or your Insights provider. Everything stays optional and encrypted."
"body": "Open Settings to link Withings (auto-sync weight, BP and pulse), moodLog, or your Insights provider. Everything stays optional and encrypted — and under Settings → Modules you can turn whole areas like sleep, mood or achievements on or off to keep HealthLog focused on what you track."
},
"achievements": {
"title": "Earn streaks and badges",
@@ -27,6 +27,8 @@ vi.mock("@/lib/modules/gate", () => ({
// Plain async fn (not a `vi.fn`) so `vi.resetAllMocks()` in beforeEach
// can't blank it out and flip the gate to a falsy decision.
requireModuleEnabled: async () => ({ enabled: true }),
// v1.18.0 B5 — all modules on so the per-module badge filter is inert.
resolveModuleMap: async () => ({}),
MODULE_DISABLED_ERROR_CODE: "module.disabled",
}));
@@ -26,6 +26,9 @@ vi.mock("@/lib/auth/session", () => ({ getSession: vi.fn() }));
// in module-gate.test.ts.
vi.mock("@/lib/modules/gate", () => ({
requireModuleEnabled: async () => ({ enabled: true }),
// v1.18.0 B5 — all modules enabled by default so the badge filter is
// a no-op here; the skip behaviour is asserted in module-gate.test.ts.
resolveModuleMap: async () => ({}),
MODULE_DISABLED_ERROR_CODE: "module.disabled",
}));
@@ -112,6 +115,11 @@ describe("GET /api/gamification/achievements?format=ios", () => {
unlocked: boolean;
unlockedAt: string | null;
progress: number;
category: string;
points: number;
target: number;
current: number;
isHidden: boolean;
}>;
};
expect(Array.isArray(body.data)).toBe(true);
@@ -123,6 +131,12 @@ describe("GET /api/gamification/achievements?format=ios", () => {
expect(first.iconName).toEqual(expect.any(String));
expect(first.progress).toBeGreaterThanOrEqual(0);
expect(first.progress).toBeLessThanOrEqual(1);
// v1.18.0 B5 — parity fields the iOS DTO previously dropped.
expect(first.category).toEqual(expect.any(String));
expect(first.points).toEqual(expect.any(Number));
expect(first.target).toEqual(expect.any(Number));
expect(first.current).toEqual(expect.any(Number));
expect(first.isHidden).toEqual(expect.any(Boolean));
});
it("falls back to the legacy wrapped shape when no format is given", async () => {
@@ -49,13 +49,14 @@ vi.mock("next/headers", () => ({
// their own unit coverage in src/lib/modules/__tests__/gate.test.ts).
vi.mock("@/lib/modules/gate", () => ({
requireModuleEnabled: vi.fn(),
resolveModuleMap: vi.fn(),
MODULE_DISABLED_ERROR_CODE: "module.disabled",
}));
import { GET } from "../route";
import { prisma } from "@/lib/db";
import { getSession } from "@/lib/auth/session";
import { requireModuleEnabled } from "@/lib/modules/gate";
import { requireModuleEnabled, resolveModuleMap } from "@/lib/modules/gate";
import { apiError } from "@/lib/api-response";
import { __resetAllCachesForTests } from "@/lib/cache/server-cache";
@@ -89,6 +90,9 @@ beforeEach(() => {
vi.mocked(prisma.userHealthProfile.findUnique).mockResolvedValue(
null as never,
);
// v1.18.0 B5 — default to "every module on" so individual cases only
// flip the one key under test.
vi.mocked(resolveModuleMap).mockResolvedValue({} as never);
});
describe("GET /api/gamification/achievements — achievements module gate", () => {
@@ -151,3 +155,70 @@ describe("GET /api/gamification/achievements — achievements module gate", () =
expect(requireModuleEnabled).not.toHaveBeenCalled();
});
});
describe("GET /api/gamification/achievements — per-module badge skipping", () => {
// Three mood entries make the mood badges earnable (hasMood = true) so
// they survive the discovery filter when the mood module is ON — which
// means a later disappearance can only be the B5 module filter.
const MOOD_ENTRIES = [
{
date: "2026-03-02",
score: 4,
moodLoggedAt: new Date("2026-03-02T08:00:00.000Z"),
},
{
date: "2026-03-03",
score: 5,
moodLoggedAt: new Date("2026-03-03T08:00:00.000Z"),
},
{
date: "2026-03-04",
score: 5,
moodLoggedAt: new Date("2026-03-04T08:00:00.000Z"),
},
];
type Badge = { id: string; category: string; metric: string };
async function badgesFor(
moduleMap: Record<string, boolean>,
): Promise<Badge[]> {
vi.mocked(getSession).mockResolvedValue(SESSION_OK as never);
vi.mocked(requireModuleEnabled).mockResolvedValue({ enabled: true });
vi.mocked(resolveModuleMap).mockResolvedValue(moduleMap as never);
vi.mocked(prisma.moodEntry.findMany).mockResolvedValue(
MOOD_ENTRIES as never,
);
const res = await GET(
new NextRequest("http://localhost/api/gamification/achievements"),
);
expect(res.status).toBe(200);
const body = (await res.json()) as { data: { achievements: Badge[] } };
return body.data.achievements;
}
it("includes mood badges when the mood module is enabled", async () => {
const badges = await badgesFor({ mood: true });
expect(badges.some((b) => b.category === "mood")).toBe(true);
});
it("drops every mood badge when the mood module is disabled", async () => {
const badges = await badgesFor({ mood: false });
expect(badges.some((b) => b.category === "mood")).toBe(false);
// Non-mood badges still render — the filter is surgical, not global.
expect(badges.length).toBeGreaterThan(0);
});
it("never persists an unlock for a disabled-module badge", async () => {
// A mood badge would unlock at >=1 entry; with mood OFF the createMany
// must carry no mood achievement row.
await badgesFor({ mood: false });
const createCalls = vi.mocked(prisma.userAchievement.createMany).mock
.calls;
for (const [arg] of createCalls) {
const rows = (arg as { data: Array<{ achievementId: string }> }).data;
expect(rows.some((r) => r.achievementId.includes("mood"))).toBe(false);
}
});
});
+105 -31
View File
@@ -1,6 +1,7 @@
import { prisma } from "@/lib/db";
import { apiHandler, requireAuth } from "@/lib/api-handler";
import { requireModuleEnabled } from "@/lib/modules/gate";
import { requireModuleEnabled, resolveModuleMap } from "@/lib/modules/gate";
import type { ModuleKey } from "@/lib/modules/gate";
import { annotate } from "@/lib/logging/context";
import { apiSuccess } from "@/lib/api-response";
import { cached, caches, type ServerCache } from "@/lib/cache/server-cache";
@@ -10,6 +11,7 @@ import {
applyDiscoveryFilter,
calculateLongestStreak,
evaluateAchievementsWithCompletionDates,
moduleForMetric,
toBerlinDayKey,
type AchievementMetrics,
type AchievementProgress,
@@ -41,6 +43,15 @@ interface IosAchievement {
unlocked: boolean;
unlockedAt: string | null;
progress: number;
// v1.18.0 B5 — parity fields the web payload already carries; the iOS
// client needs them to group badges by category, render the points
// tally, show absolute progress (current / target) and the opaque
// hidden-card placeholder in lock-step with the web surface.
category: string;
points: number;
target: number;
current: number;
isHidden: boolean;
}
export const dynamic = "force-dynamic";
@@ -496,23 +507,57 @@ export const GET = apiHandler(async (request: NextRequest) => {
// iOS-format branch runs the locale-aware transform after the cache
// read so the cache stays format-agnostic and the achievement-progress
// dashboard duplicate (seen twice per dashboard mount in the v1.4.33
// HAR) coalesces into one builder call. Newly-unlocked achievements
// get persisted inside the builder so the second concurrent mount
// observes the up-to-date `persisted` state.
// HAR) coalesces into one builder call. v1.18.0 B5 — unlock persistence
// now happens in the handler after the cache read (idempotent), not as
// a side effect inside the cached factory.
// v1.18.0 B5 — resolve the per-user module map once and pass it into the
// builder so badge categories whose owning module is disabled (sleep
// badges when sleep is off, mood badges when mood is off) are skipped
// from evaluation AND unlock-persistence. Resolved outside the cache so
// a toggle change is reflected on the next read.
const moduleMap = await resolveModuleMap(user.id);
const result = await cached(
caches.achievements as ServerCache<AchievementsResult>,
user.id,
() => buildAchievementsResult(user),
() => buildAchievementsResult(user, moduleMap),
annotate,
);
// v1.18.0 B5 — persist newly unlocked achievements OUTSIDE the cached
// factory. `createMany({ skipDuplicates: true })` is idempotent on the
// `(userId, achievementId)` unique, so re-running it on a cache hit is
// a no-op rather than a duplicate, and the write is never skipped just
// because the read was served from cache.
if (result.pendingUnlocks.length > 0) {
await prisma.userAchievement.createMany({
data: result.pendingUnlocks.map((u) => ({
userId: user.id,
achievementId: u.achievementId,
unlockedAt: new Date(u.unlockedAt),
})),
skipDuplicates: true,
});
annotate({
action: { name: "gamification.achievements" },
meta: { newUnlocks: result.pendingUnlocks.length },
});
}
// Strip the internal `pendingUnlocks` carrier; it never goes on the wire.
const payload = {
summary: result.summary,
achievements: result.achievements,
metrics: result.metrics,
};
if (isIosFormat) {
const locale = await resolveServerLocale({
request,
userLocale: user.locale,
});
const t = getServerTranslator(locale);
const ios: IosAchievement[] = result.achievements.map((a) => ({
const ios: IosAchievement[] = payload.achievements.map((a) => ({
id: a.id,
key: a.id,
title: t.t(a.titleKey),
@@ -521,11 +566,16 @@ export const GET = apiHandler(async (request: NextRequest) => {
unlocked: a.unlocked,
unlockedAt: a.completedAt,
progress: Math.max(0, Math.min(1, a.progressPercent / 100)),
category: a.category,
points: a.points,
target: a.target,
current: a.current,
isHidden: a.isHidden,
}));
return apiSuccess(ios);
}
return apiSuccess(result);
return apiSuccess(payload);
});
type AuthedUser = Awaited<ReturnType<typeof requireAuth>>["user"];
@@ -537,13 +587,18 @@ type AchievementsResult = Awaited<ReturnType<typeof buildAchievementsResult>>;
* the heavy aggregation. Returns the locale-agnostic web payload; the
* iOS-format transform runs in the handler after the cache read.
*
* `prisma.userAchievement.createMany` inside the builder is kept here
* (rather than moved to the cache hit path) because new unlocks are
* a side effect of the same aggregate computation. A duplicate concurrent
* read coalesces through the cache's single-flight `pending` map so the
* createMany only fires once.
* v1.18.0 B5 — this builder NO LONGER writes. It computes the not-yet-
* persisted unlocks and returns them as `pendingUnlocks`; the handler
* performs the idempotent `createMany({ skipDuplicates: true })` on every
* request after the cache read. That keeps the persistence off the cached
* path (a cache hit can't silently skip it, a concurrent miss can't
* duplicate it) while the unique `(userId, achievementId)` makes a repeat
* write a no-op.
*/
async function buildAchievementsResult(user: AuthedUser) {
async function buildAchievementsResult(
user: AuthedUser,
moduleMap: Record<ModuleKey, boolean>,
) {
const now = new Date();
const userId = user.id;
const startDate = maxDate(user.createdAt, GAMIFICATION_ROLLOUT_AT);
@@ -975,23 +1030,32 @@ async function buildAchievementsResult(user: AuthedUser) {
mergedDates,
);
// Persist newly unlocked achievements
const newUnlocks = fullResult.achievements.filter(
(a) =>
a.unlocked &&
a.completedAt &&
!persisted.some((p) => p.achievementId === a.id),
);
if (newUnlocks.length > 0) {
await prisma.userAchievement.createMany({
data: newUnlocks.map((a) => ({
userId,
achievementId: a.id,
unlockedAt: new Date(a.completedAt!),
})),
skipDuplicates: true,
});
}
// v1.18.0 B5 — drop badges whose owning module is disabled BEFORE both
// unlock-persistence and serialisation. A sleep badge must never unlock
// (or render) while the sleep module is off; same for mood badges when
// mood is off, etc. Core-domain and account-wide badges return a null
// owner from `moduleForMetric` and always pass through.
const moduleEnabledAchievements = fullResult.achievements.filter((a) => {
const owner = moduleForMetric(a.metric);
if (owner === null) return true;
return moduleMap[owner as ModuleKey] !== false;
});
// v1.18.0 B5 — compute the newly unlocked rows here but DON'T write them
// inside the cached factory: a write must not be a side effect of a
// cached GET (a cache hit would silently skip it, a concurrent miss
// could duplicate it). The handler performs the idempotent
// `createMany({ skipDuplicates: true })` on every request after the
// cache read, so the persistence runs regardless of cache outcome and
// is safe to repeat.
const pendingUnlocks: PendingUnlock[] = moduleEnabledAchievements
.filter(
(a) =>
a.unlocked &&
a.completedAt &&
!persisted.some((p) => p.achievementId === a.id),
)
.map((a) => ({ achievementId: a.id, unlockedAt: a.completedAt! }));
// v1.4.18 — apply the discovery filter so locked badges that the
// user has *no path* to earn (e.g. mood badges for someone who has
@@ -999,7 +1063,7 @@ async function buildAchievementsResult(user: AuthedUser) {
// eggs always pass through. Already-unlocked badges always pass
// through (regression guard).
const visibleAchievements = applyDiscoveryFilter(
fullResult.achievements,
moduleEnabledAchievements,
earnability,
);
@@ -1051,11 +1115,21 @@ async function buildAchievementsResult(user: AuthedUser) {
},
achievements: redactedAchievements,
metrics: redactedMetrics,
// v1.18.0 B5 — carried out of the cached factory so the handler can
// persist them on every read (idempotent), not as a write side effect
// that a cache hit would skip. NOT part of the serialised payload.
pendingUnlocks,
};
return result;
}
/** v1.18.0 B5 — a not-yet-persisted unlock the handler writes after the cache read. */
interface PendingUnlock {
achievementId: string;
unlockedAt: string;
}
/**
* Hidden Easter-eggs ship to the client *only* as opaque cards while
* locked. Returning the full `AchievementProgress` shape would let a
@@ -21,6 +21,7 @@ import { afterEach, beforeEach, describe, expect, it } from "vitest";
import {
setTourForceLaunch,
shouldAutoLaunchTour,
tourIncludesAchievements,
TOUR_AUTOLAUNCH_DELAY_MS,
tourForceLaunchKey,
} from "../tour-launcher";
@@ -132,6 +133,27 @@ describe("shouldAutoLaunchTour() — v1.4.47 W5 chain-gate", () => {
});
});
describe("tourIncludesAchievements() — v1.18.0 B5 module gate", () => {
it("includes the achievements stop by default (no module map)", () => {
expect(tourIncludesAchievements(undefined)).toBe(true);
expect(tourIncludesAchievements({})).toBe(true);
});
it("includes the achievements stop when the module is explicitly on", () => {
expect(tourIncludesAchievements({ achievements: true })).toBe(true);
});
it("drops the achievements stop when the module is explicitly off", () => {
expect(tourIncludesAchievements({ achievements: false })).toBe(false);
});
it("ignores other disabled modules — only the achievements key matters", () => {
expect(
tourIncludesAchievements({ sleep: false, mood: false }),
).toBe(true);
});
});
describe("setTourForceLaunch() — v1.4.47 W5 manual replay bypass", () => {
// (c) of the W5 acceptance: the manual "launch tour" trigger still
// works. The launcher's render-phase gate is exercised by
@@ -101,6 +101,19 @@ export function shouldAutoLaunchTour(args: {
return args.nowMs - completedMs >= TOUR_AUTOLAUNCH_DELAY_MS;
}
/**
* v1.18.0 B5 — whether the achievements tour stop should be included.
* Reads the per-user module map with the standard client default-on
* convention (`modules?.achievements !== false`): only an explicit
* `false` drops the stop. Pure + exported so the rule is unit-pinned
* without rendering the launcher.
*/
export function tourIncludesAchievements(
modules: Partial<Record<string, boolean>> | undefined,
): boolean {
return modules?.achievements !== false;
}
// v1.4.15 H4 — sessionStorage keys are now scoped by user id so an
// admin impersonating a second user does not inherit the first user's
// "tour dismissed for this session" state. The previous global keys
@@ -348,8 +361,15 @@ export function TourLauncher({ ready }: TourLauncherProps) {
if (showTour !== true) return null;
// v1.18.0 B5 — drop the achievements tour stop when the account has the
// achievements module turned off. Reads the per-user module map the same
// way every other client gate does (`modules?.<key> !== false`,
// default-on).
const includeAchievements = tourIncludesAchievements(user?.modules);
return (
<OnboardingTour
includeAchievements={includeAchievements}
onClose={async (outcome) => {
// Optimistic: hide the overlay immediately, persist in the
// background. The flag is idempotent so re-fires are safe;
+26
View File
@@ -80,6 +80,32 @@ export interface AchievementDefinition {
category: AchievementCategory;
}
/**
* v1.18.0 — module ownership for an achievement metric. Returns the
* `ModuleKey` whose enable/disable state governs the badge, or `null`
* when the badge belongs to a core domain (vitals, medications) or to
* an account-wide capability (security, login, bug reports) that has no
* toggleable module behind it.
*
* The string literals here intentionally mirror `MODULE_KEYS` in
* `@/lib/modules/registry` without importing it, so this pure lib stays
* free of the gate's transitive dependencies. The module gate (B5) reads
* this map to skip badge categories whose owning module is turned off —
* a sleep badge must not unlock while the sleep module is disabled.
*/
export function moduleForMetric(metric: AchievementMetricKey): string | null {
switch (metric) {
case "moodEntryCount":
case "moodDayStreak":
case "moodImprovementHit":
return "mood";
case "sleepLogDayStreak":
return "sleep";
default:
return null;
}
}
/**
* v1.4.18 — discovery flags. Predicate input for
* `applyDiscoveryFilter`. A flag is true iff the user has at least one