Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions apps/api/src/modules/comment/service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -675,10 +675,12 @@ describe('deleteComment — edge cases', () => {
type ModelDeps = typeof deps.model;
type NotifyRecipeCommentedParams = Parameters<typeof deps.notifyRecipeCommented>[0];
type MentionNotificationParams = Parameters<typeof deps.createMentionNotifications>[0];
type CommentNotificationParams = Parameters<typeof deps.createCommentNotification>[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 = {
Expand All @@ -690,6 +692,7 @@ describe('createComment — notification side-effects (F04 mention flow)', () =>

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) {
Expand All @@ -710,17 +713,23 @@ describe('createComment — notification side-effects (F04 mention flow)', () =>
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 () => {
Expand Down Expand Up @@ -817,6 +826,60 @@ describe('createComment — notification side-effects (F04 mention flow)', () =>
expect(notifyCalls.length).toBe(1);
expect(notifyCalls[0].recipeAuthorId).toBe('author-1');
});

// 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');
});
});

// ---------------------------------------------------------------------------
Expand Down
14 changes: 13 additions & 1 deletion apps/api/src/modules/comment/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ 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');
Expand All @@ -28,6 +28,7 @@ export const deps = {
recipeModel,
notifyRecipeCommented,
createMentionNotifications,
createCommentNotification,
evaluateBadges,
};

Expand Down Expand Up @@ -158,6 +159,8 @@ export async function runCommentNotificationSideEffects(params: {
if (!commenter?.username) return;

// Recipe-commented email only when someone ELSE comments on the recipe.
// F05 fan-out colocates an in-app `comment` notification record
// (gated on `notifyRecipeCommented` prefs — single flag gates both).
if (recipe.authorId !== userId) {
try {
await deps.notifyRecipeCommented({
Expand All @@ -169,6 +172,15 @@ export async function runCommentNotificationSideEffects(params: {
} 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
Expand Down
119 changes: 118 additions & 1 deletion apps/api/src/modules/follow/service.test.ts
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand Down Expand Up @@ -38,3 +43,115 @@ 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, sends the follow email (suppressed under APP_ENV=test), and
// persists a `follow` notification record for the followed user (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<boolean> {
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);
});
},
);
10 changes: 10 additions & 0 deletions apps/api/src/modules/follow/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ 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');
Expand Down Expand Up @@ -38,6 +39,15 @@ export async function followUser(followerId: string, followingId: string) {
followingId,
followerUsername: follower.username,
});
// F05 fan-out: persist a `follow` notification record for the followed
// user (gated on `notifyNewFollower` prefs — single flag gates both).
createFollowNotification({
followerId,
followerUsername: follower.username,
followingId,
}).catch((err) =>
logger.error({ err, followerId, followingId }, 'createFollowNotification failed')
);
})().catch((err) => logger.error({ err }, 'notifyNewFollower failed'));

evaluateBadges(followerId).catch((err) => logger.error({ err }, 'evaluateBadges failed'));
Expand Down
76 changes: 74 additions & 2 deletions apps/api/src/modules/notification/model.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ describe('findMentionTargets', { sanitizeOps: false, sanitizeResources: false },
await db.insert(userPreferences).values({
id: prefsId,
userId: optedOutUserId,
mentionedInComment: false,
notifyMentionedInComment: false,
});
});

Expand All @@ -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 () => {
Expand Down Expand Up @@ -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();
});
});
Loading
Loading