diff --git a/AGENTS.md b/AGENTS.md index 6c44ed4..3d44268 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -90,6 +90,7 @@ stay complete. This is mandatory, like logging — a route without `describeRout - Connection pool: `max: 10` via `postgres-js` driver in `packages/db/src/index.ts`. - Migrations: `deno task db:generate` (creates SQL) then `deno task db:migrate` (applies); seed is `deno run -A packages/db/src/seed.ts`. - **Schema changes:** All schema changes (tables, columns, indexes, enums, constraints) MUST be made in `packages/db/src/schema.ts` (the Drizzle TypeScript schema). Then run `make db-generate && make db-migrate` to auto-generate and apply the migration. **Never manually edit the generated SQL migration files** — Drizzle's hash-based migration tracking depends on them being unmodified, and manual edits cause silent migration failures. The only exception is `make db-push` for lightweight rename/enum-addition syncs (but it does NOT detect new CHECK constraints or indexes). +- **`drizzle-kit generate --custom` workaround (TTY-less environments):** Drizzle Kit 0.31's interactive `generate` prompts for column renames require a TTY — non-interactive shells (CI, piped stdin, agents without a PTY) error out with "Interactive prompts require a TTY terminal". The non-interactive pivot is `drizzle-kit generate --custom --name=` which creates an empty SQL migration shell + a snapshot that is an EXACT COPY of the previous snapshot (NOT regenerated from the current schema). After writing the SQL by hand (per the design's reference template), you MUST also **manually update `meta/_snapshot.json`** to reflect EVERY schema change introduced by the migration — including but not limited to: renamed columns (both the JSON object key AND the `name` value inside), new enum values (under the `enums` section — Drizzle's snapshot stores them as a `values` array), new/dropped columns, new indexes, new constraints. After editing, **run `make db-generate` again and assert it outputs "No schema changes, nothing to migrate 😴"** — if it produces a new migration file, your snapshot is missing a change the schema declares; diff the new migration to find the gap, fold it into the snapshot, delete the stray migration + its snapshot + journal entry, and re-run until clean. CI runs `make db-generate` as a freshness check; a snapshot that doesn't match the schema fails the build. The hand-written SQL in the `.sql` file is NOT cross-checked against the snapshot — both must be authored to agree. - **Seed idempotency:** `packages/db/src/seed.ts` must be safe to run repeatedly. All seed helpers that insert into tables with unique constraints MUST use `onConflictDoNothing({ target: [...] })` keyed on those constraints, or select-and-reuse existing rows for tables without usable unique keys. This lets `make db-seed` recover when containers are recreated but the Postgres named volume still holds previous seed data. The seed script entrypoint MUST be guarded with `if (import.meta.main)` so the file can be imported by tests without executing the full seed. - **Full DB reset:** `make db-reset` drops and recreates the database, pushes the schema fresh, re-seeds, and flushes the Deno KV cache. diff --git a/apps/api/src/modules/comment/service.test.ts b/apps/api/src/modules/comment/service.test.ts index 2f633ad..4b5f873 100644 --- a/apps/api/src/modules/comment/service.test.ts +++ b/apps/api/src/modules/comment/service.test.ts @@ -663,9 +663,12 @@ describe('deleteComment — edge cases', () => { // // Exercises the REAL runCommentNotificationSideEffects exported by service.ts // via the service-layer `deps` proxy (same idiom as notification/service.test.ts): -// model lookups and the two notification functions are replaced with in-memory -// recorders and restored after each test. Verifies: -// - notifyRecipeCommented remains gated on recipe.authorId !== userId +// model lookups and the notification functions are replaced with in-memory +// recorders and restored after each test. As of the comment/notification +// ownership refactor, createCommentNotification is the SOLE owner of both the +// in-app comment record AND the recipe-commented email; notifyRecipeCommented +// is no longer called from the comment service. Verifies: +// - createCommentNotification is gated on recipe.authorId !== userId // - the mention scan ALWAYS runs (even when the commenter IS the recipe // author) on the effective content, forwarding recipeAuthorId // - each notification call has independent error handling: a rejection in @@ -673,12 +676,12 @@ describe('deleteComment — edge cases', () => { // --------------------------------------------------------------------------- type ModelDeps = typeof deps.model; -type NotifyRecipeCommentedParams = Parameters[0]; type MentionNotificationParams = Parameters[0]; +type CommentNotificationParams = Parameters[0]; const originalModel = deps.model; -const originalNotifyRecipeCommented = deps.notifyRecipeCommented; const originalCreateMentionNotifications = deps.createMentionNotifications; +const originalCreateCommentNotification = deps.createCommentNotification; describe('createComment — notification side-effects (F04 mention flow)', () => { const recipe = { @@ -688,8 +691,8 @@ describe('createComment — notification side-effects (F04 mention flow)', () => authorId: 'author-1', }; - let notifyCalls: NotifyRecipeCommentedParams[]; let mentionCalls: MentionNotificationParams[]; + let commentCalls: CommentNotificationParams[]; /** Replace deps with in-memory recorders; pass null to simulate a missing recipe. */ function stubDeps(recipeResult: typeof recipe | null) { @@ -702,25 +705,25 @@ describe('createComment — notification side-effects (F04 mention flow)', () => getCommenterById: (userId: string) => Promise.resolve({ id: userId, username: 'commenter' }), }; deps.model = modelStub; - deps.notifyRecipeCommented = (params) => { - notifyCalls.push(params); - return Promise.resolve(); - }; deps.createMentionNotifications = (params) => { mentionCalls.push(params); return Promise.resolve(); }; + deps.createCommentNotification = (params) => { + commentCalls.push(params); + return Promise.resolve(); + }; } beforeEach(() => { - notifyCalls = []; mentionCalls = []; + commentCalls = []; }); afterEach(() => { deps.model = originalModel; - deps.notifyRecipeCommented = originalNotifyRecipeCommented; deps.createMentionNotifications = originalCreateMentionNotifications; + deps.createCommentNotification = originalCreateCommentNotification; }); it('sends recipe-commented AND triggers the mention flow when the commenter is not the author', async () => { @@ -731,8 +734,16 @@ describe('createComment — notification side-effects (F04 mention flow)', () => commentId: 'comment-1', effectiveContent: 'Nice one @alice and @bob-2!', }); - expect(notifyCalls.length).toBe(1); - expect(notifyCalls[0].recipeAuthorId).toBe('author-1'); + expect(commentCalls.length).toBe(1); + expect(commentCalls[0]).toEqual({ + commenterId: 'commenter-1', + commenterUsername: 'commenter', + recipeAuthorId: 'author-1', + recipeId: 'recipe-1', + recipeSlug: 'recipe-slug', + recipeTitle: 'Recipe Title', + commentId: 'comment-1', + }); expect(mentionCalls.length).toBe(1); expect(mentionCalls[0].mentions).toEqual(['alice', 'bob-2']); expect(mentionCalls[0].commentId).toBe('comment-1'); @@ -749,7 +760,7 @@ describe('createComment — notification side-effects (F04 mention flow)', () => commentId: 'comment-2', effectiveContent: 'Thanks @alice for the tip', }); - expect(notifyCalls.length).toBe(0); // gated: author commenting on own recipe + expect(commentCalls.length).toBe(0); // gated: author commenting on own recipe expect(mentionCalls.length).toBe(1); // mention scan must NOT be skipped expect(mentionCalls[0].mentions).toEqual(['alice']); expect(mentionCalls[0].recipeAuthorId).toBe('author-1'); @@ -775,7 +786,7 @@ describe('createComment — notification side-effects (F04 mention flow)', () => commentId: 'comment-4', effectiveContent: 'no mentions here', }); - expect(notifyCalls.length).toBe(1); + expect(commentCalls.length).toBe(1); expect(mentionCalls.length).toBe(1); expect(mentionCalls[0].mentions).toEqual([]); }); @@ -788,13 +799,13 @@ describe('createComment — notification side-effects (F04 mention flow)', () => commentId: 'comment-5', effectiveContent: 'hello @alice', }); - expect(notifyCalls.length).toBe(0); + expect(commentCalls.length).toBe(0); expect(mentionCalls.length).toBe(0); }); - it('still runs the mention flow when notifyRecipeCommented rejects', async () => { + it('still runs the mention flow when createCommentNotification rejects', async () => { stubDeps(recipe); - deps.notifyRecipeCommented = () => Promise.reject(new Error('smtp down')); + deps.createCommentNotification = () => Promise.reject(new Error('db down')); await runCommentNotificationSideEffects({ userId: 'commenter-1', recipeId: 'recipe-1', @@ -814,8 +825,70 @@ describe('createComment — notification side-effects (F04 mention flow)', () => commentId: 'comment-7', effectiveContent: 'cc @alice', }); - expect(notifyCalls.length).toBe(1); - expect(notifyCalls[0].recipeAuthorId).toBe('author-1'); + expect(commentCalls.length).toBe(1); + expect(commentCalls[0]).toEqual({ + commenterId: 'commenter-1', + commenterUsername: 'commenter', + recipeAuthorId: 'author-1', + recipeId: 'recipe-1', + recipeSlug: 'recipe-slug', + recipeTitle: 'Recipe Title', + commentId: 'comment-7', + }); + }); + + // F05 — single-recipient comment fan-out (createCommentNotification) is + // gated on commenter != recipe author. The F04 mention flow is independent + // of that gate and must keep firing for @mentioned users regardless of the + // commenter/author relationship (regression guard). + + it('createCommentNotification invoked when commenter != recipe author', async () => { + stubDeps(recipe); + await runCommentNotificationSideEffects({ + userId: 'commenter-1', + recipeId: 'recipe-1', + commentId: 'comment-8', + effectiveContent: 'nice recipe', + }); + expect(commentCalls.length).toBe(1); + expect(commentCalls[0]).toEqual({ + commenterId: 'commenter-1', + commenterUsername: 'commenter', + recipeAuthorId: 'author-1', + recipeId: 'recipe-1', + recipeSlug: 'recipe-slug', + recipeTitle: 'Recipe Title', + commentId: 'comment-8', + }); + }); + + it('createCommentNotification NOT invoked when commenter === recipe author (self-comment)', async () => { + stubDeps(recipe); + await runCommentNotificationSideEffects({ + userId: 'author-1', + recipeId: 'recipe-1', + commentId: 'comment-9', + effectiveContent: 'thanks for the tips', + }); + expect(commentCalls.length).toBe(0); + // The mention flow (F04) is NOT gated on the commenter/author relationship. + expect(mentionCalls.length).toBe(1); + }); + + it('createMentionNotifications STILL fires for @mentioned users (F04 regression)', async () => { + stubDeps(recipe); + await runCommentNotificationSideEffects({ + userId: 'commenter-1', + recipeId: 'recipe-1', + commentId: 'comment-10', + effectiveContent: 'cc @alice agree', + }); + // F05 comment fan-out fires (non-author commenter) ... + expect(commentCalls.length).toBe(1); + // ... AND the F04 mention fan-out still fires for the @mentioned user. + expect(mentionCalls.length).toBe(1); + expect(mentionCalls[0].mentions).toEqual(['alice']); + expect(mentionCalls[0].recipeAuthorId).toBe('author-1'); }); }); @@ -839,7 +912,6 @@ type GateRecipeModelDeps = typeof deps.recipeModel; const gateOriginalModel = deps.model; const gateOriginalRecipeModel = deps.recipeModel; -const gateOriginalNotify = deps.notifyRecipeCommented; const gateOriginalMentions = deps.createMentionNotifications; const gateOriginalBadges = deps.evaluateBadges; @@ -857,7 +929,6 @@ describe('D99.9 — comment visibility gate', () => { let createCalls: unknown[]; let incrementCalls: unknown[]; let findByRecipeCalls: unknown[]; - let notifyCalls: unknown[]; let mentionCalls: unknown[]; /** Replace deps with in-memory recorders over `fixture` (null = missing recipe). */ @@ -885,10 +956,6 @@ describe('D99.9 — comment visibility gate', () => { return Promise.resolve(); }, } as GateRecipeModelDeps; - deps.notifyRecipeCommented = (params) => { - notifyCalls.push(params); - return Promise.resolve(); - }; deps.createMentionNotifications = (params) => { mentionCalls.push(params); return Promise.resolve(); @@ -905,14 +972,12 @@ describe('D99.9 — comment visibility gate', () => { createCalls = []; incrementCalls = []; findByRecipeCalls = []; - notifyCalls = []; mentionCalls = []; }); afterEach(() => { deps.model = gateOriginalModel; deps.recipeModel = gateOriginalRecipeModel; - deps.notifyRecipeCommented = gateOriginalNotify; deps.createMentionNotifications = gateOriginalMentions; deps.evaluateBadges = gateOriginalBadges; }); @@ -920,7 +985,6 @@ describe('D99.9 — comment visibility gate', () => { function expectZeroSideEffects() { expect(createCalls.length).toBe(0); expect(incrementCalls.length).toBe(0); - expect(notifyCalls.length).toBe(0); expect(mentionCalls.length).toBe(0); } diff --git a/apps/api/src/modules/comment/service.ts b/apps/api/src/modules/comment/service.ts index a27f36a..719aff4 100644 --- a/apps/api/src/modules/comment/service.ts +++ b/apps/api/src/modules/comment/service.ts @@ -11,8 +11,7 @@ import * as model from './model.ts'; import * as recipeModel from '../recipe/model.ts'; import { canViewRecipe } from '../recipe/service.ts'; import { createLogger } from '../../utils/logger/index.ts'; -import { notifyRecipeCommented } from '../../utils/notify/index.ts'; -import { createMentionNotifications } from '../notification/service.ts'; +import { createCommentNotification, createMentionNotifications } from '../notification/service.ts'; import { evaluateBadges } from '../badge/service.ts'; const logger = createLogger('comment-service'); @@ -26,8 +25,8 @@ const logger = createLogger('comment-service'); export const deps = { model, recipeModel, - notifyRecipeCommented, createMentionNotifications, + createCommentNotification, evaluateBadges, }; @@ -128,8 +127,11 @@ export async function createComment( * Flow: * 1. Load the recipe (early return when it cannot be found). * 2. Load the commenter (early return when no username is available). - * 3. Send the recipe-commented email ONLY when the commenter is not the - * recipe author (`recipe.authorId !== userId`). + * 3. `createCommentNotification` owns BOTH the in-app `comment` record AND + * the recipe-commented email — gated on `notifyRecipeCommented` prefs + * (single flag gates both). It self-skips when the commenter IS the + * recipe author, so the `recipe.authorId !== userId` guard here only + * avoids the call; the creator's own self-check is belt-and-braces. * 4. ALWAYS run the mention flow — including when the commenter IS the * recipe author. Mentions are parsed from the effective (post-prepend, * sanitized) content; createMentionNotifications no-ops on empty lists. @@ -157,18 +159,20 @@ export async function runCommentNotificationSideEffects(params: { const commenter = await deps.model.getCommenterById(userId); if (!commenter?.username) return; - // Recipe-commented email only when someone ELSE comments on the recipe. + // F05: createCommentNotification owns BOTH the in-app `comment` record AND + // the recipe-commented email (gated on `notifyRecipeCommented` prefs — + // single flag gates both). No direct notifyRecipeCommented call here — + // that would double-send and bypass the preference gate. if (recipe.authorId !== userId) { - try { - await deps.notifyRecipeCommented({ - recipeAuthorId: recipe.authorId, - commenterUsername: commenter.username, - recipeTitle: recipe.title, - recipeSlug: recipe.slug, - }); - } catch (err) { - logger.error({ err, commentId }, 'notifyRecipeCommented failed'); - } + deps.createCommentNotification({ + commenterId: userId, + commenterUsername: commenter.username, + recipeAuthorId: recipe.authorId, + recipeId: recipe.id, + recipeSlug: recipe.slug, + recipeTitle: recipe.title, + commentId, + }).catch((err) => logger.error({ err, commentId }, 'createCommentNotification failed')); } // Mention notifications always run — including when the commenter IS the diff --git a/apps/api/src/modules/follow/service.test.ts b/apps/api/src/modules/follow/service.test.ts index e7c2cad..86d4664 100644 --- a/apps/api/src/modules/follow/service.test.ts +++ b/apps/api/src/modules/follow/service.test.ts @@ -1,5 +1,10 @@ -import { describe, it } from 'jsr:@std/testing/bdd'; +import '../../test-setup.ts'; +import { afterAll, afterEach, beforeAll, describe, it } from 'jsr:@std/testing/bdd'; import { expect } from 'jsr:@std/expect'; +import { db } from '@brewform/db'; +import { notifications, userBadges, userFollows, users } from '@brewform/db/schema'; +import { and, eq, inArray } from 'drizzle-orm'; +import { followUser } from './service.ts'; describe('Follow Service Logic', () => { describe('Self-follow prevention', () => { @@ -38,3 +43,116 @@ describe('Follow Service Logic', () => { }); }); }); + +// --------------------------------------------------------------------------- +// F05 — DB-backed integration: followUser fires createFollowNotification. +// +// The follow service calls createFollowNotification via a DIRECT import (no +// deps proxy), so the only way to assert the fan-out here is against the real +// DB. `followUser` spawns a fire-and-forget IIFE that loads the follower's +// username and delegates to `createFollowNotification`, which owns BOTH the +// follow email (suppressed under APP_ENV=test) AND the in-app `follow` +// notification record (gated on `notifyNewFollower` prefs — missing prefs +// counts as opted-in, which is the state for the freshly-inserted `followed` +// user below). +// --------------------------------------------------------------------------- + +describe( + 'Follow Service — DB integration (F05 follow notification fan-out)', + { sanitizeOps: false, sanitizeResources: false }, + () => { + let follower: typeof users.$inferSelect; + let followed: typeof users.$inferSelect; + const createdUsers: string[] = []; + + async function makeUser(prefix: string) { + const id = crypto.randomUUID(); + const [user] = await db.insert(users).values({ + id, + email: `${prefix}-${id}@example.com`, + username: `${prefix}-${id.slice(0, 8)}`, + passwordHash: 'hash', + }).returning(); + createdUsers.push(user.id); + return user; + } + + // Drain the fire-and-forget badge-evaluation race before deleting a user. + async function deleteUserWithBadges(userId: string) { + for (let attempt = 0;; attempt++) { + await db.delete(userBadges).where(eq(userBadges.userId, userId)); + try { + await db.delete(users).where(eq(users.id, userId)); + return; + } catch (err) { + if (attempt >= 9) throw err; + await new Promise((resolve) => setTimeout(resolve, 20)); + } + } + } + + // Poll for the `follow` notification row up to ~1.5s; the fan-out is async. + async function followNotificationAppeared(timeoutMs = 1500): Promise { + const start = Date.now(); + while (Date.now() - start < timeoutMs) { + const rows = await db.select().from(notifications).where(and( + eq(notifications.userId, followed.id), + eq(notifications.actorId, follower.id), + eq(notifications.type, 'follow'), + )); + if (rows.length > 0) return true; + await new Promise((resolve) => setTimeout(resolve, 10)); + } + return false; + } + + beforeAll(async () => { + follower = await makeUser('follower'); + followed = await makeUser('followed'); + }); + + afterEach(async () => { + // Reset the follow graph + any notifications so each test starts clean. + // notifications.userId cascades on user delete, but user_follow.followerId + // / followingId have NO ON DELETE CASCADE — delete the follow row explicitly. + await db.delete(notifications).where(eq(notifications.userId, followed.id)); + await db.delete(userFollows).where(eq(userFollows.followerId, follower.id)); + }); + + afterAll(async () => { + await db.delete(userFollows).where(inArray(userFollows.followerId, createdUsers)); + await db.delete(userFollows).where(inArray(userFollows.followingId, createdUsers)); + for (const userId of createdUsers) { + await deleteUserWithBadges(userId); + } + }); + + it('createFollowNotification is invoked after follow creation', async () => { + await followUser(follower.id, followed.id); + expect(await followNotificationAppeared()).toBe(true); + }); + + it('createFollowNotification NOT invoked when follow already exists (idempotent)', async () => { + // First follow creates the notification; drain the fan-out. + await followUser(follower.id, followed.id); + expect(await followNotificationAppeared()).toBe(true); + + const before = await db.select().from(notifications).where(and( + eq(notifications.userId, followed.id), + eq(notifications.actorId, follower.id), + eq(notifications.type, 'follow'), + )); + + // Second follow throws ALREADY_FOLLOWING BEFORE the IIFE — no new row. + await expect(followUser(follower.id, followed.id)).rejects.toThrow('ALREADY_FOLLOWING'); + await new Promise((resolve) => setTimeout(resolve, 50)); + + const after = await db.select().from(notifications).where(and( + eq(notifications.userId, followed.id), + eq(notifications.actorId, follower.id), + eq(notifications.type, 'follow'), + )); + expect(after.length).toBe(before.length); + }); + }, +); diff --git a/apps/api/src/modules/follow/service.ts b/apps/api/src/modules/follow/service.ts index 81b50b4..2d9f6bc 100644 --- a/apps/api/src/modules/follow/service.ts +++ b/apps/api/src/modules/follow/service.ts @@ -10,7 +10,7 @@ import type { CursorResult } from '../recipe/model.ts'; import * as recipeModel from '../recipe/model.ts'; import * as userModel from '../user/model.ts'; import { createLogger } from '../../utils/logger/index.ts'; -import { notifyNewFollower } from '../../utils/notify/index.ts'; +import { createFollowNotification } from '../notification/service.ts'; import { evaluateBadges } from '../badge/service.ts'; const logger = createLogger('follow-service'); @@ -34,11 +34,18 @@ export async function followUser(followerId: string, followingId: string) { (async () => { const follower = await userModel.findById(followerId); if (!follower?.username) return; - await notifyNewFollower({ - followingId, + // F05: createFollowNotification owns BOTH the in-app `follow` record AND + // the follow email (gated on `notifyNewFollower` prefs — single flag gates + // both). No direct notifyNewFollower call here — that would double-send + // and bypass the preference gate. + await createFollowNotification({ + followerId, followerUsername: follower.username, + followingId, }); - })().catch((err) => logger.error({ err }, 'notifyNewFollower failed')); + })().catch((err) => + logger.error({ err, followerId, followingId }, 'createFollowNotification failed') + ); evaluateBadges(followerId).catch((err) => logger.error({ err }, 'evaluateBadges failed')); diff --git a/apps/api/src/modules/notification/model.test.ts b/apps/api/src/modules/notification/model.test.ts index 450b2c1..d69f499 100644 --- a/apps/api/src/modules/notification/model.test.ts +++ b/apps/api/src/modules/notification/model.test.ts @@ -409,7 +409,7 @@ describe('findMentionTargets', { sanitizeOps: false, sanitizeResources: false }, await db.insert(userPreferences).values({ id: prefsId, userId: optedOutUserId, - mentionedInComment: false, + notifyMentionedInComment: false, }); }); @@ -429,7 +429,7 @@ describe('findMentionTargets', { sanitizeOps: false, sanitizeResources: false }, const optedOut = result.find((r) => r.id === optedOutUserId); expect(optedOut!.username).toBe(`testuser-${optedOutUserId}`); expect(optedOut!.prefs).not.toBeNull(); - expect(optedOut!.prefs!.mentionedInComment).toBe(false); + expect(optedOut!.prefs!.notifyMentionedInComment).toBe(false); }); it('should return prefs null for a user without a preferences row', async () => { @@ -458,3 +458,75 @@ describe('findMentionTargets', { sanitizeOps: false, sanitizeResources: false }, expect(result).toEqual([]); }); }); + +/** + * findNotifyTarget — Loads a single notification recipient with their + * preferences row left-joined (F05 single-recipient fan-out). Returns null + * when the user does not exist, is soft-deleted, or is banned; Drizzle + * collapses the nested `prefs` object to null when no preferences row matched. + */ +describe('findNotifyTarget', { sanitizeOps: false, sanitizeResources: false }, () => { + let withPrefsUserId: string; + let noPrefsUserId: string; + let deletedUserId: string; + let bannedUserId: string; + let prefsId: string; + + beforeEach(async () => { + withPrefsUserId = await insertUser(); + noPrefsUserId = await insertUser(); + deletedUserId = await insertUser({ deletedAt: new Date() }); + bannedUserId = await insertUser({ isBanned: true }); + prefsId = crypto.randomUUID(); + await db.insert(userPreferences).values({ + id: prefsId, + userId: withPrefsUserId, + notifyMentionedInComment: false, + }); + }); + + afterEach(async () => { + await db.delete(userPreferences).where(eq(userPreferences.id, prefsId)); + await db.delete(users).where( + inArray(users.id, [withPrefsUserId, noPrefsUserId, deletedUserId, bannedUserId]), + ); + }); + + it('should resolve a user with prefs joined when user exists and has preferences row', async () => { + const result = await model.findNotifyTarget(withPrefsUserId); + expect(result).not.toBeNull(); + expect(result!.id).toBe(withPrefsUserId); + expect(result!.username).toBe(`testuser-${withPrefsUserId}`); + expect(result!.prefs).not.toBeNull(); + expect(result!.prefs!.notifyMentionedInComment).toBe(false); + // Columns not set in the insert default to true at the DB level. + expect(result!.prefs!.notifyNewFollower).toBe(true); + expect(result!.prefs!.notifyRecipeLiked).toBe(true); + expect(result!.prefs!.notifyRecipeCommented).toBe(true); + expect(result!.prefs!.notifyFollowedUserPosted).toBe(true); + }); + + it('should return prefs null for a user without a preferences row', async () => { + const result = await model.findNotifyTarget(noPrefsUserId); + expect(result).not.toBeNull(); + expect(result!.id).toBe(noPrefsUserId); + expect(result!.username).toBe(`testuser-${noPrefsUserId}`); + // Drizzle collapses the nested prefs object to null when the LEFT JOIN row is absent. + expect(result!.prefs).toBeNull(); + }); + + it('should return null when user does not exist', async () => { + const result = await model.findNotifyTarget(crypto.randomUUID()); + expect(result).toBeNull(); + }); + + it('should return null when user is soft-deleted', async () => { + const result = await model.findNotifyTarget(deletedUserId); + expect(result).toBeNull(); + }); + + it('should return null when user is banned', async () => { + const result = await model.findNotifyTarget(bannedUserId); + expect(result).toBeNull(); + }); +}); diff --git a/apps/api/src/modules/notification/model.ts b/apps/api/src/modules/notification/model.ts index 852bde4..25c7e8d 100644 --- a/apps/api/src/modules/notification/model.ts +++ b/apps/api/src/modules/notification/model.ts @@ -174,7 +174,7 @@ export async function findMentionTargets(usernames: string[]) { id: users.id, username: users.username, prefs: { - mentionedInComment: userPreferences.mentionedInComment, + notifyMentionedInComment: userPreferences.notifyMentionedInComment, }, }) .from(users) @@ -185,3 +185,37 @@ export async function findMentionTargets(usernames: string[]) { eq(users.isBanned, false), )); } + +/** + * Look up a single notification recipient with their preferences row + * (parallel to `findMentionTargets`, used by the single-recipient fan-out + * creators in `service.ts` — `createFollow/Like/CommentNotification`). + * Returns `null` when the user does not exist, is soft-deleted, or is + * banned. Missing prefs (`null`) is treated as opted-in by the caller + * (DB-column default-true semantics). + * + * @param userId - The recipient's UUID. + * @returns The `{ id, username, prefs }` row, or `null` if not found. + */ +export async function findNotifyTarget(userId: string) { + const rows = await db.select({ + id: users.id, + username: users.username, + prefs: { + notifyNewFollower: userPreferences.notifyNewFollower, + notifyRecipeLiked: userPreferences.notifyRecipeLiked, + notifyRecipeCommented: userPreferences.notifyRecipeCommented, + notifyFollowedUserPosted: userPreferences.notifyFollowedUserPosted, + notifyMentionedInComment: userPreferences.notifyMentionedInComment, + }, + }) + .from(users) + .leftJoin(userPreferences, eq(users.id, userPreferences.userId)) + .where(and( + eq(users.id, userId), + isNull(users.deletedAt), + eq(users.isBanned, false), + )) + .limit(1); + return rows[0] ?? null; +} diff --git a/apps/api/src/modules/notification/service.test.ts b/apps/api/src/modules/notification/service.test.ts index 418fa6b..5f05f5e 100644 --- a/apps/api/src/modules/notification/service.test.ts +++ b/apps/api/src/modules/notification/service.test.ts @@ -13,26 +13,60 @@ import { deps } from './service.ts'; type ModelDeps = typeof deps.model; type NotifyMentioned = typeof deps.notifyMentioned; +type NotifyNewFollower = typeof deps.notifyNewFollower; +type NotifyRecipeLiked = typeof deps.notifyRecipeLiked; +type NotifyRecipeCommented = typeof deps.notifyRecipeCommented; type MentionTarget = Awaited>[number]; +type NotifyTarget = NonNullable>>; type NotificationRow = NonNullable>>; type CreateData = Parameters[0]; +/** Default "all notify flags on" preferences for {@link makeNotifyTarget}. */ +const ALL_NOTIFY_PREFS_TRUE = { + notifyNewFollower: true, + notifyRecipeLiked: true, + notifyRecipeCommented: true, + notifyFollowedUserPosted: true, + notifyMentionedInComment: true, +}; + /** In-memory call recorder for the model + email stubs. */ interface Calls { findMentionTargets: string[][]; + findNotifyTarget: string[]; create: CreateData[]; notifyMentioned: Parameters[0][]; + notifyNewFollower: Parameters[0][]; + notifyRecipeLiked: Parameters[0][]; + notifyRecipeCommented: Parameters[0][]; } function makeCalls(): Calls { - return { findMentionTargets: [], create: [], notifyMentioned: [] }; + return { + findMentionTargets: [], + findNotifyTarget: [], + create: [], + notifyMentioned: [], + notifyNewFollower: [], + notifyRecipeLiked: [], + notifyRecipeCommented: [], + }; } function makeTarget(overrides: Partial = {}): MentionTarget { return { id: 'target-1', username: 'target', - prefs: { mentionedInComment: true }, + prefs: { notifyMentionedInComment: true }, + ...overrides, + }; +} + +function makeNotifyTarget(overrides: Partial = {}): NotifyTarget { + return { + id: 'target-1', + username: 'target', + prefs: { ...ALL_NOTIFY_PREFS_TRUE }, ...overrides, }; } @@ -63,6 +97,10 @@ function makeModel(overrides: Partial, calls: Calls): ModelDeps { calls.findMentionTargets.push(usernames); return Promise.resolve([]); }, + findNotifyTarget: (userId: string) => { + calls.findNotifyTarget.push(userId); + return Promise.resolve(makeNotifyTarget({ id: userId })); + }, create: (data: CreateData) => { calls.create.push(data); return Promise.resolve( @@ -86,6 +124,9 @@ function makeModel(overrides: Partial, calls: Calls): ModelDeps { const originalModel = deps.model; const originalNotifyMentioned = deps.notifyMentioned; +const originalNotifyNewFollower = deps.notifyNewFollower; +const originalNotifyRecipeLiked = deps.notifyRecipeLiked; +const originalNotifyRecipeCommented = deps.notifyRecipeCommented; let calls: Calls; @@ -95,11 +136,26 @@ beforeEach(() => { calls.notifyMentioned.push(params); return Promise.resolve(); }; + deps.notifyNewFollower = (params) => { + calls.notifyNewFollower.push(params); + return Promise.resolve(); + }; + deps.notifyRecipeLiked = (params) => { + calls.notifyRecipeLiked.push(params); + return Promise.resolve(); + }; + deps.notifyRecipeCommented = (params) => { + calls.notifyRecipeCommented.push(params); + return Promise.resolve(); + }; }); afterEach(() => { deps.model = originalModel; deps.notifyMentioned = originalNotifyMentioned; + deps.notifyNewFollower = originalNotifyNewFollower; + deps.notifyRecipeLiked = originalNotifyRecipeLiked; + deps.notifyRecipeCommented = originalNotifyRecipeCommented; }); /** Default params for createMentionNotifications; recipe author is a third user. */ @@ -163,10 +219,10 @@ describe('createMentionNotifications', () => { expect(calls.notifyMentioned).toEqual([]); }); - it('skips record AND email when mentionedInComment preference is false', async () => { + it('skips record AND email when notifyMentionedInComment preference is false', async () => { deps.model = makeModel({ findMentionTargets: () => - Promise.resolve([makeTarget({ prefs: { mentionedInComment: false } })]), + Promise.resolve([makeTarget({ prefs: { notifyMentionedInComment: false } })]), }, calls); await service.createMentionNotifications(mentionParams()); expect(calls.create).toEqual([]); @@ -205,7 +261,11 @@ describe('createMentionNotifications', () => { findMentionTargets: () => Promise.resolve([ makeTarget({ id: 'mentioner-1', username: 'self' }), - makeTarget({ id: 'opted-out', username: 'quiet', prefs: { mentionedInComment: false } }), + makeTarget({ + id: 'opted-out', + username: 'quiet', + prefs: { notifyMentionedInComment: false }, + }), makeTarget({ id: 'author-1', username: 'author' }), makeTarget({ id: 'plain-1', username: 'plain' }), ]), @@ -317,3 +377,325 @@ describe('markAllAsRead / getUnreadCount', () => { expect(await service.getUnreadCount('user-1')).toBe(7); }); }); + +// --------------------------------------------------------------------------- +// F05 — single-recipient fan-out creators (follow / like / comment). +// +// Mirrors `createMentionNotifications` but targets ONE recipient (the followed +// user / recipe author) and gates on a single `notify*` preference that, when +// false, skips BOTH the DB record and the email. Missing prefs (`null`) counts +// as opted-in. Per-call failures are isolated: a thrown `create` is logged and +// execution continues to the email (independent try/catch), and vice-versa. +// --------------------------------------------------------------------------- + +describe('createFollowNotification', () => { + function followerParams( + overrides: Partial[0]> = {}, + ) { + return { + followerId: 'follower-1', + followerUsername: 'follower-username', + followingId: 'target-1', + ...overrides, + }; + } + + it('skips record and email when notifyNewFollower preference is false', async () => { + deps.model = makeModel({ + findNotifyTarget: (userId: string) => { + calls.findNotifyTarget.push(userId); + return Promise.resolve( + makeNotifyTarget({ prefs: { ...ALL_NOTIFY_PREFS_TRUE, notifyNewFollower: false } }), + ); + }, + }, calls); + await service.createFollowNotification(followerParams()); + expect(calls.findNotifyTarget).toEqual(['target-1']); + expect(calls.create).toEqual([]); + expect(calls.notifyNewFollower).toEqual([]); + }); + + it('treats missing preferences row (prefs null) as opted in', async () => { + deps.model = makeModel({ + findNotifyTarget: () => Promise.resolve(makeNotifyTarget({ prefs: null })), + }, calls); + await service.createFollowNotification(followerParams()); + expect(calls.create.length).toBe(1); + expect(calls.notifyNewFollower.length).toBe(1); + }); + + it('skips self-follow (no record, no email)', async () => { + deps.model = makeModel({}, calls); + await service.createFollowNotification(followerParams({ + followerId: 'self-1', + followingId: 'self-1', + })); + expect(calls.findNotifyTarget).toEqual([]); + expect(calls.create).toEqual([]); + expect(calls.notifyNewFollower).toEqual([]); + }); + + it('target not found returns early without record or email', async () => { + deps.model = makeModel({ findNotifyTarget: () => Promise.resolve(null as never) }, calls); + await service.createFollowNotification(followerParams()); + expect(calls.create).toEqual([]); + expect(calls.notifyNewFollower).toEqual([]); + }); + + it('creates record and sends email when opted in', async () => { + deps.model = makeModel({}, calls); + await service.createFollowNotification(followerParams()); + expect(calls.findNotifyTarget).toEqual(['target-1']); + expect(calls.create.length).toBe(1); + expect(calls.create[0]).toEqual({ + userId: 'target-1', + type: 'follow', + actorId: 'follower-1', + referenceId: null, + referenceType: 'actor', + metadata: JSON.stringify({ followerUsername: 'follower-username' }), + }); + expect(calls.notifyNewFollower).toEqual([{ + followingId: 'target-1', + followerUsername: 'follower-username', + }]); + }); + + it('creates record but logs error if email send throws', async () => { + deps.model = makeModel({}, calls); + deps.notifyNewFollower = (params) => { + calls.notifyNewFollower.push(params); + return Promise.reject(new Error('smtp down')); + }; + await service.createFollowNotification(followerParams()); + expect(calls.create.length).toBe(1); + expect(calls.notifyNewFollower.length).toBe(1); + }); + + it('logs error but does not throw if model.create throws (email still fires)', async () => { + deps.model = makeModel( + { + create: (data) => { + calls.create.push(data); + return Promise.reject(new Error('db down')); + }, + }, + calls, + ); + await service.createFollowNotification(followerParams()); + expect(calls.create.length).toBe(1); + // Independent try/catch: the email fires even after the record insert failed. + expect(calls.notifyNewFollower.length).toBe(1); + }); +}); + +describe('createLikeNotification', () => { + function likeParams( + overrides: Partial[0]> = {}, + ) { + return { + likerId: 'liker-1', + likerUsername: 'liker-username', + recipeAuthorId: 'target-1', + recipeId: 'recipe-1', + recipeSlug: 'slug-1', + recipeTitle: 'Title 1', + ...overrides, + }; + } + + it('skips record and email when notifyRecipeLiked preference is false', async () => { + deps.model = makeModel({ + findNotifyTarget: (userId: string) => { + calls.findNotifyTarget.push(userId); + return Promise.resolve( + makeNotifyTarget({ prefs: { ...ALL_NOTIFY_PREFS_TRUE, notifyRecipeLiked: false } }), + ); + }, + }, calls); + await service.createLikeNotification(likeParams()); + expect(calls.findNotifyTarget).toEqual(['target-1']); + expect(calls.create).toEqual([]); + expect(calls.notifyRecipeLiked).toEqual([]); + }); + + it('treats missing preferences row (prefs null) as opted in', async () => { + deps.model = makeModel({ + findNotifyTarget: () => Promise.resolve(makeNotifyTarget({ prefs: null })), + }, calls); + await service.createLikeNotification(likeParams()); + expect(calls.create.length).toBe(1); + expect(calls.notifyRecipeLiked.length).toBe(1); + }); + + it('skips self-like (no record, no email)', async () => { + deps.model = makeModel({}, calls); + await service.createLikeNotification(likeParams({ + likerId: 'self-1', + recipeAuthorId: 'self-1', + })); + expect(calls.findNotifyTarget).toEqual([]); + expect(calls.create).toEqual([]); + expect(calls.notifyRecipeLiked).toEqual([]); + }); + + it('target not found returns early without record or email', async () => { + deps.model = makeModel({ findNotifyTarget: () => Promise.resolve(null as never) }, calls); + await service.createLikeNotification(likeParams()); + expect(calls.create).toEqual([]); + expect(calls.notifyRecipeLiked).toEqual([]); + }); + + it('creates record and sends email when opted in', async () => { + deps.model = makeModel({}, calls); + await service.createLikeNotification(likeParams()); + expect(calls.findNotifyTarget).toEqual(['target-1']); + expect(calls.create.length).toBe(1); + expect(calls.create[0]).toEqual({ + userId: 'target-1', + type: 'like', + actorId: 'liker-1', + referenceId: 'recipe-1', + referenceType: 'recipe', + metadata: JSON.stringify({ recipeSlug: 'slug-1', recipeTitle: 'Title 1' }), + }); + expect(calls.notifyRecipeLiked).toEqual([{ + recipeAuthorId: 'target-1', + likerUsername: 'liker-username', + recipeTitle: 'Title 1', + recipeSlug: 'slug-1', + }]); + }); + + it('creates record but logs error if email send throws', async () => { + deps.model = makeModel({}, calls); + deps.notifyRecipeLiked = (params) => { + calls.notifyRecipeLiked.push(params); + return Promise.reject(new Error('smtp down')); + }; + await service.createLikeNotification(likeParams()); + expect(calls.create.length).toBe(1); + expect(calls.notifyRecipeLiked.length).toBe(1); + }); + + it('logs error but does not throw if model.create throws (email still fires)', async () => { + deps.model = makeModel( + { + create: (data) => { + calls.create.push(data); + return Promise.reject(new Error('db down')); + }, + }, + calls, + ); + await service.createLikeNotification(likeParams()); + expect(calls.create.length).toBe(1); + expect(calls.notifyRecipeLiked.length).toBe(1); + }); +}); + +describe('createCommentNotification', () => { + function commentParams( + overrides: Partial[0]> = {}, + ) { + return { + commenterId: 'commenter-1', + commenterUsername: 'commenter-username', + recipeAuthorId: 'target-1', + recipeId: 'recipe-1', + recipeSlug: 'slug-1', + recipeTitle: 'Title 1', + commentId: 'comment-1', + ...overrides, + }; + } + + it('skips record and email when notifyRecipeCommented preference is false', async () => { + deps.model = makeModel({ + findNotifyTarget: (userId: string) => { + calls.findNotifyTarget.push(userId); + return Promise.resolve( + makeNotifyTarget({ prefs: { ...ALL_NOTIFY_PREFS_TRUE, notifyRecipeCommented: false } }), + ); + }, + }, calls); + await service.createCommentNotification(commentParams()); + expect(calls.findNotifyTarget).toEqual(['target-1']); + expect(calls.create).toEqual([]); + expect(calls.notifyRecipeCommented).toEqual([]); + }); + + it('treats missing preferences row (prefs null) as opted in', async () => { + deps.model = makeModel({ + findNotifyTarget: () => Promise.resolve(makeNotifyTarget({ prefs: null })), + }, calls); + await service.createCommentNotification(commentParams()); + expect(calls.create.length).toBe(1); + expect(calls.notifyRecipeCommented.length).toBe(1); + }); + + it('skips self-comment (no record, no email)', async () => { + deps.model = makeModel({}, calls); + await service.createCommentNotification(commentParams({ + commenterId: 'self-1', + recipeAuthorId: 'self-1', + })); + expect(calls.findNotifyTarget).toEqual([]); + expect(calls.create).toEqual([]); + expect(calls.notifyRecipeCommented).toEqual([]); + }); + + it('target not found returns early without record or email', async () => { + deps.model = makeModel({ findNotifyTarget: () => Promise.resolve(null as never) }, calls); + await service.createCommentNotification(commentParams()); + expect(calls.create).toEqual([]); + expect(calls.notifyRecipeCommented).toEqual([]); + }); + + it('creates record and sends email when opted in', async () => { + deps.model = makeModel({}, calls); + await service.createCommentNotification(commentParams()); + expect(calls.findNotifyTarget).toEqual(['target-1']); + expect(calls.create.length).toBe(1); + expect(calls.create[0]).toEqual({ + userId: 'target-1', + type: 'comment', + actorId: 'commenter-1', + referenceId: 'comment-1', + referenceType: 'comment', + metadata: JSON.stringify({ recipeSlug: 'slug-1', recipeTitle: 'Title 1' }), + }); + expect(calls.notifyRecipeCommented).toEqual([{ + recipeAuthorId: 'target-1', + commenterUsername: 'commenter-username', + recipeTitle: 'Title 1', + recipeSlug: 'slug-1', + }]); + }); + + it('creates record but logs error if email send throws', async () => { + deps.model = makeModel({}, calls); + deps.notifyRecipeCommented = (params) => { + calls.notifyRecipeCommented.push(params); + return Promise.reject(new Error('smtp down')); + }; + await service.createCommentNotification(commentParams()); + expect(calls.create.length).toBe(1); + expect(calls.notifyRecipeCommented.length).toBe(1); + }); + + it('logs error but does not throw if model.create throws (email still fires)', async () => { + deps.model = makeModel( + { + create: (data) => { + calls.create.push(data); + return Promise.reject(new Error('db down')); + }, + }, + calls, + ); + await service.createCommentNotification(commentParams()); + expect(calls.create.length).toBe(1); + expect(calls.notifyRecipeCommented.length).toBe(1); + }); +}); diff --git a/apps/api/src/modules/notification/service.ts b/apps/api/src/modules/notification/service.ts index ab78ba4..82a5f24 100644 --- a/apps/api/src/modules/notification/service.ts +++ b/apps/api/src/modules/notification/service.ts @@ -1,24 +1,41 @@ /** - * Notification business logic for BrewForm (F04 — @mention notifications). + * Notification business logic for BrewForm (F04 + F05 — `mention`, `follow`, + * `like`, `comment` notification types). * * Orchestrates mention-notification fan-out (resolve mentioned usernames, - * drop self-mentions, respect the `mentionedInComment` preference, persist - * records, and send mention emails) plus the read-state operations behind the + * drop self-mentions, respect the `notifyMentionedInComment` preference, + * persist records, and send mention emails) plus single-recipient fan-out + * for `follow` / `like` / `comment` (mirror pattern: load prefs, skip + * self-action and opted-out recipients, persist record, send email via the + * matching `notify*` helper), plus the read-state operations behind the * notification endpoints (list, unread count, mark read, mark all read). */ import * as model from './model.ts'; -import { notifyMentioned } from '../../utils/notify/index.ts'; +import { + notifyMentioned, + notifyNewFollower, + notifyRecipeCommented, + notifyRecipeLiked, +} from '../../utils/notify/index.ts'; import { createLogger } from '../../utils/logger/index.ts'; const logger = createLogger('notification-service'); /** * Dependency-injection proxy for test stubbing (data access + email - * side-effect). Mirrors the `deps` idiom used by router modules + * side-effects). Mirrors the `deps` idiom used by router modules * (e.g. `coffee-variety/index.ts`), applied at the service layer so unit * tests can exercise the real service functions without a database or SMTP. + * F05 extends: `notifyNewFollower` / `notifyRecipeLiked` / `notifyRecipeCommented` + * joined for the three new fan-out creators. */ -export const deps = { model, notifyMentioned }; +export const deps = { + model, + notifyMentioned, + notifyNewFollower, + notifyRecipeLiked, + notifyRecipeCommented, +}; /** A notification row as returned by the model layer (actor username joined). */ type NotificationRow = NonNullable>>; @@ -101,7 +118,7 @@ export async function createMentionNotifications(params: { let created = 0; for (const target of targets) { if (target.id === mentionerUserId) continue; - if (target.prefs?.mentionedInComment === false) continue; + if (target.prefs?.notifyMentionedInComment === false) continue; try { await deps.model.create({ @@ -138,6 +155,266 @@ export async function createMentionNotifications(params: { } } +/** + * Create a follow notification (and follow email) when a user follows another. + * + * Flow per F05 D1/D2/D4: + * 1. Resolve the followed user + their preferences row via the model. + * 2. Drop self-follow (should never happen at the call-site, belt-and-braces). + * 3. The `notifyNewFollower` preference gates BOTH the DB record and the + * email: an opted-out target is skipped entirely (missing prefs row + * counts as enabled — the column defaults to true). + * 4. Insert a `follow` notification row with `actorId = followerId`, + * `referenceType = 'actor'`, `metadata = { followerUsername }`. + * 5. Send the follow email (`notifyNewFollower`). SOLE OWNER of the email — + * callers must NOT also call `notifyNewFollower`, or they bypass the + * preference gate above and double-send. + * + * Per-target failures are isolated: a failed insert or email is logged and + * skipped without aborting. Designed fire-and-forget from the follow service. + * + * @param params - `{ followerId, followerUsername, followingId }`. + */ +export async function createFollowNotification(params: { + followerId: string; + followerUsername: string; + followingId: string; +}): Promise { + const { followerId, followerUsername, followingId } = params; + logger.debug({ followerId, followingId }, 'createFollowNotification started'); + if (followerId === followingId) { + logger.debug( + { followerId, followingId, created: 0 }, + 'createFollowNotification completed (self-follow skipped)', + ); + return; + } + + try { + const target = await deps.model.findNotifyTarget(followingId); + if (!target) { + logger.debug( + { followingId, created: 0 }, + 'createFollowNotification completed (target not found)', + ); + return; + } + if (target.prefs?.notifyNewFollower === false) { + logger.debug( + { followingId, created: 0 }, + 'createFollowNotification completed (opted out)', + ); + return; + } + + let created = 0; + try { + await deps.model.create({ + userId: followingId, + type: 'follow', + actorId: followerId, + referenceId: null, + referenceType: 'actor', + metadata: JSON.stringify({ followerUsername }), + }); + created++; + } catch (err) { + logger.error({ err, followerId, followingId }, 'follow notification create failed'); + } + + try { + await deps.notifyNewFollower({ followingId, followerUsername }); + } catch (err) { + logger.error({ err, followerId, followingId }, 'follow email failed'); + } + + logger.debug({ followingId, created }, 'createFollowNotification completed'); + } catch (err) { + logger.error({ err, followerId, followingId }, 'createFollowNotification failed'); + throw err; + } +} + +/** + * Create a like notification (and recipe-liked email) when a user likes + * someone else's recipe. Skips self-likes. Single-recipient: targets the + * recipe author only. SOLE OWNER of the recipe-liked email — callers must + * NOT also call `notifyRecipeLiked`. Flow mirrors `createFollowNotification`. + * + * @param params - `{ likerId, likerUsername, recipeAuthorId, recipeId, recipeSlug, recipeTitle }`. + */ +export async function createLikeNotification(params: { + likerId: string; + likerUsername: string; + recipeAuthorId: string; + recipeId: string; + recipeSlug: string; + recipeTitle: string; +}): Promise { + const { likerId, likerUsername, recipeAuthorId, recipeId, recipeSlug, recipeTitle } = params; + logger.debug({ likerId, recipeId, recipeAuthorId }, 'createLikeNotification started'); + if (likerId === recipeAuthorId) { + logger.debug( + { likerId, recipeId, created: 0 }, + 'createLikeNotification completed (self-like skipped)', + ); + return; + } + + try { + const target = await deps.model.findNotifyTarget(recipeAuthorId); + if (!target) { + logger.debug( + { recipeAuthorId, created: 0 }, + 'createLikeNotification completed (target not found)', + ); + return; + } + if (target.prefs?.notifyRecipeLiked === false) { + logger.debug( + { recipeAuthorId, created: 0 }, + 'createLikeNotification completed (opted out)', + ); + return; + } + + let created = 0; + try { + await deps.model.create({ + userId: recipeAuthorId, + type: 'like', + actorId: likerId, + referenceId: recipeId, + referenceType: 'recipe', + metadata: JSON.stringify({ recipeSlug, recipeTitle }), + }); + created++; + } catch (err) { + logger.error( + { err, likerId, recipeId, recipeAuthorId }, + 'like notification create failed', + ); + } + + try { + await deps.notifyRecipeLiked({ + recipeAuthorId, + likerUsername, + recipeTitle, + recipeSlug, + }); + } catch (err) { + logger.error({ err, likerId, recipeId }, 'like email failed'); + } + + logger.debug({ recipeAuthorId, created }, 'createLikeNotification completed'); + } catch (err) { + logger.error( + { err, likerId, recipeId, recipeAuthorId }, + 'createLikeNotification failed', + ); + throw err; + } +} + +/** + * Create a comment-on-recipe notification (and recipe-commented email) for + * the recipe author when someone else comments on their recipe. Skips + * self-comments. SOLE OWNER of the recipe-commented email — callers must + * NOT also call `notifyRecipeCommented`. This path is DISTINCT from + * `createMentionNotifications` (F04), which targets each `@username` in the + * comment body. The same comment can trigger BOTH fan-outs: this one targets + * the recipe author ("X commented on your recipe"); the mention path targets + * each mentioned user ("X mentioned you"). See design D6. + * + * @param params - `{ commenterId, commenterUsername, recipeAuthorId, recipeId, recipeSlug, recipeTitle, commentId }`. + */ +export async function createCommentNotification(params: { + commenterId: string; + commenterUsername: string; + recipeAuthorId: string; + recipeId: string; + recipeSlug: string; + recipeTitle: string; + commentId: string; +}): Promise { + const { + commenterId, + commenterUsername, + recipeAuthorId, + recipeId, + recipeSlug, + recipeTitle, + commentId, + } = params; + logger.debug( + { commenterId, recipeId, recipeAuthorId }, + 'createCommentNotification started', + ); + if (commenterId === recipeAuthorId) { + logger.debug( + { commenterId, recipeId, created: 0 }, + 'createCommentNotification completed (self-comment skipped)', + ); + return; + } + + try { + const target = await deps.model.findNotifyTarget(recipeAuthorId); + if (!target) { + logger.debug( + { recipeAuthorId, created: 0 }, + 'createCommentNotification completed (target not found)', + ); + return; + } + if (target.prefs?.notifyRecipeCommented === false) { + logger.debug( + { recipeAuthorId, created: 0 }, + 'createCommentNotification completed (opted out)', + ); + return; + } + + let created = 0; + try { + await deps.model.create({ + userId: recipeAuthorId, + type: 'comment', + actorId: commenterId, + referenceId: commentId, + referenceType: 'comment', + metadata: JSON.stringify({ recipeSlug, recipeTitle }), + }); + created++; + } catch (err) { + logger.error( + { err, commentId, recipeId, recipeAuthorId }, + 'comment notification create failed', + ); + } + + try { + await deps.notifyRecipeCommented({ + recipeAuthorId, + commenterUsername, + recipeTitle, + recipeSlug, + }); + } catch (err) { + logger.error({ err, commentId, recipeId }, 'comment email failed'); + } + + logger.debug({ recipeAuthorId, created }, 'createCommentNotification completed'); + } catch (err) { + logger.error( + { err, commenterId, recipeId, recipeAuthorId }, + 'createCommentNotification failed', + ); + throw err; + } +} + /** * List a user's notifications (paginated, newest first). * diff --git a/apps/api/src/modules/preference/index.ts b/apps/api/src/modules/preference/index.ts index b5fb4f0..e999a58 100644 --- a/apps/api/src/modules/preference/index.ts +++ b/apps/api/src/modules/preference/index.ts @@ -1,7 +1,7 @@ import { Hono } from 'hono'; import { zValidator } from '@hono/zod-validator'; import { describeRoute, resolver } from 'hono-openapi'; -import { UserPreferencesSchema } from '@brewform/shared/schemas'; +import { UserPreferencesPatchSchema } from '@brewform/shared/schemas'; import { ErrorEnvelopeSchema, successEnvelope, @@ -64,7 +64,7 @@ preference.patch( summary: 'Update preferences', description: "Updates the authenticated user's preferences.", security: [{ bearerAuth: [] }], - requestBody: jsonRequestBody(UserPreferencesSchema), + requestBody: jsonRequestBody(UserPreferencesPatchSchema), responses: { 200: { description: 'Updated user preferences', @@ -79,11 +79,17 @@ preference.patch( }, }), authMiddleware, - zValidator('json', UserPreferencesSchema), + zValidator('json', UserPreferencesPatchSchema), async (c) => { const userId = c.get('userId') as string; const body = c.req.valid('json'); + // F05: flat `notify*` field identity-copy from the PATCH-only schema. + // `UserPreferencesPatchSchema` makes every field optional with NO defaults, + // so omitted fields parse to `undefined` and the `!== undefined` guards + // below skip them — omitted preferences remain unchanged. The earlier + // `UserPreferencesSchema` (with `.default(true)` on booleans) would fill + // omitted fields to `true` and silently overwrite stored values. const flatData: PreferenceUpdate = {}; if (body.unitSystem !== undefined) flatData.unitSystem = body.unitSystem; if (body.temperatureUnit !== undefined) flatData.temperatureUnit = body.temperatureUnit; @@ -91,12 +97,16 @@ preference.patch( if (body.locale !== undefined) flatData.locale = body.locale; if (body.timezone !== undefined) flatData.timezone = body.timezone; if (body.dateFormat !== undefined) flatData.dateFormat = body.dateFormat; - if (body.emailNotifications !== undefined) { - flatData.newFollower = body.emailNotifications.newFollower; - flatData.recipeLiked = body.emailNotifications.recipeLiked; - flatData.recipeCommented = body.emailNotifications.recipeCommented; - flatData.followedUserPosted = body.emailNotifications.followedUserPosted; - flatData.mentionedInComment = body.emailNotifications.mentionedInComment; + if (body.notifyNewFollower !== undefined) flatData.notifyNewFollower = body.notifyNewFollower; + if (body.notifyRecipeLiked !== undefined) flatData.notifyRecipeLiked = body.notifyRecipeLiked; + if (body.notifyRecipeCommented !== undefined) { + flatData.notifyRecipeCommented = body.notifyRecipeCommented; + } + if (body.notifyFollowedUserPosted !== undefined) { + flatData.notifyFollowedUserPosted = body.notifyFollowedUserPosted; + } + if (body.notifyMentionedInComment !== undefined) { + flatData.notifyMentionedInComment = body.notifyMentionedInComment; } const prefs = await service.updatePreferences(userId, flatData); diff --git a/apps/api/src/modules/preference/model.test.ts b/apps/api/src/modules/preference/model.test.ts index af4469c..aa25644 100644 --- a/apps/api/src/modules/preference/model.test.ts +++ b/apps/api/src/modules/preference/model.test.ts @@ -123,4 +123,20 @@ describe('upsert', { sanitizeOps: false, sanitizeResources: false }, () => { .where(eq(userPreferences.userId, userId)); expect(row.userId).toBe(userId); }); + + it('toggles notifyMentionedInComment via upsert and round-trips through findByUserId', async () => { + const inserted = await model.upsert(userId, { notifyMentionedInComment: false }); + expect(inserted.notifyMentionedInComment).toBe(false); + + const refetched = await model.findByUserId(userId); + expect(refetched).not.toBeNull(); + expect(refetched!.notifyMentionedInComment).toBe(false); + + const updated = await model.upsert(userId, { notifyMentionedInComment: true }); + expect(updated.notifyMentionedInComment).toBe(true); + + const refetchedAgain = await model.findByUserId(userId); + expect(refetchedAgain).not.toBeNull(); + expect(refetchedAgain!.notifyMentionedInComment).toBe(true); + }); }); diff --git a/apps/api/src/modules/recipe/service.test.ts b/apps/api/src/modules/recipe/service.test.ts index 8601393..866cdc5 100644 --- a/apps/api/src/modules/recipe/service.test.ts +++ b/apps/api/src/modules/recipe/service.test.ts @@ -4,6 +4,7 @@ import { expect } from 'jsr:@std/expect'; import fc from 'npm:fast-check'; import { db } from '@brewform/db'; import { + notifications, recipes, recipeVersions, userBadges, @@ -11,7 +12,7 @@ import { userRecipeLikes, users, } from '@brewform/db/schema'; -import { eq, inArray } from 'drizzle-orm'; +import { and, eq, inArray } from 'drizzle-orm'; import * as model from './model.ts'; import * as service from './service.ts'; import { canViewRecipe } from './service.ts'; @@ -630,6 +631,10 @@ describe( }); afterEach(async () => { + // F05 fan-out: toggleLike may leave a `like` notification row for the author. + // Clean these between tests so the polling assertions below stay isolated + // (userId cascades on user delete in afterAll; this just prevents leakage). + await db.delete(notifications).where(inArray(notifications.userId, [author.id, other.id])); if (createdRecipes.length) { await db.delete(userRecipeLikes).where(inArray(userRecipeLikes.recipeId, createdRecipes)); await db.delete(userRecipeFavourites).where( @@ -703,6 +708,70 @@ describe( 'RECIPE_NOT_FOUND', ); }); + + // F05 fan-out polling helper: toggleLike fires createLikeNotification via + // a fire-and-forget IIFE. Returns true if the `like` notification row + // appears within ~1.5s. + async function likeNotificationAppeared( + authorId: string, + likerId: string, + recipeId: string, + timeoutMs = 1500, + ): Promise { + const start = Date.now(); + while (Date.now() - start < timeoutMs) { + const rows = await db.select().from(notifications).where(and( + eq(notifications.userId, authorId), + eq(notifications.actorId, likerId), + eq(notifications.type, 'like'), + eq(notifications.referenceId, recipeId), + )); + if (rows.length > 0) return true; + await new Promise((resolve) => setTimeout(resolve, 10)); + } + return false; + } + + it('createLikeNotification invoked when like toggles ON and liker != author', async () => { + const recipe = await makeRecipe(author.id, 'Like Notify Fanout'); + await service.toggleLike(other.id, recipe.id); + expect(await likeNotificationAppeared(author.id, other.id, recipe.id)).toBe(true); + }); + + it('createLikeNotification NOT invoked when like toggles OFF', async () => { + const recipe = await makeRecipe(author.id, 'Like Off No Notify'); + // First toggle ON creates a like + notification; drain the fan-out. + await service.toggleLike(other.id, recipe.id); + await likeNotificationAppeared(author.id, other.id, recipe.id); + const before = await db.select().from(notifications).where(and( + eq(notifications.userId, author.id), + eq(notifications.actorId, other.id), + eq(notifications.type, 'like'), + eq(notifications.referenceId, recipe.id), + )); + // Toggle OFF — `result.liked` is false, the notification IIFE is not spawned. + await service.toggleLike(other.id, recipe.id); + await new Promise((resolve) => setTimeout(resolve, 50)); + const after = await db.select().from(notifications).where(and( + eq(notifications.userId, author.id), + eq(notifications.actorId, other.id), + eq(notifications.type, 'like'), + eq(notifications.referenceId, recipe.id), + )); + expect(after.length).toBe(before.length); + }); + + it('createLikeNotification NOT invoked when liker === recipe author (self-like)', async () => { + const recipe = await makeRecipe(author.id, 'Self Like No Notify'); + await service.toggleLike(author.id, recipe.id); + await new Promise((resolve) => setTimeout(resolve, 50)); + const rows = await db.select().from(notifications).where(and( + eq(notifications.userId, author.id), + eq(notifications.actorId, author.id), + eq(notifications.type, 'like'), + )); + expect(rows.length).toBe(0); + }); }); describe('toggleFavourite', () => { diff --git a/apps/api/src/modules/recipe/service.ts b/apps/api/src/modules/recipe/service.ts index fb6fef8..dcd8322 100644 --- a/apps/api/src/modules/recipe/service.ts +++ b/apps/api/src/modules/recipe/service.ts @@ -21,7 +21,8 @@ import { import type { RecipeMerge } from '@brewform/shared/schemas'; import { createLogger } from '../../utils/logger/index.ts'; import { decodeCursor } from '@brewform/shared/utils'; -import { notifyFollowersOfNewRecipe, notifyRecipeLiked } from '../../utils/notify/index.ts'; +import { notifyFollowersOfNewRecipe } from '../../utils/notify/index.ts'; +import { createLikeNotification } from '../notification/service.ts'; import { evaluateBadges } from '../badge/service.ts'; import type { BrewMethod } from '@brewform/shared/types'; @@ -562,7 +563,9 @@ export async function listStarredRecipes( * Toggle a user's like on a recipe. * * Returns the new liked state. When liking (not un-liking) a recipe by a - * different author, fires an asynchronous `notifyRecipeLiked` side effect. + * different author, fires `createLikeNotification` (F05), which owns BOTH + * the in-app `like` notification record AND the recipe-liked email (gated + * on `notifyRecipeLiked` prefs — single flag gates both channels). */ export async function toggleLike(userId: string, recipeId: string) { logger.debug({ userId, recipeId }, 'toggleLike started'); @@ -574,13 +577,19 @@ export async function toggleLike(userId: string, recipeId: string) { (async () => { const liker = await model.getUserById(userId); if (!liker?.username) return; - await notifyRecipeLiked({ - recipeAuthorId: recipe.authorId, + // F05: createLikeNotification owns BOTH the in-app `like` record AND + // the recipe-liked email (gated on `notifyRecipeLiked` prefs — single + // flag gates both). No direct notifyRecipeLiked call here — that would + // double-send and bypass the preference gate. + await createLikeNotification({ + likerId: userId, likerUsername: liker.username, - recipeTitle: recipe.title, + recipeAuthorId: recipe.authorId, + recipeId, recipeSlug: recipe.slug, + recipeTitle: recipe.title, }); - })().catch((err) => logger.error({ err }, 'notifyRecipeLiked failed')); + })().catch((err) => logger.error({ err, recipeId }, 'createLikeNotification failed')); } logger.debug({ userId, recipeId }, 'toggleLike completed'); diff --git a/apps/api/src/modules/user/model.test.ts b/apps/api/src/modules/user/model.test.ts index 421cf0e..56814ca 100644 --- a/apps/api/src/modules/user/model.test.ts +++ b/apps/api/src/modules/user/model.test.ts @@ -27,10 +27,11 @@ describe('User Model', { sanitizeOps: false, sanitizeResources: false }, () => { locale: 'tr', timezone: 'Europe/Istanbul', dateFormat: 'DD_MM_YYYY', - newFollower: false, - recipeLiked: false, - recipeCommented: false, - followedUserPosted: false, + notifyNewFollower: false, + notifyRecipeLiked: false, + notifyRecipeCommented: false, + notifyFollowedUserPosted: false, + notifyMentionedInComment: false, }); }); @@ -53,10 +54,11 @@ describe('User Model', { sanitizeOps: false, sanitizeResources: false }, () => { expect(result!.preferences!.locale).toBe('tr'); expect(result!.preferences!.timezone).toBe('Europe/Istanbul'); expect(result!.preferences!.dateFormat).toBe('DD_MM_YYYY'); - expect(result!.preferences!.emailNotifications.newFollower).toBe(false); - expect(result!.preferences!.emailNotifications.recipeLiked).toBe(false); - expect(result!.preferences!.emailNotifications.recipeCommented).toBe(false); - expect(result!.preferences!.emailNotifications.followedUserPosted).toBe(false); + expect(result!.preferences!.notifyNewFollower).toBe(false); + expect(result!.preferences!.notifyRecipeLiked).toBe(false); + expect(result!.preferences!.notifyRecipeCommented).toBe(false); + expect(result!.preferences!.notifyFollowedUserPosted).toBe(false); + expect(result!.preferences!.notifyMentionedInComment).toBe(false); }); }); diff --git a/apps/api/src/modules/user/model.ts b/apps/api/src/modules/user/model.ts index a4f0f8f..fe379b9 100644 --- a/apps/api/src/modules/user/model.ts +++ b/apps/api/src/modules/user/model.ts @@ -33,12 +33,14 @@ export async function findById(id: string) { locale: prefsRow.locale, timezone: prefsRow.timezone, dateFormat: prefsRow.dateFormat, - emailNotifications: { - newFollower: prefsRow.newFollower, - recipeLiked: prefsRow.recipeLiked, - recipeCommented: prefsRow.recipeCommented, - followedUserPosted: prefsRow.followedUserPosted, - }, + // F05: flat `notify*` fields — `/me` and `/preferences` now share the + // same shape. The F04 latent `mentionedInComment` omission (the 4-flag + // nest here forgot it) is structurally fixed by the flatten. + notifyNewFollower: prefsRow.notifyNewFollower, + notifyRecipeLiked: prefsRow.notifyRecipeLiked, + notifyRecipeCommented: prefsRow.notifyRecipeCommented, + notifyFollowedUserPosted: prefsRow.notifyFollowedUserPosted, + notifyMentionedInComment: prefsRow.notifyMentionedInComment, } : null, }; diff --git a/apps/api/src/utils/notify/index.ts b/apps/api/src/utils/notify/index.ts index 3d0778c..c1038f9 100644 --- a/apps/api/src/utils/notify/index.ts +++ b/apps/api/src/utils/notify/index.ts @@ -128,7 +128,7 @@ export async function notifyNewFollower(params: { }): Promise { const recipient = await loadRecipient(params.followingId); if (!recipient) return; - if (recipient.prefs.newFollower === false) return; + if (recipient.prefs.notifyNewFollower === false) return; const html = renderTemplate(newFollowerTemplate, { app_name: 'BrewForm', @@ -151,7 +151,7 @@ export async function notifyRecipeLiked(params: { }): Promise { const recipient = await loadRecipient(params.recipeAuthorId); if (!recipient) return; - if (recipient.prefs.recipeLiked === false) return; + if (recipient.prefs.notifyRecipeLiked === false) return; const html = renderTemplate(recipeLikedTemplate, { app_name: 'BrewForm', @@ -175,7 +175,7 @@ export async function notifyRecipeCommented(params: { }): Promise { const recipient = await loadRecipient(params.recipeAuthorId); if (!recipient) return; - if (recipient.prefs.recipeCommented === false) return; + if (recipient.prefs.notifyRecipeCommented === false) return; const html = renderTemplate(recipeCommentedTemplate, { app_name: 'BrewForm', @@ -201,7 +201,7 @@ export async function notifyMentioned(params: { logger.debug({ mentionedUserId: params.mentionedUserId }, 'notifyMentioned started'); const recipient = await loadRecipient(params.mentionedUserId); if (!recipient) return; - if (recipient.prefs.mentionedInComment === false) return; + if (recipient.prefs.notifyMentionedInComment === false) return; const html = renderTemplate(mentionedInCommentTemplate, { app_name: 'BrewForm', @@ -243,7 +243,7 @@ export async function notifyFollowersOfNewRecipe(params: { username: u.user.username, prefs: u.user_preferences ?? {}, })) - .filter((r) => r.prefs.followedUserPosted !== false); + .filter((r) => r.prefs.notifyFollowedUserPosted !== false); if (recipients.length === 0) return; diff --git a/apps/web/src/components/layout/NotificationItem.test.tsx b/apps/web/src/components/layout/NotificationItem.test.tsx index 13ed3fb..17962db 100644 --- a/apps/web/src/components/layout/NotificationItem.test.tsx +++ b/apps/web/src/components/layout/NotificationItem.test.tsx @@ -52,6 +52,9 @@ const mockNotify = vi.mocked(notifyNotificationsChanged); const templates: Record = { 'notifications.mention': '{username} mentioned you in a comment on {recipeTitle}', 'notifications.mentionGeneric': '{username} mentioned you in a comment', + 'notifications.follow': '{actorUsername} started following you', + 'notifications.like': '{actorUsername} liked your recipe {recipeTitle}', + 'notifications.comment': '{actorUsername} commented on {recipeTitle}', 'notifications.markRead': 'Mark as read', }; @@ -132,4 +135,44 @@ describe('NotificationItem', () => { expect(mockNotify).toHaveBeenCalled(); }); }); + + it('renders follow text with actorUsername interpolated and links to the actor profile', () => { + render( + , + ); + expect(screen.getByText('alice started following you')).toBeInTheDocument(); + const link = screen.getByRole('link'); + expect(link).toHaveAttribute('href', '/u/alice'); + }); + + it('renders like text with actorUsername and recipeTitle and links to the recipe', () => { + render(); + expect(screen.getByText('alice liked your recipe Pour Over')).toBeInTheDocument(); + const link = screen.getByRole('link'); + expect(link).toHaveAttribute('href', '/recipes/pour-over'); + }); + + it('renders comment text and links to the recipe with #commentId anchor', () => { + render( + , + ); + expect(screen.getByText('alice commented on Pour Over')).toBeInTheDocument(); + const link = screen.getByRole('link'); + expect(link).toHaveAttribute('href', '/recipes/pour-over#c-123'); + }); + + it('falls back to mentionGeneric for an unknown notification type', () => { + // ponytail: `as never` deliberately crosses the type union for forward-compat testing. + render( + , + ); + expect(screen.getByText('alice mentioned you in a comment')).toBeInTheDocument(); + }); }); diff --git a/apps/web/src/components/layout/NotificationItem.tsx b/apps/web/src/components/layout/NotificationItem.tsx index 51f38bc..48f1afb 100644 --- a/apps/web/src/components/layout/NotificationItem.tsx +++ b/apps/web/src/components/layout/NotificationItem.tsx @@ -8,10 +8,16 @@ import { formatDate } from '../../utils/format.ts'; const log = createLogger('NotificationItem'); -/** Mention-notification metadata payload (`{recipeSlug, recipeTitle}` JSON string). */ -interface MentionMetadata { +/** + * Notification metadata payload (JSON string). Each notification type stores + * a different shape: mention/like/comment carry `{ recipeSlug, recipeTitle }`; + * follow carries `{ followerUsername }` (though the actor's username is also + * available directly on `notification.actorUsername`). + */ +interface NotificationMetadata { recipeSlug?: string; recipeTitle?: string; + followerUsername?: string; } /** @@ -19,15 +25,16 @@ interface MentionMetadata { * Malformed or non-object payloads yield `{}` so the item falls back * to the generic mention text instead of crashing. */ -function parseMetadata(metadata: string | null): MentionMetadata { +function parseMetadata(metadata: string | null): NotificationMetadata { if (!metadata) return {}; try { const parsed: unknown = JSON.parse(metadata); if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) { - const { recipeSlug, recipeTitle } = parsed as Record; + const { recipeSlug, recipeTitle, followerUsername } = parsed as Record; return { recipeSlug: typeof recipeSlug === 'string' ? recipeSlug : undefined, recipeTitle: typeof recipeTitle === 'string' ? recipeTitle : undefined, + followerUsername: typeof followerUsername === 'string' ? followerUsername : undefined, }; } } catch { @@ -58,11 +65,45 @@ export function NotificationItem({ notification, onRead, onNavigate }: Notificat const username = notification.actorUsername ?? ''; const isUnread = !notification.readAt; - const text = notification.type === 'mention' && meta.recipeTitle - ? t('notifications.mention') - .replace('{username}', username) - .replace('{recipeTitle}', meta.recipeTitle) - : t('notifications.mentionGeneric').replace('{username}', username); + // F05: per-type text pattern. `follow` / `like` / `comment` use the + // matching i18n key; mention and unknown types fall back to mentionGeneric. + const text = (() => { + switch (notification.type) { + case 'follow': + return t('notifications.follow').replace('{actorUsername}', username); + case 'like': + return meta.recipeTitle + ? t('notifications.like') + .replace('{actorUsername}', username) + .replace('{recipeTitle}', meta.recipeTitle) + : t('notifications.mentionGeneric').replace('{username}', username); + case 'comment': + return meta.recipeTitle + ? t('notifications.comment') + .replace('{actorUsername}', username) + .replace('{recipeTitle}', meta.recipeTitle) + : t('notifications.mentionGeneric').replace('{username}', username); + case 'mention': + return meta.recipeTitle + ? t('notifications.mention') + .replace('{username}', username) + .replace('{recipeTitle}', meta.recipeTitle) + : t('notifications.mentionGeneric').replace('{username}', username); + default: + return t('notifications.mentionGeneric').replace('{username}', username); + } + })(); + + // F05: per-type link target. Follow links to the actor's profile; + // comment links to the recipe with `#commentId` anchor; like / mention + // link to the recipe; missing slug renders a ` + + + {items.length === 0 ? : ( diff --git a/apps/web/src/pages/settings/SettingsPage.test.tsx b/apps/web/src/pages/settings/SettingsPage.test.tsx index 45e1726..30bee18 100644 --- a/apps/web/src/pages/settings/SettingsPage.test.tsx +++ b/apps/web/src/pages/settings/SettingsPage.test.tsx @@ -75,11 +75,11 @@ const mockPreferences = { locale: 'en', timezone: 'UTC', dateFormat: 'YYYY_MM_DD', - newFollower: true, - recipeLiked: true, - recipeCommented: false, - followedUserPosted: true, - mentionedInComment: true, + notifyNewFollower: true, + notifyRecipeLiked: true, + notifyRecipeCommented: false, + notifyFollowedUserPosted: true, + notifyMentionedInComment: true, createdAt: '2024-01-01T00:00:00Z', updatedAt: '2024-01-01T00:00:00Z', }; @@ -102,7 +102,7 @@ const enT = (key: string) => { 'settings.unitSystem.imperial': 'Imperial (oz, fl oz, °F)', 'settings.temperatureUnit.celsius': 'Celsius', 'settings.temperatureUnit.fahrenheit': 'Fahrenheit', - 'settings.emailNotifications': 'Email Notifications', + 'settings.notifications': 'Notifications', 'settings.notif.newFollower': 'New follower', 'settings.notif.recipeLiked': 'Recipe liked', 'settings.notif.recipeCommented': 'Recipe commented', @@ -362,7 +362,7 @@ describe('SettingsPage', () => { expect(mockApi.patch).toHaveBeenCalledWith( '/preferences', expect.objectContaining({ - emailNotifications: expect.objectContaining({ mentionedInComment: false }), + notifyMentionedInComment: false, }), ); }); diff --git a/apps/web/src/pages/settings/SettingsPage.tsx b/apps/web/src/pages/settings/SettingsPage.tsx index b3ab2d3..9d3f4f9 100644 --- a/apps/web/src/pages/settings/SettingsPage.tsx +++ b/apps/web/src/pages/settings/SettingsPage.tsx @@ -12,7 +12,11 @@ import { api } from '../../api/client.ts'; import { createLogger } from '@/utils/logger.ts'; import type { UserPreferences, UserPreferencesOutput } from '@brewform/shared/schemas'; -/** Convert the flat GET response to the nested request shape used by the form. */ +/** + * Convert the flat GET response to the form state. F05 flatten: the request + * and response share the same flat `notify*` shape (no `emailNotifications` + * re-nest step anymore). + */ function toUserPreferences(out: UserPreferencesOutput): UserPreferences { return { unitSystem: out.unitSystem as UserPreferences['unitSystem'], @@ -21,13 +25,11 @@ function toUserPreferences(out: UserPreferencesOutput): UserPreferences { locale: out.locale, timezone: out.timezone, dateFormat: out.dateFormat as UserPreferences['dateFormat'], - emailNotifications: { - newFollower: out.newFollower, - recipeLiked: out.recipeLiked, - recipeCommented: out.recipeCommented, - followedUserPosted: out.followedUserPosted, - mentionedInComment: out.mentionedInComment, - }, + notifyNewFollower: out.notifyNewFollower, + notifyRecipeLiked: out.notifyRecipeLiked, + notifyRecipeCommented: out.notifyRecipeCommented, + notifyFollowedUserPosted: out.notifyFollowedUserPosted, + notifyMentionedInComment: out.notifyMentionedInComment, }; } @@ -76,13 +78,18 @@ export function SettingsPage() { setMessage(''); setMessageType(null); try { + // F05: flat `notify*` flags sent directly (no `emailNotifications` nest). await api.patch('/preferences', { unitSystem: prefs.unitSystem, temperatureUnit: prefs.temperatureUnit, locale: prefs.locale, timezone: prefs.timezone, dateFormat: prefs.dateFormat, - emailNotifications: prefs.emailNotifications, + notifyNewFollower: prefs.notifyNewFollower, + notifyRecipeLiked: prefs.notifyRecipeLiked, + notifyRecipeCommented: prefs.notifyRecipeCommented, + notifyFollowedUserPosted: prefs.notifyFollowedUserPosted, + notifyMentionedInComment: prefs.notifyMentionedInComment, }); setMessage(t('settings.savedMsg')); setMessageType('success'); @@ -257,52 +264,32 @@ export function SettingsPage() { )} {prefs && ( -
+
- setPrefs({ - ...prefs, - emailNotifications: { ...prefs.emailNotifications, newFollower: v }, - })} + checked={prefs.notifyNewFollower} + onChange={(v) => setPrefs({ ...prefs, notifyNewFollower: v })} /> - setPrefs({ - ...prefs, - emailNotifications: { ...prefs.emailNotifications, recipeLiked: v }, - })} + checked={prefs.notifyRecipeLiked} + onChange={(v) => setPrefs({ ...prefs, notifyRecipeLiked: v })} /> - setPrefs({ - ...prefs, - emailNotifications: { ...prefs.emailNotifications, recipeCommented: v }, - })} + checked={prefs.notifyRecipeCommented} + onChange={(v) => setPrefs({ ...prefs, notifyRecipeCommented: v })} /> - setPrefs({ - ...prefs, - emailNotifications: { ...prefs.emailNotifications, followedUserPosted: v }, - })} + checked={prefs.notifyFollowedUserPosted} + onChange={(v) => setPrefs({ ...prefs, notifyFollowedUserPosted: v })} /> - setPrefs({ - ...prefs, - emailNotifications: { ...prefs.emailNotifications, mentionedInComment: v }, - })} + checked={prefs.notifyMentionedInComment} + onChange={(v) => setPrefs({ ...prefs, notifyMentionedInComment: v })} />