add api basics#278
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation is missing approximately 90% of the required functionality. Only the most basic registration endpoint exists, but critical authentication features are completely absent: - Password validation: No logic to validate against required rules (min length, special characters, etc.) and users aren't informed about these rules. Additionally, passwords are stored as plain text instead of being hashed with bcrypt. - Activation flow: No isActive field in the User model, no activation token generation, no email sending, and no /activate route. - All authentication routes: Missing login (/login), logout (/logout), password reset (request + confirmation + success), and profile (/profile) endpoints. - Authentication middleware: No middleware to protect routes (profile/logout should require auth; login/registration should be for non-authenticated users only). - 404 handler: No catch-all handler for undefined routes as required by the task. Also fix this blocker: Your .eslintrc.js uses CommonJS syntax (module.exports) but package.json has `
✨ 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
| import dotenv from 'dotenv'; | ||
| import { client } from './src/backend/src/utils/db.js'; | ||
|
|
||
| dotenv.config({ path: './src/backend/.env' }); |
There was a problem hiding this comment.
Missing: Registration route requires authController.register only. Need to implement and add routes for: activation, login, logout, password reset (request email, reset confirmation, success), and profile page.
| import { authRouter } from './routes/auth.route.js'; | ||
|
|
||
| const PORT = process.env.PORT || 3005; | ||
|
|
There was a problem hiding this comment.
Missing password validation. Requirements state you must inform users about password rules and check/validate against them. Add validation logic (e.g., min length, special characters) before creating the user.
| import { authRouter } from './routes/auth.route.js'; | ||
|
|
||
| const PORT = process.env.PORT || 3005; | ||
|
|
||
| const app = express(); | ||
|
|
There was a problem hiding this comment.
Missing activation email sending. Requirements state 'send an activation email' after registration. Implement email sending logic with an activation token.
| @@ -0,0 +1,16 @@ | |||
| import express from 'express'; | |||
There was a problem hiding this comment.
Missing routes for: login, logout, activation, password reset (request, confirm, success), and profile. Only registration route exists.
| import express from 'express'; | ||
| import 'dotenv/config'; | ||
| import { authRouter } from './routes/auth.route.js'; | ||
|
|
||
| const PORT = process.env.PORT || 3005; | ||
|
|
||
| const app = express(); | ||
|
|
||
| app.use(express.json()); | ||
|
|
||
| app.use(authRouter); |
There was a problem hiding this comment.
Missing authentication middleware. No implementation for session management, protected routes, or checking if users are authenticated/unauthenticated as required.
| const register = async (req, res) => { | ||
| const { name, email, password } = req.body; | ||
|
|
||
| const newUser = await User.create({ name, email, password }); | ||
|
|
||
| res.send(newUser); |
There was a problem hiding this comment.
Registration is missing activation email sending functionality
| import { Sequelize } from 'sequelize'; | ||
|
|
||
| export const client = new Sequelize({ | ||
| host: process.env.DB_HOST, | ||
| username: process.env.DB_USER, | ||
| password: process.env.DB_PASSWORD + '', |
There was a problem hiding this comment.
Missing routes per task requirements: activation (/activate), login (/login), logout (/logout), password reset (/reset-password, /reset-password/confirm), profile page, and 404 handler.
| import { Sequelize } from 'sequelize'; | ||
|
|
||
| export const client = new Sequelize({ | ||
| host: process.env.DB_HOST, | ||
| username: process.env.DB_USER, | ||
| password: process.env.DB_PASSWORD + '', |
There was a problem hiding this comment.
Missing: Authentication middleware is not used. Routes like /login, /registration, /reset-password should only be accessible to non-authenticated users, while /profile, /logout should require authentication.
| import express from 'express'; | ||
| import { authController } from '../controllers/auth.controller.js'; | ||
|
|
||
| export const authRouter = new express.Router(); | ||
|
|
||
| authRouter.post('/registration', authController.register); |
There was a problem hiding this comment.
Missing routes: Only /registration exists. Required routes not implemented: login (/login), logout (/logout), activation (/activate), password reset (request, reset page, success), and profile page.
|
|
||
| authRouter.post('/registration', authController.register); |
There was a problem hiding this comment.
The + '' on password is unnecessary - process.env.DB_PASSWORD is already a string. This could be simplified to just password: process.env.DB_PASSWORD.
|
There is a minor issue in the email change flow. When the user opens the email change confirmation link, the email is successfully updated and the backend correctly returns status 200. However, after that, an additional refresh/auth request is triggered automatically, which causes confirmEmailChange to run again with the same token. Since the token has already been cleared after the first successful confirmation, the second request results in User not found. The main functionality works correctly, but there is still an edge-case issue related to repeated requests after the confirmation flow. |
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation covers most authentication features well, but two critical requirements from the task are not met:
-
Missing 404 catch-all handler: The task explicitly requires "404 for all the other pages" but
src/backend/src/index.jshas no catch-all route likeapp.use((req, res) => res.status(404).send({ message: 'Not found' }))to handle undefined endpoints. -
Missing password rules display: The task explicitly requires "Inform the users about the rules for a password and check them", but the registration page never shows password requirements (minimum length, special characters, uppercase, etc.). Users must guess what constitutes a valid password.
Additionally, these medium issues should be addressed:
src/backend/src/controllers/profile.controller.jsline 85: missingawaitbeforeuser.save()causing potential race conditionssrc/backend/src/pages/ActivationPage.tsx:navigate('/profile')is called before the activation promise resolves, causing premature navigationsrc/frontend/src/pages/ProfilePage/components/ProfileChangePassword.tsxline 68: h3 heading says "Change email" but should say "Change password"- Backend's
changePasswordendpoint doesn't validate password confirmation (should checknewPassword === confirmation)
The linting failure in check runs is due to ESLint configuration issues (flat config vs deprecated flags), not your code. Once the missing 404 handler and password rules display are added, the implementation should meet all task requirements.
✨ 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
| import { User } from './src/models/user.js' | ||
| import { Token } from './src/models/token.js' |
There was a problem hiding this comment.
The import path './src/models/user.js' is incorrect. Since setup.js is already in 'src/backend/', the correct path should be './models/user.js'. This same issue applies to other imports in this file.
| import dotenv from 'dotenv/config'; | ||
| import { User } from './src/models/user.js' | ||
| import { Token } from './src/models/token.js' | ||
| import { client } from './src/utils/db.js'; | ||
|
|
||
|
|
||
| async function setup() { |
There was a problem hiding this comment.
The auth routes (registration, login, password reset) should only be accessible to non-authenticated users. The backend doesn't implement middleware to check if a user is already authenticated and redirect them.
| import { User } from '../models/user.js'; | ||
| import 'dotenv/config'; | ||
| import { userService } from '../services/user.service.js'; | ||
| import { jwtService } from '../services/jwt.service.js'; | ||
| import { ApiError } from '../exeptions/api.error.js'; | ||
| import bcrypt from 'bcrypt'; | ||
| import { tokenService } from '../services/token.service.js'; | ||
| import { emailService } from '../services/email.service.js'; | ||
| import { validateValues } from '../utils/validateValues.js'; | ||
|
|
||
| const register = async (req, res) => { | ||
| const { name, email, password } = req.body; | ||
|
|
||
| const errors = { | ||
| name: validateValues.name(name), | ||
| email: validateValues.email(email), | ||
| password: validateValues.password(password), | ||
| }; | ||
|
|
||
| if (errors.name || errors.email || errors.password) { | ||
| throw ApiError.badRequest('Bad request', errors); | ||
| } | ||
|
|
||
| const hashedPassword = await bcrypt.hash(password, 10); | ||
|
|
||
| await userService.register(name, email, hashedPassword); | ||
|
|
||
| res.send({ message: 'OK' }); |
There was a problem hiding this comment.
Missing 404 handler: There is no catch-all route handler for undefined pages. The task explicitly requires '404 for all the other pages'. Add app.use((req, res) => res.status(404).send({ message: 'Not found' })) before the error middleware and after other routes to handle requests to non-existent endpoints.
| export class ApiError extends Error { | ||
| constructor({ message, status }) { | ||
| super(message); | ||
|
|
||
| this.status = status; | ||
| } | ||
|
|
||
| static badRequest(message) { | ||
| return new ApiError({ | ||
| message, | ||
| status: 400, | ||
| }); | ||
| } | ||
|
|
||
| static unauthorized(message) { | ||
| return new ApiError({ | ||
| message, | ||
| status: 401, | ||
| }); | ||
| } | ||
|
|
||
| static notFound(message) { | ||
| return new ApiError({ | ||
| message, | ||
| status: 404, | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing 404 handler: The task explicitly requires '404 for all the other pages'. No catch-all route exists for undefined endpoints. Add app.use((req, res) => res.status(404).send({ message: 'Not found' })) after all routes but before errorMiddleware.
| @@ -0,0 +1,28 @@ | |||
| export class ApiError extends Error { | |||
There was a problem hiding this comment.
Unused import: The verify import from 'node:crypto' on line 1 is never used in this file. Remove it to avoid linting errors.
|
|
||
| const changePassword = async (req, res) => { | ||
| const { currentPassword, newPassword } = req.body; | ||
|
|
||
| const currentPasswordError = validateValues.password(currentPassword); | ||
| const newPasswordError = validateValues.password(newPassword); | ||
|
|
||
| if (currentPasswordError) { | ||
| throw ApiError.badRequest(currentPasswordError); | ||
| } | ||
|
|
||
| if (newPasswordError) { | ||
| throw ApiError.badRequest(newPasswordError); | ||
| } | ||
|
|
||
| const user = await userService.findById(req.user.id); | ||
|
|
||
| const isCurrentPasswordCorrect = await bcrypt.compare( | ||
| currentPassword, | ||
| user.password, | ||
| ); | ||
|
|
||
| if (!isCurrentPasswordCorrect) { | ||
| throw ApiError.badRequest('Incorrect current password'); | ||
| } | ||
|
|
||
| const newHashedPassword = await bcrypt.hash(newPassword, 10); | ||
|
|
||
| user.password = newHashedPassword; | ||
| await user.save(); | ||
|
|
||
| await tokenService.generateTokens(res, user); |
There was a problem hiding this comment.
The task requires changing password with 'old one, new password and confirmation'. The backend only accepts currentPassword and newPassword but does not require or validate confirmation. The backend should also validate that newPassword === confirmation before updating.
| user.pendingEmail = newEmail; | ||
| user.save(); |
There was a problem hiding this comment.
Missing await before user.save() - this is a fire-and-forget operation that could cause issues.
| })) | ||
| app.use(authRouter); | ||
| app.use('/profile', profileRouter) | ||
|
|
||
| app.use(errorMiddleware); | ||
|
|
||
| app.listen(PORT, () => { | ||
| // eslint-disable-next-line no-console | ||
| console.log(`Server is running on port:${PORT}`); | ||
| }); |
There was a problem hiding this comment.
Missing 404 catch-all handler: The task explicitly requires '404 for all the other pages'. Add app.use((req, res) => res.status(404).send({ message: 'Not found' })) after app.use(errorMiddleware) and before app.listen().
| @@ -0,0 +1,26 @@ | |||
| import { verify } from 'node:crypto'; | |||
There was a problem hiding this comment.
Unused import: verify from 'node:crypto' is imported but never used. This should be removed to avoid dead code.
| import { DataTypes } from 'sequelize'; | ||
| import { client } from '../utils/db.js'; | ||
|
|
||
| export const User = client.define('user', { | ||
| name: { | ||
| type: DataTypes.STRING, | ||
| allowNull: false, | ||
| }, | ||
| email: { | ||
| type: DataTypes.STRING, | ||
| allowNull: false, | ||
| unique: true, | ||
| }, | ||
| password: { |
There was a problem hiding this comment.
Missing 404 handler: The task explicitly requires '404 for all the other pages'. No catch-all route exists in index.js to handle undefined endpoints. Add app.use((req, res) => res.status(404).send({ message: 'Not found' })) before errorMiddleware.
| import { DataTypes } from 'sequelize'; | ||
| import { client } from '../utils/db.js'; | ||
|
|
||
| export const User = client.define('user', { | ||
| name: { | ||
| type: DataTypes.STRING, | ||
| allowNull: false, | ||
| }, | ||
| email: { | ||
| type: DataTypes.STRING, | ||
| allowNull: false, | ||
| unique: true, | ||
| }, | ||
| password: { | ||
| type: DataTypes.STRING, | ||
| allowNull: false, | ||
| }, | ||
| activationToken: { | ||
| type: DataTypes.STRING, | ||
| }, | ||
| resetToken: { | ||
| type: DataTypes.STRING, | ||
| }, | ||
| pendingEmail: { | ||
| type: DataTypes.STRING, | ||
| }, | ||
| emailChangeToken: { | ||
| type: DataTypes.STRING, | ||
| } |
There was a problem hiding this comment.
Missing isActive field: The task mentions checking if user is active and redirecting them if not. The User model doesn't have an explicit isActive boolean field - it uses activationToken being null to indicate active status. This works, but consider if an explicit isActive field would be clearer.
| import { DataTypes } from 'sequelize'; | ||
| import { client } from '../utils/db.js'; | ||
|
|
||
| export const User = client.define('user', { | ||
| name: { | ||
| type: DataTypes.STRING, | ||
| allowNull: false, | ||
| }, |
There was a problem hiding this comment.
Missing password confirmation validation: The task requires profile password change to require 'old one, new password and confirmation'. The backend changePassword only accepts currentPassword and newPassword without validating confirmation. While the frontend validates this, the backend should enforce it too.
| import { DataTypes } from 'sequelize'; | ||
| import { client } from '../utils/db.js'; | ||
|
|
||
| export const User = client.define('user', { | ||
| name: { | ||
| type: DataTypes.STRING, | ||
| allowNull: false, | ||
| }, |
There was a problem hiding this comment.
The password validation function only checks for minimum length (6 characters). The task requires informing users about password rules and validating against them. Consider adding rules like special characters, uppercase, etc.
| @@ -0,0 +1,13 @@ | |||
| import { DataTypes } from 'sequelize'; | |||
| import { client } from '../utils/db.js'; | |||
| import { User } from './user.js'; | |||
There was a problem hiding this comment.
Syntax error: Missing semicolon at the end of the import statement. This will cause the module to fail to load. Change line 3 to: import { catchError } from '../utils/catchError.js';
|
|
||
| if (error) { | ||
| res.statusCode = 500; | ||
| res.send({ | ||
| message: 'Server error', | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
Unreachable code: The next() call on line 20 is never reached because res.send() on line 15 already sends a response. Additionally, when error is falsy, nothing is sent and next() is still called. Consider removing the next() call or restructuring the logic.
| import { ApiError } from '../exeptions/api.error.js'; | ||
|
|
||
| export const errorMiddleware = (error, req, res, next) => { | ||
|
|
||
| if (error instanceof ApiError) { | ||
| res.status(error.status).send({ | ||
| message: error.message, | ||
| errors: error.errors, | ||
| }); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| if (error) { |
There was a problem hiding this comment.
Routes for non-authenticated users (registration, login, password reset) should include middleware to check if a user is already authenticated and redirect them. Currently, authenticated users could still access these routes.
| @@ -0,0 +1,37 @@ | |||
| import jwt from 'jsonwebtoken'; | |||
| import 'dotenv/config'; | |||
There was a problem hiding this comment.
Syntax error: Missing semicolon at the end of the import statement. This will prevent the module from loading. Change line 2 to: import { catchError } from '../utils/catchError.js';
| } | ||
|
|
||
| function verify(token) { | ||
| try { |
There was a problem hiding this comment.
The confirm-email-change route is correctly placed outside authMiddleware since users access it via email link. However, if an authenticated user accesses this route directly, they should be redirected to their profile. Consider adding logic to check if user is already authenticated.
| @@ -0,0 +1,88 @@ | |||
| import 'dotenv/config'; | |||
| import nodemailer from 'nodemailer'; | |||
There was a problem hiding this comment.
Missing semicolon: The import statement on line 2 is missing a semicolon. Change to: import { catchError } from '../utils/catchError.js';
| const html = ` | ||
| <h2>Activate account</h1> | ||
| <a href="${href}">${href}</a> |
There was a problem hiding this comment.
HTML tag mismatch: The h2 opening tag on line 23 is closed with </h1>. This should be </h2>.
| const html = ` | ||
| <h2>Reset password</h1> | ||
| <a href="${href}">${href}</a> |
There was a problem hiding this comment.
HTML tag mismatch: The h2 opening tag on line 37 is closed with </h1>. This should be </h2>.
| const html = ` | ||
| <h2>Change email</h1> | ||
| <a href="${href}">${href}</a> |
There was a problem hiding this comment.
HTML tag mismatch: The h2 opening tag on line 51 is closed with </h1>. This should be </h2>.
| @@ -0,0 +1,11 @@ | |||
| import express from 'express'; | |||
| import { catchError } from '../utils/catchError.js' | |||
There was a problem hiding this comment.
Syntax error: Missing semicolon at the end of the import statement on line 2. Change to: import { catchError } from '../utils/catchError.js';
| import express from 'express'; | ||
| import { catchError } from '../utils/catchError.js' | ||
| import { profileController } from '../controllers/profile.controller.js'; | ||
| import { authMiddleware } from '../middlewares/authMiddleware.js'; | ||
|
|
||
| export const profileRouter = new express.Router(); | ||
|
|
||
| profileRouter.patch('/name', authMiddleware, catchError(profileController.changeName)); | ||
| profileRouter.patch('/password', authMiddleware, catchError(profileController.changePassword)); | ||
| profileRouter.patch('/email', authMiddleware, catchError(profileController.requestEmailChange)); | ||
| profileRouter.get('/confirm-email-change/:emailChangeToken', catchError(profileController.confirmEmailChange)); |
There was a problem hiding this comment.
Missing 404 handler: The task explicitly requires '404 for all the other pages'. Add app.use((req, res) => res.status(404).send({ message: 'Not found' })) to index.js after all routes.
| @@ -0,0 +1,54 @@ | |||
| import { Token } from '../models/token.js'; | |||
| import { userService } from '../services/user.service.js'; | |||
There was a problem hiding this comment.
Syntax error: Missing semicolon at the end of the import statement. This will cause the module to fail to load. Change line 2 to: import { catchError } from '../utils/catchError.js';
| where: { userId }, | ||
| }); | ||
|
|
There was a problem hiding this comment.
The password change endpoint doesn't require or validate password confirmation. The task requires 'new password and confirmation' fields. While the frontend validates this, the backend should also enforce it by checking that newPassword === confirmation before updating.
| import { Token } from '../models/token.js'; | ||
| import { userService } from '../services/user.service.js'; | ||
| import { jwtService } from '../services/jwt.service.js'; | ||
|
|
||
| async function save(userId, refreshToken) { | ||
| const token = await Token.findOne({ | ||
| where: { userId }, | ||
| }); | ||
|
|
||
| if (token) { | ||
| token.refreshToken = refreshToken; |
There was a problem hiding this comment.
Missing 404 handler: The task explicitly requires '404 for all the other pages'. No catch-all route exists in index.js to handle undefined endpoints. Add app.use((req, res) => res.status(404).send({ message: 'Not found' })) in index.js before errorMiddleware.
| @@ -0,0 +1,78 @@ | |||
| import { where } from 'sequelize'; | |||
There was a problem hiding this comment.
Unused import: where is imported from 'sequelize' on line 1 but never used in this file. Remove it to avoid dead code.
|
|
||
| async function register(name, email, password) { | ||
| const existUser = await findByEmail(email); | ||
|
|
||
| if (existUser) { | ||
| throw ApiError.badRequest('User already exists'); | ||
| } | ||
|
|
||
| const activationToken = uuidvv4(); | ||
|
|
||
| await User.create({ |
There was a problem hiding this comment.
Password validation too minimal: The password function only checks for minimum 6 characters. The task requires informing users about password rules and validating against them (special characters, uppercase, etc.). Add rules like: at least one uppercase, one lowercase, one number, one special character, minimum 8 characters.
| import { User } from '../models/user.js'; | ||
| import { emailService } from '../services/email.service.js'; | ||
| import { v4 as uuidvv4 } from 'uuid'; | ||
|
|
There was a problem hiding this comment.
Unnecessary workaround: Adding + '' to convert DB_PASSWORD assumes it's a number, but environment variables are always strings. Simply use process.env.DB_PASSWORD directly.
| import { where } from 'sequelize'; | ||
| import { ApiError } from '../exeptions/api.error.js'; | ||
| import { User } from '../models/user.js'; | ||
| import { emailService } from '../services/email.service.js'; | ||
| import { v4 as uuidvv4 } from 'uuid'; | ||
|
|
||
| function getAllActivated() { | ||
| return User.findAll({ | ||
| where: { | ||
| activationToken: null, | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| function normalizeData({ id, name, email }) { | ||
| return { id, name, email }; | ||
| } | ||
|
|
||
| function findByEmail(email) { | ||
| return User.findOne({ where: { email } }); | ||
| } | ||
|
|
||
| function findById(id) { | ||
| return User.findByPk(id); | ||
| } | ||
|
|
||
| async function register(name, email, password) { | ||
| const existUser = await findByEmail(email); | ||
|
|
||
| if (existUser) { | ||
| throw ApiError.badRequest('User already exists'); | ||
| } | ||
|
|
||
| const activationToken = uuidvv4(); | ||
|
|
||
| await User.create({ | ||
| name, | ||
| email, | ||
| password, | ||
| activationToken, | ||
| }); | ||
|
|
||
| await emailService.sendActivationEmail(email, activationToken); |
There was a problem hiding this comment.
Missing 404 handler: The task explicitly requires '404 for all the other pages'. This should be added in index.js after all routes and before errorMiddleware: app.use((req, res) => res.status(404).send({ message: 'Not found' }))
| @@ -0,0 +1,10 @@ | |||
| import { Sequelize } from 'sequelize'; | |||
There was a problem hiding this comment.
Unused import: The { where } import from 'sequelize' on line 1 is never used. The where clause is used directly in object form. Remove this import.
| @@ -0,0 +1,9 @@ | |||
| export const catchError = (action) => { | |||
There was a problem hiding this comment.
Unused import: where is imported from 'sequelize' on line 1 but never used anywhere in the file. Remove this import to avoid dead code.
| @@ -0,0 +1,43 @@ | |||
| function name(value) { | |||
There was a problem hiding this comment.
Unused import: The where import from 'sequelize' on line 1 is never used in this file. Remove it to avoid linting errors.
|
|
||
| function password(value) { | ||
| if (!value) { | ||
| return 'Password is required'; | ||
| } | ||
|
|
||
| if (value.length < 6) { | ||
| return 'At least 6 characters'; | ||
| } | ||
|
|
||
| return ''; |
There was a problem hiding this comment.
Missing password rules: The password validation only checks for minimum length (6 characters). The task requires informing users about password rules and validating against them. Consider adding validation for special characters, uppercase letters, etc., and the frontend should display these rules to users during registration.
| <!doctype html> | ||
| <html lang="en"> | ||
| <head> | ||
| <meta charset="UTF-8" /> | ||
| <link rel="icon" type="image/svg+xml" href="/favicon.svg" /> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0" /> |
There was a problem hiding this comment.
Missing semicolons: All import statements (lines 1-5) are missing semicolons. While modern JavaScript can parse these without semicolons due to ASI (Automatic Semicolon Insertion), ESLint config files often have issues. Add semicolons to all import statements for consistency.
| # React + TypeScript + Vite | ||
|
|
||
| This template provides a minimal setup to get React working in Vite with HMR and some ESLint rules. | ||
|
|
||
| Currently, two official plugins are available: | ||
|
|
There was a problem hiding this comment.
Missing semicolons: All import statements (lines 1-6) are missing semicolons. Add semicolons at the end of each import statement to fix syntax errors.
| return ( | ||
| <> | ||
| <nav className="nav"> | ||
| <span className="nav-title">Auth application</span> | ||
| <div className="nav-wrapper"> | ||
| <NavLink | ||
| to="/" | ||
| className={`nav-link ${pathname === '/' ? 'nav-link-active' : ''}`} | ||
| > | ||
| Home | ||
| </NavLink> | ||
| {user && ( | ||
| <NavLink | ||
| to="/profile" | ||
| className={`nav-link nav-link-profile ${pathname === '/profile' ? 'nav-link-active' : ''}`} | ||
| > | ||
| Profile Page | ||
| </NavLink> | ||
| )} | ||
|
|
||
| {user ? ( | ||
| <button className="form-button" onClick={handleLogout}> | ||
| Log out | ||
| </button> | ||
| ) : ( | ||
| <> | ||
| <NavLink | ||
| to="/register" | ||
| className={`nav-link ${pathname === '/register' ? 'nav-link-active' : ''}`} | ||
| > | ||
| Register | ||
| </NavLink> | ||
| <NavLink | ||
| to="/login" | ||
| className={`nav-link ${pathname === '/login' ? 'nav-link-active' : ''}`} | ||
| > | ||
| Sign in | ||
| </NavLink> | ||
| </> | ||
| )} | ||
| </div> | ||
| </nav> | ||
| <div className="gradient-wrapper"> | ||
| <main className="main"> | ||
| <section className="routes"> |
There was a problem hiding this comment.
Missing password rules display: The task requires informing users about password rules during registration. The registration page only shows validation errors but doesn't display what the password rules are (e.g., minimum length, special characters, uppercase). Consider adding a password requirements hint near the password field.
| if (authUser) { | ||
| setUser(authUser); | ||
| } | ||
| } catch (error) { | ||
| console.log(error); | ||
| } finally { | ||
| setIsLoading(false); | ||
| } | ||
| } | ||
| fetchUserData(); | ||
| }, [setUser, setIsLoading]); | ||
|
|
||
| const handleLogout = async () => { | ||
| await logout(); | ||
| setUser(null); | ||
| navigate('/login'); | ||
| }; |
There was a problem hiding this comment.
Inconsistent error handling: The login function (line 48-64) doesn't have a final throw new Error for non-Axios errors, unlike other functions. If a non-Axios error occurs, the function silently returns undefined. Consider adding: throw new Error('Unknown error', { cause: error });
| <Routes> | ||
| <Route path="/" element={<HomePage />} /> | ||
| <Route path="register" element={<RegisterPage />} /> | ||
| <Route | ||
| path="login" | ||
| element={user ? <Navigate to="/profile" /> : <LoginPage />} | ||
| /> | ||
| <Route | ||
| path="activation/:activationToken" | ||
| element={<ActivationPage />} | ||
| /> | ||
| <Route | ||
| path="profile" | ||
| element={user ? <ProfilePage /> : <Navigate to="/login" />} | ||
| /> | ||
| <Route | ||
| path="/forgot-password" | ||
| element={ | ||
| user ? <Navigate to="/login" /> : <ResetPasswordForm /> | ||
| } | ||
| /> | ||
| <Route | ||
| path="/reset-password/:resetToken" | ||
| element={user ? <Navigate to="/" /> : <ResetPasswordPage />} | ||
| /> | ||
| <Route | ||
| path="/confirm-email-change/:emailChangeToken" | ||
| element={<ConfirmEmailChange />} | ||
| /> | ||
| <Route path="*" element={<NotFoundPage />} /> |
There was a problem hiding this comment.
Missing 404 handler: The task explicitly requires '404 for all the other pages'. While the frontend has a NotFoundPage, the backend index.js needs a catch-all route to return 404 for undefined endpoints: app.use((req, res) => res.status(404).send({ message: 'Not found' }))
| throw new Error(error.response?.data.message || 'Change failed', { | ||
| cause: error, | ||
| }); | ||
| } | ||
|
|
||
| throw new Error('Unknown error', { | ||
| cause: error, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| export async function changeEmail(newEmail: string, password: string) { | ||
| try { | ||
| const response = await api.patch('/profile/email', { | ||
| newEmail, | ||
| password, | ||
| }); | ||
|
|
||
| return response.data; | ||
| } catch (error) { | ||
| if (axios.isAxiosError(error)) { | ||
| throw new Error(error.response?.data.message || 'Change failed', { | ||
| cause: error, |
There was a problem hiding this comment.
The useEffect dependency array is missing navigate which could cause React warnings about missing dependencies. Consider adding navigate to the dependency array or using useCallback for the effect.
|
|
||
| export async function changePassword( | ||
| currentPassword: string, | ||
| newPassword: string, | ||
| ) { | ||
| try { | ||
| const response = await api.patch('/profile/password', { | ||
| currentPassword, | ||
| newPassword, | ||
| }); | ||
|
|
||
| return response.data; | ||
| } catch (error) { | ||
| if (axios.isAxiosError(error)) { | ||
| throw new Error(error.response?.data.message || 'Change failed', { | ||
| cause: error, | ||
| }); | ||
| } | ||
|
|
||
| throw new Error('Unknown error', { | ||
| cause: error, | ||
| }); | ||
| } |
There was a problem hiding this comment.
The changePassword function only sends currentPassword and newPassword but the task requires 'new password and confirmation'. The frontend should also collect and send confirmation field, and validate that newPassword === confirmation before making the API call.
| try { | ||
| await api.get('/logout'); | ||
|
|
||
| accessTokenService.remove(); | ||
| } catch (error) { | ||
| if (axios.isAxiosError(error)) { | ||
| throw new Error(error.response?.data.message || 'Logout failed', { | ||
| cause: error, | ||
| }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| export async function activation(activationToken: string) { | ||
| try { |
There was a problem hiding this comment.
Missing password rules display: The task requires informing users about password rules. The registration page doesn't display what the password requirements are (minimum length, special characters, etc.). Add a helper text or list showing the password requirements near the password field.
|
|
||
| export async function logout() { | ||
| try { | ||
| await api.get('/logout'); | ||
|
|
||
| accessTokenService.remove(); | ||
| } catch (error) { | ||
| if (axios.isAxiosError(error)) { | ||
| throw new Error(error.response?.data.message || 'Logout failed', { | ||
| cause: error, | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Inconsistent error handling: The logout function (lines 66-77) doesn't throw an error for non-Axios errors (the catch block only handles Axios errors). Other functions like login have the same issue. Consider adding a generic error throw in the catch block.
| accessToken: string; | ||
| user: { | ||
| email: string; | ||
| id: number; | ||
| name: string; | ||
| }; | ||
| }; | ||
|
|
||
|
|
||
|
|
||
| export async function register(data: CreatePostBody) { | ||
| try { | ||
| const response = await api.post<CreatePostResponse>('/registration', data); | ||
|
|
||
| return response.data; | ||
| } catch (error) { | ||
| if (axios.isAxiosError(error)) { | ||
| throw new Error(error.response?.data.message || 'Registration failed', { | ||
| cause: error, | ||
| }); | ||
| } | ||
|
|
||
| throw new Error('Unknown error', { |
There was a problem hiding this comment.
Missing password confirmation in API: The changePassword function sends currentPassword and newPassword, but the task requires a confirmation field. The backend should also validate the confirmation. Consider adding a confirmPassword parameter and validating it matches newPassword.
|
|
||
| useEffect(() => { | ||
| if (!activationToken) { | ||
| return; | ||
| } | ||
|
|
||
| activation(activationToken) | ||
| .then(setUser) | ||
| .catch((error) => { | ||
| if (error instanceof Error) { | ||
| setError(error.message); | ||
| } | ||
| }) | ||
| .finally(() => setDone(true)); | ||
| navigate('/profile'); |
There was a problem hiding this comment.
Navigation timing issue: The navigate('/profile') call on line 26 happens immediately after starting the activation promise, not after it resolves. This means the user navigates to profile before the activation actually completes. Move navigate inside a .then() block after the activation completes, or call it only in the .finally() block after setting done to true.
| import { useContext, useEffect, useState } from 'react'; | ||
| import { useNavigate, useParams } from 'react-router-dom'; | ||
| import { AuthContext } from '../context/AuthContext'; | ||
| import { activation } from '../api/auth'; | ||
| import { Loader } from './components/Loader'; | ||
|
|
||
| export const ActivationPage: React.FC = () => { | ||
| const [done, setDone] = useState(false); | ||
| const [error, setError] = useState(''); | ||
| const { activationToken } = useParams(); | ||
| const { setUser } = useContext(AuthContext); | ||
| const navigate = useNavigate(); | ||
|
|
||
| useEffect(() => { | ||
| if (!activationToken) { | ||
| return; | ||
| } | ||
|
|
||
| activation(activationToken) | ||
| .then(setUser) | ||
| .catch((error) => { | ||
| if (error instanceof Error) { | ||
| setError(error.message); | ||
| } | ||
| }) | ||
| .finally(() => setDone(true)); | ||
| navigate('/profile'); | ||
| }, [setUser, activationToken, navigate]); | ||
|
|
||
| if (!done) { | ||
| return <Loader />; | ||
| } | ||
|
|
||
| return ( | ||
| <div> | ||
| <h2 className="title">Account activation</h2> | ||
|
|
||
| {error ? ( | ||
| <p className="notification">{error}</p> | ||
| ) : ( | ||
| <p className="notification"> | ||
| Your account is now active | ||
| </p> | ||
| )} | ||
| </div> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Missing password rules display: The task requires informing users about password rules during registration. No component reviewed yet displays these requirements to users.
| localStorage.setItem(key, JSON.stringify(value)); | ||
| } catch { | ||
| throw new Error('Error'); |
There was a problem hiding this comment.
Throwing in useEffect: The throw new Error('Error') in the catch block inside useEffect (line 20) is problematic. Throwing errors in effects can cause React warnings and unexpected behavior. Instead of throwing, consider logging the error or setting a state to display an error message to the user.
| type Props = { | ||
| children: React.ReactNode; | ||
| }; |
There was a problem hiding this comment.
Error handling in useEffect: Throwing an error inside a useEffect callback (line 20) is problematic. The error won't be caught by React error boundaries and will crash the app. Consider handling the error differently, such as setting an error state instead of throwing.
| user: null, | ||
| setUser: () => {}, | ||
| isLoading: false, | ||
| setIsLoading: () => {}, | ||
| }); | ||
|
|
||
| type Props = { | ||
| children: React.ReactNode; | ||
| }; | ||
|
|
||
| export const AuthProvider: React.FC<Props> = ({ children }) => { | ||
| const [user, setUser] = useState<ProfileUser | null>(null); | ||
| const [isLoading, setIsLoading] = useState(false); | ||
|
|
||
| const value = useMemo( |
There was a problem hiding this comment.
Navigation timing issue: The navigate('/profile') on line 26 is called before the activation promise resolves. This means the user navigates to profile even if activation fails. Move the navigation inside .then() after setUser() to ensure it only happens on success.
| <div className="profile-information-done"> | ||
| <h3 className="profile-change-email-title"> | ||
| Confirmation link sent! Please check your new email address to | ||
| complete the change |
There was a problem hiding this comment.
Incorrect heading: The h3 on line 68 says 'Change email' but this is the password change page. It should say 'Change password'.
| user: ProfileUser; | ||
| }; | ||
|
|
||
| export const ProfileChangeEmail: React.FC<Props> = ({ user }) => { |
There was a problem hiding this comment.
Unused prop: The Props type on line 8 defines user: ProfileUser, but the component on line 11 doesn't destructure it. Either remove the unused prop or use it.
|
|
||
| const newErrors: ChangeEmailErrors = { |
There was a problem hiding this comment.
Variable shadowing: On line 26, the user variable is reassigned with the result of changeName(), which shadows the user prop. This can cause confusion. Consider using a different variable name like updatedUser.
| } | ||
|
|
||
| return ( | ||
| <div className="profile-information"> |
There was a problem hiding this comment.
Incorrect form title: The h3 on line 68 says 'Change email' but this component is for changing password. Change to: <h3>Change password</h3>
| type Props = { | ||
| user: ProfileUser; | ||
| }; | ||
|
|
There was a problem hiding this comment.
Unused Props type: The Props type defines user: ProfileUser but the component doesn't destructure or use it (line 11). Either use the user prop or remove it from the Props type.
| event.preventDefault(); | ||
|
|
There was a problem hiding this comment.
Variable shadowing: On line 26, const user = await changeName(name) shadows the props user parameter. While this works, it could cause confusion. Consider renaming to const updatedUser.
| return ( | ||
| <div className="profile-information-done"> | ||
| <CircleAlert size={50} color="red" /> | ||
| <h3 className="profile-information-title"> |
There was a problem hiding this comment.
Typo in heading: The h3 on line 68 says 'Change email' but this is the password change page. It should say 'Change password'.
| type Status = 'success' | 'error' | null; | ||
|
|
||
| export const ConfirmEmailChange: React.FC = () => { | ||
| const { emailChangeToken } = useParams(); | ||
| const [isLoading, setIsLoading] = useState(false); |
There was a problem hiding this comment.
Unused type definition: The Props type and user prop (lines 7-11) are never used in the component. Either remove them or use the user prop if needed.
| } | ||
|
|
There was a problem hiding this comment.
Variable shadowing: The variable user on line 26 shadows the user prop parameter. While this works, it could be confusing. Consider renaming to updatedUser or similar.
| type Props = { | ||
| user: ProfileUser; | ||
| }; | ||
|
|
||
| export const ProfileInfo: React.FC<Props> = ({ user }) => { |
There was a problem hiding this comment.
Unused type definition: The Props type is defined (lines 7-9) but the component on line 11 doesn't destructure the user prop from props. Either use the prop or remove the type definition.
| </span> | ||
| )} | ||
| {nameForm ? ( | ||
| <div className="profile-buttons"> |
There was a problem hiding this comment.
Wrong heading text: The h3 on line 68 says 'Change email' but this is the password change section. It should say 'Change password'.
| try { | ||
| const user = await changeName(name); |
There was a problem hiding this comment.
Variable shadowing: The user constant on line 26 shadows the user prop (line 10). While this works, it could be confusing. Consider renaming the API response variable to updatedUser or similar.
|
|
||
| if (!resetToken) { | ||
| return; |
There was a problem hiding this comment.
Missing error handling for missing token: On line 41-43, if resetToken is undefined, the function returns silently without informing the user. Consider showing an error message like 'Invalid reset link'.
| const [confirmPassword, setConfirmPassword] = useState(''); | ||
| const [error, setError] = useState(''); | ||
| const [errors, setErrors] = useState<NewPasswordErrors>({ | ||
| newPassword: '', | ||
| confirmPassword: '', |
There was a problem hiding this comment.
Missing email validation: The ResetPasswordForm doesn't validate the email format before sending the request. Consider adding email validation using validateValues.email(email) before calling resetPassword.
|
|
||
| const handleSubmit = async (event: React.SubmitEvent) => { | ||
| event.preventDefault(); | ||
|
|
There was a problem hiding this comment.
Missing email validation: The ResetPasswordForm doesn't validate the email before submitting. Consider adding validation using validateValues.email(email) to check if the email is valid before making the API call.
|
|
||
| const handleReset = async () => { | ||
| await resetPassword(email); | ||
| setRegistered(true); |
There was a problem hiding this comment.
Missing error handling: The handleReset function on lines 7-10 doesn't handle errors. If the API call fails, the user won't know. Add try-catch and error state handling.
| </div> | ||
| <div className="input-container"> | ||
| <label htmlFor="password"> | ||
| <strong>Password</strong> | ||
| </label> | ||
| <input | ||
| type="password" | ||
| name="password" | ||
| id="password" | ||
| value={password} | ||
| onChange={(e) => setPassword(e.target.value)} | ||
| className={`form-input ${errors.password ? 'input-error' : ''}`} | ||
| placeholder="Enter your password" | ||
| /> |
There was a problem hiding this comment.
Missing password rules display: The task explicitly requires 'Inform the users about the rules for a password'. The registration page should display the password requirements (e.g., minimum length, special characters, uppercase, numbers) near the password field so users know what rules apply.
| const [name, setName] = useState<string>(''); | ||
| const [email, setEmail] = useState<string>(''); | ||
| const [password, setPassword] = useState<string>(''); | ||
| const [errors, setErrors] = useState<FormErrors>({ | ||
| name: '', | ||
| email: '', | ||
| password: '', | ||
| }); | ||
| const [serverError, setServerError] = useState(''); | ||
| const [registered, setRegistered] = useState(false); | ||
| const [isLoading, setIsLoading] = useState(false); | ||
|
|
||
| if (registered) { | ||
| return ( | ||
| <section className="registered"> | ||
| <h2>Check your email</h2> | ||
| <p>We have sent you an email with the activation link</p> | ||
| </section> | ||
| ); | ||
| } | ||
|
|
||
| const handleRegister = async () => { | ||
| setIsLoading(true); | ||
| try { | ||
| setServerError(''); | ||
|
|
||
| await register({ | ||
| name, | ||
| email, | ||
| password, | ||
| }); | ||
|
|
||
| setName(''); | ||
| setEmail(''); | ||
| setPassword(''); | ||
|
|
||
| return true; | ||
| } catch (error) { | ||
| if (error instanceof Error) { | ||
| setServerError(error.message); | ||
| } | ||
|
|
||
| return false; | ||
| } finally { | ||
| setIsLoading(false); | ||
| } | ||
| }; | ||
|
|
||
| const handleSubmit = async (event: React.SubmitEvent) => { | ||
| event.preventDefault(); | ||
|
|
||
| const newErrors: FormErrors = { | ||
| name: validateValues.name(name), | ||
| email: validateValues.email(email), | ||
| password: validateValues.password(password), | ||
| }; | ||
|
|
||
| setErrors(newErrors); | ||
|
|
||
| const hasErrors = Object.values(newErrors).some((error) => error); | ||
|
|
||
| if (hasErrors) { | ||
| return; | ||
| } | ||
|
|
||
| const success = await handleRegister(); | ||
| if (success) { | ||
| setRegistered(true); | ||
| } | ||
| }; | ||
|
|
||
| return ( | ||
| <div | ||
| className={`form-wrapper background-white ${serverError ? 'form-wrapper-danger' : ''}`} | ||
| > | ||
| <h2 className="title">Sign up</h2> | ||
| <form className="form-container" onSubmit={handleSubmit}> | ||
| <div className="input-container"> | ||
| <label htmlFor="name"> | ||
| <strong>Name</strong> | ||
| </label> | ||
| <input | ||
| type="text" | ||
| name="name" | ||
| id="name" | ||
| value={name} | ||
| onChange={(e) => setName(e.target.value)} | ||
| className={`form-input ${errors.name ? 'input-error' : ''}`} | ||
| placeholder="Enter your name" | ||
| /> | ||
| {errors.name && <p className="notification">{errors.name}</p>} | ||
| </div> | ||
| <div className="input-container"> | ||
| <label htmlFor="email"> | ||
| <strong>Email</strong> | ||
| </label> | ||
| <input | ||
| type="email" | ||
| name="email" | ||
| id="email" | ||
| value={email} | ||
| onChange={(e) => setEmail(e.target.value)} | ||
| className={`form-input ${errors.email ? 'input-error' : ''}`} | ||
| placeholder="Enter your email" | ||
| /> | ||
| {errors.email && <p className="notification">{errors.email}</p>} | ||
| </div> | ||
| <div className="input-container"> | ||
| <label htmlFor="password"> | ||
| <strong>Password</strong> | ||
| </label> | ||
| <input |
There was a problem hiding this comment.
Missing password rules display: The reset password page should also inform users about password rules when they set a new password.
| export type ConfirmNewPasswordData = { | ||
| resetToken: string, | ||
| newPassword: string, |
There was a problem hiding this comment.
Missing confirmPassword field: The ConfirmNewPasswordData type only has resetToken and newPassword. The task requires validating that 'password and confirmation fields must be equal'. Add confirmPassword: string to the type definition.
| export type FormErrors = { | ||
| name: string; | ||
| email: string; | ||
| password: string; | ||
| }; | ||
|
|
There was a problem hiding this comment.
Sensitive data exposure: The User interface (lines 1-6) includes password and activationToken fields which are sensitive server-side data. This interface shouldn't be exported or used on the frontend. The ProfileUser interface is sufficient for client-side use.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation covers most authentication features well, but two critical requirements from the task are not met:
-
Missing 404 catch-all handler: The task explicitly requires "404 for all the other pages" but
src/backend/src/index.jshas no catch-all route likeapp.use((req, res) => res.status(404).send({ message: 'Not found' }))to handle undefined endpoints. -
Missing password rules display: The task explicitly requires "Inform the users about the rules for a password and check them", but the registration page never shows password requirements (minimum length, special characters, uppercase, etc.). Users must guess what constitutes a valid password.
Additionally, these medium issues should be addressed:
src/backend/src/controllers/profile.controller.jsline 85: missingawaitbeforeuser.save()causing potential race conditionssrc/backend/src/pages/ActivationPage.tsx:navigate('/profile')is called before the activation promise resolves, causing premature navigationsrc/frontend/src/pages/ProfilePage/components/ProfileChangePassword.tsxline 68: h3 heading says "Change email" but should say "Change password"- Backend's
changePasswordendpoint doesn't validate password confirmation (should checknewPassword === confirmation)
The linting failure in check runs is due to ESLint configuration issues (flat config vs deprecated flags), not your code. Once the missing 404 handler and password rules display are added, the implementation should meet all task requirements.
✨ 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
| import { User } from './src/models/user.js' | ||
| import { Token } from './src/models/token.js' |
There was a problem hiding this comment.
The import path './src/models/user.js' is incorrect. Since setup.js is already in 'src/backend/', the correct path should be './models/user.js'. This same issue applies to other imports in this file.
| import dotenv from 'dotenv/config'; | ||
| import { User } from './src/models/user.js' | ||
| import { Token } from './src/models/token.js' | ||
| import { client } from './src/utils/db.js'; | ||
|
|
||
|
|
||
| async function setup() { |
There was a problem hiding this comment.
The auth routes (registration, login, password reset) should only be accessible to non-authenticated users. The backend doesn't implement middleware to check if a user is already authenticated and redirect them.
| import { User } from '../models/user.js'; | ||
| import 'dotenv/config'; | ||
| import { userService } from '../services/user.service.js'; | ||
| import { jwtService } from '../services/jwt.service.js'; | ||
| import { ApiError } from '../exeptions/api.error.js'; | ||
| import bcrypt from 'bcrypt'; | ||
| import { tokenService } from '../services/token.service.js'; | ||
| import { emailService } from '../services/email.service.js'; | ||
| import { validateValues } from '../utils/validateValues.js'; | ||
|
|
||
| const register = async (req, res) => { | ||
| const { name, email, password } = req.body; | ||
|
|
||
| const errors = { | ||
| name: validateValues.name(name), | ||
| email: validateValues.email(email), | ||
| password: validateValues.password(password), | ||
| }; | ||
|
|
||
| if (errors.name || errors.email || errors.password) { | ||
| throw ApiError.badRequest('Bad request', errors); | ||
| } | ||
|
|
||
| const hashedPassword = await bcrypt.hash(password, 10); | ||
|
|
||
| await userService.register(name, email, hashedPassword); | ||
|
|
||
| res.send({ message: 'OK' }); |
There was a problem hiding this comment.
Missing 404 handler: There is no catch-all route handler for undefined pages. The task explicitly requires '404 for all the other pages'. Add app.use((req, res) => res.status(404).send({ message: 'Not found' })) before the error middleware and after other routes to handle requests to non-existent endpoints.
| export class ApiError extends Error { | ||
| constructor({ message, status }) { | ||
| super(message); | ||
|
|
||
| this.status = status; | ||
| } | ||
|
|
||
| static badRequest(message) { | ||
| return new ApiError({ | ||
| message, | ||
| status: 400, | ||
| }); | ||
| } | ||
|
|
||
| static unauthorized(message) { | ||
| return new ApiError({ | ||
| message, | ||
| status: 401, | ||
| }); | ||
| } | ||
|
|
||
| static notFound(message) { | ||
| return new ApiError({ | ||
| message, | ||
| status: 404, | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing 404 handler: The task explicitly requires '404 for all the other pages'. No catch-all route exists for undefined endpoints. Add app.use((req, res) => res.status(404).send({ message: 'Not found' })) after all routes but before errorMiddleware.
| @@ -0,0 +1,28 @@ | |||
| export class ApiError extends Error { | |||
There was a problem hiding this comment.
Unused import: The verify import from 'node:crypto' on line 1 is never used in this file. Remove it to avoid linting errors.
|
|
||
| const changePassword = async (req, res) => { | ||
| const { currentPassword, newPassword } = req.body; | ||
|
|
||
| const currentPasswordError = validateValues.password(currentPassword); | ||
| const newPasswordError = validateValues.password(newPassword); | ||
|
|
||
| if (currentPasswordError) { | ||
| throw ApiError.badRequest(currentPasswordError); | ||
| } | ||
|
|
||
| if (newPasswordError) { | ||
| throw ApiError.badRequest(newPasswordError); | ||
| } | ||
|
|
||
| const user = await userService.findById(req.user.id); | ||
|
|
||
| const isCurrentPasswordCorrect = await bcrypt.compare( | ||
| currentPassword, | ||
| user.password, | ||
| ); | ||
|
|
||
| if (!isCurrentPasswordCorrect) { | ||
| throw ApiError.badRequest('Incorrect current password'); | ||
| } | ||
|
|
||
| const newHashedPassword = await bcrypt.hash(newPassword, 10); | ||
|
|
||
| user.password = newHashedPassword; | ||
| await user.save(); | ||
|
|
||
| await tokenService.generateTokens(res, user); |
There was a problem hiding this comment.
The task requires changing password with 'old one, new password and confirmation'. The backend only accepts currentPassword and newPassword but does not require or validate confirmation. The backend should also validate that newPassword === confirmation before updating.
| user.pendingEmail = newEmail; | ||
| user.save(); |
There was a problem hiding this comment.
Missing await before user.save() - this is a fire-and-forget operation that could cause issues.
| })) | ||
| app.use(authRouter); | ||
| app.use('/profile', profileRouter) | ||
|
|
||
| app.use(errorMiddleware); | ||
|
|
||
| app.listen(PORT, () => { | ||
| // eslint-disable-next-line no-console | ||
| console.log(`Server is running on port:${PORT}`); | ||
| }); |
There was a problem hiding this comment.
Missing 404 catch-all handler: The task explicitly requires '404 for all the other pages'. Add app.use((req, res) => res.status(404).send({ message: 'Not found' })) after app.use(errorMiddleware) and before app.listen().
| @@ -0,0 +1,26 @@ | |||
| import { verify } from 'node:crypto'; | |||
There was a problem hiding this comment.
Unused import: verify from 'node:crypto' is imported but never used. This should be removed to avoid dead code.
| import { DataTypes } from 'sequelize'; | ||
| import { client } from '../utils/db.js'; | ||
|
|
||
| export const User = client.define('user', { | ||
| name: { | ||
| type: DataTypes.STRING, | ||
| allowNull: false, | ||
| }, | ||
| email: { | ||
| type: DataTypes.STRING, | ||
| allowNull: false, | ||
| unique: true, | ||
| }, | ||
| password: { |
There was a problem hiding this comment.
Missing 404 handler: The task explicitly requires '404 for all the other pages'. No catch-all route exists in index.js to handle undefined endpoints. Add app.use((req, res) => res.status(404).send({ message: 'Not found' })) before errorMiddleware.
| import { DataTypes } from 'sequelize'; | ||
| import { client } from '../utils/db.js'; | ||
|
|
||
| export const User = client.define('user', { | ||
| name: { | ||
| type: DataTypes.STRING, | ||
| allowNull: false, | ||
| }, | ||
| email: { | ||
| type: DataTypes.STRING, | ||
| allowNull: false, | ||
| unique: true, | ||
| }, | ||
| password: { | ||
| type: DataTypes.STRING, | ||
| allowNull: false, | ||
| }, | ||
| activationToken: { | ||
| type: DataTypes.STRING, | ||
| }, | ||
| resetToken: { | ||
| type: DataTypes.STRING, | ||
| }, | ||
| pendingEmail: { | ||
| type: DataTypes.STRING, | ||
| }, | ||
| emailChangeToken: { | ||
| type: DataTypes.STRING, | ||
| } |
There was a problem hiding this comment.
Missing isActive field: The task mentions checking if user is active and redirecting them if not. The User model doesn't have an explicit isActive boolean field - it uses activationToken being null to indicate active status. This works, but consider if an explicit isActive field would be clearer.
| import { DataTypes } from 'sequelize'; | ||
| import { client } from '../utils/db.js'; | ||
|
|
||
| export const User = client.define('user', { | ||
| name: { | ||
| type: DataTypes.STRING, | ||
| allowNull: false, | ||
| }, |
There was a problem hiding this comment.
Missing password confirmation validation: The task requires profile password change to require 'old one, new password and confirmation'. The backend changePassword only accepts currentPassword and newPassword without validating confirmation. While the frontend validates this, the backend should enforce it too.
| import { DataTypes } from 'sequelize'; | ||
| import { client } from '../utils/db.js'; | ||
|
|
||
| export const User = client.define('user', { | ||
| name: { | ||
| type: DataTypes.STRING, | ||
| allowNull: false, | ||
| }, |
There was a problem hiding this comment.
The password validation function only checks for minimum length (6 characters). The task requires informing users about password rules and validating against them. Consider adding rules like special characters, uppercase, etc.
| @@ -0,0 +1,13 @@ | |||
| import { DataTypes } from 'sequelize'; | |||
| import { client } from '../utils/db.js'; | |||
| import { User } from './user.js'; | |||
There was a problem hiding this comment.
Syntax error: Missing semicolon at the end of the import statement. This will cause the module to fail to load. Change line 3 to: import { catchError } from '../utils/catchError.js';
|
|
||
| if (error) { | ||
| res.statusCode = 500; | ||
| res.send({ | ||
| message: 'Server error', | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
Unreachable code: The next() call on line 20 is never reached because res.send() on line 15 already sends a response. Additionally, when error is falsy, nothing is sent and next() is still called. Consider removing the next() call or restructuring the logic.
| import { ApiError } from '../exeptions/api.error.js'; | ||
|
|
||
| export const errorMiddleware = (error, req, res, next) => { | ||
|
|
||
| if (error instanceof ApiError) { | ||
| res.status(error.status).send({ | ||
| message: error.message, | ||
| errors: error.errors, | ||
| }); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| if (error) { |
There was a problem hiding this comment.
Routes for non-authenticated users (registration, login, password reset) should include middleware to check if a user is already authenticated and redirect them. Currently, authenticated users could still access these routes.
| @@ -0,0 +1,37 @@ | |||
| import jwt from 'jsonwebtoken'; | |||
| import 'dotenv/config'; | |||
There was a problem hiding this comment.
Syntax error: Missing semicolon at the end of the import statement. This will prevent the module from loading. Change line 2 to: import { catchError } from '../utils/catchError.js';
| } | ||
|
|
||
| function verify(token) { | ||
| try { |
There was a problem hiding this comment.
The confirm-email-change route is correctly placed outside authMiddleware since users access it via email link. However, if an authenticated user accesses this route directly, they should be redirected to their profile. Consider adding logic to check if user is already authenticated.
| @@ -0,0 +1,88 @@ | |||
| import 'dotenv/config'; | |||
| import nodemailer from 'nodemailer'; | |||
There was a problem hiding this comment.
Missing semicolon: The import statement on line 2 is missing a semicolon. Change to: import { catchError } from '../utils/catchError.js';
| const html = ` | ||
| <h2>Activate account</h1> | ||
| <a href="${href}">${href}</a> |
There was a problem hiding this comment.
HTML tag mismatch: The h2 opening tag on line 23 is closed with </h1>. This should be </h2>.
| const html = ` | ||
| <h2>Reset password</h1> | ||
| <a href="${href}">${href}</a> |
There was a problem hiding this comment.
HTML tag mismatch: The h2 opening tag on line 37 is closed with </h1>. This should be </h2>.
| const html = ` | ||
| <h2>Change email</h1> | ||
| <a href="${href}">${href}</a> |
There was a problem hiding this comment.
HTML tag mismatch: The h2 opening tag on line 51 is closed with </h1>. This should be </h2>.
| @@ -0,0 +1,11 @@ | |||
| import express from 'express'; | |||
| import { catchError } from '../utils/catchError.js' | |||
There was a problem hiding this comment.
Syntax error: Missing semicolon at the end of the import statement on line 2. Change to: import { catchError } from '../utils/catchError.js';
| import express from 'express'; | ||
| import { catchError } from '../utils/catchError.js' | ||
| import { profileController } from '../controllers/profile.controller.js'; | ||
| import { authMiddleware } from '../middlewares/authMiddleware.js'; | ||
|
|
||
| export const profileRouter = new express.Router(); | ||
|
|
||
| profileRouter.patch('/name', authMiddleware, catchError(profileController.changeName)); | ||
| profileRouter.patch('/password', authMiddleware, catchError(profileController.changePassword)); | ||
| profileRouter.patch('/email', authMiddleware, catchError(profileController.requestEmailChange)); | ||
| profileRouter.get('/confirm-email-change/:emailChangeToken', catchError(profileController.confirmEmailChange)); |
There was a problem hiding this comment.
Missing 404 handler: The task explicitly requires '404 for all the other pages'. Add app.use((req, res) => res.status(404).send({ message: 'Not found' })) to index.js after all routes.
| @@ -0,0 +1,54 @@ | |||
| import { Token } from '../models/token.js'; | |||
| import { userService } from '../services/user.service.js'; | |||
There was a problem hiding this comment.
Syntax error: Missing semicolon at the end of the import statement. This will cause the module to fail to load. Change line 2 to: import { catchError } from '../utils/catchError.js';
| where: { userId }, | ||
| }); | ||
|
|
There was a problem hiding this comment.
The password change endpoint doesn't require or validate password confirmation. The task requires 'new password and confirmation' fields. While the frontend validates this, the backend should also enforce it by checking that newPassword === confirmation before updating.
| import { Token } from '../models/token.js'; | ||
| import { userService } from '../services/user.service.js'; | ||
| import { jwtService } from '../services/jwt.service.js'; | ||
|
|
||
| async function save(userId, refreshToken) { | ||
| const token = await Token.findOne({ | ||
| where: { userId }, | ||
| }); | ||
|
|
||
| if (token) { | ||
| token.refreshToken = refreshToken; |
There was a problem hiding this comment.
Missing 404 handler: The task explicitly requires '404 for all the other pages'. No catch-all route exists in index.js to handle undefined endpoints. Add app.use((req, res) => res.status(404).send({ message: 'Not found' })) in index.js before errorMiddleware.
| @@ -0,0 +1,78 @@ | |||
| import { where } from 'sequelize'; | |||
There was a problem hiding this comment.
Unused import: where is imported from 'sequelize' on line 1 but never used in this file. Remove it to avoid dead code.
|
|
||
| async function register(name, email, password) { | ||
| const existUser = await findByEmail(email); | ||
|
|
||
| if (existUser) { | ||
| throw ApiError.badRequest('User already exists'); | ||
| } | ||
|
|
||
| const activationToken = uuidvv4(); | ||
|
|
||
| await User.create({ |
There was a problem hiding this comment.
Password validation too minimal: The password function only checks for minimum 6 characters. The task requires informing users about password rules and validating against them (special characters, uppercase, etc.). Add rules like: at least one uppercase, one lowercase, one number, one special character, minimum 8 characters.
| import { User } from '../models/user.js'; | ||
| import { emailService } from '../services/email.service.js'; | ||
| import { v4 as uuidvv4 } from 'uuid'; | ||
|
|
There was a problem hiding this comment.
Unnecessary workaround: Adding + '' to convert DB_PASSWORD assumes it's a number, but environment variables are always strings. Simply use process.env.DB_PASSWORD directly.
| import { where } from 'sequelize'; | ||
| import { ApiError } from '../exeptions/api.error.js'; | ||
| import { User } from '../models/user.js'; | ||
| import { emailService } from '../services/email.service.js'; | ||
| import { v4 as uuidvv4 } from 'uuid'; | ||
|
|
||
| function getAllActivated() { | ||
| return User.findAll({ | ||
| where: { | ||
| activationToken: null, | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| function normalizeData({ id, name, email }) { | ||
| return { id, name, email }; | ||
| } | ||
|
|
||
| function findByEmail(email) { | ||
| return User.findOne({ where: { email } }); | ||
| } | ||
|
|
||
| function findById(id) { | ||
| return User.findByPk(id); | ||
| } | ||
|
|
||
| async function register(name, email, password) { | ||
| const existUser = await findByEmail(email); | ||
|
|
||
| if (existUser) { | ||
| throw ApiError.badRequest('User already exists'); | ||
| } | ||
|
|
||
| const activationToken = uuidvv4(); | ||
|
|
||
| await User.create({ | ||
| name, | ||
| email, | ||
| password, | ||
| activationToken, | ||
| }); | ||
|
|
||
| await emailService.sendActivationEmail(email, activationToken); |
There was a problem hiding this comment.
Missing 404 handler: The task explicitly requires '404 for all the other pages'. This should be added in index.js after all routes and before errorMiddleware: app.use((req, res) => res.status(404).send({ message: 'Not found' }))
| @@ -0,0 +1,10 @@ | |||
| import { Sequelize } from 'sequelize'; | |||
There was a problem hiding this comment.
Unused import: The { where } import from 'sequelize' on line 1 is never used. The where clause is used directly in object form. Remove this import.
| @@ -0,0 +1,9 @@ | |||
| export const catchError = (action) => { | |||
There was a problem hiding this comment.
Unused import: where is imported from 'sequelize' on line 1 but never used anywhere in the file. Remove this import to avoid dead code.
| @@ -0,0 +1,43 @@ | |||
| function name(value) { | |||
There was a problem hiding this comment.
Unused import: The where import from 'sequelize' on line 1 is never used in this file. Remove it to avoid linting errors.
|
|
||
| function password(value) { | ||
| if (!value) { | ||
| return 'Password is required'; | ||
| } | ||
|
|
||
| if (value.length < 6) { | ||
| return 'At least 6 characters'; | ||
| } | ||
|
|
||
| return ''; |
There was a problem hiding this comment.
Missing password rules: The password validation only checks for minimum length (6 characters). The task requires informing users about password rules and validating against them. Consider adding validation for special characters, uppercase letters, etc., and the frontend should display these rules to users during registration.
| <!doctype html> | ||
| <html lang="en"> | ||
| <head> | ||
| <meta charset="UTF-8" /> | ||
| <link rel="icon" type="image/svg+xml" href="/favicon.svg" /> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0" /> |
There was a problem hiding this comment.
Missing semicolons: All import statements (lines 1-5) are missing semicolons. While modern JavaScript can parse these without semicolons due to ASI (Automatic Semicolon Insertion), ESLint config files often have issues. Add semicolons to all import statements for consistency.
| # React + TypeScript + Vite | ||
|
|
||
| This template provides a minimal setup to get React working in Vite with HMR and some ESLint rules. | ||
|
|
||
| Currently, two official plugins are available: | ||
|
|
There was a problem hiding this comment.
Missing semicolons: All import statements (lines 1-6) are missing semicolons. Add semicolons at the end of each import statement to fix syntax errors.
| return ( | ||
| <> | ||
| <nav className="nav"> | ||
| <span className="nav-title">Auth application</span> | ||
| <div className="nav-wrapper"> | ||
| <NavLink | ||
| to="/" | ||
| className={`nav-link ${pathname === '/' ? 'nav-link-active' : ''}`} | ||
| > | ||
| Home | ||
| </NavLink> | ||
| {user && ( | ||
| <NavLink | ||
| to="/profile" | ||
| className={`nav-link nav-link-profile ${pathname === '/profile' ? 'nav-link-active' : ''}`} | ||
| > | ||
| Profile Page | ||
| </NavLink> | ||
| )} | ||
|
|
||
| {user ? ( | ||
| <button className="form-button" onClick={handleLogout}> | ||
| Log out | ||
| </button> | ||
| ) : ( | ||
| <> | ||
| <NavLink | ||
| to="/register" | ||
| className={`nav-link ${pathname === '/register' ? 'nav-link-active' : ''}`} | ||
| > | ||
| Register | ||
| </NavLink> | ||
| <NavLink | ||
| to="/login" | ||
| className={`nav-link ${pathname === '/login' ? 'nav-link-active' : ''}`} | ||
| > | ||
| Sign in | ||
| </NavLink> | ||
| </> | ||
| )} | ||
| </div> | ||
| </nav> | ||
| <div className="gradient-wrapper"> | ||
| <main className="main"> | ||
| <section className="routes"> |
There was a problem hiding this comment.
Missing password rules display: The task requires informing users about password rules during registration. The registration page only shows validation errors but doesn't display what the password rules are (e.g., minimum length, special characters, uppercase). Consider adding a password requirements hint near the password field.
| if (authUser) { | ||
| setUser(authUser); | ||
| } | ||
| } catch (error) { | ||
| console.log(error); | ||
| } finally { | ||
| setIsLoading(false); | ||
| } | ||
| } | ||
| fetchUserData(); | ||
| }, [setUser, setIsLoading]); | ||
|
|
||
| const handleLogout = async () => { | ||
| await logout(); | ||
| setUser(null); | ||
| navigate('/login'); | ||
| }; |
There was a problem hiding this comment.
Inconsistent error handling: The login function (line 48-64) doesn't have a final throw new Error for non-Axios errors, unlike other functions. If a non-Axios error occurs, the function silently returns undefined. Consider adding: throw new Error('Unknown error', { cause: error });
| <Routes> | ||
| <Route path="/" element={<HomePage />} /> | ||
| <Route path="register" element={<RegisterPage />} /> | ||
| <Route | ||
| path="login" | ||
| element={user ? <Navigate to="/profile" /> : <LoginPage />} | ||
| /> | ||
| <Route | ||
| path="activation/:activationToken" | ||
| element={<ActivationPage />} | ||
| /> | ||
| <Route | ||
| path="profile" | ||
| element={user ? <ProfilePage /> : <Navigate to="/login" />} | ||
| /> | ||
| <Route | ||
| path="/forgot-password" | ||
| element={ | ||
| user ? <Navigate to="/login" /> : <ResetPasswordForm /> | ||
| } | ||
| /> | ||
| <Route | ||
| path="/reset-password/:resetToken" | ||
| element={user ? <Navigate to="/" /> : <ResetPasswordPage />} | ||
| /> | ||
| <Route | ||
| path="/confirm-email-change/:emailChangeToken" | ||
| element={<ConfirmEmailChange />} | ||
| /> | ||
| <Route path="*" element={<NotFoundPage />} /> |
There was a problem hiding this comment.
Missing 404 handler: The task explicitly requires '404 for all the other pages'. While the frontend has a NotFoundPage, the backend index.js needs a catch-all route to return 404 for undefined endpoints: app.use((req, res) => res.status(404).send({ message: 'Not found' }))
| throw new Error(error.response?.data.message || 'Change failed', { | ||
| cause: error, | ||
| }); | ||
| } | ||
|
|
||
| throw new Error('Unknown error', { | ||
| cause: error, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| export async function changeEmail(newEmail: string, password: string) { | ||
| try { | ||
| const response = await api.patch('/profile/email', { | ||
| newEmail, | ||
| password, | ||
| }); | ||
|
|
||
| return response.data; | ||
| } catch (error) { | ||
| if (axios.isAxiosError(error)) { | ||
| throw new Error(error.response?.data.message || 'Change failed', { | ||
| cause: error, |
There was a problem hiding this comment.
The useEffect dependency array is missing navigate which could cause React warnings about missing dependencies. Consider adding navigate to the dependency array or using useCallback for the effect.
|
|
||
| export async function changePassword( | ||
| currentPassword: string, | ||
| newPassword: string, | ||
| ) { | ||
| try { | ||
| const response = await api.patch('/profile/password', { | ||
| currentPassword, | ||
| newPassword, | ||
| }); | ||
|
|
||
| return response.data; | ||
| } catch (error) { | ||
| if (axios.isAxiosError(error)) { | ||
| throw new Error(error.response?.data.message || 'Change failed', { | ||
| cause: error, | ||
| }); | ||
| } | ||
|
|
||
| throw new Error('Unknown error', { | ||
| cause: error, | ||
| }); | ||
| } |
There was a problem hiding this comment.
The changePassword function only sends currentPassword and newPassword but the task requires 'new password and confirmation'. The frontend should also collect and send confirmation field, and validate that newPassword === confirmation before making the API call.
| try { | ||
| await api.get('/logout'); | ||
|
|
||
| accessTokenService.remove(); | ||
| } catch (error) { | ||
| if (axios.isAxiosError(error)) { | ||
| throw new Error(error.response?.data.message || 'Logout failed', { | ||
| cause: error, | ||
| }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| export async function activation(activationToken: string) { | ||
| try { |
There was a problem hiding this comment.
Missing password rules display: The task requires informing users about password rules. The registration page doesn't display what the password requirements are (minimum length, special characters, etc.). Add a helper text or list showing the password requirements near the password field.
|
|
||
| export async function logout() { | ||
| try { | ||
| await api.get('/logout'); | ||
|
|
||
| accessTokenService.remove(); | ||
| } catch (error) { | ||
| if (axios.isAxiosError(error)) { | ||
| throw new Error(error.response?.data.message || 'Logout failed', { | ||
| cause: error, | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Inconsistent error handling: The logout function (lines 66-77) doesn't throw an error for non-Axios errors (the catch block only handles Axios errors). Other functions like login have the same issue. Consider adding a generic error throw in the catch block.
| accessToken: string; | ||
| user: { | ||
| email: string; | ||
| id: number; | ||
| name: string; | ||
| }; | ||
| }; | ||
|
|
||
|
|
||
|
|
||
| export async function register(data: CreatePostBody) { | ||
| try { | ||
| const response = await api.post<CreatePostResponse>('/registration', data); | ||
|
|
||
| return response.data; | ||
| } catch (error) { | ||
| if (axios.isAxiosError(error)) { | ||
| throw new Error(error.response?.data.message || 'Registration failed', { | ||
| cause: error, | ||
| }); | ||
| } | ||
|
|
||
| throw new Error('Unknown error', { |
There was a problem hiding this comment.
Missing password confirmation in API: The changePassword function sends currentPassword and newPassword, but the task requires a confirmation field. The backend should also validate the confirmation. Consider adding a confirmPassword parameter and validating it matches newPassword.
|
|
||
| useEffect(() => { | ||
| if (!activationToken) { | ||
| return; | ||
| } | ||
|
|
||
| activation(activationToken) | ||
| .then(setUser) | ||
| .catch((error) => { | ||
| if (error instanceof Error) { | ||
| setError(error.message); | ||
| } | ||
| }) | ||
| .finally(() => setDone(true)); | ||
| navigate('/profile'); |
There was a problem hiding this comment.
Navigation timing issue: The navigate('/profile') call on line 26 happens immediately after starting the activation promise, not after it resolves. This means the user navigates to profile before the activation actually completes. Move navigate inside a .then() block after the activation completes, or call it only in the .finally() block after setting done to true.
| import { useContext, useEffect, useState } from 'react'; | ||
| import { useNavigate, useParams } from 'react-router-dom'; | ||
| import { AuthContext } from '../context/AuthContext'; | ||
| import { activation } from '../api/auth'; | ||
| import { Loader } from './components/Loader'; | ||
|
|
||
| export const ActivationPage: React.FC = () => { | ||
| const [done, setDone] = useState(false); | ||
| const [error, setError] = useState(''); | ||
| const { activationToken } = useParams(); | ||
| const { setUser } = useContext(AuthContext); | ||
| const navigate = useNavigate(); | ||
|
|
||
| useEffect(() => { | ||
| if (!activationToken) { | ||
| return; | ||
| } | ||
|
|
||
| activation(activationToken) | ||
| .then(setUser) | ||
| .catch((error) => { | ||
| if (error instanceof Error) { | ||
| setError(error.message); | ||
| } | ||
| }) | ||
| .finally(() => setDone(true)); | ||
| navigate('/profile'); | ||
| }, [setUser, activationToken, navigate]); | ||
|
|
||
| if (!done) { | ||
| return <Loader />; | ||
| } | ||
|
|
||
| return ( | ||
| <div> | ||
| <h2 className="title">Account activation</h2> | ||
|
|
||
| {error ? ( | ||
| <p className="notification">{error}</p> | ||
| ) : ( | ||
| <p className="notification"> | ||
| Your account is now active | ||
| </p> | ||
| )} | ||
| </div> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Missing password rules display: The task requires informing users about password rules during registration. No component reviewed yet displays these requirements to users.
| localStorage.setItem(key, JSON.stringify(value)); | ||
| } catch { | ||
| throw new Error('Error'); |
There was a problem hiding this comment.
Throwing in useEffect: The throw new Error('Error') in the catch block inside useEffect (line 20) is problematic. Throwing errors in effects can cause React warnings and unexpected behavior. Instead of throwing, consider logging the error or setting a state to display an error message to the user.
| type Props = { | ||
| children: React.ReactNode; | ||
| }; |
There was a problem hiding this comment.
Error handling in useEffect: Throwing an error inside a useEffect callback (line 20) is problematic. The error won't be caught by React error boundaries and will crash the app. Consider handling the error differently, such as setting an error state instead of throwing.
| user: null, | ||
| setUser: () => {}, | ||
| isLoading: false, | ||
| setIsLoading: () => {}, | ||
| }); | ||
|
|
||
| type Props = { | ||
| children: React.ReactNode; | ||
| }; | ||
|
|
||
| export const AuthProvider: React.FC<Props> = ({ children }) => { | ||
| const [user, setUser] = useState<ProfileUser | null>(null); | ||
| const [isLoading, setIsLoading] = useState(false); | ||
|
|
||
| const value = useMemo( |
There was a problem hiding this comment.
Navigation timing issue: The navigate('/profile') on line 26 is called before the activation promise resolves. This means the user navigates to profile even if activation fails. Move the navigation inside .then() after setUser() to ensure it only happens on success.
| <div className="profile-information-done"> | ||
| <h3 className="profile-change-email-title"> | ||
| Confirmation link sent! Please check your new email address to | ||
| complete the change |
There was a problem hiding this comment.
Incorrect heading: The h3 on line 68 says 'Change email' but this is the password change page. It should say 'Change password'.
| user: ProfileUser; | ||
| }; | ||
|
|
||
| export const ProfileChangeEmail: React.FC<Props> = ({ user }) => { |
There was a problem hiding this comment.
Unused prop: The Props type on line 8 defines user: ProfileUser, but the component on line 11 doesn't destructure it. Either remove the unused prop or use it.
|
|
||
| const newErrors: ChangeEmailErrors = { |
There was a problem hiding this comment.
Variable shadowing: On line 26, the user variable is reassigned with the result of changeName(), which shadows the user prop. This can cause confusion. Consider using a different variable name like updatedUser.
| } | ||
|
|
||
| return ( | ||
| <div className="profile-information"> |
There was a problem hiding this comment.
Incorrect form title: The h3 on line 68 says 'Change email' but this component is for changing password. Change to: <h3>Change password</h3>
| type Props = { | ||
| user: ProfileUser; | ||
| }; | ||
|
|
There was a problem hiding this comment.
Unused Props type: The Props type defines user: ProfileUser but the component doesn't destructure or use it (line 11). Either use the user prop or remove it from the Props type.
| event.preventDefault(); | ||
|
|
There was a problem hiding this comment.
Variable shadowing: On line 26, const user = await changeName(name) shadows the props user parameter. While this works, it could cause confusion. Consider renaming to const updatedUser.
| return ( | ||
| <div className="profile-information-done"> | ||
| <CircleAlert size={50} color="red" /> | ||
| <h3 className="profile-information-title"> |
There was a problem hiding this comment.
Typo in heading: The h3 on line 68 says 'Change email' but this is the password change page. It should say 'Change password'.
| type Status = 'success' | 'error' | null; | ||
|
|
||
| export const ConfirmEmailChange: React.FC = () => { | ||
| const { emailChangeToken } = useParams(); | ||
| const [isLoading, setIsLoading] = useState(false); |
There was a problem hiding this comment.
Unused type definition: The Props type and user prop (lines 7-11) are never used in the component. Either remove them or use the user prop if needed.
| } | ||
|
|
There was a problem hiding this comment.
Variable shadowing: The variable user on line 26 shadows the user prop parameter. While this works, it could be confusing. Consider renaming to updatedUser or similar.
| type Props = { | ||
| user: ProfileUser; | ||
| }; | ||
|
|
||
| export const ProfileInfo: React.FC<Props> = ({ user }) => { |
There was a problem hiding this comment.
Unused type definition: The Props type is defined (lines 7-9) but the component on line 11 doesn't destructure the user prop from props. Either use the prop or remove the type definition.
| </span> | ||
| )} | ||
| {nameForm ? ( | ||
| <div className="profile-buttons"> |
There was a problem hiding this comment.
Wrong heading text: The h3 on line 68 says 'Change email' but this is the password change section. It should say 'Change password'.
| try { | ||
| const user = await changeName(name); |
There was a problem hiding this comment.
Variable shadowing: The user constant on line 26 shadows the user prop (line 10). While this works, it could be confusing. Consider renaming the API response variable to updatedUser or similar.
|
|
||
| if (!resetToken) { | ||
| return; |
There was a problem hiding this comment.
Missing error handling for missing token: On line 41-43, if resetToken is undefined, the function returns silently without informing the user. Consider showing an error message like 'Invalid reset link'.
| const [confirmPassword, setConfirmPassword] = useState(''); | ||
| const [error, setError] = useState(''); | ||
| const [errors, setErrors] = useState<NewPasswordErrors>({ | ||
| newPassword: '', | ||
| confirmPassword: '', |
There was a problem hiding this comment.
Missing email validation: The ResetPasswordForm doesn't validate the email format before sending the request. Consider adding email validation using validateValues.email(email) before calling resetPassword.
|
|
||
| const handleSubmit = async (event: React.SubmitEvent) => { | ||
| event.preventDefault(); | ||
|
|
There was a problem hiding this comment.
Missing email validation: The ResetPasswordForm doesn't validate the email before submitting. Consider adding validation using validateValues.email(email) to check if the email is valid before making the API call.
|
|
||
| const handleReset = async () => { | ||
| await resetPassword(email); | ||
| setRegistered(true); |
There was a problem hiding this comment.
Missing error handling: The handleReset function on lines 7-10 doesn't handle errors. If the API call fails, the user won't know. Add try-catch and error state handling.
| </div> | ||
| <div className="input-container"> | ||
| <label htmlFor="password"> | ||
| <strong>Password</strong> | ||
| </label> | ||
| <input | ||
| type="password" | ||
| name="password" | ||
| id="password" | ||
| value={password} | ||
| onChange={(e) => setPassword(e.target.value)} | ||
| className={`form-input ${errors.password ? 'input-error' : ''}`} | ||
| placeholder="Enter your password" | ||
| /> |
There was a problem hiding this comment.
Missing password rules display: The task explicitly requires 'Inform the users about the rules for a password'. The registration page should display the password requirements (e.g., minimum length, special characters, uppercase, numbers) near the password field so users know what rules apply.
| const [name, setName] = useState<string>(''); | ||
| const [email, setEmail] = useState<string>(''); | ||
| const [password, setPassword] = useState<string>(''); | ||
| const [errors, setErrors] = useState<FormErrors>({ | ||
| name: '', | ||
| email: '', | ||
| password: '', | ||
| }); | ||
| const [serverError, setServerError] = useState(''); | ||
| const [registered, setRegistered] = useState(false); | ||
| const [isLoading, setIsLoading] = useState(false); | ||
|
|
||
| if (registered) { | ||
| return ( | ||
| <section className="registered"> | ||
| <h2>Check your email</h2> | ||
| <p>We have sent you an email with the activation link</p> | ||
| </section> | ||
| ); | ||
| } | ||
|
|
||
| const handleRegister = async () => { | ||
| setIsLoading(true); | ||
| try { | ||
| setServerError(''); | ||
|
|
||
| await register({ | ||
| name, | ||
| email, | ||
| password, | ||
| }); | ||
|
|
||
| setName(''); | ||
| setEmail(''); | ||
| setPassword(''); | ||
|
|
||
| return true; | ||
| } catch (error) { | ||
| if (error instanceof Error) { | ||
| setServerError(error.message); | ||
| } | ||
|
|
||
| return false; | ||
| } finally { | ||
| setIsLoading(false); | ||
| } | ||
| }; | ||
|
|
||
| const handleSubmit = async (event: React.SubmitEvent) => { | ||
| event.preventDefault(); | ||
|
|
||
| const newErrors: FormErrors = { | ||
| name: validateValues.name(name), | ||
| email: validateValues.email(email), | ||
| password: validateValues.password(password), | ||
| }; | ||
|
|
||
| setErrors(newErrors); | ||
|
|
||
| const hasErrors = Object.values(newErrors).some((error) => error); | ||
|
|
||
| if (hasErrors) { | ||
| return; | ||
| } | ||
|
|
||
| const success = await handleRegister(); | ||
| if (success) { | ||
| setRegistered(true); | ||
| } | ||
| }; | ||
|
|
||
| return ( | ||
| <div | ||
| className={`form-wrapper background-white ${serverError ? 'form-wrapper-danger' : ''}`} | ||
| > | ||
| <h2 className="title">Sign up</h2> | ||
| <form className="form-container" onSubmit={handleSubmit}> | ||
| <div className="input-container"> | ||
| <label htmlFor="name"> | ||
| <strong>Name</strong> | ||
| </label> | ||
| <input | ||
| type="text" | ||
| name="name" | ||
| id="name" | ||
| value={name} | ||
| onChange={(e) => setName(e.target.value)} | ||
| className={`form-input ${errors.name ? 'input-error' : ''}`} | ||
| placeholder="Enter your name" | ||
| /> | ||
| {errors.name && <p className="notification">{errors.name}</p>} | ||
| </div> | ||
| <div className="input-container"> | ||
| <label htmlFor="email"> | ||
| <strong>Email</strong> | ||
| </label> | ||
| <input | ||
| type="email" | ||
| name="email" | ||
| id="email" | ||
| value={email} | ||
| onChange={(e) => setEmail(e.target.value)} | ||
| className={`form-input ${errors.email ? 'input-error' : ''}`} | ||
| placeholder="Enter your email" | ||
| /> | ||
| {errors.email && <p className="notification">{errors.email}</p>} | ||
| </div> | ||
| <div className="input-container"> | ||
| <label htmlFor="password"> | ||
| <strong>Password</strong> | ||
| </label> | ||
| <input |
There was a problem hiding this comment.
Missing password rules display: The reset password page should also inform users about password rules when they set a new password.
| export type ConfirmNewPasswordData = { | ||
| resetToken: string, | ||
| newPassword: string, |
There was a problem hiding this comment.
Missing confirmPassword field: The ConfirmNewPasswordData type only has resetToken and newPassword. The task requires validating that 'password and confirmation fields must be equal'. Add confirmPassword: string to the type definition.
| export type FormErrors = { | ||
| name: string; | ||
| email: string; | ||
| password: string; | ||
| }; | ||
|
|
There was a problem hiding this comment.
Sensitive data exposure: The User interface (lines 1-6) includes password and activationToken fields which are sensitive server-side data. This interface shouldn't be exported or used on the frontend. The ProfileUser interface is sufficient for client-side use.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation covers most authentication features well, but two critical requirements from the task are not met:
-
Missing 404 catch-all handler: The task explicitly requires "404 for all the other pages" but
src/backend/src/index.jshas no catch-all route likeapp.use((req, res) => res.status(404).send({ message: 'Not found' }))to handle undefined endpoints. -
Missing password rules display: The task explicitly requires "Inform the users about the rules for a password and check them", but the registration page never shows password requirements (minimum length, special characters, uppercase, etc.). Users must guess what constitutes a valid password.
Additionally, these medium issues should be addressed:
src/backend/src/controllers/profile.controller.jsline 85: missingawaitbeforeuser.save()causing potential race conditionssrc/backend/src/pages/ActivationPage.tsx:navigate('/profile')is called before the activation promise resolves, causing premature navigationsrc/frontend/src/pages/ProfilePage/components/ProfileChangePassword.tsxline 68: h3 heading says "Change email" but should say "Change password"- Backend's
changePasswordendpoint doesn't validate password confirmation (should checknewPassword === confirmation)
The linting failure in check runs is due to ESLint configuration issues (flat config vs deprecated flags), not your code. Once the missing 404 handler and password rules display are added, the implementation should meet all task requirements.
✨ 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
| import { User } from './src/models/user.js' | ||
| import { Token } from './src/models/token.js' |
There was a problem hiding this comment.
The import path './src/models/user.js' is incorrect. Since setup.js is already in 'src/backend/', the correct path should be './models/user.js'. This same issue applies to other imports in this file.
| import dotenv from 'dotenv/config'; | ||
| import { User } from './src/models/user.js' | ||
| import { Token } from './src/models/token.js' | ||
| import { client } from './src/utils/db.js'; | ||
|
|
||
|
|
||
| async function setup() { |
There was a problem hiding this comment.
The auth routes (registration, login, password reset) should only be accessible to non-authenticated users. The backend doesn't implement middleware to check if a user is already authenticated and redirect them.
| import { User } from '../models/user.js'; | ||
| import 'dotenv/config'; | ||
| import { userService } from '../services/user.service.js'; | ||
| import { jwtService } from '../services/jwt.service.js'; | ||
| import { ApiError } from '../exeptions/api.error.js'; | ||
| import bcrypt from 'bcrypt'; | ||
| import { tokenService } from '../services/token.service.js'; | ||
| import { emailService } from '../services/email.service.js'; | ||
| import { validateValues } from '../utils/validateValues.js'; | ||
|
|
||
| const register = async (req, res) => { | ||
| const { name, email, password } = req.body; | ||
|
|
||
| const errors = { | ||
| name: validateValues.name(name), | ||
| email: validateValues.email(email), | ||
| password: validateValues.password(password), | ||
| }; | ||
|
|
||
| if (errors.name || errors.email || errors.password) { | ||
| throw ApiError.badRequest('Bad request', errors); | ||
| } | ||
|
|
||
| const hashedPassword = await bcrypt.hash(password, 10); | ||
|
|
||
| await userService.register(name, email, hashedPassword); | ||
|
|
||
| res.send({ message: 'OK' }); |
There was a problem hiding this comment.
Missing 404 handler: There is no catch-all route handler for undefined pages. The task explicitly requires '404 for all the other pages'. Add app.use((req, res) => res.status(404).send({ message: 'Not found' })) before the error middleware and after other routes to handle requests to non-existent endpoints.
| export class ApiError extends Error { | ||
| constructor({ message, status }) { | ||
| super(message); | ||
|
|
||
| this.status = status; | ||
| } | ||
|
|
||
| static badRequest(message) { | ||
| return new ApiError({ | ||
| message, | ||
| status: 400, | ||
| }); | ||
| } | ||
|
|
||
| static unauthorized(message) { | ||
| return new ApiError({ | ||
| message, | ||
| status: 401, | ||
| }); | ||
| } | ||
|
|
||
| static notFound(message) { | ||
| return new ApiError({ | ||
| message, | ||
| status: 404, | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing 404 handler: The task explicitly requires '404 for all the other pages'. No catch-all route exists for undefined endpoints. Add app.use((req, res) => res.status(404).send({ message: 'Not found' })) after all routes but before errorMiddleware.
| @@ -0,0 +1,28 @@ | |||
| export class ApiError extends Error { | |||
There was a problem hiding this comment.
Unused import: The verify import from 'node:crypto' on line 1 is never used in this file. Remove it to avoid linting errors.
|
|
||
| const changePassword = async (req, res) => { | ||
| const { currentPassword, newPassword } = req.body; | ||
|
|
||
| const currentPasswordError = validateValues.password(currentPassword); | ||
| const newPasswordError = validateValues.password(newPassword); | ||
|
|
||
| if (currentPasswordError) { | ||
| throw ApiError.badRequest(currentPasswordError); | ||
| } | ||
|
|
||
| if (newPasswordError) { | ||
| throw ApiError.badRequest(newPasswordError); | ||
| } | ||
|
|
||
| const user = await userService.findById(req.user.id); | ||
|
|
||
| const isCurrentPasswordCorrect = await bcrypt.compare( | ||
| currentPassword, | ||
| user.password, | ||
| ); | ||
|
|
||
| if (!isCurrentPasswordCorrect) { | ||
| throw ApiError.badRequest('Incorrect current password'); | ||
| } | ||
|
|
||
| const newHashedPassword = await bcrypt.hash(newPassword, 10); | ||
|
|
||
| user.password = newHashedPassword; | ||
| await user.save(); | ||
|
|
||
| await tokenService.generateTokens(res, user); |
There was a problem hiding this comment.
The task requires changing password with 'old one, new password and confirmation'. The backend only accepts currentPassword and newPassword but does not require or validate confirmation. The backend should also validate that newPassword === confirmation before updating.
| user.pendingEmail = newEmail; | ||
| user.save(); |
There was a problem hiding this comment.
Missing await before user.save() - this is a fire-and-forget operation that could cause issues.
| })) | ||
| app.use(authRouter); | ||
| app.use('/profile', profileRouter) | ||
|
|
||
| app.use(errorMiddleware); | ||
|
|
||
| app.listen(PORT, () => { | ||
| // eslint-disable-next-line no-console | ||
| console.log(`Server is running on port:${PORT}`); | ||
| }); |
There was a problem hiding this comment.
Missing 404 catch-all handler: The task explicitly requires '404 for all the other pages'. Add app.use((req, res) => res.status(404).send({ message: 'Not found' })) after app.use(errorMiddleware) and before app.listen().
| @@ -0,0 +1,26 @@ | |||
| import { verify } from 'node:crypto'; | |||
There was a problem hiding this comment.
Unused import: verify from 'node:crypto' is imported but never used. This should be removed to avoid dead code.
| import { DataTypes } from 'sequelize'; | ||
| import { client } from '../utils/db.js'; | ||
|
|
||
| export const User = client.define('user', { | ||
| name: { | ||
| type: DataTypes.STRING, | ||
| allowNull: false, | ||
| }, | ||
| email: { | ||
| type: DataTypes.STRING, | ||
| allowNull: false, | ||
| unique: true, | ||
| }, | ||
| password: { |
There was a problem hiding this comment.
Missing 404 handler: The task explicitly requires '404 for all the other pages'. No catch-all route exists in index.js to handle undefined endpoints. Add app.use((req, res) => res.status(404).send({ message: 'Not found' })) before errorMiddleware.
| import { DataTypes } from 'sequelize'; | ||
| import { client } from '../utils/db.js'; | ||
|
|
||
| export const User = client.define('user', { | ||
| name: { | ||
| type: DataTypes.STRING, | ||
| allowNull: false, | ||
| }, | ||
| email: { | ||
| type: DataTypes.STRING, | ||
| allowNull: false, | ||
| unique: true, | ||
| }, | ||
| password: { | ||
| type: DataTypes.STRING, | ||
| allowNull: false, | ||
| }, | ||
| activationToken: { | ||
| type: DataTypes.STRING, | ||
| }, | ||
| resetToken: { | ||
| type: DataTypes.STRING, | ||
| }, | ||
| pendingEmail: { | ||
| type: DataTypes.STRING, | ||
| }, | ||
| emailChangeToken: { | ||
| type: DataTypes.STRING, | ||
| } |
There was a problem hiding this comment.
Missing isActive field: The task mentions checking if user is active and redirecting them if not. The User model doesn't have an explicit isActive boolean field - it uses activationToken being null to indicate active status. This works, but consider if an explicit isActive field would be clearer.
| import { DataTypes } from 'sequelize'; | ||
| import { client } from '../utils/db.js'; | ||
|
|
||
| export const User = client.define('user', { | ||
| name: { | ||
| type: DataTypes.STRING, | ||
| allowNull: false, | ||
| }, |
There was a problem hiding this comment.
Missing password confirmation validation: The task requires profile password change to require 'old one, new password and confirmation'. The backend changePassword only accepts currentPassword and newPassword without validating confirmation. While the frontend validates this, the backend should enforce it too.
| import { DataTypes } from 'sequelize'; | ||
| import { client } from '../utils/db.js'; | ||
|
|
||
| export const User = client.define('user', { | ||
| name: { | ||
| type: DataTypes.STRING, | ||
| allowNull: false, | ||
| }, |
There was a problem hiding this comment.
The password validation function only checks for minimum length (6 characters). The task requires informing users about password rules and validating against them. Consider adding rules like special characters, uppercase, etc.
| @@ -0,0 +1,13 @@ | |||
| import { DataTypes } from 'sequelize'; | |||
| import { client } from '../utils/db.js'; | |||
| import { User } from './user.js'; | |||
There was a problem hiding this comment.
Syntax error: Missing semicolon at the end of the import statement. This will cause the module to fail to load. Change line 3 to: import { catchError } from '../utils/catchError.js';
|
|
||
| if (error) { | ||
| res.statusCode = 500; | ||
| res.send({ | ||
| message: 'Server error', | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
Unreachable code: The next() call on line 20 is never reached because res.send() on line 15 already sends a response. Additionally, when error is falsy, nothing is sent and next() is still called. Consider removing the next() call or restructuring the logic.
| import { ApiError } from '../exeptions/api.error.js'; | ||
|
|
||
| export const errorMiddleware = (error, req, res, next) => { | ||
|
|
||
| if (error instanceof ApiError) { | ||
| res.status(error.status).send({ | ||
| message: error.message, | ||
| errors: error.errors, | ||
| }); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| if (error) { |
There was a problem hiding this comment.
Routes for non-authenticated users (registration, login, password reset) should include middleware to check if a user is already authenticated and redirect them. Currently, authenticated users could still access these routes.
| @@ -0,0 +1,37 @@ | |||
| import jwt from 'jsonwebtoken'; | |||
| import 'dotenv/config'; | |||
There was a problem hiding this comment.
Syntax error: Missing semicolon at the end of the import statement. This will prevent the module from loading. Change line 2 to: import { catchError } from '../utils/catchError.js';
| } | ||
|
|
||
| function verify(token) { | ||
| try { |
There was a problem hiding this comment.
The confirm-email-change route is correctly placed outside authMiddleware since users access it via email link. However, if an authenticated user accesses this route directly, they should be redirected to their profile. Consider adding logic to check if user is already authenticated.
| @@ -0,0 +1,88 @@ | |||
| import 'dotenv/config'; | |||
| import nodemailer from 'nodemailer'; | |||
There was a problem hiding this comment.
Missing semicolon: The import statement on line 2 is missing a semicolon. Change to: import { catchError } from '../utils/catchError.js';
| const html = ` | ||
| <h2>Activate account</h1> | ||
| <a href="${href}">${href}</a> |
There was a problem hiding this comment.
HTML tag mismatch: The h2 opening tag on line 23 is closed with </h1>. This should be </h2>.
| const html = ` | ||
| <h2>Reset password</h1> | ||
| <a href="${href}">${href}</a> |
There was a problem hiding this comment.
HTML tag mismatch: The h2 opening tag on line 37 is closed with </h1>. This should be </h2>.
| const html = ` | ||
| <h2>Change email</h1> | ||
| <a href="${href}">${href}</a> |
There was a problem hiding this comment.
HTML tag mismatch: The h2 opening tag on line 51 is closed with </h1>. This should be </h2>.
| @@ -0,0 +1,11 @@ | |||
| import express from 'express'; | |||
| import { catchError } from '../utils/catchError.js' | |||
There was a problem hiding this comment.
Syntax error: Missing semicolon at the end of the import statement on line 2. Change to: import { catchError } from '../utils/catchError.js';
| import express from 'express'; | ||
| import { catchError } from '../utils/catchError.js' | ||
| import { profileController } from '../controllers/profile.controller.js'; | ||
| import { authMiddleware } from '../middlewares/authMiddleware.js'; | ||
|
|
||
| export const profileRouter = new express.Router(); | ||
|
|
||
| profileRouter.patch('/name', authMiddleware, catchError(profileController.changeName)); | ||
| profileRouter.patch('/password', authMiddleware, catchError(profileController.changePassword)); | ||
| profileRouter.patch('/email', authMiddleware, catchError(profileController.requestEmailChange)); | ||
| profileRouter.get('/confirm-email-change/:emailChangeToken', catchError(profileController.confirmEmailChange)); |
There was a problem hiding this comment.
Missing 404 handler: The task explicitly requires '404 for all the other pages'. Add app.use((req, res) => res.status(404).send({ message: 'Not found' })) to index.js after all routes.
| @@ -0,0 +1,54 @@ | |||
| import { Token } from '../models/token.js'; | |||
| import { userService } from '../services/user.service.js'; | |||
There was a problem hiding this comment.
Syntax error: Missing semicolon at the end of the import statement. This will cause the module to fail to load. Change line 2 to: import { catchError } from '../utils/catchError.js';
| where: { userId }, | ||
| }); | ||
|
|
There was a problem hiding this comment.
The password change endpoint doesn't require or validate password confirmation. The task requires 'new password and confirmation' fields. While the frontend validates this, the backend should also enforce it by checking that newPassword === confirmation before updating.
| import { Token } from '../models/token.js'; | ||
| import { userService } from '../services/user.service.js'; | ||
| import { jwtService } from '../services/jwt.service.js'; | ||
|
|
||
| async function save(userId, refreshToken) { | ||
| const token = await Token.findOne({ | ||
| where: { userId }, | ||
| }); | ||
|
|
||
| if (token) { | ||
| token.refreshToken = refreshToken; |
There was a problem hiding this comment.
Missing 404 handler: The task explicitly requires '404 for all the other pages'. No catch-all route exists in index.js to handle undefined endpoints. Add app.use((req, res) => res.status(404).send({ message: 'Not found' })) in index.js before errorMiddleware.
| @@ -0,0 +1,78 @@ | |||
| import { where } from 'sequelize'; | |||
There was a problem hiding this comment.
Unused import: where is imported from 'sequelize' on line 1 but never used in this file. Remove it to avoid dead code.
|
|
||
| async function register(name, email, password) { | ||
| const existUser = await findByEmail(email); | ||
|
|
||
| if (existUser) { | ||
| throw ApiError.badRequest('User already exists'); | ||
| } | ||
|
|
||
| const activationToken = uuidvv4(); | ||
|
|
||
| await User.create({ |
There was a problem hiding this comment.
Password validation too minimal: The password function only checks for minimum 6 characters. The task requires informing users about password rules and validating against them (special characters, uppercase, etc.). Add rules like: at least one uppercase, one lowercase, one number, one special character, minimum 8 characters.
| import { User } from '../models/user.js'; | ||
| import { emailService } from '../services/email.service.js'; | ||
| import { v4 as uuidvv4 } from 'uuid'; | ||
|
|
There was a problem hiding this comment.
Unnecessary workaround: Adding + '' to convert DB_PASSWORD assumes it's a number, but environment variables are always strings. Simply use process.env.DB_PASSWORD directly.
| import { where } from 'sequelize'; | ||
| import { ApiError } from '../exeptions/api.error.js'; | ||
| import { User } from '../models/user.js'; | ||
| import { emailService } from '../services/email.service.js'; | ||
| import { v4 as uuidvv4 } from 'uuid'; | ||
|
|
||
| function getAllActivated() { | ||
| return User.findAll({ | ||
| where: { | ||
| activationToken: null, | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| function normalizeData({ id, name, email }) { | ||
| return { id, name, email }; | ||
| } | ||
|
|
||
| function findByEmail(email) { | ||
| return User.findOne({ where: { email } }); | ||
| } | ||
|
|
||
| function findById(id) { | ||
| return User.findByPk(id); | ||
| } | ||
|
|
||
| async function register(name, email, password) { | ||
| const existUser = await findByEmail(email); | ||
|
|
||
| if (existUser) { | ||
| throw ApiError.badRequest('User already exists'); | ||
| } | ||
|
|
||
| const activationToken = uuidvv4(); | ||
|
|
||
| await User.create({ | ||
| name, | ||
| email, | ||
| password, | ||
| activationToken, | ||
| }); | ||
|
|
||
| await emailService.sendActivationEmail(email, activationToken); |
There was a problem hiding this comment.
Missing 404 handler: The task explicitly requires '404 for all the other pages'. This should be added in index.js after all routes and before errorMiddleware: app.use((req, res) => res.status(404).send({ message: 'Not found' }))
| @@ -0,0 +1,10 @@ | |||
| import { Sequelize } from 'sequelize'; | |||
There was a problem hiding this comment.
Unused import: The { where } import from 'sequelize' on line 1 is never used. The where clause is used directly in object form. Remove this import.
| @@ -0,0 +1,9 @@ | |||
| export const catchError = (action) => { | |||
There was a problem hiding this comment.
Unused import: where is imported from 'sequelize' on line 1 but never used anywhere in the file. Remove this import to avoid dead code.
| @@ -0,0 +1,43 @@ | |||
| function name(value) { | |||
There was a problem hiding this comment.
Unused import: The where import from 'sequelize' on line 1 is never used in this file. Remove it to avoid linting errors.
|
|
||
| function password(value) { | ||
| if (!value) { | ||
| return 'Password is required'; | ||
| } | ||
|
|
||
| if (value.length < 6) { | ||
| return 'At least 6 characters'; | ||
| } | ||
|
|
||
| return ''; |
There was a problem hiding this comment.
Missing password rules: The password validation only checks for minimum length (6 characters). The task requires informing users about password rules and validating against them. Consider adding validation for special characters, uppercase letters, etc., and the frontend should display these rules to users during registration.
| <!doctype html> | ||
| <html lang="en"> | ||
| <head> | ||
| <meta charset="UTF-8" /> | ||
| <link rel="icon" type="image/svg+xml" href="/favicon.svg" /> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0" /> |
There was a problem hiding this comment.
Missing semicolons: All import statements (lines 1-5) are missing semicolons. While modern JavaScript can parse these without semicolons due to ASI (Automatic Semicolon Insertion), ESLint config files often have issues. Add semicolons to all import statements for consistency.
| # React + TypeScript + Vite | ||
|
|
||
| This template provides a minimal setup to get React working in Vite with HMR and some ESLint rules. | ||
|
|
||
| Currently, two official plugins are available: | ||
|
|
There was a problem hiding this comment.
Missing semicolons: All import statements (lines 1-6) are missing semicolons. Add semicolons at the end of each import statement to fix syntax errors.
| return ( | ||
| <> | ||
| <nav className="nav"> | ||
| <span className="nav-title">Auth application</span> | ||
| <div className="nav-wrapper"> | ||
| <NavLink | ||
| to="/" | ||
| className={`nav-link ${pathname === '/' ? 'nav-link-active' : ''}`} | ||
| > | ||
| Home | ||
| </NavLink> | ||
| {user && ( | ||
| <NavLink | ||
| to="/profile" | ||
| className={`nav-link nav-link-profile ${pathname === '/profile' ? 'nav-link-active' : ''}`} | ||
| > | ||
| Profile Page | ||
| </NavLink> | ||
| )} | ||
|
|
||
| {user ? ( | ||
| <button className="form-button" onClick={handleLogout}> | ||
| Log out | ||
| </button> | ||
| ) : ( | ||
| <> | ||
| <NavLink | ||
| to="/register" | ||
| className={`nav-link ${pathname === '/register' ? 'nav-link-active' : ''}`} | ||
| > | ||
| Register | ||
| </NavLink> | ||
| <NavLink | ||
| to="/login" | ||
| className={`nav-link ${pathname === '/login' ? 'nav-link-active' : ''}`} | ||
| > | ||
| Sign in | ||
| </NavLink> | ||
| </> | ||
| )} | ||
| </div> | ||
| </nav> | ||
| <div className="gradient-wrapper"> | ||
| <main className="main"> | ||
| <section className="routes"> |
There was a problem hiding this comment.
Missing password rules display: The task requires informing users about password rules during registration. The registration page only shows validation errors but doesn't display what the password rules are (e.g., minimum length, special characters, uppercase). Consider adding a password requirements hint near the password field.
| if (authUser) { | ||
| setUser(authUser); | ||
| } | ||
| } catch (error) { | ||
| console.log(error); | ||
| } finally { | ||
| setIsLoading(false); | ||
| } | ||
| } | ||
| fetchUserData(); | ||
| }, [setUser, setIsLoading]); | ||
|
|
||
| const handleLogout = async () => { | ||
| await logout(); | ||
| setUser(null); | ||
| navigate('/login'); | ||
| }; |
There was a problem hiding this comment.
Inconsistent error handling: The login function (line 48-64) doesn't have a final throw new Error for non-Axios errors, unlike other functions. If a non-Axios error occurs, the function silently returns undefined. Consider adding: throw new Error('Unknown error', { cause: error });
| <Routes> | ||
| <Route path="/" element={<HomePage />} /> | ||
| <Route path="register" element={<RegisterPage />} /> | ||
| <Route | ||
| path="login" | ||
| element={user ? <Navigate to="/profile" /> : <LoginPage />} | ||
| /> | ||
| <Route | ||
| path="activation/:activationToken" | ||
| element={<ActivationPage />} | ||
| /> | ||
| <Route | ||
| path="profile" | ||
| element={user ? <ProfilePage /> : <Navigate to="/login" />} | ||
| /> | ||
| <Route | ||
| path="/forgot-password" | ||
| element={ | ||
| user ? <Navigate to="/login" /> : <ResetPasswordForm /> | ||
| } | ||
| /> | ||
| <Route | ||
| path="/reset-password/:resetToken" | ||
| element={user ? <Navigate to="/" /> : <ResetPasswordPage />} | ||
| /> | ||
| <Route | ||
| path="/confirm-email-change/:emailChangeToken" | ||
| element={<ConfirmEmailChange />} | ||
| /> | ||
| <Route path="*" element={<NotFoundPage />} /> |
There was a problem hiding this comment.
Missing 404 handler: The task explicitly requires '404 for all the other pages'. While the frontend has a NotFoundPage, the backend index.js needs a catch-all route to return 404 for undefined endpoints: app.use((req, res) => res.status(404).send({ message: 'Not found' }))
| throw new Error(error.response?.data.message || 'Change failed', { | ||
| cause: error, | ||
| }); | ||
| } | ||
|
|
||
| throw new Error('Unknown error', { | ||
| cause: error, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| export async function changeEmail(newEmail: string, password: string) { | ||
| try { | ||
| const response = await api.patch('/profile/email', { | ||
| newEmail, | ||
| password, | ||
| }); | ||
|
|
||
| return response.data; | ||
| } catch (error) { | ||
| if (axios.isAxiosError(error)) { | ||
| throw new Error(error.response?.data.message || 'Change failed', { | ||
| cause: error, |
There was a problem hiding this comment.
The useEffect dependency array is missing navigate which could cause React warnings about missing dependencies. Consider adding navigate to the dependency array or using useCallback for the effect.
|
|
||
| export async function changePassword( | ||
| currentPassword: string, | ||
| newPassword: string, | ||
| ) { | ||
| try { | ||
| const response = await api.patch('/profile/password', { | ||
| currentPassword, | ||
| newPassword, | ||
| }); | ||
|
|
||
| return response.data; | ||
| } catch (error) { | ||
| if (axios.isAxiosError(error)) { | ||
| throw new Error(error.response?.data.message || 'Change failed', { | ||
| cause: error, | ||
| }); | ||
| } | ||
|
|
||
| throw new Error('Unknown error', { | ||
| cause: error, | ||
| }); | ||
| } |
There was a problem hiding this comment.
The changePassword function only sends currentPassword and newPassword but the task requires 'new password and confirmation'. The frontend should also collect and send confirmation field, and validate that newPassword === confirmation before making the API call.
| try { | ||
| await api.get('/logout'); | ||
|
|
||
| accessTokenService.remove(); | ||
| } catch (error) { | ||
| if (axios.isAxiosError(error)) { | ||
| throw new Error(error.response?.data.message || 'Logout failed', { | ||
| cause: error, | ||
| }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| export async function activation(activationToken: string) { | ||
| try { |
There was a problem hiding this comment.
Missing password rules display: The task requires informing users about password rules. The registration page doesn't display what the password requirements are (minimum length, special characters, etc.). Add a helper text or list showing the password requirements near the password field.
|
|
||
| export async function logout() { | ||
| try { | ||
| await api.get('/logout'); | ||
|
|
||
| accessTokenService.remove(); | ||
| } catch (error) { | ||
| if (axios.isAxiosError(error)) { | ||
| throw new Error(error.response?.data.message || 'Logout failed', { | ||
| cause: error, | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Inconsistent error handling: The logout function (lines 66-77) doesn't throw an error for non-Axios errors (the catch block only handles Axios errors). Other functions like login have the same issue. Consider adding a generic error throw in the catch block.
| accessToken: string; | ||
| user: { | ||
| email: string; | ||
| id: number; | ||
| name: string; | ||
| }; | ||
| }; | ||
|
|
||
|
|
||
|
|
||
| export async function register(data: CreatePostBody) { | ||
| try { | ||
| const response = await api.post<CreatePostResponse>('/registration', data); | ||
|
|
||
| return response.data; | ||
| } catch (error) { | ||
| if (axios.isAxiosError(error)) { | ||
| throw new Error(error.response?.data.message || 'Registration failed', { | ||
| cause: error, | ||
| }); | ||
| } | ||
|
|
||
| throw new Error('Unknown error', { |
There was a problem hiding this comment.
Missing password confirmation in API: The changePassword function sends currentPassword and newPassword, but the task requires a confirmation field. The backend should also validate the confirmation. Consider adding a confirmPassword parameter and validating it matches newPassword.
|
|
||
| useEffect(() => { | ||
| if (!activationToken) { | ||
| return; | ||
| } | ||
|
|
||
| activation(activationToken) | ||
| .then(setUser) | ||
| .catch((error) => { | ||
| if (error instanceof Error) { | ||
| setError(error.message); | ||
| } | ||
| }) | ||
| .finally(() => setDone(true)); | ||
| navigate('/profile'); |
There was a problem hiding this comment.
Navigation timing issue: The navigate('/profile') call on line 26 happens immediately after starting the activation promise, not after it resolves. This means the user navigates to profile before the activation actually completes. Move navigate inside a .then() block after the activation completes, or call it only in the .finally() block after setting done to true.
| import { useContext, useEffect, useState } from 'react'; | ||
| import { useNavigate, useParams } from 'react-router-dom'; | ||
| import { AuthContext } from '../context/AuthContext'; | ||
| import { activation } from '../api/auth'; | ||
| import { Loader } from './components/Loader'; | ||
|
|
||
| export const ActivationPage: React.FC = () => { | ||
| const [done, setDone] = useState(false); | ||
| const [error, setError] = useState(''); | ||
| const { activationToken } = useParams(); | ||
| const { setUser } = useContext(AuthContext); | ||
| const navigate = useNavigate(); | ||
|
|
||
| useEffect(() => { | ||
| if (!activationToken) { | ||
| return; | ||
| } | ||
|
|
||
| activation(activationToken) | ||
| .then(setUser) | ||
| .catch((error) => { | ||
| if (error instanceof Error) { | ||
| setError(error.message); | ||
| } | ||
| }) | ||
| .finally(() => setDone(true)); | ||
| navigate('/profile'); | ||
| }, [setUser, activationToken, navigate]); | ||
|
|
||
| if (!done) { | ||
| return <Loader />; | ||
| } | ||
|
|
||
| return ( | ||
| <div> | ||
| <h2 className="title">Account activation</h2> | ||
|
|
||
| {error ? ( | ||
| <p className="notification">{error}</p> | ||
| ) : ( | ||
| <p className="notification"> | ||
| Your account is now active | ||
| </p> | ||
| )} | ||
| </div> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Missing password rules display: The task requires informing users about password rules during registration. No component reviewed yet displays these requirements to users.
| localStorage.setItem(key, JSON.stringify(value)); | ||
| } catch { | ||
| throw new Error('Error'); |
There was a problem hiding this comment.
Throwing in useEffect: The throw new Error('Error') in the catch block inside useEffect (line 20) is problematic. Throwing errors in effects can cause React warnings and unexpected behavior. Instead of throwing, consider logging the error or setting a state to display an error message to the user.
| type Props = { | ||
| children: React.ReactNode; | ||
| }; |
There was a problem hiding this comment.
Error handling in useEffect: Throwing an error inside a useEffect callback (line 20) is problematic. The error won't be caught by React error boundaries and will crash the app. Consider handling the error differently, such as setting an error state instead of throwing.
| user: null, | ||
| setUser: () => {}, | ||
| isLoading: false, | ||
| setIsLoading: () => {}, | ||
| }); | ||
|
|
||
| type Props = { | ||
| children: React.ReactNode; | ||
| }; | ||
|
|
||
| export const AuthProvider: React.FC<Props> = ({ children }) => { | ||
| const [user, setUser] = useState<ProfileUser | null>(null); | ||
| const [isLoading, setIsLoading] = useState(false); | ||
|
|
||
| const value = useMemo( |
There was a problem hiding this comment.
Navigation timing issue: The navigate('/profile') on line 26 is called before the activation promise resolves. This means the user navigates to profile even if activation fails. Move the navigation inside .then() after setUser() to ensure it only happens on success.
| <div className="profile-information-done"> | ||
| <h3 className="profile-change-email-title"> | ||
| Confirmation link sent! Please check your new email address to | ||
| complete the change |
There was a problem hiding this comment.
Incorrect heading: The h3 on line 68 says 'Change email' but this is the password change page. It should say 'Change password'.
| user: ProfileUser; | ||
| }; | ||
|
|
||
| export const ProfileChangeEmail: React.FC<Props> = ({ user }) => { |
There was a problem hiding this comment.
Unused prop: The Props type on line 8 defines user: ProfileUser, but the component on line 11 doesn't destructure it. Either remove the unused prop or use it.
|
|
||
| const newErrors: ChangeEmailErrors = { |
There was a problem hiding this comment.
Variable shadowing: On line 26, the user variable is reassigned with the result of changeName(), which shadows the user prop. This can cause confusion. Consider using a different variable name like updatedUser.
| } | ||
|
|
||
| return ( | ||
| <div className="profile-information"> |
There was a problem hiding this comment.
Incorrect form title: The h3 on line 68 says 'Change email' but this component is for changing password. Change to: <h3>Change password</h3>
| type Props = { | ||
| user: ProfileUser; | ||
| }; | ||
|
|
There was a problem hiding this comment.
Unused Props type: The Props type defines user: ProfileUser but the component doesn't destructure or use it (line 11). Either use the user prop or remove it from the Props type.
| event.preventDefault(); | ||
|
|
There was a problem hiding this comment.
Variable shadowing: On line 26, const user = await changeName(name) shadows the props user parameter. While this works, it could cause confusion. Consider renaming to const updatedUser.
| return ( | ||
| <div className="profile-information-done"> | ||
| <CircleAlert size={50} color="red" /> | ||
| <h3 className="profile-information-title"> |
There was a problem hiding this comment.
Typo in heading: The h3 on line 68 says 'Change email' but this is the password change page. It should say 'Change password'.
| type Status = 'success' | 'error' | null; | ||
|
|
||
| export const ConfirmEmailChange: React.FC = () => { | ||
| const { emailChangeToken } = useParams(); | ||
| const [isLoading, setIsLoading] = useState(false); |
There was a problem hiding this comment.
Unused type definition: The Props type and user prop (lines 7-11) are never used in the component. Either remove them or use the user prop if needed.
| } | ||
|
|
There was a problem hiding this comment.
Variable shadowing: The variable user on line 26 shadows the user prop parameter. While this works, it could be confusing. Consider renaming to updatedUser or similar.
| type Props = { | ||
| user: ProfileUser; | ||
| }; | ||
|
|
||
| export const ProfileInfo: React.FC<Props> = ({ user }) => { |
There was a problem hiding this comment.
Unused type definition: The Props type is defined (lines 7-9) but the component on line 11 doesn't destructure the user prop from props. Either use the prop or remove the type definition.
| </span> | ||
| )} | ||
| {nameForm ? ( | ||
| <div className="profile-buttons"> |
There was a problem hiding this comment.
Wrong heading text: The h3 on line 68 says 'Change email' but this is the password change section. It should say 'Change password'.
| try { | ||
| const user = await changeName(name); |
There was a problem hiding this comment.
Variable shadowing: The user constant on line 26 shadows the user prop (line 10). While this works, it could be confusing. Consider renaming the API response variable to updatedUser or similar.
|
|
||
| if (!resetToken) { | ||
| return; |
There was a problem hiding this comment.
Missing error handling for missing token: On line 41-43, if resetToken is undefined, the function returns silently without informing the user. Consider showing an error message like 'Invalid reset link'.
| const [confirmPassword, setConfirmPassword] = useState(''); | ||
| const [error, setError] = useState(''); | ||
| const [errors, setErrors] = useState<NewPasswordErrors>({ | ||
| newPassword: '', | ||
| confirmPassword: '', |
There was a problem hiding this comment.
Missing email validation: The ResetPasswordForm doesn't validate the email format before sending the request. Consider adding email validation using validateValues.email(email) before calling resetPassword.
|
|
||
| const handleSubmit = async (event: React.SubmitEvent) => { | ||
| event.preventDefault(); | ||
|
|
There was a problem hiding this comment.
Missing email validation: The ResetPasswordForm doesn't validate the email before submitting. Consider adding validation using validateValues.email(email) to check if the email is valid before making the API call.
|
|
||
| const handleReset = async () => { | ||
| await resetPassword(email); | ||
| setRegistered(true); |
There was a problem hiding this comment.
Missing error handling: The handleReset function on lines 7-10 doesn't handle errors. If the API call fails, the user won't know. Add try-catch and error state handling.
| </div> | ||
| <div className="input-container"> | ||
| <label htmlFor="password"> | ||
| <strong>Password</strong> | ||
| </label> | ||
| <input | ||
| type="password" | ||
| name="password" | ||
| id="password" | ||
| value={password} | ||
| onChange={(e) => setPassword(e.target.value)} | ||
| className={`form-input ${errors.password ? 'input-error' : ''}`} | ||
| placeholder="Enter your password" | ||
| /> |
There was a problem hiding this comment.
Missing password rules display: The task explicitly requires 'Inform the users about the rules for a password'. The registration page should display the password requirements (e.g., minimum length, special characters, uppercase, numbers) near the password field so users know what rules apply.
| const [name, setName] = useState<string>(''); | ||
| const [email, setEmail] = useState<string>(''); | ||
| const [password, setPassword] = useState<string>(''); | ||
| const [errors, setErrors] = useState<FormErrors>({ | ||
| name: '', | ||
| email: '', | ||
| password: '', | ||
| }); | ||
| const [serverError, setServerError] = useState(''); | ||
| const [registered, setRegistered] = useState(false); | ||
| const [isLoading, setIsLoading] = useState(false); | ||
|
|
||
| if (registered) { | ||
| return ( | ||
| <section className="registered"> | ||
| <h2>Check your email</h2> | ||
| <p>We have sent you an email with the activation link</p> | ||
| </section> | ||
| ); | ||
| } | ||
|
|
||
| const handleRegister = async () => { | ||
| setIsLoading(true); | ||
| try { | ||
| setServerError(''); | ||
|
|
||
| await register({ | ||
| name, | ||
| email, | ||
| password, | ||
| }); | ||
|
|
||
| setName(''); | ||
| setEmail(''); | ||
| setPassword(''); | ||
|
|
||
| return true; | ||
| } catch (error) { | ||
| if (error instanceof Error) { | ||
| setServerError(error.message); | ||
| } | ||
|
|
||
| return false; | ||
| } finally { | ||
| setIsLoading(false); | ||
| } | ||
| }; | ||
|
|
||
| const handleSubmit = async (event: React.SubmitEvent) => { | ||
| event.preventDefault(); | ||
|
|
||
| const newErrors: FormErrors = { | ||
| name: validateValues.name(name), | ||
| email: validateValues.email(email), | ||
| password: validateValues.password(password), | ||
| }; | ||
|
|
||
| setErrors(newErrors); | ||
|
|
||
| const hasErrors = Object.values(newErrors).some((error) => error); | ||
|
|
||
| if (hasErrors) { | ||
| return; | ||
| } | ||
|
|
||
| const success = await handleRegister(); | ||
| if (success) { | ||
| setRegistered(true); | ||
| } | ||
| }; | ||
|
|
||
| return ( | ||
| <div | ||
| className={`form-wrapper background-white ${serverError ? 'form-wrapper-danger' : ''}`} | ||
| > | ||
| <h2 className="title">Sign up</h2> | ||
| <form className="form-container" onSubmit={handleSubmit}> | ||
| <div className="input-container"> | ||
| <label htmlFor="name"> | ||
| <strong>Name</strong> | ||
| </label> | ||
| <input | ||
| type="text" | ||
| name="name" | ||
| id="name" | ||
| value={name} | ||
| onChange={(e) => setName(e.target.value)} | ||
| className={`form-input ${errors.name ? 'input-error' : ''}`} | ||
| placeholder="Enter your name" | ||
| /> | ||
| {errors.name && <p className="notification">{errors.name}</p>} | ||
| </div> | ||
| <div className="input-container"> | ||
| <label htmlFor="email"> | ||
| <strong>Email</strong> | ||
| </label> | ||
| <input | ||
| type="email" | ||
| name="email" | ||
| id="email" | ||
| value={email} | ||
| onChange={(e) => setEmail(e.target.value)} | ||
| className={`form-input ${errors.email ? 'input-error' : ''}`} | ||
| placeholder="Enter your email" | ||
| /> | ||
| {errors.email && <p className="notification">{errors.email}</p>} | ||
| </div> | ||
| <div className="input-container"> | ||
| <label htmlFor="password"> | ||
| <strong>Password</strong> | ||
| </label> | ||
| <input |
There was a problem hiding this comment.
Missing password rules display: The reset password page should also inform users about password rules when they set a new password.
| export type ConfirmNewPasswordData = { | ||
| resetToken: string, | ||
| newPassword: string, |
There was a problem hiding this comment.
Missing confirmPassword field: The ConfirmNewPasswordData type only has resetToken and newPassword. The task requires validating that 'password and confirmation fields must be equal'. Add confirmPassword: string to the type definition.
| export type FormErrors = { | ||
| name: string; | ||
| email: string; | ||
| password: string; | ||
| }; | ||
|
|
There was a problem hiding this comment.
Sensitive data exposure: The User interface (lines 1-6) includes password and activationToken fields which are sensitive server-side data. This interface shouldn't be exported or used on the frontend. The ProfileUser interface is sufficient for client-side use.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your solution is approved. All previously identified issues have been successfully resolved: the 404 catch-all handler is properly implemented, password rules are displayed via tooltip on registration and reset password pages, all await keywords are correctly placed before user.save() calls, the ActivationPage awaits the promise before navigation, and password confirmation validation works in both frontend and backend. The only check run failure is a tooling issue with ESLint's --ext flag incompatibility in the mate-scripts package—this is unrelated to your code quality. Excellent work implementing all authentication features including registration with activation, login/logout, password reset, and profile management with name, password, and email changes.
✨ 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.
No description provided.