Skip to content

Create backend for notifications#2957

Draft
ErlendSae wants to merge 2 commits into
mainfrom
mono-859-create-notification-backend
Draft

Create backend for notifications#2957
ErlendSae wants to merge 2 commits into
mainfrom
mono-859-create-notification-backend

Conversation

@ErlendSae
Copy link
Copy Markdown
Contributor

No description provided.

@linear
Copy link
Copy Markdown

linear Bot commented Feb 25, 2026

@ErlendSae ErlendSae changed the title created backend for notifications Create backend for notifications Feb 25, 2026
@ErlendSae ErlendSae force-pushed the mono-859-create-notification-backend branch from 3b82bf2 to cae2aaf Compare February 25, 2026 21:09
Comment thread apps/rpc/src/modules/notification/notification.ts
@ErlendSae ErlendSae force-pushed the mono-859-create-notification-backend branch 2 times, most recently from c61379d to d6e23e1 Compare March 4, 2026 17:48
@Terbau Terbau force-pushed the mono-859-create-notification-backend branch from d6e23e1 to 6c25f83 Compare March 18, 2026 18:35
@JacobKH123 JacobKH123 force-pushed the mono-859-create-notification-backend branch from 6c25f83 to 04e80f7 Compare March 18, 2026 19:44
@Terbau Terbau force-pushed the mono-859-create-notification-backend branch from 04e80f7 to d21d581 Compare March 29, 2026 18:13
@Terbau Terbau force-pushed the mono-859-create-notification-backend branch from d21d581 to 0cd8e9e Compare April 15, 2026 12:35
@ErlendSae ErlendSae force-pushed the mono-859-create-notification-backend branch from 0cd8e9e to ec90836 Compare April 22, 2026 06:14
@simponm simponm force-pushed the mono-859-create-notification-backend branch from ec90836 to 8907be3 Compare April 27, 2026 18:40
@brage-andreas brage-andreas force-pushed the mono-859-create-notification-backend branch 6 times, most recently from 0e902a4 to 2a3952e Compare May 14, 2026 17:24
Comment on lines +46 to +50
async create(handle, notificationData) {
const { recipientIds, ...data } = notificationData
const notification = await handle.notification.create({ data })
return notification
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critical bug: recipientIds are extracted from notificationData but never used to create the recipient records. When a notification is created, no recipients will be added to the database, making the notification invisible to users.

Fix:

async create(handle, notificationData) {
  const { recipientIds, ...data } = notificationData
  const notification = await handle.notification.create({ data })
  await this.addRecipients(handle, notification.id, recipientIds)
  return notification
}
Suggested change
async create(handle, notificationData) {
const { recipientIds, ...data } = notificationData
const notification = await handle.notification.create({ data })
return notification
},
async create(handle, notificationData) {
const { recipientIds, ...data } = notificationData
const notification = await handle.notification.create({ data })
await this.addRecipients(handle, notification.id, recipientIds)
return notification
},

Spotted by Graphite

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

Comment on lines +51 to +58
async update(handle, notificationId, notificationData) {
const { recipientIds, ...data } = notificationData
const notification = await handle.notification.update({
where: { id: notificationId },
data,
})
return notification
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: recipientIds are extracted from notificationData but ignored during update. If the API allows passing recipientIds in updates, they will be silently ignored, causing unexpected behavior.

Either remove recipientIds from the destructuring if updates shouldn't modify recipients, or implement the recipient update logic:

async update(handle, notificationId, notificationData) {
  const { recipientIds, ...data } = notificationData
  const notification = await handle.notification.update({
    where: { id: notificationId },
    data,
  })
  if (recipientIds !== undefined) {
    await this.addRecipients(handle, notification.id, recipientIds)
  }
  return notification
}
Suggested change
async update(handle, notificationId, notificationData) {
const { recipientIds, ...data } = notificationData
const notification = await handle.notification.update({
where: { id: notificationId },
data,
})
return notification
},
async update(handle, notificationId, notificationData) {
const { recipientIds, ...data } = notificationData
const notification = await handle.notification.update({
where: { id: notificationId },
data,
})
if (recipientIds !== undefined) {
await this.addRecipients(handle, notification.id, recipientIds)
}
return notification
},

Spotted by Graphite

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

@brage-andreas brage-andreas force-pushed the mono-859-create-notification-backend branch 4 times, most recently from 24a10f4 to 11bd0a5 Compare May 19, 2026 23:33
@brage-andreas brage-andreas force-pushed the mono-859-create-notification-backend branch from 11bd0a5 to b36a014 Compare May 21, 2026 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants