Merged
Conversation
Owner
Author
|
@sourcery-ai title |
Owner
Author
|
@sourcery-ai review |
Reviewer's GuideThis PR refactors route definitions to leverage dedicated controller classes, consolidates business logic into a new controllers directory, introduces a full notification subsystem (model, controller, routes), enhances the comment module with pagination, replies, and likes, and extends the frontend Auth hook/context to support user profile and preference updates. Sequence diagram for comment creation with reply and likesequenceDiagram
actor User
participant Frontend
participant Fastify
participant Comments
participant CommentModel
participant TaskModel
participant RoomModel
participant UserModel
User->>Frontend: Submit new comment (with reply/like)
Frontend->>Fastify: POST /add/comments
Fastify->>Comments: addComment(request)
Comments->>CommentModel: Save comment
Comments->>UserModel: Update user stats
Comments->>TaskModel: Update task (if taskId)
Comments->>RoomModel: Update room (if roomId)
Comments-->>Fastify: Return populated comment
Fastify-->>Frontend: Response
User->>Frontend: Like comment
Frontend->>Fastify: POST /comments/:commentId/like
Fastify->>Comments: addLike(request)
Comments->>CommentModel: Update likes
Comments-->>Fastify: Return updated like count
Fastify-->>Frontend: Response
ER diagram for updated User, Task, and Comment modelserDiagram
USER {
string userName
string email
string password
object profile
object stats
object notification
object preference
object rooms
object myTasks
object collaborators
object comment
}
TASK {
string title
string description
string status
string priority
date dueDate
number estimatedHours
object author
object assignees
object room
object comments
}
COMMENT {
string content
object author
object task
object room
object replies
object likes
}
USER ||--o{ TASK : "author"
USER ||--o{ COMMENT : "comment"
TASK ||--o{ COMMENT : "comments"
COMMENT }o--|| USER : "author"
COMMENT }o--|| TASK : "task"
COMMENT }o--|| ROOM : "room"
COMMENT ||--o{ COMMENT : "replies"
COMMENT ||--o{ USER : "likes"
USER ||--o{ ROOM : "rooms"
ROOM ||--o{ USER : "members"
ROOM ||--o{ TASK : "tasks"
ROOM ||--o{ COMMENT : "comments"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
SourceryAI
reviewed
Sep 7, 2025
SourceryAI
left a comment
SourceryAI
left a comment
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- src/controllers/room.controller.js is missing the
import mongoosestatement before usingmongoose.startSession, which will cause runtime errors. - RoomController defines
deleteRoomtwice—remove the duplicate method at the end to avoid unintentionally overriding the intended logic. - TaskController methods reference
fastify.ioand callemitTaskNotificationdirectly—consider injecting the Fastify instance into the controller (e.g. via constructor) and usingthis.emitTaskNotificationso socket emissions work correctly.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- src/controllers/room.controller.js is missing the `import mongoose` statement before using `mongoose.startSession`, which will cause runtime errors.
- RoomController defines `deleteRoom` twice—remove the duplicate method at the end to avoid unintentionally overriding the intended logic.
- TaskController methods reference `fastify.io` and call `emitTaskNotification` directly—consider injecting the Fastify instance into the controller (e.g. via constructor) and using `this.emitTaskNotification` so socket emissions work correctly.
## Individual Comments
### Comment 1
<location> `src/controllers/user.controller.js:157` </location>
<code_context>
+ .send({ success: false, message: "Erreur au niveau de l'objet" });
+ }
+
+ const updatePrefer = await User.findByIdAndUpdate(
+ profileId,
+ { $set: { preferences } },
+ { $new: true }
+ );
</code_context>
<issue_to_address>
Incorrect option '$new' in Mongoose update may prevent returning updated document.
Use 'new: true' instead of '$new: true' to ensure the updated document is returned.
</issue_to_address>
### Comment 2
<location> `src/controllers/user.controller.js:126` </location>
<code_context>
+ $push: { replies: newReply._id },
+ });
+
+ return reply.code(201).send({
+ success: true,
+ message: "Effectué avec success",
</code_context>
<issue_to_address>
updatePreference response incorrectly sets 'success' to false on success.
Please change 'success' to true in the response to correctly indicate a successful update.
</issue_to_address>
### Comment 3
<location> `src/controllers/task.controller.js:9` </location>
<code_context>
+import User from "../models/user.model.js";
+
+export class TaskController {
+ emitTaskNotification = (events, data, userId = null) => {
+ try {
+ const notification = {
</code_context>
<issue_to_address>
emitTaskNotification references 'fastify' which is not defined in the controller context.
Please pass 'fastify' as a parameter to this function or refactor to use an alternative notification approach.
</issue_to_address>
### Comment 4
<location> `src/controllers/task.controller.js:204` </location>
<code_context>
+ session.endSession();
+
+ // Notifications
+ emitTaskNotification("taskCreated", {
+ task: savedTask,
+ message: `Nouvelle tâche créée: ${savedTask.title}`,
</code_context>
<issue_to_address>
emitTaskNotification is called without 'this.', which may cause a ReferenceError.
Call 'emitTaskNotification' with 'this.' to avoid context issues and potential ReferenceErrors.
</issue_to_address>
### Comment 5
<location> `src/controllers/task.controller.js:366` </location>
<code_context>
+ });
+
+ //Notification spécifique à la tache (si d'autre utilisateur collabore)
+ fastify.io.to(`task_${id}`).emit("taskRoomUpdate", {
+ taskId: id,
+ task: updatedTask,
</code_context>
<issue_to_address>
Direct use of 'fastify.io' in controller may break separation of concerns.
Consider moving notification logic out of the controller or injecting dependencies to maintain separation of concerns.
Suggested implementation:
```javascript
//Notification spécifique à la tache (si d'autre utilisateur collabore)
+ await taskNotificationService.notifyTaskRoomUpdate({
+ taskId: id,
+ task: updatedTask,
```
1. Create a new file `src/services/taskNotification.service.js` with a class or object that exposes a `notifyTaskRoomUpdate` method, which uses `fastify.io` to emit the event.
2. In your controller, import and instantiate (or inject) `taskNotificationService`.
3. Ensure that `fastify.io` is available to the service, either by passing it in the constructor or as a parameter to the method.
4. Update any tests or usages to use the new service.
</issue_to_address>
### Comment 6
<location> `src/controllers/task.controller.js:464` </location>
<code_context>
+ });
+
+ // Informer les utilisateurs de la tâche qu'elle a été supprimée
+ fastify.io.to(`task_${id}`).emit("taskRoomDeleted", {
+ taskId: id,
+ message: "Cette tache a été suprimé",
</code_context>
<issue_to_address>
Same issue as above: 'fastify.io' is not defined in controller context.
</issue_to_address>
### Comment 7
<location> `src/controllers/comment.controller.js:361` </location>
<code_context>
+ });
+ }
+
+ // Créer l'objet de réponse
+ const replyData = {
+ content: content.trim(),
+ author: userId,
+ likes: [],
+ };
+
+ // Ajouter la réponse au tableau replies du commentaire parent
+ parentComment.replies.push(replyData);
+ await parentComment.save();
</code_context>
<issue_to_address>
Variable 'newReply' is used before assignment in replyComment.
Referencing 'newReply' before it is defined will result in a ReferenceError.
</issue_to_address>
### Comment 8
<location> `src/controllers/comment.controller.js:373` </location>
<code_context>
+ await parentComment.save();
+
+ // Mettre à jour les statistiques de l'utilisateur
+ await User.findByIdAndUpdate(userId, {
+ $push: { comment: newReply._id },
+ });
</code_context>
<issue_to_address>
User comment statistics update in replyComment may not work as intended.
'newReply' is undefined here; use the correct reply object or its ID when updating the user's comment array.
</issue_to_address>
### Comment 9
<location> `src/controllers/comment.controller.js:378` </location>
<code_context>
+ });
+
+ // Mettre à jour les données du commentaire parent
+ await Comment.findByIdAndUpdate(commentId, {
+ $push: { replies: newReply._id },
+ });
</code_context>
<issue_to_address>
Redundant update to parent comment's replies array in replyComment.
Since the reply is already added and saved, this extra update could cause duplicates in 'parentComment.replies'.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
// Mettre à jour les données du commentaire parent
=======
// Mettre à jour les données du commentaire parent
// (Suppression de la mise à jour redondante pour éviter les doublons)
>>>>>>> REPLACE
</suggested_fix>
### Comment 10
<location> `src/controllers/comment.controller.js:382` </location>
<code_context>
+ $push: { replies: newReply._id },
+ });
+
+ return reply.code(201).send({
+ success: true,
+ message: "Effectué avec success",
</code_context>
<issue_to_address>
Response in replyComment includes 'newReply', which is not defined.
'newReply' is used but never assigned, which will cause runtime errors due to its undefined value.
</issue_to_address>
### Comment 11
<location> `src/models/user.model.js:128` </location>
<code_context>
},
],
},
+ comment: {
+ type: mongoose.Schema.Types.ObjectId,
+ ref: "Comment",
</code_context>
<issue_to_address>
User model 'comment' field is defined as ObjectId but default is an array.
Ensure the field type matches its intended use. If multiple comments are allowed, define the field as an array of ObjectIds; otherwise, update the default value to match the type.
</issue_to_address>Hi @elhalj! 👋
Thanks for trying out Sourcery by commenting with @sourcery-ai review! 🚀
Install the sourcery-ai bot to get automatic code reviews on every pull request ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+157
to
+159
| const updatePrefer = await User.findByIdAndUpdate( | ||
| profileId, | ||
| { $set: { preferences } }, |
There was a problem hiding this comment.
issue (bug_risk): Incorrect option '$new' in Mongoose update may prevent returning updated document.
Use 'new: true' instead of '$new: true' to ensure the updated document is returned.
| { new: true } | ||
| ); | ||
|
|
||
| return reply.code(201).send({ |
There was a problem hiding this comment.
issue (bug_risk): updatePreference response incorrectly sets 'success' to false on success.
Please change 'success' to true in the response to correctly indicate a successful update.
| import User from "../models/user.model.js"; | ||
|
|
||
| export class TaskController { | ||
| emitTaskNotification = (events, data, userId = null) => { |
There was a problem hiding this comment.
issue (bug_risk): emitTaskNotification references 'fastify' which is not defined in the controller context.
Please pass 'fastify' as a parameter to this function or refactor to use an alternative notification approach.
| session.endSession(); | ||
|
|
||
| // Notifications | ||
| emitTaskNotification("taskCreated", { |
There was a problem hiding this comment.
issue (bug_risk): emitTaskNotification is called without 'this.', which may cause a ReferenceError.
Call 'emitTaskNotification' with 'this.' to avoid context issues and potential ReferenceErrors.
| }); | ||
|
|
||
| //Notification spécifique à la tache (si d'autre utilisateur collabore) | ||
| fastify.io.to(`task_${id}`).emit("taskRoomUpdate", { |
There was a problem hiding this comment.
suggestion: Direct use of 'fastify.io' in controller may break separation of concerns.
Consider moving notification logic out of the controller or injecting dependencies to maintain separation of concerns.
Suggested implementation:
//Notification spécifique à la tache (si d'autre utilisateur collabore)
+ await taskNotificationService.notifyTaskRoomUpdate({
+ taskId: id,
+ task: updatedTask,- Create a new file
src/services/taskNotification.service.jswith a class or object that exposes anotifyTaskRoomUpdatemethod, which usesfastify.ioto emit the event. - In your controller, import and instantiate (or inject)
taskNotificationService. - Ensure that
fastify.iois available to the service, either by passing it in the constructor or as a parameter to the method. - Update any tests or usages to use the new service.
| async updateTask(request, reply) { | ||
| try { | ||
| const { id } = request.params; | ||
| const userId = request.user.userId; |
There was a problem hiding this comment.
suggestion (code-quality): Prefer object destructuring when accessing and using properties. (use-object-destructuring)
Suggested change
| const userId = request.user.userId; | |
| const {userId} = request.user; |
Explanation
Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.From the Airbnb Javascript Style Guide
| async delete(request, reply) { | ||
| try { | ||
| const { id } = request.params; | ||
| const userId = request.user.userId; |
There was a problem hiding this comment.
suggestion (code-quality): Prefer object destructuring when accessing and using properties. (use-object-destructuring)
Suggested change
| const userId = request.user.userId; | |
| const {userId} = request.user; |
Explanation
Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.From the Airbnb Javascript Style Guide
Comment on lines
+6
to
+18
| const users = await User.find({}) | ||
| .populate("userName email stats") | ||
| .populate({ | ||
| path: "notification", | ||
| select: "message type", | ||
| populate: { | ||
| path: "user", | ||
| select: "userName email", | ||
| }, | ||
| }) | ||
| .sort({ updatedAt: -1 }); | ||
|
|
||
| return users; |
There was a problem hiding this comment.
suggestion (code-quality): Inline variable that is immediately returned (inline-immediately-returned-variable)
Suggested change
| const users = await User.find({}) | |
| .populate("userName email stats") | |
| .populate({ | |
| path: "notification", | |
| select: "message type", | |
| populate: { | |
| path: "user", | |
| select: "userName email", | |
| }, | |
| }) | |
| .sort({ updatedAt: -1 }); | |
| return users; | |
| return await User.find({}) | |
| .populate("userName email stats") | |
| .populate({ | |
| path: "notification", | |
| select: "message type", | |
| populate: { | |
| path: "user", | |
| select: "userName email", | |
| }, | |
| }) | |
| .sort({ updatedAt: -1 }); | |
Explanation
Something that we often see in people's code is assigning to a result variableand then immediately returning it.
Returning the result directly shortens the code and removes an unnecessary
variable, reducing the mental load of reading the function.
Where intermediate variables can be useful is if they then get used as a
parameter or a condition, and the name can act like a comment on what the
variable represents. In the case where you're returning it from a function, the
function name is there to tell you what the result is, so the variable name
is unnecessary.
|
|
||
| async update(request, reply) { | ||
| const { profileId } = request.params; | ||
| const userId = request.user.userId; |
There was a problem hiding this comment.
suggestion (code-quality): Prefer object destructuring when accessing and using properties. (use-object-destructuring)
Suggested change
| const userId = request.user.userId; | |
| const {userId} = request.user; |
Explanation
Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.From the Airbnb Javascript Style Guide
|
|
||
| async updatePreference(request, reply) { | ||
| const { profileId } = request.params; | ||
| const userId = request.user.userId; |
There was a problem hiding this comment.
suggestion (code-quality): Prefer object destructuring when accessing and using properties. (use-object-destructuring)
Suggested change
| const userId = request.user.userId; | |
| const {userId} = request.user; |
Explanation
Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.From the Airbnb Javascript Style Guide
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
###Feature Back
#Add notification an comment route