Skip to content

Commit 877d2cf

Browse files
feat(notifications): notify users of their own actions by default (#909)
* feat(notifications): notify users of their own actions by default Users requested notifications for their own actions as a personal record/reminder in their inbox. This changes the default behavior of createNotification to include the actor in the recipient list. Changes: - notifications.ts: Set includeActor default to true - issues.ts: Remove redundant includeActor: true flags (now default) - issues.test.ts: Remove includeActor from test expectations - Integration tests: Add explicit includeActor: false to tests that specifically test actor exclusion or recipient preference behavior - database-queries.test.ts: Fix invalid UUID that was using "some-other-user-id" instead of a valid UUID format Fixes #849 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test: add test for new includeActor default behavior Addresses Copilot review feedback: adds a test that verifies actors ARE notified by default when includeActor is not specified. This ensures the new default behavior is properly tested. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 8203051 commit 877d2cf

File tree

5 files changed

+54
-8
lines changed

5 files changed

+54
-8
lines changed

src/lib/notifications.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ export async function createNotification(
4444
resourceId,
4545
resourceType,
4646
actorId,
47-
includeActor,
47+
includeActor = true,
4848
issueTitle,
4949
machineName,
5050
formattedIssueId,

src/services/issues.test.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,6 @@ describe("Issue Service", () => {
104104
resourceId: issueId,
105105
resourceType: "issue",
106106
actorId,
107-
includeActor: true,
108107
issueTitle: "Test Issue",
109108
machineName: "Test Machine",
110109
formattedIssueId: "MM-01",
@@ -144,7 +143,6 @@ describe("Issue Service", () => {
144143
resourceId: "issue-new",
145144
resourceType: "issue",
146145
actorId: "user-1",
147-
includeActor: true,
148146
issueTitle: "New Issue",
149147
machineName: "Test Machine",
150148
formattedIssueId: "MM-01",

src/services/issues.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,6 @@ export async function createIssue({
196196
resourceId: issue.id,
197197
resourceType: "issue",
198198
...(reportedBy ? { actorId: reportedBy } : {}),
199-
includeActor: true,
200199
issueTitle: title,
201200
machineName: updatedMachine.name,
202201
formattedIssueId: formatIssueId(machineInitials, issueNumber),
@@ -282,7 +281,6 @@ export async function updateIssueStatus({
282281
resourceId: issueId,
283282
resourceType: "issue",
284283
actorId: userId,
285-
includeActor: true,
286284
issueTitle: currentIssue.title,
287285
machineName: currentIssue.machine.name,
288286
formattedIssueId: formatIssueId(
@@ -506,7 +504,6 @@ export async function assignIssue({
506504
resourceId: issueId,
507505
resourceType: "issue",
508506
actorId,
509-
includeActor: true,
510507
issueTitle: currentIssue.title,
511508
machineName: currentIssue.machine.name,
512509
formattedIssueId: formatIssueId(

src/test/integration/database-queries.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,8 @@ describe("Database Queries (PGlite)", () => {
277277
type: "new_issue",
278278
resourceId: issue.id,
279279
resourceType: "issue",
280-
actorId: "some-other-user-id", // Actor is someone else
280+
actorId: "00000000-0000-0000-0000-000000000002", // Actor is someone else
281+
includeActor: false, // Exclude actor to test global watch behavior
281282
issueTitle: "Test Issue",
282283
machineName: "Test Machine",
283284
issueContext: {

src/test/integration/notifications.test.ts

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, it, expect, vi, beforeEach } from "vitest";
2-
import { eq } from "drizzle-orm";
2+
import { eq, sql } from "drizzle-orm";
33
import { createNotification } from "~/lib/notifications";
44
import { sendEmail } from "~/lib/email/client";
55
import { getTestDb, setupTestDb } from "~/test/setup/pglite";
@@ -66,6 +66,7 @@ describe("createNotification (Integration)", () => {
6666
resourceId: issue.id,
6767
resourceType: "issue",
6868
actorId: actor.id,
69+
includeActor: false, // Explicitly test actor exclusion
6970
commentContent: "Test comment",
7071
},
7172
db
@@ -77,6 +78,53 @@ describe("createNotification (Integration)", () => {
7778
expect(sendEmail).not.toHaveBeenCalled();
7879
});
7980

81+
it("should notify the actor by default", async () => {
82+
const db = await getTestDb();
83+
84+
// Setup: Actor is also a watcher
85+
const [actor] = await db
86+
.insert(userProfiles)
87+
.values(createTestUser())
88+
.returning();
89+
const [machine] = await db
90+
.insert(machines)
91+
.values(createTestMachine({ initials: "DEF" }))
92+
.returning();
93+
const [issue] = await db
94+
.insert(issues)
95+
.values(createTestIssue(machine.initials, { issueNumber: 1 }))
96+
.returning();
97+
98+
await db.insert(issueWatchers).values({
99+
issueId: issue.id,
100+
userId: actor.id,
101+
});
102+
103+
// Mock auth.users email for actor
104+
await db.execute(
105+
sql`INSERT INTO auth.users (id, email) VALUES (${actor.id}, 'actor@test.com')`
106+
);
107+
108+
// Don't specify includeActor - should default to true
109+
await createNotification(
110+
{
111+
type: "new_comment",
112+
resourceId: issue.id,
113+
resourceType: "issue",
114+
actorId: actor.id,
115+
commentContent: "Test comment",
116+
},
117+
db
118+
);
119+
120+
// Verify actor receives notification (new default behavior)
121+
const result = await db.query.notifications.findMany({
122+
where: eq(notifications.userId, actor.id),
123+
});
124+
expect(result).toHaveLength(1);
125+
expect(result[0].type).toBe("new_comment");
126+
});
127+
80128
it("should respect main switches", async () => {
81129
const db = await getTestDb();
82130

@@ -127,6 +175,7 @@ describe("createNotification (Integration)", () => {
127175
resourceId: issue.id,
128176
resourceType: "issue",
129177
actorId: actor.id,
178+
includeActor: false, // Exclude actor to test recipient preferences
130179
commentContent: "Test comment",
131180
},
132181
db
@@ -186,6 +235,7 @@ describe("createNotification (Integration)", () => {
186235
resourceId: issue.id,
187236
resourceType: "issue",
188237
actorId: actor.id,
238+
includeActor: false, // Exclude actor to test recipient granular toggles
189239
commentContent: "Test comment",
190240
},
191241
db

0 commit comments

Comments
 (0)