feat(notifications): add notifications for @mentions and task assignm… #217
Conversation
…ents This adds a notification system for user mentions and task assignments: - Add MENTION and TASK_ASSIGNED notification types to schema - Create user_mentions table to track when users are @mentioned in pages - Add createMentionNotification and createTaskAssignedNotification helpers - Update page-mention-service to parse user mentions and create notifications - Add notification creation when tasks are assigned to users - Update NotificationDropdown with icons and navigation for new types - Include database migration for schema changes Users will now receive notifications in the notification bell when: - They are @mentioned in a page (using @[Name](userId:user) format) - A task is assigned to them in a task list
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughAdds mention and task-assigned notifications: DB migration and schema for user mentions; mention extraction/sync returns structured user/page IDs; notification creators for MENTION and TASK_ASSIGNED; code paths fire-and-forget notifications on new mentions and task assignee changes; UI shows icons and navigation for these notifications. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant API as Page Mutation API
participant MentionSvc as Mention Service
participant DB as Database
participant NotifSvc as Notification Service
participant Email as Email System
User->>API: PATCH page content (with mentions)
API->>MentionSvc: syncMentions(sourcePageId, content, tx, {mentionedByUserId})
MentionSvc->>MentionSvc: Extract pageIds & userIds (HTML/Markdown)
MentionSvc->>DB: Update page_mentions & user_mentions within tx
DB-->>MentionSvc: tx commit
MentionSvc->>NotifSvc: createMentionNotification for newlyMentionedUserIds (fire-and-forget)
NotifSvc->>DB: Insert MENTION notification
NotifSvc->>Email: Trigger email send (async)
API-->>User: HTTP response (no wait for notifications)
sequenceDiagram
actor User
participant API as Task PATCH Endpoint
participant DB as Database
participant NotifSvc as Notification Service
participant Email as Email System
User->>API: PATCH task (change assignee)
API->>DB: Update task record
DB-->>API: Task updated
API->>API: Detect assignee change (new non-null assignee)
API->>NotifSvc: createTaskAssignedNotification(targetUserId, taskId, ...) (fire-and-forget)
NotifSvc->>DB: Insert TASK_ASSIGNED notification
NotifSvc->>Email: Trigger email send (async)
API-->>User: HTTP response (no wait for notification)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🧰 Additional context used📓 Path-based instructions (4)**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.ts📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,js,jsx,json}📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/web/src/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧬 Code graph analysis (3)apps/web/src/services/api/page-mutation-service.ts (3)
apps/web/src/services/api/rollback-service.ts (2)
apps/web/src/services/api/page-mention-service.ts (1)
🔇 Additional comments (9)
✏️ Tip: You can disable this entire section by setting 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/web/src/services/api/page-mention-service.ts`:
- Around line 140-152: The setTimeout-based dispatch in syncUserMentions can
send notifications before the DB transaction commits; remove the setTimeout
block and instead return the toCreate array (and sourcePageId/mentionedByUserId
context as needed) from syncUserMentions so the caller can iterate over
newlyMentioned user IDs and call createMentionNotification for each only after
the transaction successfully commits; update callers to perform the notification
loop and error-logging (loggers.api.error) after commit to avoid race/rollback
issues.
🧹 Nitpick comments (1)
packages/db/src/schema/core.ts (1)
148-161: Consider using a unique constraint instead of a regular index forsourceUserKey.The
sourceUserKeyindex on(sourcePageId, targetUserId)appears to enforce uniqueness semantically (one mention record per user per page). If duplicate mentions for the same user on the same page should be prevented at the database level, consider usinguniqueIndexinstead ofindex.♻️ Optional: Use uniqueIndex for deduplication at DB level
}, (table) => { return { - sourceUserKey: index('user_mentions_source_page_id_target_user_id_key').on(table.sourcePageId, table.targetUserId), + sourceUserKey: uniqueIndex('user_mentions_source_page_id_target_user_id_key').on(table.sourcePageId, table.targetUserId), sourcePageIdx: index('user_mentions_source_page_id_idx').on(table.sourcePageId), targetUserIdx: index('user_mentions_target_user_id_idx').on(table.targetUserId), } });Note: This would require importing
uniqueIndexfromdrizzle-orm/pg-coreand regenerating the migration.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/web/src/app/api/pages/[pageId]/tasks/[taskId]/route.tsapps/web/src/components/notifications/NotificationDropdown.tsxapps/web/src/services/api/page-mention-service.tsapps/web/src/services/api/page-mutation-service.tsapps/web/src/services/api/rollback-service.tspackages/db/drizzle/0038_salty_phalanx.sqlpackages/db/drizzle/meta/0038_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema/core.tspackages/db/src/schema/notifications.tspackages/lib/package.jsonpackages/lib/src/notifications/notifications.tspackages/lib/src/notifications/types.tspackages/lib/src/services/notification-email-service.ts
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier
Files:
packages/db/drizzle/meta/_journal.jsonapps/web/src/services/api/rollback-service.tsapps/web/src/services/api/page-mutation-service.tspackages/lib/package.jsonpackages/db/src/schema/notifications.tsapps/web/src/components/notifications/NotificationDropdown.tsxapps/web/src/app/api/pages/[pageId]/tasks/[taskId]/route.tspackages/lib/src/notifications/types.tspackages/lib/src/services/notification-email-service.tspackages/db/src/schema/core.tspackages/lib/src/notifications/notifications.tsapps/web/src/services/api/page-mention-service.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Never useanytypes - always use proper TypeScript types
Use camelCase for variable and function names
Use UPPER_SNAKE_CASE for constants
Use PascalCase for type and enum names
Use kebab-case for filenames, except React hooks (camelCase withuseprefix), Zustand stores (camelCase withuseprefix), and React components (PascalCase)
Lint with Next/ESLint as configured inapps/web/eslint.config.mjs
Message content should always use the message parts structure with{ parts: [{ type: 'text', text: '...' }] }
Use centralized permission functions from@pagespace/lib/permissions(e.g.,getUserAccessLevel,canUserEditPage) instead of implementing permission logic locally
Always use Drizzle client from@pagespace/dbpackage for database access
Use ESM modules throughout the codebase
**/*.{ts,tsx}: Never useanytypes - always use proper TypeScript types
Write code that is explicit over implicit and self-documenting
Files:
apps/web/src/services/api/rollback-service.tsapps/web/src/services/api/page-mutation-service.tspackages/db/src/schema/notifications.tsapps/web/src/components/notifications/NotificationDropdown.tsxapps/web/src/app/api/pages/[pageId]/tasks/[taskId]/route.tspackages/lib/src/notifications/types.tspackages/lib/src/services/notification-email-service.tspackages/db/src/schema/core.tspackages/lib/src/notifications/notifications.tsapps/web/src/services/api/page-mention-service.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: React hook files should use camelCase matching the exported hook name (e.g.,useAuth.ts)
Zustand store files should use camelCase withuseprefix (e.g.,useAuthStore.ts)
Files:
apps/web/src/services/api/rollback-service.tsapps/web/src/services/api/page-mutation-service.tspackages/db/src/schema/notifications.tsapps/web/src/app/api/pages/[pageId]/tasks/[taskId]/route.tspackages/lib/src/notifications/types.tspackages/lib/src/services/notification-email-service.tspackages/db/src/schema/core.tspackages/lib/src/notifications/notifications.tsapps/web/src/services/api/page-mention-service.ts
apps/web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/src/**/*.{ts,tsx}: Use message parts structure for message content:{ parts: [{ type: 'text', text: '...' }] }
For database access, always use Drizzle client from@pagespace/db:import { db, pages } from '@pagespace/db';
Use centralized Drizzle ORM with PostgreSQL for all database operations - no direct SQL or other ORMs
Use Socket.IO for real-time collaboration features - imported from the realtime service at port 3001
Use Vercel AI SDK with async/await for all AI operations and streaming
Use Next.js 15 App Router and TypeScript for all routes and components
Files:
apps/web/src/services/api/rollback-service.tsapps/web/src/services/api/page-mutation-service.tsapps/web/src/components/notifications/NotificationDropdown.tsxapps/web/src/app/api/pages/[pageId]/tasks/[taskId]/route.tsapps/web/src/services/api/page-mention-service.ts
packages/db/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for database queries with PostgreSQL
Files:
packages/db/src/schema/notifications.tspackages/db/src/schema/core.ts
packages/db/src/schema/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Database schema changes must be made in
packages/db/src/schema/and thenpnpm db:generatemust be run to create migrations
Files:
packages/db/src/schema/notifications.tspackages/db/src/schema/core.ts
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/*.tsx: React component files should use PascalCase (e.g.,UserProfile.tsx)
Use@dnd-kitfor drag-and-drop functionality
Use Zustand for client state management
Use SWR for server state management and caching
Files:
apps/web/src/components/notifications/NotificationDropdown.tsx
**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Tailwind CSS and shadcn/ui components for styling and UI
Files:
apps/web/src/components/notifications/NotificationDropdown.tsx
apps/web/src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/src/components/**/*.{ts,tsx}: When document editing, register editing state withuseEditingStore.getState().startEditing()to prevent UI refreshes, and clean up in return statement
When AI is streaming, register streaming state withuseEditingStore.getState().startStreaming()to prevent UI refreshes, and clean up in return statement
For SWR data fetching with editing protection, useisPaused: () => hasLoadedRef.current && isEditingActive()to allow initial fetch and only pause after, withrevalidateOnFocus: false
Use Zustand for client-side state management as the primary state solution
Use SWR for server state and caching with proper configuration includingrevalidateOnFocus: falsefor editing protection
Use TipTap rich text editor with markdown support for document editing
Use Monaco Editor for code editing features
Use@dnd-kitfor drag-and-drop functionality instead of other libraries
Use Tailwind CSS with shadcn/ui components for all UI styling and components
Files:
apps/web/src/components/notifications/NotificationDropdown.tsx
apps/web/src/app/**/route.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/src/app/**/route.{ts,tsx}: In Next.js 15 route handlers,paramsin dynamic routes are Promise objects and MUST be awaited before destructuring
UseResponse.json()orNextResponse.json()for returning JSON from route handlers
Get request body usingconst body = await request.json();
Get search parameters usingconst { searchParams } = new URL(request.url);
apps/web/src/app/**/route.{ts,tsx}: In Next.js 15 dynamic routes,paramsare Promise objects and MUST be awaited before destructuring:const { id } = await context.params;
In Route Handlers, get request body withconst body = await request.json();
In Route Handlers, get search parameters withconst { searchParams } = new URL(request.url);
In Route Handlers, return JSON usingResponse.json(data)orNextResponse.json(data)
For permission logic, use centralized functions from@pagespace/lib/permissions:getUserAccessLevel(),canUserEditPage()
Files:
apps/web/src/app/api/pages/[pageId]/tasks/[taskId]/route.ts
**/*ai*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Vercel AI SDK for AI integrations
Files:
packages/lib/src/services/notification-email-service.ts
🧠 Learnings (6)
📚 Learning: 2025-12-22T20:04:40.910Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T20:04:40.910Z
Learning: Applies to **/*.{ts,tsx} : Use ESM modules throughout the codebase
Applied to files:
packages/lib/package.json
📚 Learning: 2025-12-14T14:54:45.713Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:45.713Z
Learning: Use TypeScript strict mode and ESM modules throughout the codebase
Applied to files:
packages/lib/package.json
📚 Learning: 2025-12-22T20:04:40.910Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T20:04:40.910Z
Learning: Applies to packages/db/src/schema.ts : Update database schema in `packages/db/src/schema.ts` and generate migrations with `pnpm db:generate`
Applied to files:
packages/db/src/schema/notifications.tspackages/db/src/schema/core.ts
📚 Learning: 2025-12-14T14:54:45.713Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:45.713Z
Learning: Applies to packages/db/src/schema.ts : Maintain the Drizzle ORM database schema in `packages/db/src/schema.ts` as the single entry point for schema definitions
Applied to files:
packages/db/src/schema/core.ts
📚 Learning: 2025-12-22T20:04:40.910Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T20:04:40.910Z
Learning: Applies to packages/db/src/schema.ts : Database schema entry point is at `packages/db/src/schema.ts`; migrations emit to `packages/db/drizzle/`
Applied to files:
packages/db/src/schema/core.ts
📚 Learning: 2025-12-23T18:49:41.966Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-23T18:49:41.966Z
Learning: Applies to packages/db/src/schema/**/*.{ts,tsx} : Database schema changes must be made in `packages/db/src/schema/` and then `pnpm db:generate` must be run to create migrations
Applied to files:
packages/db/src/schema/core.ts
🧬 Code graph analysis (6)
apps/web/src/services/api/rollback-service.ts (1)
apps/web/src/services/api/page-mention-service.ts (1)
syncMentions(70-83)
apps/web/src/services/api/page-mutation-service.ts (1)
apps/web/src/services/api/page-mention-service.ts (1)
syncMentions(70-83)
apps/web/src/app/api/pages/[pageId]/tasks/[taskId]/route.ts (1)
packages/lib/src/notifications/notifications.ts (1)
createTaskAssignedNotification(397-440)
packages/db/src/schema/core.ts (1)
packages/db/src/schema/auth.ts (1)
users(10-33)
packages/lib/src/notifications/notifications.ts (2)
packages/db/src/index.ts (1)
db(20-20)packages/db/src/schema/core.ts (1)
pages(24-68)
apps/web/src/services/api/page-mention-service.ts (2)
packages/db/src/schema/core.ts (1)
userMentions(149-161)packages/lib/src/notifications/notifications.ts (1)
createMentionNotification(353-392)
⏰ 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). (2)
- GitHub Check: Unit Tests
- GitHub Check: Lint & TypeScript Check
🔇 Additional comments (23)
packages/db/drizzle/meta/_journal.json (1)
270-277: LGTM!The new migration journal entry follows the established pattern and is properly formatted.
packages/lib/package.json (2)
210-215: LGTM!The new
./notificationsexport follows the established pattern used by other exports like./auth, with proper ESM/CommonJS support and TypeScript type declarations.
332-335: LGTM!The
typesVersionsentry correctly maps to the notifications type declarations, ensuring proper TypeScript resolution.packages/db/src/schema/core.ts (2)
196-196: LGTM!The
userMentionsFromrelation correctly links pages to their user mentions using the appropriate relation name.
252-269: LGTM!The
userMentionsRelationsare correctly defined with appropriate relation names that match the corresponding relations inpagesRelations. The three-way relationship (sourcePage, targetUser, mentionedByUser) is properly modeled.packages/db/src/schema/notifications.ts (1)
21-23: LGTM!The new
MENTIONandTASK_ASSIGNEDnotification types follow the existing naming convention and align with the PR objectives. The existingnotificationstable structure (withpageIdandtriggeredByUserId) supports the metadata needed for these notification types.packages/db/drizzle/0038_salty_phalanx.sql (2)
1-2: LGTM!The enum type extension is performed before the table creation, which is the correct order of operations.
3-31: LGTM!The migration is well-structured with:
- Proper
IF NOT EXISTSfor idempotent table creation- Foreign key constraints with appropriate
ON DELETEbehavior (CASCADE for page/target user, SET NULL for mentioner)- Exception handling for duplicate constraints ensures rerunnability
- Indexes aligned with the Drizzle schema definition
As per coding guidelines, database schema changes in
packages/db/src/schema/have been paired with migration generation viapnpm db:generate.apps/web/src/services/api/rollback-service.ts (1)
393-395: LGTM!The change correctly passes
mentionedByUserIdtosyncMentionswhen restoring content during rollback, ensuring mention notifications are properly attributed to the user performing the rollback operation. The nullish coalescing handles the case whereoptions?.userIdmight benull.apps/web/src/services/api/page-mutation-service.ts (1)
214-216: LGTM!The change correctly passes the acting user's ID to
syncMentions, enabling proper attribution for mention notifications when page content is updated.apps/web/src/components/notifications/NotificationDropdown.tsx (3)
20-23: LGTM!Clean import additions for the new notification icon types.
58-61: LGTM!Appropriate icon choices:
AtSignfor mentions andListTodofor task assignments are intuitive and consistent with the notification semantics.
202-207: LGTM!The navigation logic correctly handles both new notification types by routing to the relevant page. The check for both
pageIdanddriveIdprovides graceful fallback to the generic drive navigation (line 208-211) when either is missing.apps/web/src/app/api/pages/[pageId]/tasks/[taskId]/route.ts (2)
8-8: LGTM!Clean import of the new notification helper from the library package.
224-234: LGTM!The notification logic is well-implemented:
- The condition correctly triggers only when
assigneeIdchanges to a non-null value (new assignment, not unassignment)- Fire-and-forget with
voidis appropriate since notification delivery shouldn't block the API response- The
createTaskAssignedNotificationfunction already handles self-assignment checks internallypackages/lib/src/services/notification-email-service.ts (1)
30-32: Email templates not yet implemented for new notification types.The
MENTIONandTASK_ASSIGNEDtypes are correctly added to the union, but there are no corresponding email templates ingetEmailTemplate(). Currently, these notification types will silently skip email sending (lines 291-294 handle this gracefully).If email notifications for mentions and task assignments are desired, templates will need to be added in a follow-up PR.
Is intentionally skipping email notifications for these types the desired behavior, or should email templates be added as part of this PR?
packages/lib/src/notifications/types.ts (2)
197-225: LGTM! Well-structured notification types.The new
MentionNotificationandTaskAssignedNotificationtypes follow the established discriminated union pattern consistently. Metadata fields appropriately capture all context needed for notification display and navigation.
241-243: LGTM!Union type correctly extended with the new notification types.
apps/web/src/services/api/page-mention-service.ts (3)
14-64: LGTM! Well-structured mention parsing.The
findMentionNodesfunction correctly:
- Parses both HTML (
data-page-id,data-user-id) and markdown-style mentions- Falls back to regex parsing if HTML parsing fails
- Returns deduplicated results using
Set- Handles the type discriminator (
uservspage) appropriately
85-114: LGTM!The
syncPageMentionsfunction correctly implements the diff-based synchronization pattern with proper transaction support.
66-83: LGTM!Clean orchestration with a well-defined options interface.
packages/lib/src/notifications/notifications.ts (2)
350-392: LGTM! Clean implementation following established patterns.The
createMentionNotificationfunction:
- Correctly prevents self-mention notifications
- Properly fetches related entities with null checks
- Provides appropriate fallback for missing user names
- Metadata aligns with
MentionNotificationtype definition
394-440: LGTM! Consistent implementation.The
createTaskAssignedNotificationfunction follows the same well-established pattern as other notification creators in this file, with appropriate self-assignment checks and complete metadata population.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Fixes race condition where notifications could be sent before the database transaction commits. If the transaction rolled back, incorrect notifications would have already been sent. Changes: - Remove setTimeout-based notification dispatch from syncUserMentions - Return newly mentioned user IDs from syncMentions - Send notifications in callers after transaction commits - Update page-mutation-service and rollback-service to handle notifications after their respective transaction boundaries
…ents
This adds a notification system for user mentions and task assignments:
Users will now receive notifications in the notification bell when:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.