solution #218

OleksandrY0 · 2025-12-28T20:38:02Z

No description provided.

mateacademy-ai-mentor

Review Summary

You've built a solid architectural foundation, and the core JWT authentication flow is well-implemented. However, changes are needed because several critical features are missing and there's a significant bug in the error handling.

Here are the main points to address:

Missing Core Features: The password reset and profile management (changing name, password, email) functionalities are not implemented. You'll need to add the corresponding routes, controllers, and service logic.
Incomplete Registration: The name field is a requirement for registration but is missing from the user.model.js and the registration logic in auth.controller.js.
Login Logic: The login process in auth.controller.js must check if a user's account has been activated before allowing them to log in.
Critical Error Handling Bug: In app-back/src/middlewares/error.midleware.js, you must add a return after sending the JSON response for an ApiError to prevent the server from crashing.
Missing 404 Handler: The application lacks a catch-all route in index.js to handle requests for non-existent pages as required.

Once these key features are implemented, your project will be in great shape. Keep up the good work!

✨ Please rate this review in the Platform's notification center¹. 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 button² 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 tag³.
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.

mateacademy-ai-mentor · 2025-12-28T20:40:05Z

app-back/src/index.js

+app.use(cookieParser());
+
+app.use('/auth', authRouter);
+app.use('/users', userRouter);


The task requires you to provide a 404 page for all other pages. You should add a catch-all middleware here to handle any requests that don't match the /auth or /users routes.

mateacademy-ai-mentor · 2025-12-28T20:40:05Z

app-back/src/controllers/auth.controller.js

