Notifications - #36
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements a comprehensive notifications system for user mentions in posts and comments, addressing issues #28, #29, and #34. The implementation includes frontend UI for viewing mentions, backend APIs for logging and retrieving mention events, a scheduler service for sending weekly digest emails, and user analytics features.
Key changes:
- Added user mention detection and tagging in posts/comments with autocomplete
- Created notifications page showing mention events
- Implemented scheduled jobs (weekly digest, daily mentions, hourly cleanup)
- Added user analytics endpoint showing post interactions
- Updated database schema with new event types (event-mentioned, event-notice-acknowledged, event-digest-email)
Reviewed changes
Copilot reviewed 40 out of 41 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| app/src/types/index.ts | Added Notification type and mentions count fields to User/Post interfaces |
| app/src/services/api-client.ts | Added searchUsers, getNotifications, and getUserAnalytics API methods |
| app/src/pages/notifications.tsx | New notifications page displaying mention events with timeframe filtering |
| app/src/pages/create-post.tsx | Integrated user mention autocomplete in markdown editor |
| app/src/pages/analytics.tsx | Added user analytics tab with search functionality |
| app/src/hooks/use-user-mentions.ts | Custom hook for detecting and inserting @mentions |
| app/src/components/ui/*.tsx | New components for mention suggestions and notification bell |
| app/package.json | Added mermaid ^11.12.2 and upgraded axios to 1.13.2 |
| apis/src/services/scheduler.py | New distributed scheduler service with database locking |
| apis/src/services/email_service.py | SMTP email service with connection pooling and rate limiting |
| apis/src/services/jobs/*.py | Scheduled job implementations (weekly digest, mentions, cleanup) |
| apis/src/services/database/*.py | Added log_mention_events method to both SQLite and PostgreSQL services |
| apis/src/routers/*.py | Updated users, posts, comments, events routers to support mentions |
| apis/src/models/*.py | Added mentioned_user_ids fields to PostCreate, CommentCreate models |
| apis/src/services/database/migration.py | Database version bump to 2.1.0 with new event types |
| apis/src/config/settings.py | Added SMTP configuration settings |
| apis/main.py | Integrated scheduler initialization on startup |
Files not reviewed (1)
- app/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Handle both datetime objects and ISO strings | ||
| if isinstance(updated_ts, str): | ||
| # Remove timezone info if present for parsing | ||
| if updated_ts.endswith('+00:00') or updated_ts.endswith('Z'): | ||
| updated_ts = updated_ts.replace('+00:00', '').replace('Z', '') | ||
| lock_time = datetime.fromisoformat(updated_ts) | ||
| else: | ||
| lock_time = updated_ts |
There was a problem hiding this comment.
Potential string parsing issue with timezone handling. The code strips timezone info (+00:00 or Z) from ISO strings before parsing, which could lead to incorrect datetime comparisons if timestamps from different timezones are mixed. Consider using dateutil.parser or properly handling timezone-aware datetime objects throughout.
| if (query.length < 4) { | ||
| setSearchResults([]); | ||
| setShowSearchDropdown(false); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Magic number used for minimum search query length. The hardcoded value 4 should be extracted to a named constant for better maintainability and to make it easier to adjust this threshold in the future.
| 'event-notice-acknowledged', | ||
| 'notice-acknowledged', | ||
| 'engagement', | ||
| 'User acknowledged a notice', | ||
| CURRENT_TIMESTAMP, | ||
| 'ef85dcf4-97dd-4ccb-b481-93067b0cfd27' | ||
| ),( |
There was a problem hiding this comment.
SQL syntax error in the INSERT statement. There's a missing opening parenthesis before 'event-notice-acknowledged' on line 101, which will cause the migration to fail.
| 'event-notice-acknowledged', | |
| 'notice-acknowledged', | |
| 'engagement', | |
| 'User acknowledged a notice', | |
| CURRENT_TIMESTAMP, | |
| 'ef85dcf4-97dd-4ccb-b481-93067b0cfd27' | |
| ),( | |
| ( | |
| 'event-notice-acknowledged', | |
| 'notice-acknowledged', | |
| 'engagement', | |
| 'User acknowledged a notice', | |
| CURRENT_TIMESTAMP, | |
| 'ef85dcf4-97dd-4ccb-b481-93067b0cfd27' | |
| ), |
| export const extractMentionedUserIds = (markdown: string): string[] => { | ||
| const mentionRegex = /\[@[^\]]+\]\(\/content\/profile\/([a-zA-Z0-9._-]+)\)/g; | ||
| const matches = [...markdown.matchAll(mentionRegex)]; | ||
| const usernames = matches.map(match => match[1]); | ||
| return [...new Set(usernames)]; // Remove duplicates |
There was a problem hiding this comment.
The regex pattern for extracting mentions from markdown could incorrectly match usernames that contain special regex characters. The pattern should escape special characters in the captured username to prevent false matches or regex errors.
| const handleTextareaKeyDown = (e: React.KeyboardEvent<HTMLTextAreaElement>) => { | ||
| //console.log('Textarea keydown event:', e.key); | ||
| // Only trigger mention detection on @ key | ||
| //if (e.key === '@') { | ||
| const textarea = e.currentTarget; | ||
| const cursorPos = textarea.selectionStart; | ||
| //console.debug('Cursor position for mention:', cursorPos); | ||
| mentions.detectMention(markdownContent, cursorPos, textarea); | ||
| // } | ||
| }; |
There was a problem hiding this comment.
The commented-out code and debug logic makes the intention unclear. Lines 71-78 contain commented logic with an active function call on line 77 that always executes regardless of key press. Either remove the commented sections or properly implement the conditional logic for @ key detection.
| """ | ||
|
|
||
| import threading | ||
| import logging |
There was a problem hiding this comment.
Import of 'logging' is not used.
| import logging |
|
|
||
| import threading | ||
| import logging | ||
| from datetime import datetime, timedelta |
There was a problem hiding this comment.
Import of 'timedelta' is not used.
| from datetime import datetime, timedelta | |
| from datetime import datetime |
| if self._connection is not None: | ||
| try: | ||
| self._connection.quit() | ||
| except: |
There was a problem hiding this comment.
Except block directly handles BaseException.
| except: | |
| except Exception: |
| try: | ||
| dt = datetime.fromisoformat(timestamp.replace('Z', '+00:00')) | ||
| return dt.strftime("%b %d, %Y") | ||
| except: |
There was a problem hiding this comment.
Except block directly handles BaseException.
| except: | |
| except (ValueError, TypeError): |
| except: | ||
| pass |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
| except: | |
| pass | |
| except Exception as close_exc: | |
| logger.debug(f"Exception occurred while closing SMTP connection: {close_exc}") |
Fixes for below issues
#34
#29
#28