feat: implement notification centre solves #90 #91
Conversation
WalkthroughA new centralized Notification Centre feature is implemented in the frontend. This includes a dedicated notifications page accessible via a protected route, a navigation badge indicating unread notifications, and a UI for viewing, selecting, and deleting notifications. The notifications page fetches data from the backend and provides dialog-based details for each notification. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant NavBar
participant AppRouter
participant NotificationsPage
participant BackendAPI
User->>NavBar: Clicks "Notifications"
NavBar->>AppRouter: Navigates to /notifications
AppRouter->>NotificationsPage: Renders component (protected)
NotificationsPage->>BackendAPI: GET /notifications
BackendAPI-->>NotificationsPage: Returns notifications
NotificationsPage->>User: Displays notification list
User->>NotificationsPage: Clicks notification item
NotificationsPage->>BackendAPI: PATCH /notifications/mark-read (if unread)
BackendAPI-->>NotificationsPage: Confirms read status update
NotificationsPage->>User: Shows notification details dialog
User->>NotificationsPage: Selects notifications, clicks delete
NotificationsPage->>BackendAPI: DELETE /notifications (single/bulk)
BackendAPI-->>NotificationsPage: Confirms deletion
NotificationsPage->>User: Updates notification list
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learningsBackend/app/routes/notification.py (2)🧬 Code Graph Analysis (1)Backend/app/routes/notification.py (4)
🪛 Ruff (0.11.9)Backend/app/routes/notification.py106-106: Do not perform function call (B008) 107-107: Do not perform function call (B008) 135-135: Do not perform function call (B008) 136-136: Do not perform function call (B008) 137-137: Do not perform function call (B008) 197-197: Do not perform function call (B008) 198-198: Do not perform function call (B008) 199-199: Do not perform function call (B008) 222-222: Do not perform function call (B008) 223-223: Do not perform function call (B008) 224-224: Do not perform function call (B008) 🔇 Additional comments (1)
✨ Finishing Touches
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
Frontend/src/pages/Notifications.tsx (1)
127-156: Consider adding pagination or virtualization for large notification lists.The current implementation loads all notifications at once, which could cause performance issues with large datasets.
For better performance with large notification lists, consider implementing:
- Pagination: Load notifications in chunks
- Virtualization: Use libraries like
react-windowfor large lists- Infinite scrolling: Load more notifications as user scrolls
- Caching: Implement proper caching strategy for notification data
This will improve both performance and user experience, especially on mobile devices.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Frontend/src/App.tsx(2 hunks)Frontend/src/components/user-nav.tsx(1 hunks)Frontend/src/pages/Notifications.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
Frontend/src/App.tsx (1)
Frontend/src/pages/Notifications.tsx (1)
NotificationsPage(14-178)
🔇 Additional comments (2)
Frontend/src/App.tsx (2)
15-15: LGTM: Import statement is correct.The import follows the existing pattern and is properly placed with other page imports.
78-82: LGTM: Route implementation follows best practices.The notifications route is properly wrapped with
ProtectedRouteto ensure authentication and follows the existing routing patterns in the application.
…robust backend handling
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (5)
Frontend/src/pages/Notifications.tsx (5)
17-21: Improve type safety by defining proper interfaces.
42-46: Replace hardcoded API endpoint with environment variable.
53-54: Improve error handling and type safety.
240-240: Add error handling for date formatting.
227-234: Improve accessibility for checkbox interactions.
🧹 Nitpick comments (2)
Backend/app/models/models.py (1)
166-177: Consider adding relationship and indexes for better performance.The Notification model lacks a relationship back to the User model and indexes for common query patterns.
Add a relationship for easier navigation:
class Notification(Base): __tablename__ = "notifications" id = Column(String, primary_key=True, default=generate_uuid) user_id = Column(String, ForeignKey("users.id"), nullable=False) type = Column(String, nullable=True) title = Column(String, nullable=False) message = Column(Text, nullable=False) link = Column(String, nullable=True) is_read = Column(Boolean, default=False) category = Column(String, nullable=True) created_at = Column(DateTime(timezone=True), default=lambda: datetime.now(timezone.utc)) + + user = relationship("User", back_populates="notifications")And update the User model to include:
notifications = relationship("Notification", back_populates="user")Consider adding indexes in your migration for common queries:
CREATE INDEX idx_notifications_user_created ON notifications(user_id, created_at DESC); CREATE INDEX idx_notifications_user_unread ON notifications(user_id, is_read) WHERE is_read = FALSE;Backend/app/routes/notification.py (1)
193-193: Use explicit None return for clarity.Empty return statements should explicitly return None.
- return + return None
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
Backend/app/main.py(2 hunks)Backend/app/models/models.py(1 hunks)Backend/app/routes/notification.py(1 hunks)Frontend/src/App.tsx(2 hunks)Frontend/src/components/user-nav.tsx(2 hunks)Frontend/src/pages/Notifications.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- Backend/app/main.py
🚧 Files skipped from review as they are similar to previous changes (1)
- Frontend/src/components/user-nav.tsx
🧰 Additional context used
🧬 Code Graph Analysis (3)
Backend/app/models/models.py (1)
Backend/app/models/chat.py (1)
generate_uuid(9-10)
Frontend/src/pages/Notifications.tsx (4)
Frontend/src/context/AuthContext.tsx (1)
useAuth(216-222)Frontend/src/utils/supabase.tsx (1)
supabase(11-11)Frontend/src/components/ui/button.tsx (1)
Button(54-54)Frontend/src/components/ui/dialog.tsx (6)
Dialog(123-123)DialogContent(125-125)DialogHeader(128-128)DialogTitle(131-131)DialogDescription(126-126)DialogFooter(127-127)
Backend/app/routes/notification.py (3)
Backend/app/models/models.py (1)
Notification(166-177)Backend/app/db/db.py (1)
get_db(38-40)Frontend/src/utils/supabase.tsx (1)
supabase(11-11)
🪛 Ruff (0.11.9)
Backend/app/routes/notification.py
1-1: fastapi.Request imported but unused
Remove unused import: fastapi.Request
(F401)
96-96: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
97-97: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
121-121: Do not perform function call Body in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
122-122: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
123-123: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
183-183: Do not perform function call Body in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
184-184: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
185-185: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
197-197: Do not perform function call Body in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
198-198: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
199-199: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🔇 Additional comments (2)
Frontend/src/pages/Notifications.tsx (1)
62-89: Real-time subscription implementation looks good.The Supabase real-time subscription is properly set up with:
- Correct filtering by user ID
- Duplicate prevention logic
- Proper cleanup in the return function
Backend/app/routes/notification.py (1)
27-35: Good error handling in Supabase insertion.The function properly handles Supabase insertion failures with logging and returns a success indicator.
…ge by hiding global UserNav
…tifications API for cleaner, more maintainable code
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
Backend/app/routes/notification.py (1)
48-48: Avoid logging sensitive authorization tokens.Logging the full authorization header can expose JWT tokens in logs, which poses a security risk.
- logger.info(f"Authorization header received: {authorization}") + logger.debug(f"Authorization header received: {authorization[:20]}...")
🧹 Nitpick comments (2)
Backend/app/routes/notification.py (2)
28-37: Add type hints and improve error handling in the Supabase helper function.The function lacks type hints and could benefit from more specific error handling.
-def insert_notification_to_supabase(notification_dict): +def insert_notification_to_supabase(notification_dict: dict) -> bool: try: supabase.table("notifications").insert(notification_dict).execute() logger.info(f"Notification {notification_dict['id']} inserted into Supabase.") return True except Exception as e: - logger.error(f"Failed to insert notification {notification_dict['id']} into Supabase: {e}") + logger.error(f"Failed to insert notification {notification_dict.get('id', 'unknown')} into Supabase: {e}") # Optionally, add to a retry queue here return False
170-170: Simplify datetime serialization with consistent timezone handling.The datetime serialization logic can be simplified since
created_atis already ensured to be timezone-aware.- "created_at": notif_obj.created_at.astimezone(timezone.utc).isoformat() if notif_obj.created_at else None, + "created_at": notif_obj.created_at.isoformat() if notif_obj.created_at else None,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
Backend/.env-example(1 hunks)Backend/app/routes/notification.py(1 hunks)Frontend/env-example(1 hunks)Frontend/src/App.tsx(2 hunks)Frontend/src/components/user-nav.tsx(2 hunks)Frontend/src/pages/Notifications.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- Backend/.env-example
- Frontend/env-example
🚧 Files skipped from review as they are similar to previous changes (3)
- Frontend/src/components/user-nav.tsx
- Frontend/src/pages/Notifications.tsx
- Frontend/src/App.tsx
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Saahi30
PR: AOSSIE-Org/InPactAI#91
File: Backend/app/models/models.py:177-177
Timestamp: 2025-07-08T21:54:53.683Z
Learning: In the Backend/app/models/models.py file, the Notification model intentionally uses datetime.utcnow for the created_at field instead of timezone-aware datetime for easier debugging and error tracking, as preferred by the user for notification timestamp handling.
Backend/app/routes/notification.py (1)
Learnt from: Saahi30
PR: AOSSIE-Org/InPactAI#91
File: Backend/app/models/models.py:177-177
Timestamp: 2025-07-08T21:54:53.683Z
Learning: In the Backend/app/models/models.py file, the Notification model intentionally uses datetime.utcnow for the created_at field instead of timezone-aware datetime for easier debugging and error tracking, as preferred by the user for notification timestamp handling.
🧬 Code Graph Analysis (1)
Backend/app/routes/notification.py (4)
Frontend/src/pages/Notifications.tsx (1)
Notification(13-22)Backend/app/models/models.py (1)
Notification(166-177)Backend/app/db/db.py (1)
get_db(38-40)Frontend/src/utils/supabase.tsx (1)
supabase(11-11)
🪛 Ruff (0.11.9)
Backend/app/routes/notification.py
98-98: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
99-99: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
123-123: Do not perform function call Body in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
124-124: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
125-125: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
185-185: Do not perform function call Body in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
186-186: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
187-187: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
199-199: Do not perform function call Body in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
200-200: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
201-201: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🔇 Additional comments (1)
Backend/app/routes/notification.py (1)
121-182: LGTM! Good implementation of notification creation with proper error handling.The create notification endpoint has solid error handling, proper datetime conversion, and graceful fallback when Supabase insertion fails. The use of JSONResponse for different status codes is appropriate.
…o notifications API
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
Backend/app/routes/notification.py (2)
48-48: Avoid logging the full authorization header containing JWT tokens.The authorization header contains the JWT token which is sensitive information that should not be logged in full.
- logger.info(f"Authorization header received: {authorization}") + logger.debug(f"Authorization header received: {authorization[:20]}...")
63-63: Avoid logging sensitive JWT payload information.The JWT payload may contain sensitive user information that should not be logged.
- logger.info("RS256 verification succeeded.") + logger.debug("RS256 verification succeeded")- logger.info("HS256 verification succeeded.") + logger.debug("HS256 verification succeeded")Also applies to: 83-83
🧹 Nitpick comments (3)
Backend/app/routes/notification.py (3)
123-124: Move JSONResponse import to the top of the file.The
JSONResponseimport is repeated multiple times inside functions. Move it to the top with other imports for better organization.from fastapi import APIRouter, Depends, HTTPException, status, Body, Header +from fastapi.responses import JSONResponseThen remove the local imports:
- from fastapi.responses import JSONResponseAlso applies to: 194-194, 219-219
175-175: Simplify timezone handling for UTC datetime.The
astimezone(timezone.utc)call is redundant if the datetime is already in UTC (as ensured by the logic above).- "created_at": notif_obj.created_at.astimezone(timezone.utc).isoformat() if notif_obj.created_at else None, + "created_at": notif_obj.created_at.isoformat() if notif_obj.created_at else None,
208-208: Ensure consistent return types for success responses.The delete endpoint returns nothing (implicit None) while the mark-read endpoint returns a dict. Consider consistent return types.
- return + return JSONResponse(status_code=204, content=None)Also applies to: 234-234
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Backend/app/routes/notification.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Saahi30
PR: AOSSIE-Org/InPactAI#91
File: Backend/app/models/models.py:177-177
Timestamp: 2025-07-08T21:54:53.683Z
Learning: In the Backend/app/models/models.py file, the Notification model intentionally uses datetime.utcnow for the created_at field instead of timezone-aware datetime for easier debugging and error tracking, as preferred by the user for notification timestamp handling.
Backend/app/routes/notification.py (1)
Learnt from: Saahi30
PR: AOSSIE-Org/InPactAI#91
File: Backend/app/models/models.py:177-177
Timestamp: 2025-07-08T21:54:53.683Z
Learning: In the Backend/app/models/models.py file, the Notification model intentionally uses datetime.utcnow for the created_at field instead of timezone-aware datetime for easier debugging and error tracking, as preferred by the user for notification timestamp handling.
🧬 Code Graph Analysis (1)
Backend/app/routes/notification.py (4)
Frontend/src/pages/Notifications.tsx (1)
Notification(13-22)Backend/app/models/models.py (1)
Notification(166-177)Backend/app/db/db.py (1)
get_db(38-40)Frontend/src/utils/supabase.tsx (1)
supabase(11-11)
🪛 Ruff (0.11.9)
Backend/app/routes/notification.py
98-98: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
99-99: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
128-128: Do not perform function call Body in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
129-129: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
130-130: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
190-190: Do not perform function call Body in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
191-191: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
192-192: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
215-215: Do not perform function call Body in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
216-216: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
217-217: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🔇 Additional comments (1)
Backend/app/routes/notification.py (1)
96-237: LGTM - Well-structured notification API implementation.The notification endpoints are well-implemented with proper error handling, input validation, and consistent response formats. The integration with Supabase for real-time updates is a nice touch, and the JWT authentication dependency is robust with RS256/HS256 fallback.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
Backend/app/routes/notification.py (3)
96-124: Consider moving JSONResponse import to top of file.The JSONResponse import is already available at the top of the file (line 13), making this local import redundant.
except Exception as e: logger.error(f"Failed to fetch notifications: {e}") - from fastapi.responses import JSONResponse return JSONResponse(status_code=500, content={"error": "Failed to fetch notifications."})
188-212: Remove redundant JSONResponse import.Similar to the get endpoint, the JSONResponse import is already available at the top of the file.
): - from fastapi.responses import JSONResponse if not ids or not isinstance(ids, list) or len(ids) == 0:
214-239: Remove redundant JSONResponse import.Consistent with other endpoints, the JSONResponse import is already available at the top of the file.
): - from fastapi.responses import JSONResponse if not ids or not isinstance(ids, list) or len(ids) == 0:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Backend/app/routes/notification.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Saahi30
PR: AOSSIE-Org/InPactAI#91
File: Backend/app/models/models.py:177-177
Timestamp: 2025-07-08T21:54:53.683Z
Learning: In the Backend/app/models/models.py file, the Notification model intentionally uses datetime.utcnow for the created_at field instead of timezone-aware datetime for easier debugging and error tracking, as preferred by the user for notification timestamp handling.
Backend/app/routes/notification.py (1)
Learnt from: Saahi30
PR: AOSSIE-Org/InPactAI#91
File: Backend/app/models/models.py:177-177
Timestamp: 2025-07-08T21:54:53.683Z
Learning: In the Backend/app/models/models.py file, the Notification model intentionally uses datetime.utcnow for the created_at field instead of timezone-aware datetime for easier debugging and error tracking, as preferred by the user for notification timestamp handling.
🧬 Code Graph Analysis (1)
Backend/app/routes/notification.py (4)
Frontend/src/pages/Notifications.tsx (1)
Notification(13-22)Backend/app/models/models.py (1)
Notification(166-177)Backend/app/db/db.py (1)
get_db(38-40)Frontend/src/utils/supabase.tsx (1)
supabase(11-11)
🪛 Ruff (0.11.9)
Backend/app/routes/notification.py
98-98: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
99-99: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
128-128: Do not perform function call Body in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
129-129: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
130-130: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
190-190: Do not perform function call Body in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
191-191: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
192-192: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
216-216: Do not perform function call Body in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
217-217: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
218-218: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🔇 Additional comments (3)
Backend/app/routes/notification.py (3)
47-94: LGTM - JWT authentication implementation.The JWT authentication function properly handles both RS256 and HS256 verification with appropriate logging and error handling. Previous security concerns about logging sensitive JWT payload information have been addressed.
126-186: LGTM - Comprehensive notification creation with proper error handling.The create notification endpoint properly handles timezone conversion, UUID generation, database operations, and Supabase integration with appropriate error handling and rollback mechanisms.
28-36: LGTM - Well-structured Supabase integration function.The helper function properly handles Supabase insertion with appropriate error handling and logging for debugging and monitoring purposes.
There was a problem hiding this comment.
@Saahi30 Please check coderabbit reviews and resolve relevant ones.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Backend/app/routes/notification.py (1)
51-51: Avoid logging full authorization header for security.The complete authorization header contains sensitive JWT tokens that should not be logged in full.
- logger.info(f"Authorization header received: {authorization}") + logger.debug(f"Authorization header received: {authorization[:20]}...")
🧹 Nitpick comments (4)
Backend/app/routes/notification.py (4)
126-127: Move import statements to the top of the file.Importing modules inside functions reduces readability and can impact performance. The
JSONResponseimport should be moved to the top with other imports.from fastapi import APIRouter, Depends, HTTPException, status, Body, Header +from fastapi.responses import JSONResponse from sqlalchemy.ext.asyncio import AsyncSessionThen remove the local imports:
except Exception as e: logger.error(f"Failed to fetch notifications: {e}") - from fastapi.responses import JSONResponse return JSONResponse(status_code=500, content={"error": "Failed to fetch notifications."})Apply similar changes to lines 197 and 223.
Also applies to: 197-197, 223-223
178-178: Simplify timezone handling for created_at.Based on the retrieved learning, the Notification model uses
datetime.utcnowfor consistency. The timezone conversion here is unnecessary since the timestamp is already in UTC.- "created_at": notif_obj.created_at.astimezone(timezone.utc).isoformat() if notif_obj.created_at else None, + "created_at": notif_obj.created_at.isoformat() if notif_obj.created_at else None,
31-40: Consider adding retry mechanism for Supabase failures.The function includes a TODO comment about adding notifications to a retry queue, which would improve reliability for real-time synchronization.
The comment suggests implementing a retry queue for failed Supabase insertions. This would be valuable for ensuring notifications don't get lost during temporary connectivity issues.
Would you like me to help implement a basic retry mechanism or create an issue to track this enhancement?
99-242: Consider the static analysis warnings about function calls in defaults.While the
Depends()andBody()calls in function parameter defaults are common in FastAPI, they technically violate Python best practices. However, this is widely accepted in the FastAPI ecosystem and documented in their examples.If you want to address the static analysis warnings, you could move the dependency injection inside the function bodies, but this would make the code less readable and go against FastAPI conventions. I recommend keeping the current approach unless your team has specific guidelines against it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Backend/app/routes/notification.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Saahi30
PR: AOSSIE-Org/InPactAI#91
File: Backend/app/models/models.py:177-177
Timestamp: 2025-07-08T21:54:53.683Z
Learning: In the Backend/app/models/models.py file, the Notification model intentionally uses datetime.utcnow for the created_at field instead of timezone-aware datetime for easier debugging and error tracking, as preferred by the user for notification timestamp handling.
Backend/app/routes/notification.py (2)
Learnt from: Saahi30
PR: AOSSIE-Org/InPactAI#91
File: Backend/app/models/models.py:177-177
Timestamp: 2025-07-08T21:54:53.683Z
Learning: In the Backend/app/models/models.py file, the Notification model intentionally uses datetime.utcnow for the created_at field instead of timezone-aware datetime for easier debugging and error tracking, as preferred by the user for notification timestamp handling.
Learnt from: muntaxir4
PR: AOSSIE-Org/InPactAI#56
File: Backend/app/services/redis_client.py:1-4
Timestamp: 2025-05-07T21:28:06.358Z
Learning: Hardcoded Redis connection parameters in Backend/app/services/redis_client.py are intentional during development, with plans to implement environment variable configuration later during production preparation.
🪛 Ruff (0.11.9)
Backend/app/routes/notification.py
18-18: Undefined name logger
(F821)
101-101: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
102-102: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
131-131: Do not perform function call Body in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
132-132: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
133-133: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
193-193: Do not perform function call Body in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
194-194: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
195-195: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
219-219: Do not perform function call Body in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
220-220: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
221-221: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
…ures and improve security
Closes #90
📝 Description
This pull request implements a fully functional Notification Centre for the platform.
Users can now view, manage, and interact with their notifications in a centralized UI.
The Notification Centre supports real-time data fetching, marking notifications as read, bulk deletion, and detailed notification dialogs.
This feature lays the groundwork for future enhancements such as real-time updates, push notifications, and advanced user workflows.
🔧 Changes Made
📷 Screenshots or Visual Changes (if applicable)
✅ Checklist
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Refactor
Style
Chores