+};
+
+const register = async (req, res) => {
+  const { email, password } = req.body;


The task requires registration with name, email, and password. The name field is missing from this implementation. You'll need to update this to accept the user's name, and likely update your user model and service as well.

mateacademy-ai-mentor · 2025-12-28T20:40:05Z

app-back/src/controllers/auth.controller.js

+
+  if (!user) {
+    return res.sendStatus(404);
+  }


The requirements state that if a user is not active, you should ask them to activate their email. Before proceeding with the password check, you should verify if the user's account has been activated (i.e., if user.activationToken is null).

mateacademy-ai-mentor · 2025-12-28T20:40:06Z

app-back/src/controllers/user.controller.js

+export const userController = {
+  getAllActivated,
+};


This controller is missing the required functions for profile management. According to the requirements, you need to implement endpoints that allow an authenticated user to change their name, password, and email.

mateacademy-ai-mentor · 2025-12-28T20:40:06Z

app-back/src/middlewares/error.midleware.js

+  if (error instanceof ApiError) {
+    res.status(error.status).send({
+      message: error.message,
+      errors: error.message,


In your ApiError class, you have a distinct errors property. To provide more detailed error information to the client, you should use error.errors here instead of repeating error.message.

mateacademy-ai-mentor · 2025-12-28T20:40:06Z

app-back/src/services/email.service.js

+export const emailService = {
+  sendActivationEmail,
+  send,
+};


This service is missing functions for other required email notifications. According to the task description, you also need to send an email for password resets and another to notify a user when their email address has been changed.

mateacademy-ai-mentor · 2025-12-28T20:40:06Z

app-back/src/setup.js

@@ -0,0 +1,3 @@
+import { client } from './utils/db.utils.js';
+
+client.sync({ force: true });


Using { force: true } is great for development as it drops and recreates tables, ensuring a clean state. However, it's crucial to remember that this is a destructive operation. If this script were ever run against a production database, it would wipe out all data. It's a good practice to prevent such scripts from running in production, for example, by checking if process.env.NODE_ENV !== 'production'.

mateacademy-ai-mentor · 2025-12-28T20:40:06Z

app-back/src/services/user.service.js

+function normalizedUser({ id, email }) {
+  return { id, email };


The task requirements specify that a user has a name. This normalization function should also include the user's name in the returned object.

mateacademy-ai-mentor · 2025-12-28T20:40:06Z

app-back/src/services/user.service.js

+  });
+}
+
+async function register(email, password) {


According to the task description, registration must use name, email, and password. This function is missing the name parameter and the logic to save it to the new user record.

mateacademy-ai-mentor · 2025-12-28T20:40:06Z

app-back/src/services/user.service.js

+export const userService = {
+  register,
+  getAllActivated,
+  normalizedUser,
+  findByEmail,
+};


This service is missing the implementation for several key features. You need to add functions to handle the password reset flow and to allow users to update their profile information (name, password, and email) as required by the task.

mateacademy-ai-mentor

Review Summary

Great progress on the application! You've implemented many of the core features and the overall structure is looking good. However, a few critical requirements are still missing, so I'm requesting some changes before approval.

Here are the main issues to address:

Runtime Error: The application will crash when trying to manage the user profile because ApiError is used without being imported in src/controllers/user.controller.js.
Missing 404 Handler: The catch-all route for handling non-existent pages in src/index.js is still commented out. This is a core requirement that needs to be implemented.
Incomplete Password Logic: Both the password reset (auth.controller.js) and change password (user.controller.js) features are missing the required validation to check the new password against a confirmation field.
Missing Email Change Notification: When a user changes their email, the old email address must be notified. This functionality is missing from user.service.js and email.service.js.

Please fix these issues to meet the task requirements. You're very close to finishing!

✨ Please rate this review in the Platform's notification center¹. 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 button² 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 tag³.
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.

mateacademy-ai-mentor · 2025-12-29T01:02:05Z

app-back/.github/workflows/test.yml

+    - uses: actions/checkout@v2
+    - name: Use Node.js ${{ matrix.node-version }}
+      uses: actions/setup-node@v1


It's a good practice to use the latest major versions of GitHub Actions to benefit from new features, bug fixes, and performance improvements. Consider updating actions/checkout to v4 and actions/setup-node to v4. The v1 version of setup-node is quite outdated and may not support newer Node.js features efficiently.

mateacademy-ai-mentor · 2025-12-29T01:02:05Z

app-back/src/index.js

+// app.all('/', (req, res) => {
+//   res.status(404).json({ message: 'Page not found' });
+// });


The catch-all route for handling 404 errors is commented out, which means the application doesn't meet the requirement to display a 404 page for non-existent routes. This handler should be placed after all other routes to function correctly. Also, consider using app.all('*', ...) to catch all paths and methods that haven't been matched by other routes.

mateacademy-ai-mentor · 2025-12-29T01:02:06Z

app-back/src/controllers/user.controller.js

+  const userId = req.user.id;
+
+  if (!name || name.trim().length < 2) {
+    throw ApiError.badRequest('Name is invalid');


The ApiError class is used here, but it hasn't been imported into this file. This will cause a ReferenceError at runtime.

mateacademy-ai-mentor · 2025-12-29T01:02:06Z

app-back/src/controllers/user.controller.js

+};
+
+const changePassword = async (req, res) => {
+  const { oldPassword, newPassword } = req.body;


The requirements state that changing a password requires an old password, a new password, and a confirmation field. This implementation is missing the logic to check if newPassword and its confirmation match.

mateacademy-ai-mentor · 2025-12-29T01:02:06Z

app-back/src/controllers/user.controller.js

+};
+
+const changeEmail = async (req, res) => {
+  const { password, newEmail } = req.body;


According to the task description, the user must confirm their new email address. This controller is missing the validation to check the newEmail against a confirmation field from the request body.

mateacademy-ai-mentor · 2025-12-29T01:02:06Z

app-back/src/services/jwt.service.js

+
+function sign(user) {
+  const token = jwt.sign(user, process.env.JWT_KEY, {
+    expiresIn: '5s',


An expiration of 5 seconds is very short for an access token and might lead to a poor user experience, as it would require frequent refreshing. While this is fine for testing purposes, you might want to consider a longer duration, like '15m' or '30m', for a real application.

mateacademy-ai-mentor · 2025-12-29T01:02:06Z

app-back/src/services/email.service.js

+export const emailService = {
+  sendActivationEmail,
+  sendResetPasswordEmail,
+  send,
+};


The task requirements state that when a user changes their email, a notification must be sent to the old email address. This service is missing a function to handle that specific notification. Consider adding a new function, like sendEmailChangeNotification(oldEmail, newEmail), to create and send this email.

mateacademy-ai-mentor · 2025-12-29T01:02:06Z

app-back/src/services/user.service.js

+async function changeEmail(user, email) {
+  user.email = email;
+
+  await user.save();
+
+  return user;


This function is missing a crucial validation step. Before updating the email, you must check if the new email address is already in use by another user. If it is, you should throw an ApiError.badRequest to prevent a unique constraint violation in the database.

mateacademy-ai-mentor · 2025-12-29T01:02:06Z

app-back/src/services/user.service.js

+async function changeEmail(user, email) {
+  user.email = email;
+
+  await user.save();
+
+  return user;


According to the task requirements, when an email is changed, a notification must be sent to the old email address. This implementation is missing that step. You should capture the old email address from the user object before updating it and then call a method on the emailService to send the notification.

mateacademy-ai-mentor · 2025-12-29T01:02:06Z

app-back/src/setup.js

@@ -0,0 +1,3 @@
+import { client } from './utils/db.utils.js';
+
+client.sync({ force: true });


Using { force: true } is a destructive operation that drops all existing tables and recreates them from the models. While this is very useful for development and testing to ensure a clean state, it must never be used in a production environment as it will cause irreversible data loss.

mateacademy-ai-mentor

Review Summary

Excellent progress on the application! You've successfully addressed most of the previous feedback, which is great to see. I am approving this submission as an educational override to help you move forward, but please take a moment to review a few critical points.

In app-back/src/controllers/user.controller.js, you should throw the ApiError instead of returning it. Returning the error prevents our error-handling middleware from catching it, which will cause the request to hang without a response.
The 404 handler in app-back/src/index.js currently only catches requests to the root path (/). To meet the requirement, it needs to be a true catch-all for any unhandled route, like app.all('*', ...).
There's a small bit of redundant password validation in app-back/src/controllers/auth.controller.js that can be removed for cleaner code.

Keep up the fantastic work, you're very close to a perfect solution!

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Rate AI review example ↩

mateacademy-ai-mentor · 2025-12-29T01:19:46Z

app-back/src/controllers/user.controller.js

+  const userId = req.user.id;
+
+  if (authController.validatePassword(newPassword)) {
+    return ApiError.badRequest();


When validatePassword returns an error message (a truthy value), you should throw the ApiError instead of returning it. The catchError utility is designed to catch thrown errors, but returning a value here will not send a proper response to the client.

mateacademy-ai-mentor · 2025-12-29T01:19:46Z

app-back/src/controllers/user.controller.js

+  }
+
+  if (newPassword !== confirmPassword) {
+    return ApiError.badRequest();


Similar to the previous point, you should throw ApiError.badRequest() here. Returning the error object directly will not be handled correctly by Express or your error middleware.

mateacademy-ai-mentor · 2025-12-29T01:19:46Z

app-back/src/controllers/auth.controller.js

+  if (validatePassword(password)) {
+    return res
+      .status(400)
+      .json({ message: 'Password must be at least 6 characters' });
+  }


This password validation check is redundant because the same check is already performed on line 148. You can safely remove this block of code.

mateacademy-ai-mentor · 2025-12-29T01:19:46Z

app-back/src/index.js

+app.all('/', (req, res) => {
+  res.status(404).json({ message: 'Page not found' });
+});


This route will only handle requests to / and return a 404. To meet the requirement of handling all non-existent pages, this should be a catch-all route like app.all('*', ...). Make sure to place it after all other route definitions.

solution

4c93a60

mateacademy-ai-mentor suggested changes Dec 28, 2025

View reviewed changes

solution

44159c9

OleksandrY0 requested a review from mateacademy-ai-mentor December 29, 2025 00:59

mateacademy-ai-mentor suggested changes Dec 29, 2025

View reviewed changes

solution

124a463

OleksandrY0 requested a review from mateacademy-ai-mentor December 29, 2025 01:17

mateacademy-ai-mentor approved these changes Dec 29, 2025

View reviewed changes

		@@ -0,0 +1,3 @@
		import { client } from './utils/db.utils.js';

		client.sync({ force: true });

		function normalizedUser({ id, email }) {
		return { id, email };

solution #218

Are you sure you want to change the base?

solution #218

Uh oh!

Conversation

OleksandrY0 commented Dec 28, 2025

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants