Conversation
WalkthroughThis update introduces a centralized toast notification system to the frontend, adds a CommentService for API interactions, and refactors comment management and error handling to use the new toast mechanism. It also standardizes authentication hook imports and simplifies backend comment route paths while enforcing authentication middleware on all comment endpoints. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant ToastProvider
participant CommentService
participant Backend
User->>UI: Triggers comment action (add/edit/delete)
UI->>CommentService: Calls relevant method (publishComment, etc.)
CommentService->>Backend: Sends API request
Backend-->>CommentService: Responds with success/error
CommentService-->>UI: Returns result or Error
alt Error occurs
UI->>ToastProvider: Calls notify(message, "error")
ToastProvider-->>UI: Displays toast notification
end
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
✨ 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 (5)
frontend/src/pages/profile/ManageProfile.tsx (1)
21-22: Consider migrating to the new toast notification system.This component still uses local state for error and success messages. Consider migrating to the new
useToasthook for consistent user feedback across the application.Example refactor to use toast notifications:
+import { useToast } from "../../hooks/ToastProvider/useToast"; export default function ManageProfile() { + const { showToast } = useToast(); - const [errorEditProfile, setErrorEditProfile] = useState(""); - const [successEditProfile, setSuccessEditProfile] = useState(""); - const [errorEditPassword, setErrorEditPassword] = useState(""); - const [successEditPassord, setSuccessEditPassord] = useState(""); - const [errorEditEmail, setErrorEditEmail] = useState(""); - const [successEditEmail, setSuccessEditEmail] = useState(""); - const [errorEditUsername, setErrorEditUsername] = useState(""); - const [successEditUsername, setSuccessEditUsername] = useState(""); - const [errorDeleteAccount, setErrorDeleteAccount] = useState(""); - const [successDeleteAccount, setSuccessDeleteAccount] = useState(""); // Then replace setState calls with: // showToast("Error message", "error") // showToast("Success message", "success")Also applies to: 31-32, 35-36, 39-40, 44-45
frontend/src/pages/content/ContentEditor.tsx (1)
36-36: Consider migrating to the new toast notification system for better UX.This component uses local error state and console.error logging. Consider migrating to the new
useToasthook for consistent user feedback.Example refactor for error handling:
+import { useToast } from "../../hooks/ToastProvider/useToast"; export default function ContentEditor() { + const { showToast } = useToast(); - const [error, setError] = useState(""); // Replace error handling: - setError("Failed to summarize provided content..."); + showToast("Failed to summarize provided content. Please try again.", "error"); - console.error(`Error sending notifications: ${error}`); + showToast("Failed to send notifications to followers", "error");Also applies to: 158-164, 236-238, 249-251, 279-281, 288-291
frontend/src/hooks/ToastProvider/ToastProvider.tsx (1)
7-31: LGTM! Clean implementation with room for enhancement.The ToastProvider implementation is well-structured and follows React best practices. However, consider these potential improvements for better user experience:
Optional enhancements to consider:
Toast queuing system: Currently only one toast can be displayed at a time. Consider implementing a queue to handle multiple simultaneous notifications.
Duplicate prevention: Add logic to prevent showing identical toast messages when they're triggered in quick succession.
Auto-dismiss configuration: Make the 5-second auto-dismiss timeout configurable per toast type.
export function ToastProvider({ children }: { children: ReactNode }) { - const [toast, setToast] = useState<{ + const [toasts, setToasts] = useState<Array<{ + id: string; message: string; type: ToastType; - } | null>(null); + }>([]); - const notify = (message: string, type: ToastType) => { - setToast({ message, type }); + const notify = (message: string, type: ToastType) => { + // Prevent duplicates + if (toasts.some(toast => toast.message === message)) return; + + const newToast = { id: Date.now().toString(), message, type }; + setToasts(prev => [...prev, newToast]); };frontend/src/services/CommentService.ts (2)
5-5: Consider using standalone functions instead of a static-only class.The static analysis tool correctly identifies that this class contains only static members. Consider refactoring to use standalone functions for better simplicity and tree-shaking.
-export default class CommentService { +export const CommentService = {Or even better, export individual functions:
export async function publishComment(/* params */) { /* implementation */ } export async function deleteComment(/* params */) { /* implementation */ } // ... other functions🧰 Tools
🪛 Biome (1.9.4)
[error] 5-118: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
18-21: Extract configuration constants for better maintainability.The axios configuration is repeated across all methods. Consider extracting these constants.
Add constants at the top of the file:
+const API_TIMEOUT = 5000; +const DEFAULT_CONFIG = { + withCredentials: true, + timeout: API_TIMEOUT, +};Then use in requests:
- { - withCredentials: true, - timeout: 5000, - } + DEFAULT_CONFIG
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
backend/src/modules/comment/routes/comment.routes.ts(1 hunks)frontend/src/App.tsx(2 hunks)frontend/src/components/Navbar.tsx(5 hunks)frontend/src/components/content/CommentList.tsx(4 hunks)frontend/src/components/content/ContentPreviewPopup.tsx(1 hunks)frontend/src/components/content/ContentTile.tsx(1 hunks)frontend/src/components/feed/ContentList.tsx(1 hunks)frontend/src/components/notification/NotificationView.tsx(1 hunks)frontend/src/components/search/SearchListResults.tsx(4 hunks)frontend/src/hooks/AuthProvider/AuthContext.ts(1 hunks)frontend/src/hooks/AuthProvider/AuthProvider.tsx(4 hunks)frontend/src/hooks/ToastProvider/ToastContext.ts(1 hunks)frontend/src/hooks/ToastProvider/ToastProvider.tsx(1 hunks)frontend/src/hooks/ToastProvider/ToastType.ts(1 hunks)frontend/src/hooks/ToastProvider/useToast.ts(1 hunks)frontend/src/pages/Feed.tsx(1 hunks)frontend/src/pages/authentication/CallbackAuthentication.tsx(1 hunks)frontend/src/pages/authentication/Login.tsx(1 hunks)frontend/src/pages/authentication/Register.tsx(1 hunks)frontend/src/pages/content/ContentEditor.tsx(1 hunks)frontend/src/pages/content/ContentView.tsx(6 hunks)frontend/src/pages/pro/ManageSubscription.tsx(1 hunks)frontend/src/pages/pro/ProDetails.tsx(1 hunks)frontend/src/pages/pro/SubscribePro.tsx(1 hunks)frontend/src/pages/profile/ManageProfile.tsx(1 hunks)frontend/src/pages/profile/Profile.tsx(8 hunks)frontend/src/services/CommentService.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (11)
frontend/src/hooks/ToastProvider/useToast.ts (1)
frontend/src/hooks/ToastProvider/ToastContext.ts (1)
ToastContext(8-10)
frontend/src/components/search/SearchListResults.tsx (1)
frontend/src/hooks/ToastProvider/useToast.ts (1)
useToast(4-8)
backend/src/modules/comment/routes/comment.routes.ts (2)
backend/src/shared/middleware/auth.ts (1)
authenticateToken(20-67)backend/src/modules/comment/controllers/comment.controller.ts (5)
createCommentController(13-25)getCommentsByPostController(90-105)getCommentByIdController(27-38)updateCommentController(40-55)deleteCommentController(57-71)
frontend/src/App.tsx (2)
frontend/src/hooks/ToastProvider/ToastProvider.tsx (1)
ToastProvider(7-31)frontend/src/hooks/AuthProvider/AuthProvider.tsx (1)
AuthProvider(10-85)
frontend/src/hooks/ToastProvider/ToastProvider.tsx (3)
frontend/src/hooks/ToastProvider/ToastType.ts (1)
ToastType(1-1)frontend/src/hooks/ToastProvider/ToastContext.ts (1)
ToastContext(8-10)frontend/src/components/ToastNotification.tsx (1)
ToastNotification(15-35)
frontend/src/pages/content/ContentView.tsx (1)
frontend/src/hooks/ToastProvider/useToast.ts (1)
useToast(4-8)
frontend/src/hooks/ToastProvider/ToastContext.ts (1)
frontend/src/hooks/ToastProvider/ToastType.ts (1)
ToastType(1-1)
frontend/src/components/Navbar.tsx (1)
frontend/src/hooks/ToastProvider/useToast.ts (1)
useToast(4-8)
frontend/src/pages/profile/Profile.tsx (1)
frontend/src/hooks/ToastProvider/useToast.ts (1)
useToast(4-8)
frontend/src/components/content/CommentList.tsx (5)
frontend/src/models/Content.ts (1)
Content(3-24)frontend/src/models/User.ts (1)
User(1-22)frontend/src/models/Comment.ts (1)
Comment(1-10)frontend/src/hooks/ToastProvider/useToast.ts (1)
useToast(4-8)frontend/src/services/CommentService.ts (1)
CommentService(5-118)
frontend/src/services/CommentService.ts (1)
frontend/src/scripts/api.ts (1)
apiURL(8-8)
🪛 Biome (1.9.4)
frontend/src/services/CommentService.ts
[error] 5-118: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
🔇 Additional comments (43)
frontend/src/pages/Feed.tsx (1)
3-3: StandardizeuseAuthimport path
The import foruseAuthhas been updated to the newAuthProviderdirectory, aligning with the consolidated authentication context structure.frontend/src/pages/authentication/CallbackAuthentication.tsx (1)
3-3: StandardizeuseAuthimport path
Updated theuseAuthimport to reflect the move intoAuthProvider. This change is consistent with other authentication-related components.frontend/src/components/feed/ContentList.tsx (1)
7-7: StandardizeuseAuthimport path
TheuseAuthhook import has been adjusted to the newAuthProviderpath, maintaining consistency across frontend modules.frontend/src/pages/authentication/Register.tsx (1)
3-3: StandardizeuseAuthimport path
Aligned theuseAuthimport with the centralizedAuthProviderdirectory for authentication logic.frontend/src/pages/pro/ManageSubscription.tsx (1)
2-2: StandardizeuseAuthimport path
Refactored theuseAuthimport to the newAuthProviderstructure, consistent with the overall authentication context refactor.frontend/src/components/content/ContentPreviewPopup.tsx (1)
4-4: Consistent authentication import
The import path foruseAuthhas been correctly updated to"../../hooks/AuthProvider/useAuth", aligning with the newAuthProviderstructure.frontend/src/pages/pro/SubscribePro.tsx (1)
2-2: Authentication hook import updated
TheuseAuthhook is now imported from"../../hooks/AuthProvider/useAuth", ensuring consistency with the centralized AuthProvider directory.frontend/src/pages/authentication/Login.tsx (1)
3-3: Standardize authentication hook path
UpdateduseAuthimport to reflect its new location underAuthProvider. This keeps the authentication logic centralized.frontend/src/components/notification/NotificationView.tsx (1)
2-2: AlignuseAuthimport with AuthProvider
Correctly updated the import path ofuseAuthto"../../hooks/AuthProvider/useAuth", matching the refactored hook placement.frontend/src/pages/pro/ProDetails.tsx (1)
2-2: Refactored auth hook import
TheuseAuthhook import has been updated to the newAuthProviderdirectory structure, ensuring consistent authentication usage.frontend/src/pages/profile/ManageProfile.tsx (1)
2-2: LGTM! Import path correctly updated for authentication hook refactoring.The import path change aligns with the centralized AuthProvider structure.
frontend/src/components/content/ContentTile.tsx (1)
3-3: LGTM! Import path correctly updated for authentication hook refactoring.The import path change aligns with the centralized AuthProvider structure.
frontend/src/hooks/AuthProvider/AuthContext.ts (1)
2-2: LGTM! Import path correctly adjusted for new directory structure.The path update from
"../models/User"to"../../models/User"correctly accounts for the AuthProvider being moved to a subdirectory.frontend/src/hooks/ToastProvider/ToastType.ts (1)
1-1: LGTM! Well-defined type for toast notifications.The
ToastTypedefinition provides clear, type-safe options for the toast notification system with appropriate categories for user feedback.frontend/src/pages/content/ContentEditor.tsx (1)
2-2:Details
✅ Verification successful
LGTM! Import path correctly updated for authentication hook refactoring.
The import path change aligns with the centralized AuthProvider structure.
Let me verify that all the new import paths are valid across the codebase:
🏁 Script executed:
#!/bin/bash # Description: Verify that the new AuthProvider import paths are valid and consistent # Check if AuthProvider directory and files exist echo "Checking AuthProvider directory structure:" fd -t d "AuthProvider" frontend/src/hooks/ # Check all useAuth imports to ensure consistency echo -e "\nChecking all useAuth import paths:" rg -A 1 'import.*useAuth.*from' frontend/src/ # Check for any remaining old import paths echo -e "\nChecking for any remaining old useAuth import paths:" rg 'hooks/useAuth"' frontend/src/Length of output: 3670
Import path refactor verified – changes approved
Verified that the
AuthProvider/useAuthdirectory exists and alluseAuthimports acrossfrontend/src/now point to"../../hooks/AuthProvider/useAuth". No outdated import paths remain.Approve the import path change.
• Optional improvement: In
frontend/src/pages/content/ContentEditor.tsx, consider replacingconsole.errorand localerrorstate with the new toast notifications for more consistent user feedback.frontend/src/hooks/ToastProvider/useToast.ts (1)
4-8: LGTM! Well-implemented custom hook following React best practices.The
useToasthook correctly implements the standard pattern for consuming React context with proper error handling when used outside the provider. The error message is clear and helpful for developers.frontend/src/App.tsx (2)
3-4: LGTM! Proper integration of the toast provider at the application root.The import path update and addition of
ToastProvidercorrectly establishes the toast notification system for the entire application.
64-66: Correct provider nesting order.The
ToastProvideras the outermost provider wrappingAuthProviderensures toast notifications are available throughout the entire component tree, including within authentication flows.frontend/src/components/search/SearchListResults.tsx (2)
8-8: Good addition of toast notification import.Importing the
useToasthook enables user-facing error notifications instead of console logging.
77-77: Excellent improvement to user experience with toast notifications.Replacing
console.errorcalls with user-facing toast notifications significantly improves the user experience by providing visible feedback when data fetching fails. The error messages are appropriately user-friendly.Also applies to: 125-125
frontend/src/hooks/ToastProvider/ToastContext.ts (2)
4-6: Well-defined context type interface.The
ToastContextTypeinterface clearly defines the expected shape with a singlenotifyfunction that accepts appropriate parameters for message and type.
8-10: Proper context initialization following React best practices.Creating the context with
undefinedas the initial value ensures that components must be wrapped in theToastProviderto access the toast functionality, preventing runtime errors from missing context.backend/src/modules/comment/routes/comment.routes.ts (1)
9-9: Excellent security enhancement.Adding
authenticateTokenmiddleware to all comment routes is a great security improvement that ensures only authenticated users can perform comment operations.frontend/src/components/Navbar.tsx (3)
6-6: LGTM! Import path update is consistent.The updated import path aligns with the new organized hook structure.
16-16: Excellent integration of the toast notification system.The import and instantiation of the
useToasthook is correctly implemented.Also applies to: 38-38
129-129: Significant UX improvement with toast notifications.Replacing
console.errorcalls with user-facing toast notifications greatly improves the user experience by providing immediate, visible feedback when errors occur.The error messages are clear and actionable:
- "An error occurred while searching users."
- "An error occurred while searching contents."
- "Failed to fetch subscription status."
This change aligns perfectly with the centralized toast notification system implementation.
Also applies to: 135-135, 186-186
frontend/src/pages/content/ContentView.tsx (5)
2-2: LGTM! Import path update is consistent.The updated import path aligns with the new organized hook structure.
27-27: Excellent integration of the toast notification system.The import and instantiation of the
useToasthook follows the established pattern correctly.Also applies to: 68-68
109-112: Great UX improvement with descriptive error messaging.Replacing console error logging with user-facing toast notifications provides much better feedback when creator data fetching fails.
431-436: Excellent UX improvement with toast notifications.Replacing alert dialogs with toast notifications provides a much better user experience for follow/unfollow error handling.
The error message is clear and contextual, dynamically indicating whether the error occurred during follow or unfollow operation.
211-211:Details
✅ Verification successful
Critical issue: Potential security vulnerability in delete logic.
The change from
content?.creatorUIDtouser?.uidfor the delete request could introduce a security issue or break the deletion functionality.Analysis:
- Before: Used
content?.creatorUID(the original creator's ID)- After: Uses
user?.uid(the current user's ID)- Risk: This might allow any authenticated user to delete content by sending their own ID
The backend should validate ownership, but this change seems inconsistent with the permission check on line 208:
if (localStorage.getItem("userUID") === content?.creatorUID)Please verify the backend deletion endpoint expects the current user's ID or the content creator's ID:
If the backend expects the current user's ID and validates ownership server-side, this change is correct. Otherwise, it should be reverted.
🏁 Script executed:
#!/bin/bash # Description: Check the content deletion endpoint implementation # Expected: Understand what user ID the backend expects and how it validates ownership echo "Checking content deletion controller implementation..." ast-grep --pattern $'async function $_(req, res) { $$$ delete$$$ $$$ }' echo -e "\nChecking content deletion route..." rg -A 5 -B 2 "delete.*content" backend/src/ echo -e "\nChecking user validation in content operations..." rg -A 3 -B 1 "userId.*req" backend/src/Length of output: 19442
🏁 Script executed:
#!/bin/bash # Display deleteContent handler in ContentController rg -n -A 20 -B 2 "static async deleteContent" backend/src/modules/content/controllers/content.controller.tsLength of output: 892
Ownership validation confirmed – no changes needed
The backend’s
deleteContenthandler (backend/src/modules/content/controllers/content.controller.ts lines 173–179) correctly readsuserIdfrom the request body and compares it tocreatorUIDbefore performing the deletion. Passinguser?.uidfrom the frontend aligns with this logic and does not introduce a security risk.• File: backend/src/modules/content/controllers/content.controller.ts
• Lines 173–179:
ts const { userId } = req.body; const owner_id = confirmation?.creatorUID; if (userId == owner_id) { await ContentService.deleteContent(contentId); res.status(200).json(response); } else { throw new Error("You don't have the permission to delete this!!"); }No further changes are required.
frontend/src/pages/profile/Profile.tsx (3)
3-3: Import path updates look good.The import path changes align with the new directory structure mentioned in the summary.
Also applies to: 14-14
32-32: Toast hook instantiation is correct.The toast function is properly obtained from the useToast hook.
50-53: Error handling improvements enhance user experience.All error reporting has been consistently updated to use toast notifications with user-friendly messages instead of console errors or alerts. This provides better feedback to users and aligns with the centralized notification system.
Also applies to: 105-108, 253-258, 315-318, 336-339
frontend/src/hooks/AuthProvider/AuthProvider.tsx (3)
3-8: Import path updates are consistent with the new directory structure.All imports have been properly updated to reflect the new directory organization, and the toast hook import is correctly added.
17-17: Toast notification system properly integrated.The component now uses the centralized toast system instead of managing toast state locally, which simplifies the code and provides consistency across the application.
Also applies to: 26-26, 40-40
62-64: Good error handling for user not found scenario.This addition properly handles the case where a user is not found by showing an error message, clearing the user state, and redirecting to the login page. This improves the user experience in edge cases.
frontend/src/components/content/CommentList.tsx (5)
14-15: Service and toast imports properly added.The CommentService import centralizes API calls, and the toast hook provides consistent error handling.
Also applies to: 36-36
26-28: State management simplified effectively.Consolidating editing states into a single
selectedCommentEditobject is cleaner and makes the component logic easier to follow.
49-60: Comments fetching properly refactored with service integration.The function now uses the centralized CommentService and provides user-friendly error messages via toast notifications.
89-117: Comment editing implementation is well done.The function properly handles edge cases (empty text triggers deletion) and efficiently updates local state without refetching. The UX improvement of allowing deletion by clearing text is intuitive.
123-136: Comment deletion properly implemented with local state updates.The function correctly removes the comment from local state after successful deletion, providing immediate UI feedback.
frontend/src/services/CommentService.ts (1)
1-4: LGTM! Clean imports and dependencies.The imports are well-structured and include all necessary dependencies for the service implementation.
| router.post('/:contentId', authenticateToken, createCommentController, ); | ||
| router.get( | ||
| "/:contentId", | ||
| authenticateToken, | ||
| getCommentsByPostController | ||
| ); | ||
| router.get("/:commentId", authenticateToken, getCommentByIdController); | ||
| router.put("/:commentId", authenticateToken, updateCommentController); | ||
| router.delete("/:commentId", authenticateToken, deleteCommentController); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Critical issue: Route parameter mismatch with controllers.
The simplified route parameters don't align with what the controllers expect. This will cause runtime errors.
Issue analysis:
- Routes now use
:contentIdand:commentId - Controllers still expect
post_id,comment_id, anduser_idfromreq.params - Line 18:
router.get("/:commentId", ...)conflicts with line 14'srouter.get("/:contentId", ...)
Two GET routes with similar patterns will cause routing conflicts.
Run this script to verify controller parameter usage:
Suggested fix:
Either update the controllers to use the new parameter names, or modify the routes to match controller expectations:
-router.get("/:commentId", authenticateToken, getCommentByIdController);
+router.get("/:contentId/:commentId", authenticateToken, getCommentByIdController);
-router.put("/:commentId", authenticateToken, updateCommentController);
+router.put("/:contentId/:commentId", authenticateToken, updateCommentController);
-router.delete("/:commentId", authenticateToken, deleteCommentController);
+router.delete("/:contentId/:commentId", authenticateToken, deleteCommentController);🏁 Script executed:
#!/bin/bash
# Description: Check how controllers use request parameters
# Expected: Find mismatches between route params and controller usage
echo "Checking controller parameter usage..."
ast-grep --pattern $'req.params.$_'
echo -e "\nChecking specific parameter names in controllers..."
rg -A 3 -B 1 "post_id|comment_id|user_id" backend/src/modules/comment/controllers/
echo -e "\nChecking route definitions..."
rg -A 1 -B 1 "router\.(get|post|put|delete)" backend/src/modules/comment/routes/Length of output: 7487
Fix route–controller parameter mismatch
There’s a clear mismatch between the route parameters and what the controllers are trying to read, plus two GET handlers with conflicting patterns:
- Routes define only
:contentIdand:commentId - Controllers expect
post_id,comment_idand evenuser_idinreq.params router.get("/:contentId")vsrouter.get("/:commentId")will collide at runtime
To resolve, either rename your route params to match the controllers, or (better) standardize naming and pull user_id from the authenticated token rather than the URL.
Key locations to update:
backend/src/modules/comment/routes/comment.routes.tsbackend/src/modules/comment/controllers/comment.controller.ts
Suggested diffs:
--- a/backend/src/modules/comment/routes/comment.routes.ts
+++ b/backend/src/modules/comment/routes/comment.routes.ts
@@
-router.post('/:contentId', authenticateToken, createCommentController);
+router.post('/:post_id', authenticateToken, createCommentController);
-router.get('/:contentId', authenticateToken, getCommentsByPostController);
+router.get('/:post_id', authenticateToken, getCommentsByPostController);
-router.get('/:commentId', authenticateToken, getCommentByIdController);
+router.get('/:post_id/:comment_id', authenticateToken, getCommentByIdController);
-router.put('/:commentId', authenticateToken, updateCommentController);
+router.put('/:post_id/:comment_id', authenticateToken, updateCommentController);
-router.delete('/:commentId', authenticateToken, deleteCommentController);
+router.delete('/:post_id/:comment_id', authenticateToken, deleteCommentController);--- a/backend/src/modules/comment/controllers/comment.controller.ts
+++ b/backend/src/modules/comment/controllers/comment.controller.ts
@@ createCommentController
- const { post_id } = req.params;
+ const { post_id } = req.params;
@@ getCommentByIdController
- const { post_id, comment_id } = req.params;
+ const { post_id, comment_id } = req.params;
@@ updateCommentController, deleteCommentController
- const { post_id, comment_id, user_id } = req.params;
+ const { post_id, comment_id } = req.params;
+ const user_id = req.userId; // provided by authenticateToken middlewareThis aligns route definitions with controller expectations and avoids duplicate route patterns.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In backend/src/modules/comment/routes/comment.routes.ts around lines 12 to 20,
the route parameters :contentId and :commentId do not match the parameter names
expected by the controllers (post_id, comment_id, user_id), and the two GET
routes with :contentId and :commentId conflict causing routing issues. To fix
this, rename the route parameters to match the controller expectations (e.g.,
use :post_id and :comment_id) and ensure user_id is obtained from the
authenticated token rather than the URL. Also, adjust the routes to avoid
overlapping GET patterns by clearly distinguishing the paths for fetching
comments by post and by comment ID.
| static async getPostComments(contentId: string): Promise<Comment[] | Error> { | ||
| try { | ||
| const response = await axios.get( | ||
| `${apiURL}/comment/content/${contentId}`, | ||
| { | ||
| withCredentials: true, | ||
| timeout: 5000, | ||
| } | ||
| ); | ||
|
|
||
| return response.data; | ||
| } catch (error) { | ||
| const message = | ||
| axios.isAxiosError(error) && error.response?.data?.error | ||
| ? error.response.data.error | ||
| : "Failed to get comments"; | ||
|
|
||
| return new Error(message); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Apply consistent improvements as discussed in previous methods.
This method follows the same patterns and would benefit from the same improvements regarding error handling, input validation, and configuration extraction.
🤖 Prompt for AI Agents
In frontend/src/services/CommentService.ts around lines 80 to 99, the
getPostComments method lacks input validation, consistent error handling, and
does not use centralized configuration for axios options. Add validation to
ensure contentId is a non-empty string before making the request. Extract axios
options like withCredentials and timeout into a shared config variable or
method. Improve error handling by distinguishing between different error types
and returning consistent error objects or messages as done in other methods.
| static async getComment(commentId: string): Promise<Comment | Error> { | ||
| try { | ||
| const response = await axios.get(`${apiURL}/comment/${commentId}`, { | ||
| withCredentials: true, | ||
| timeout: 5000, | ||
| }); | ||
|
|
||
| return response.data; | ||
| } catch (error) { | ||
| const message = | ||
| axios.isAxiosError(error) && error.response?.data?.error | ||
| ? error.response.data.error | ||
| : "Failed to get comment"; | ||
|
|
||
| return new Error(message); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Apply consistent improvements as discussed in previous methods.
This method follows the same patterns and would benefit from the same improvements regarding error handling, input validation, and configuration extraction.
🤖 Prompt for AI Agents
In frontend/src/services/CommentService.ts around lines 101 to 117, the
getComment method lacks input validation for commentId, does not extract
configuration values like apiURL consistently, and has basic error handling. Add
validation to ensure commentId is a non-empty string before making the request,
extract apiURL and other constants to a centralized config, and improve error
handling by logging or processing errors uniformly as done in other methods.
| static async updateComment( | ||
| commentId: string, | ||
| commentText: string | ||
| ): Promise<{ message: string } | Error> { | ||
| try { | ||
| const response = await axios.put( | ||
| `${apiURL}/comment/${commentId}`, | ||
| { text: commentText }, | ||
| { | ||
| withCredentials: true, | ||
| timeout: 5000, | ||
| } | ||
| ); | ||
|
|
||
| return response.data; | ||
| } catch (error) { | ||
| const message = | ||
| axios.isAxiosError(error) && error.response?.data?.error | ||
| ? error.response.data.error | ||
| : "Failed to update comment"; | ||
|
|
||
| return new Error(message); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Apply consistent improvements and consider extracting error handling logic.
This method has the same patterns as previous methods. Additionally, the error handling logic is duplicated across all methods.
Consider extracting a common error handler:
private static handleError(error: unknown, defaultMessage: string): never {
const message =
axios.isAxiosError(error) && error.response?.data?.error
? error.response.data.error
: defaultMessage;
throw new Error(message);
}Then use it in methods:
} catch (error) {
- const message =
- axios.isAxiosError(error) && error.response?.data?.error
- ? error.response.data.error
- : "Failed to update comment";
-
- return new Error(message);
+ this.handleError(error, "Failed to update comment");
}🤖 Prompt for AI Agents
In frontend/src/services/CommentService.ts around lines 55 to 78, the
updateComment method duplicates error handling logic found in other methods. To
fix this, extract the error handling into a private static method named
handleError that takes the error and a default message, determines the
appropriate error message, and throws a new Error. Replace the catch block in
updateComment to call this new handleError method instead of duplicating the
logic.
| export default class CommentService { | ||
| static async publishComment( | ||
| contentId: string, | ||
| commentText: string, | ||
| ownerId: string | ||
| ): Promise<{ message: string; comment: Comment } | Error> { | ||
| try { | ||
| const response = await axios.post( | ||
| `${apiURL}/comment/${contentId}`, | ||
| { | ||
| text: commentText, | ||
| ownerId: ownerId, | ||
| }, | ||
| { | ||
| withCredentials: true, | ||
| timeout: 5000, | ||
| } | ||
| ); | ||
|
|
||
| return response.data; | ||
| } catch (error) { | ||
| const message = | ||
| axios.isAxiosError(error) && error.response?.data?.error | ||
| ? error.response.data.error | ||
| : "Failed to publish comment"; | ||
|
|
||
| return new Error(message); | ||
| } | ||
| } | ||
|
|
||
| static async deleteComment( | ||
| commentId: string | ||
| ): Promise<{ message: string } | Error> { | ||
| try { | ||
| const response = await axios.delete(`${apiURL}/comment/${commentId}`, { | ||
| withCredentials: true, | ||
| timeout: 5000, | ||
| }); | ||
|
|
||
| return response.data; | ||
| } catch (error) { | ||
| const message = | ||
| axios.isAxiosError(error) && error.response?.data?.error | ||
| ? error.response.data.error | ||
| : "Failed to delete comment"; | ||
|
|
||
| return new Error(message); | ||
| } | ||
| } | ||
|
|
||
| static async updateComment( | ||
| commentId: string, | ||
| commentText: string | ||
| ): Promise<{ message: string } | Error> { | ||
| try { | ||
| const response = await axios.put( | ||
| `${apiURL}/comment/${commentId}`, | ||
| { text: commentText }, | ||
| { | ||
| withCredentials: true, | ||
| timeout: 5000, | ||
| } | ||
| ); | ||
|
|
||
| return response.data; | ||
| } catch (error) { | ||
| const message = | ||
| axios.isAxiosError(error) && error.response?.data?.error | ||
| ? error.response.data.error | ||
| : "Failed to update comment"; | ||
|
|
||
| return new Error(message); | ||
| } | ||
| } | ||
|
|
||
| static async getPostComments(contentId: string): Promise<Comment[] | Error> { | ||
| try { | ||
| const response = await axios.get( | ||
| `${apiURL}/comment/content/${contentId}`, | ||
| { | ||
| withCredentials: true, | ||
| timeout: 5000, | ||
| } | ||
| ); | ||
|
|
||
| return response.data; | ||
| } catch (error) { | ||
| const message = | ||
| axios.isAxiosError(error) && error.response?.data?.error | ||
| ? error.response.data.error | ||
| : "Failed to get comments"; | ||
|
|
||
| return new Error(message); | ||
| } | ||
| } | ||
|
|
||
| static async getComment(commentId: string): Promise<Comment | Error> { | ||
| try { | ||
| const response = await axios.get(`${apiURL}/comment/${commentId}`, { | ||
| withCredentials: true, | ||
| timeout: 5000, | ||
| }); | ||
|
|
||
| return response.data; | ||
| } catch (error) { | ||
| const message = | ||
| axios.isAxiosError(error) && error.response?.data?.error | ||
| ? error.response.data.error | ||
| : "Failed to get comment"; | ||
|
|
||
| return new Error(message); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Overall architecture suggestion: Comprehensive refactor for better maintainability.
Based on the review of individual methods, here's a comprehensive refactor suggestion that addresses all identified concerns:
import axios from "axios";
import { apiURL } from "../scripts/api";
import { Comment } from "../models/Comment";
const API_TIMEOUT = 5000;
const DEFAULT_CONFIG = {
withCredentials: true,
timeout: API_TIMEOUT,
};
class CommentServiceError extends Error {
constructor(message: string, public originalError?: unknown) {
super(message);
this.name = 'CommentServiceError';
}
}
function handleError(error: unknown, defaultMessage: string): never {
const message =
axios.isAxiosError(error) && error.response?.data?.error
? error.response.data.error
: defaultMessage;
throw new CommentServiceError(message, error);
}
export const CommentService = {
async publishComment(
contentId: string,
commentText: string,
ownerId: string
): Promise<{ message: string; comment: Comment }> {
if (!contentId || !commentText || !ownerId) {
throw new CommentServiceError('Missing required parameters');
}
try {
const response = await axios.post(
`${apiURL}/comment/${contentId}`,
{ text: commentText, ownerId },
DEFAULT_CONFIG
);
return response.data;
} catch (error) {
handleError(error, "Failed to publish comment");
}
},
async deleteComment(commentId: string): Promise<{ message: string }> {
if (!commentId) {
throw new CommentServiceError('Comment ID is required');
}
try {
const response = await axios.delete(
`${apiURL}/comment/${commentId}`,
DEFAULT_CONFIG
);
return response.data;
} catch (error) {
handleError(error, "Failed to delete comment");
}
},
// ... similar pattern for other methods
};
export default CommentService;🧰 Tools
🪛 Biome (1.9.4)
[error] 5-118: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
🤖 Prompt for AI Agents
In frontend/src/services/CommentService.ts from lines 5 to 118, refactor the
CommentService class to improve maintainability by extracting common axios
config into a constant, creating a custom CommentServiceError class for
consistent error handling, and centralizing error processing in a handleError
function that throws this custom error. Replace the class with an exported
object containing async methods that validate input parameters and use the
shared config and error handler, ensuring all methods follow this pattern for
consistency and clarity.
| static async publishComment( | ||
| contentId: string, | ||
| commentText: string, | ||
| ownerId: string | ||
| ): Promise<{ message: string; comment: Comment } | Error> { | ||
| try { | ||
| const response = await axios.post( | ||
| `${apiURL}/comment/${contentId}`, | ||
| { | ||
| text: commentText, | ||
| ownerId: ownerId, | ||
| }, | ||
| { | ||
| withCredentials: true, | ||
| timeout: 5000, | ||
| } | ||
| ); | ||
|
|
||
| return response.data; | ||
| } catch (error) { | ||
| const message = | ||
| axios.isAxiosError(error) && error.response?.data?.error | ||
| ? error.response.data.error | ||
| : "Failed to publish comment"; | ||
|
|
||
| return new Error(message); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Review error handling pattern and consider input validation.
The method implementation is functional but has some concerns:
-
Error handling pattern: Returning
Errorobjects instead of throwing exceptions is unconventional in JavaScript/TypeScript and may complicate error handling for consumers. -
Missing input validation: Consider validating required parameters before making API calls.
Consider this improved approach:
static async publishComment(
contentId: string,
commentText: string,
ownerId: string
): Promise<{ message: string; comment: Comment }> {
+ if (!contentId || !commentText || !ownerId) {
+ throw new Error('Missing required parameters');
+ }
+
try {
const response = await axios.post(
`${apiURL}/comment/${contentId}`,
{
text: commentText,
ownerId: ownerId,
},
{
withCredentials: true,
timeout: 5000,
}
);
return response.data;
} catch (error) {
const message =
axios.isAxiosError(error) && error.response?.data?.error
? error.response.data.error
: "Failed to publish comment";
- return new Error(message);
+ throw new Error(message);
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| static async publishComment( | |
| contentId: string, | |
| commentText: string, | |
| ownerId: string | |
| ): Promise<{ message: string; comment: Comment } | Error> { | |
| try { | |
| const response = await axios.post( | |
| `${apiURL}/comment/${contentId}`, | |
| { | |
| text: commentText, | |
| ownerId: ownerId, | |
| }, | |
| { | |
| withCredentials: true, | |
| timeout: 5000, | |
| } | |
| ); | |
| return response.data; | |
| } catch (error) { | |
| const message = | |
| axios.isAxiosError(error) && error.response?.data?.error | |
| ? error.response.data.error | |
| : "Failed to publish comment"; | |
| return new Error(message); | |
| } | |
| } | |
| static async publishComment( | |
| contentId: string, | |
| commentText: string, | |
| ownerId: string | |
| ): Promise<{ message: string; comment: Comment }> { | |
| + if (!contentId || !commentText || !ownerId) { | |
| + throw new Error('Missing required parameters'); | |
| + } | |
| + | |
| try { | |
| const response = await axios.post( | |
| `${apiURL}/comment/${contentId}`, | |
| { | |
| text: commentText, | |
| ownerId: ownerId, | |
| }, | |
| { | |
| withCredentials: true, | |
| timeout: 5000, | |
| } | |
| ); | |
| return response.data; | |
| } catch (error) { | |
| const message = | |
| axios.isAxiosError(error) && error.response?.data?.error | |
| ? error.response.data.error | |
| : "Failed to publish comment"; | |
| - return new Error(message); | |
| + throw new Error(message); | |
| } | |
| } |
🤖 Prompt for AI Agents
In frontend/src/services/CommentService.ts lines 6 to 33, the publishComment
method returns Error objects instead of throwing exceptions, which is
unconventional and complicates error handling. Refactor the method to throw
errors instead of returning them. Additionally, add input validation at the
start of the method to check that contentId, commentText, and ownerId are
non-empty strings, and throw descriptive errors if validation fails before
making the API call.
| static async deleteComment( | ||
| commentId: string | ||
| ): Promise<{ message: string } | Error> { | ||
| try { | ||
| const response = await axios.delete(`${apiURL}/comment/${commentId}`, { | ||
| withCredentials: true, | ||
| timeout: 5000, | ||
| }); | ||
|
|
||
| return response.data; | ||
| } catch (error) { | ||
| const message = | ||
| axios.isAxiosError(error) && error.response?.data?.error | ||
| ? error.response.data.error | ||
| : "Failed to delete comment"; | ||
|
|
||
| return new Error(message); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Apply consistent improvements from publishComment method.
This method shares the same architectural concerns as publishComment regarding error handling patterns and missing input validation.
Apply similar improvements for consistency:
static async deleteComment(
commentId: string
): Promise<{ message: string }> {
+ if (!commentId) {
+ throw new Error('Comment ID is required');
+ }
+
try {
const response = await axios.delete(`${apiURL}/comment/${commentId}`, {
withCredentials: true,
timeout: 5000,
});
return response.data;
} catch (error) {
const message =
axios.isAxiosError(error) && error.response?.data?.error
? error.response.data.error
: "Failed to delete comment";
- return new Error(message);
+ throw new Error(message);
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| static async deleteComment( | |
| commentId: string | |
| ): Promise<{ message: string } | Error> { | |
| try { | |
| const response = await axios.delete(`${apiURL}/comment/${commentId}`, { | |
| withCredentials: true, | |
| timeout: 5000, | |
| }); | |
| return response.data; | |
| } catch (error) { | |
| const message = | |
| axios.isAxiosError(error) && error.response?.data?.error | |
| ? error.response.data.error | |
| : "Failed to delete comment"; | |
| return new Error(message); | |
| } | |
| } | |
| static async deleteComment( | |
| commentId: string | |
| ): Promise<{ message: string }> { | |
| if (!commentId) { | |
| throw new Error('Comment ID is required'); | |
| } | |
| try { | |
| const response = await axios.delete(`${apiURL}/comment/${commentId}`, { | |
| withCredentials: true, | |
| timeout: 5000, | |
| }); | |
| return response.data; | |
| } catch (error) { | |
| const message = | |
| axios.isAxiosError(error) && error.response?.data?.error | |
| ? error.response.data.error | |
| : "Failed to delete comment"; | |
| throw new Error(message); | |
| } | |
| } |
🤖 Prompt for AI Agents
In frontend/src/services/CommentService.ts between lines 35 and 53, the
deleteComment method lacks input validation and consistent error handling as
seen in the publishComment method. Add validation to check if commentId is
provided and valid before making the axios request. Refactor the error handling
to unify the pattern used in publishComment, ensuring errors are properly
caught, parsed, and returned or thrown consistently. This will improve
reliability and maintainability.
Mindful-Developer
left a comment
Mindful-Developer
left a comment
There was a problem hiding this comment.
Left some inline comments on routes that should be reviewed/fixed.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
frontend/src/services/CommentService.ts (1)
12-182: 🛠️ Refactor suggestionAddress persistent architectural issues identified in previous reviews.
The implementation still contains the same architectural concerns raised in previous reviews:
- Static-only class pattern (confirmed by Biome linter)
- Unconventional error handling - returning Error objects instead of throwing
- Missing input validation for required parameters
- Duplicated error handling logic across all methods
- Repeated configuration - axios config duplicated in every method
Please refer to the comprehensive refactor suggestion from the previous review that addresses all these concerns by:
- Converting to an exported object with functions instead of a static class
- Extracting shared axios configuration
- Implementing consistent error handling with a custom error class
- Adding input validation to all methods
- Centralizing error processing logic
This refactor will significantly improve maintainability and follow TypeScript/JavaScript best practices.
🧰 Tools
🪛 Biome (1.9.4)
[error] 12-182: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
🧹 Nitpick comments (1)
backend/src/modules/content/controllers/comment.controller.ts (1)
31-31: Improve error response consistency.Error responses have inconsistent formats - some return the raw error object while others return formatted error messages.
Standardize error responses:
- res.status(500).json({ error: error }); + res.status(500).json({ error: "Failed to create comment" });Also applies to: 73-73, 92-92
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
backend/src/app.ts(0 hunks)backend/src/modules/comment/controllers/comment.controller.ts(0 hunks)backend/src/modules/content/controllers/comment.controller.ts(1 hunks)backend/src/modules/content/routes/comment.routes.ts(1 hunks)backend/src/modules/content/routes/content.routes.ts(2 hunks)backend/src/modules/content/services/comment.service.ts(1 hunks)frontend/src/services/CommentService.ts(1 hunks)
💤 Files with no reviewable changes (2)
- backend/src/app.ts
- backend/src/modules/comment/controllers/comment.controller.ts
✅ Files skipped from review due to trivial changes (2)
- backend/src/modules/content/services/comment.service.ts
- backend/src/modules/content/routes/comment.routes.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
frontend/src/services/CommentService.ts (2)
backend/src/modules/content/models/comment.model.ts (1)
Comment(12-12)frontend/src/scripts/api.ts (1)
apiURL(8-8)
🪛 Biome (1.9.4)
frontend/src/services/CommentService.ts
[error] 12-182: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
🔇 Additional comments (1)
backend/src/modules/content/routes/content.routes.ts (1)
3-3: LGTM! Clean integration of comment routes.The comment router integration follows proper Express.js patterns and maintains the modular structure of the codebase.
Also applies to: 56-57
| export async function updateCommentController(req: Request, res: Response) { | ||
| const { post_id, comment_id, user_id } = req.params; | ||
| const comment = await getComment(post_id, comment_id); | ||
| if (comment?.owner_id == user_id) { |
There was a problem hiding this comment.
Critical: Use strict equality for authorization checks.
Using loose equality (==) for user ID comparison can lead to security vulnerabilities due to type coercion. An attacker could potentially bypass authorization by exploiting JavaScript's type conversion rules.
Apply this fix:
- if (comment?.owner_id == user_id) {
+ if (comment?.owner_id === user_id) {- if (comment?.owner_id == user_id) {
+ if (comment?.owner_id === user_id) {Also applies to: 68-68
🤖 Prompt for AI Agents
In backend/src/modules/content/controllers/comment.controller.ts at lines 51 and
68, replace the loose equality operator (==) with the strict equality operator
(===) for user ID comparisons to prevent security risks from type coercion.
Update both instances to use === to ensure type-safe authorization checks.
| console.log(error); | ||
| console.log(post_id, owner_id, text); | ||
| res.status(500).json({ error: error }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove console.log statements for production.
Multiple console.log statements are present throughout the file. These should be removed or replaced with proper logging for production environments.
Consider using a proper logging library like Winston or implementing a centralized logging service instead of console.log statements.
Also applies to: 44-44, 58-58, 74-74, 93-93, 116-116
🤖 Prompt for AI Agents
In backend/src/modules/content/controllers/comment.controller.ts around lines
29-31 and also lines 44, 58, 74, 93, and 116, remove all console.log statements
as they are not suitable for production. Replace them with calls to a proper
logging library such as Winston or a centralized logging service to ensure
consistent and configurable logging. Initialize and use the logger to record
errors and important information instead of using console.log.
| const { post_id, user_id } = req.params; | ||
| const post = await ContentService.getContent(post_id); | ||
| const creator_id = post?.creatorUID; | ||
| if (creator_id == user_id) { |
There was a problem hiding this comment.
Critical: Use strict equality for authorization checks.
Same security issue with loose equality in the post deletion authorization check.
Apply this fix:
- if (creator_id == user_id) {
+ if (creator_id === user_id) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (creator_id == user_id) { | |
| if (creator_id === user_id) { |
🤖 Prompt for AI Agents
In backend/src/modules/content/controllers/comment.controller.ts at line 85,
replace the loose equality operator (==) with the strict equality operator (===)
in the authorization check comparing creator_id and user_id. This ensures
type-safe comparison and improves security by preventing unintended type
coercion during the check.
Mindful-Developer
left a comment
Mindful-Developer
left a comment
There was a problem hiding this comment.
Looks good now!
2 participants
Summary by CodeRabbit
New Features
Improvements
Refactor