Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your project has a solid foundation, and the overall structure is well-organized. However, I'm requesting a few changes because some critical email functionalities are missing, which are required to complete the user workflows.
Here are the key areas to address:
- Send Activation Email on Registration: In
src/controllers/authController.js, the activation email is not being sent after a new user is created. You need to call your mail service to send the activation link before responding with success. - Notify Old Email on Change: The requirements state that when a user changes their email, a notification must be sent to their old address. This security feature is currently missing from the
updateEmailfunction insrc/controllers/authController.js. - Generalize the Mail Service: Your
sendActivationMailfunction insrc/utils/mailService.jsis hardcoded for one purpose. This leads to sending incorrect emails, like an 'activation' email for a password reset. Please refactor it into a more generic function that accepts parameters like the subject and body, so it can be reused for different types of notifications.
Addressing these points will ensure all user flows work as expected. Great job so far!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
src/controllers/authController.js
Outdated
| await pool.query(query, values); | ||
|
|
||
| res.status(201).json({ | ||
| message: | ||
| 'Registration successful! ' + | ||
| 'Please check your email to activate your account.', | ||
| }); |
There was a problem hiding this comment.
The task requires sending an activation email after a user registers. While the user and activation token are created correctly, the call to sendActivationMail is missing here. You'll need to construct the activation link and send it to the user's email before sending the success response.
src/controllers/authController.js
Outdated
|
|
||
| const link = `http://localhost:3000/api/activate/${activationToken}`; | ||
|
|
||
| await sendActivationMail(newEmail, link); |
There was a problem hiding this comment.
According to the requirements, when a user changes their email, a notification must be sent to their old email address. This is an important security measure. Currently, an activation email is sent to the new address, but there's no notification sent to the old one (user.email), which was fetched from the database earlier.
src/utils/mailService.js
Outdated
| export const sendActivationMail = async (to, link) => { | ||
| await transporter.sendMail({ | ||
| from: process.env.EMAIL_USER, | ||
| to, | ||
| subject: 'Account activation on our service', | ||
| text: '', | ||
| html: ` | ||
| <div> | ||
| <h1>To activate your account, please follow the link.</h1> | ||
| <a href="${link}">${link}</a> | ||
| </div> | ||
| `, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
This function is well-implemented for sending activation emails. However, its name and hardcoded subject/HTML content make it difficult to reuse for other types of emails required by the application, such as password reset links or notifying the old email address of a change. Consider making this function more generic by accepting the subject and HTML body as parameters. This would allow you to handle all email-sending requirements from a single, flexible function.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Excellent work on this update! I am happy to approve your submission. You've successfully addressed all the feedback from the previous review, resulting in a robust and complete application. The mail service is now well-designed and reusable, and all required email notifications are implemented correctly. Your project now meets all the core functional requirements. Keep up the great work!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
No description provided.