-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat(ee): notify team members of new conversation messages 311d #1794
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(ee): notify team members of new conversation messages 311d #1794
Conversation
Co-authored-by: marcftone <marcftone@gmail.com>
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes introduce a new notification system for conversations, targeting both individual viewers and team members (admins/managers). New tasks and API endpoints handle delayed, deduplicated notification scheduling and sending. Email templates and logic for team notifications are added, and imports/exports are updated for integration. Minor refactoring and cleanup are also performed. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant API (conversations-route)
participant Trigger System
participant Notification Task
participant Team Notification API
participant Email Sender
User->>API (conversations-route): Create conversation or add message
API (conversations-route)->>Trigger System: Cancel existing notification runs (by conversationId)
API (conversations-route)->>Trigger System: Schedule new notification task (5 min delay)
Trigger System->>Notification Task: Invoke after delay
Notification Task->>Team Notification API: POST send-conversation-team-member-notification
Team Notification API->>Email Sender: Send notification email to team admins/managers
Email Sender-->>Team Notification API: Email sent
Team Notification API-->>Notification Task: Success/failure response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (4)
ee/features/conversations/emails/lib/send-conversation-team-notification.ts (2)
24-36: Verify CC field handling and improve logging.The CC field receives a comma-separated string from
teamMemberEmails.slice(1).join(","). Based on thesendEmailfunction signature inlib/resend.ts(lines 11-94), theccparameter acceptsstring | string[], so this approach is valid.However, consider using the
logutility from@/lib/utilsinstead ofconsole.logfor consistency with the rest of the codebase.await sendEmail({ to: teamMemberEmails[0], // Send to first team member - cc: teamMemberEmails.slice(1).join(","), // Send to all other team members + cc: teamMemberEmails.slice(1), // Send to all other team members as array subject: `New visitor message in ${dataroomName}`, react: ConversationTeamNotification({ senderEmail, conversationTitle, dataroomName, url, }), test: process.env.NODE_ENV === "development", system: true, });
20-21: Replace console logging with proper logging utility.Use the
logutility from@/lib/utilsfor consistent logging across the codebase, as seen in other parts of the application.+ import { log } from "@/lib/utils"; if (!teamMemberEmails || teamMemberEmails.length === 0) { - console.log("No team member emails provided"); + await log({ + message: "No team member emails provided for conversation notification", + type: "info", + }); return; }} catch (e) { - console.error("Failed to send team member notification:", e); + await log({ + message: `Failed to send team member notification: ${e}`, + type: "error", + mention: true, + }); }Also applies to: 38-39
ee/features/conversations/lib/trigger/conversation-message-notification.ts (1)
247-269: Consider more specific error handling for different HTTP status codes.The current error handling is good, but you could provide more specific handling for different HTTP status codes to improve debugging.
if (!response.ok) { + const errorText = await response.text(); logger.error("Failed to send team member notifications", { dataroomId: payload.dataroomId, teamId: payload.teamId, - error: await response.text(), + status: response.status, + statusText: response.statusText, + error: errorText, }); + + // Log different status codes with appropriate severity + if (response.status >= 500) { + logger.error("Server error occurred", { status: response.status }); + } else if (response.status === 404) { + logger.warn("Resource not found", { status: response.status }); + } } else {ee/features/conversations/api/send-conversation-team-member-notification.ts (1)
111-122: Clarify sender lookup logic.The comment on Line 111 says "First try to find as a user" but the query is against the
viewertable. This seems intentional based on the context, but the comment could be clearer.- // First try to find as a user + // Find the sender in the viewer table const userSender = await prisma.viewer.findUnique({ where: { id: senderUserId }, select: { email: true }, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
ee/features/conversations/api/conversations-route.ts(5 hunks)ee/features/conversations/api/send-conversation-team-member-notification.ts(1 hunks)ee/features/conversations/components/conversation-view-sidebar.tsx(1 hunks)ee/features/conversations/emails/components/conversation-notification.tsx(1 hunks)ee/features/conversations/emails/components/conversation-team-notification.tsx(1 hunks)ee/features/conversations/emails/lib/send-conversation-team-notification.ts(1 hunks)ee/features/conversations/lib/trigger/conversation-message-notification.ts(1 hunks)lib/trigger/conversation-message-notification.ts(1 hunks)pages/api/jobs/send-conversation-team-member-notification.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/trigger/**/*.ts
📄 CodeRabbit Inference Engine (.cursor/rules/rule-trigger-typescript.mdc)
**/trigger/**/*.ts: You MUST use@trigger.dev/sdk/v3when implementing Trigger.dev tasks.
You MUST NEVER useclient.defineJobin Trigger.dev task files, as it is deprecated and will break the application.
You MUSTexportevery task, including subtasks, in Trigger.dev task files.
If you are able to generate an example payload for a task, do so.
When implementing a Trigger.dev task, always use thetaskfunction from@trigger.dev/sdk/v3and follow the correct pattern for task definition.
When implementing scheduled (cron) tasks, useschedules.taskfrom@trigger.dev/sdk/v3and follow the correct pattern for schedule definition.
When implementing schema-validated tasks, useschemaTaskfrom@trigger.dev/sdk/v3and provide a schema using Zod or another supported library.
When triggering tasks from backend code, use thetasks.trigger,tasks.batchTrigger, ortasks.triggerAndPollmethods from@trigger.dev/sdk/v3and use type-only imports for type safety.
When triggering a task from inside another task, use the correct methods (trigger,batchTrigger,triggerAndWait,batchTriggerAndWait) on the task instance as shown in the guide.
When using metadata in tasks, use themetadataAPI from@trigger.dev/sdk/v3only inside run functions or task lifecycle hooks.
When using idempotency, use theidempotencyKeysAPI from@trigger.dev/sdk/v3and provide anidempotencyKeywhen triggering tasks.
When logging inside tasks, use theloggerAPI from@trigger.dev/sdk/v3and provide a message and a key-value object.
Files:
lib/trigger/conversation-message-notification.tsee/features/conversations/lib/trigger/conversation-message-notification.ts
🧠 Learnings (11)
📚 Learning: applies to **/trigger/**/*.ts : you must `export` every task, including subtasks, in trigger.dev tas...
Learnt from: CR
PR: mfts/papermark#0
File: .cursor/rules/rule-trigger-typescript.mdc:0-0
Timestamp: 2025-07-19T07:46:44.421Z
Learning: Applies to **/trigger/**/*.ts : You MUST `export` every task, including subtasks, in Trigger.dev task files.
Applied to files:
lib/trigger/conversation-message-notification.tsee/features/conversations/lib/trigger/conversation-message-notification.ts
📚 Learning: applies to **/trigger/**/*.ts : when triggering tasks from backend code, use the `tasks.trigger`, `t...
Learnt from: CR
PR: mfts/papermark#0
File: .cursor/rules/rule-trigger-typescript.mdc:0-0
Timestamp: 2025-07-19T07:46:44.421Z
Learning: Applies to **/trigger/**/*.ts : When triggering tasks from backend code, use the `tasks.trigger`, `tasks.batchTrigger`, or `tasks.triggerAndPoll` methods from `@trigger.dev/sdk/v3` and use type-only imports for type safety.
Applied to files:
lib/trigger/conversation-message-notification.tsee/features/conversations/api/conversations-route.tsee/features/conversations/lib/trigger/conversation-message-notification.ts
📚 Learning: applies to **/trigger/**/*.ts : when triggering a task from inside another task, use the correct met...
Learnt from: CR
PR: mfts/papermark#0
File: .cursor/rules/rule-trigger-typescript.mdc:0-0
Timestamp: 2025-07-19T07:46:44.421Z
Learning: Applies to **/trigger/**/*.ts : When triggering a task from inside another task, use the correct methods (`trigger`, `batchTrigger`, `triggerAndWait`, `batchTriggerAndWait`) on the task instance as shown in the guide.
Applied to files:
lib/trigger/conversation-message-notification.tsee/features/conversations/api/conversations-route.tsee/features/conversations/lib/trigger/conversation-message-notification.ts
📚 Learning: applies to **/trigger/**/*.ts : when implementing a trigger.dev task, always use the `task` function...
Learnt from: CR
PR: mfts/papermark#0
File: .cursor/rules/rule-trigger-typescript.mdc:0-0
Timestamp: 2025-07-19T07:46:44.421Z
Learning: Applies to **/trigger/**/*.ts : When implementing a Trigger.dev task, always use the `task` function from `@trigger.dev/sdk/v3` and follow the correct pattern for task definition.
Applied to files:
lib/trigger/conversation-message-notification.tsee/features/conversations/lib/trigger/conversation-message-notification.ts
📚 Learning: before generating any code for trigger.dev tasks, verify that you are importing from `@trigger.dev/s...
Learnt from: CR
PR: mfts/papermark#0
File: .cursor/rules/rule-trigger-typescript.mdc:0-0
Timestamp: 2025-07-19T07:46:44.421Z
Learning: Before generating any code for Trigger.dev tasks, verify that you are importing from `@trigger.dev/sdk/v3`, exporting every task, and not generating any deprecated code patterns.
Applied to files:
lib/trigger/conversation-message-notification.ts
📚 Learning: applies to **/trigger/**/*.ts : when implementing scheduled (cron) tasks, use `schedules.task` from ...
Learnt from: CR
PR: mfts/papermark#0
File: .cursor/rules/rule-trigger-typescript.mdc:0-0
Timestamp: 2025-07-19T07:46:44.421Z
Learning: Applies to **/trigger/**/*.ts : When implementing scheduled (cron) tasks, use `schedules.task` from `@trigger.dev/sdk/v3` and follow the correct pattern for schedule definition.
Applied to files:
lib/trigger/conversation-message-notification.tsee/features/conversations/api/conversations-route.tsee/features/conversations/lib/trigger/conversation-message-notification.ts
📚 Learning: applies to **/trigger/**/*.ts : you must use `@trigger.dev/sdk/v3` when implementing trigger.dev tas...
Learnt from: CR
PR: mfts/papermark#0
File: .cursor/rules/rule-trigger-typescript.mdc:0-0
Timestamp: 2025-07-19T07:46:44.421Z
Learning: Applies to **/trigger/**/*.ts : You MUST use `@trigger.dev/sdk/v3` when implementing Trigger.dev tasks.
Applied to files:
lib/trigger/conversation-message-notification.tsee/features/conversations/lib/trigger/conversation-message-notification.ts
📚 Learning: applies to **/trigger/**/*.ts : if you are able to generate an example payload for a task, do so....
Learnt from: CR
PR: mfts/papermark#0
File: .cursor/rules/rule-trigger-typescript.mdc:0-0
Timestamp: 2025-07-19T07:46:44.421Z
Learning: Applies to **/trigger/**/*.ts : If you are able to generate an example payload for a task, do so.
Applied to files:
lib/trigger/conversation-message-notification.tsee/features/conversations/lib/trigger/conversation-message-notification.ts
📚 Learning: applies to **/trigger/**/*.ts : when implementing schema-validated tasks, use `schematask` from `@tr...
Learnt from: CR
PR: mfts/papermark#0
File: .cursor/rules/rule-trigger-typescript.mdc:0-0
Timestamp: 2025-07-19T07:46:44.421Z
Learning: Applies to **/trigger/**/*.ts : When implementing schema-validated tasks, use `schemaTask` from `@trigger.dev/sdk/v3` and provide a schema using Zod or another supported library.
Applied to files:
lib/trigger/conversation-message-notification.tsee/features/conversations/lib/trigger/conversation-message-notification.ts
📚 Learning: applies to **/trigger/**/*.ts : when using idempotency, use the `idempotencykeys` api from `@trigger...
Learnt from: CR
PR: mfts/papermark#0
File: .cursor/rules/rule-trigger-typescript.mdc:0-0
Timestamp: 2025-07-19T07:46:44.421Z
Learning: Applies to **/trigger/**/*.ts : When using idempotency, use the `idempotencyKeys` API from `@trigger.dev/sdk/v3` and provide an `idempotencyKey` when triggering tasks.
Applied to files:
lib/trigger/conversation-message-notification.tsee/features/conversations/api/conversations-route.ts
📚 Learning: applies to **/trigger/**/*.ts : when using metadata in tasks, use the `metadata` api from `@trigger....
Learnt from: CR
PR: mfts/papermark#0
File: .cursor/rules/rule-trigger-typescript.mdc:0-0
Timestamp: 2025-07-19T07:46:44.421Z
Learning: Applies to **/trigger/**/*.ts : When using metadata in tasks, use the `metadata` API from `@trigger.dev/sdk/v3` only inside run functions or task lifecycle hooks.
Applied to files:
ee/features/conversations/api/conversations-route.ts
🧬 Code Graph Analysis (2)
ee/features/conversations/emails/lib/send-conversation-team-notification.ts (2)
lib/resend.ts (1)
sendEmail(12-95)ee/features/conversations/emails/components/conversation-team-notification.tsx (1)
ConversationTeamNotification(16-80)
ee/features/conversations/api/send-conversation-team-member-notification.ts (2)
ee/features/conversations/emails/lib/send-conversation-team-notification.ts (1)
sendConversationTeamNotification(5-40)lib/utils.ts (1)
log(52-112)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (13)
ee/features/conversations/components/conversation-view-sidebar.tsx (2)
11-11: Import cleanup looks good
cnwas unused and is now removed, leaving only the still-usedfetcher. This reduces dead code with no functional impact.
16-16: Pruned unused sheet componentsEliminating
SheetHeaderandSheetTitlekeeps the import list minimal and avoids pulling unnecessary code into the bundle.ee/features/conversations/emails/components/conversation-notification.tsx (1)
65-65: LGTM!The simplification of the copyright footer to plain text is consistent with the new
ConversationTeamNotificationcomponent and follows common email template practices.pages/api/jobs/send-conversation-team-member-notification.ts (1)
1-2: LGTM!Clean re-export pattern that properly exposes the EE feature through the API route structure.
lib/trigger/conversation-message-notification.ts (1)
1-9: LGTM!The import and export of both notification tasks follows the required pattern for trigger files. Both tasks are properly exported as mandated by the coding guidelines.
ee/features/conversations/emails/components/conversation-team-notification.tsx (1)
66-74: Verify the absence of unsubscribe functionality.Unlike the
ConversationNotificationcomponent, this team notification doesn't include an unsubscribe link. Please confirm this is intentional, as team members might still want to manage their notification preferences.ee/features/conversations/api/conversations-route.ts (3)
124-154: Well-implemented notification deduplication logic!The implementation correctly:
- Queries for existing delayed/queued runs within a 5-minute window
- Cancels them to prevent duplicate notifications
- Uses comprehensive idempotency keys and tags
- Employs
waitUntilfor non-blocking execution
164-164: Good change making viewerId required.Making
viewerIdrequired ensures proper message attribution and prevents potential null reference errors.
180-210: Consistent notification scheduling implementation!The notification scheduling logic properly mirrors the conversation creation endpoint with the same deduplication strategy and delay mechanism.
ee/features/conversations/lib/trigger/conversation-message-notification.ts (2)
183-191: LGTM! Follows Trigger.dev v3 guidelines correctly.The task definition properly uses the
taskfunction from@trigger.dev/sdk/v3, includes appropriate retry configuration, and uses the logger API correctly throughout the implementation.
194-220: Well-structured database query with proper filtering.The query correctly filters for ADMIN/MANAGER roles and excludes blocked team members. The early return with logging when no team members are found is appropriate.
ee/features/conversations/api/send-conversation-team-member-notification.ts (2)
35-148: Well-structured API handler with comprehensive functionality.The handler properly validates team members, filters by notification preferences, fetches required data, and integrates well with the email notification system. The error handling and logging are appropriate.
149-156: Excellent error handling with detailed logging.The error handling uses the proper logging utility with appropriate metadata and mention flag for critical errors. The structured error response is also appropriate.
Summary by CodeRabbit
New Features
Bug Fixes
Style
Chores