Merge pull request #5 from isdi-coders-2023/feature/routeFix

remove unnecessary bind

src/controllers/files.controller.ts

@@ -11,8 +11,6 @@ export class FilesController {
   }
 
   fileHandler(req: Request, res: Response, next: NextFunction) {
-    console.log('File', req.file);
-    console.log('Body', req.body);
     if (!req.file) {
       next(new HttpError(400, 'Bad request', 'No file uploaded'));
       return;
@@ -21,7 +19,7 @@ export class FilesController {
     res.json({
       message: 'File uploaded',
       field: req.file.fieldname,
-      width: req.body.cloudinary.height,
+      width: req.body.cloudinary.width,
       height: req.body.cloudinary.height,
       file: req.body.cloudinary.public_id,
       format: req.body.cloudinary.format,

src/controllers/users. controller.spec.ts

@@ -1,9 +1,9 @@
 import { type Request, type Response } from 'express';
-import { UsersController } from './users.controller';
-import { type UsersSqlRepo } from '../repositories/users.sql.repo';
+import { UsersController } from './users.controller.js';
+import { type UsersSqlRepo } from '../repositories/users.sql.repo.js';
 import { Auth } from '../services/auth.service.js';
 import { type ObjectSchema } from 'joi';
-import { type UserCreateDto } from '../entities/user';
+import { type UserCreateDto } from '../entities/user.js';
 
 jest.mock('../entities/user.schema.js', () => ({
   userCreateDtoSchema: {
@@ -134,14 +134,16 @@ describe('Given a instance of the class UsersController', () => {
 
     describe('And body is ok', () => {
       test('Then it should call repo.create', async () => {
-        const user = { name: 'test', password: 'test' };
+        Auth.hash = jest.fn().mockReturnValue('hashedPassword');
+        const user = { name: 'test', password: 'test', repeatPassword: 'test' };
+
         req.body = user;
         req.body.cloudinary = { url: '' };
         req.body.avatar = req.body.cloudinary?.url as string;
-        Auth.hash = jest.fn().mockResolvedValue('hashedPassword');
+        Auth.hash = jest.fn().mockResolvedValue(user.password);
         (repo.create as jest.Mock).mockResolvedValue(user);
         await controller.create(req, res, next);
-        expect(Auth.hash).toHaveBeenCalledWith('test');
+        expect(Auth.hash).toHaveBeenCalled();
         expect(repo.create).toHaveBeenCalledWith({});
         expect(res.status).toHaveBeenCalledWith(201);
         expect(res.json).toHaveBeenCalledWith(user);

src/controllers/users.controller.ts

@@ -67,6 +67,11 @@ export class UsersController extends BaseController<User, UserCreateDto> {
   }
 
   async create(req: Request, res: Response, next: NextFunction) {
+    if (req.body.password !== req.body.repeatPassword) {
+      next(new HttpError(400, 'Bad Request', 'Passwords do not match'));
+      return;
+    }
+
     if (!req.body.password || typeof req.body.password !== 'string') {
       next(
         new HttpError(
@@ -80,8 +85,6 @@ export class UsersController extends BaseController<User, UserCreateDto> {
 
     req.body.password = await Auth.hash(req.body.password as string);
 
-    req.body.avatar = req.body.cloudinary?.url as string;
-
     await super.create(req, res, next);
   }
 

src/middleware/files.interceptor.spec.ts

@@ -1,9 +1,7 @@
-/* eslint-disable @typescript-eslint/no-unsafe-call */
 import { type Request, type Response } from 'express';
-import { FilesInterceptor } from './files.interceptor.js';
-import multer, { type MulterError } from 'multer';
+import { FilesInterceptor } from './files.interceptor';
+import multer from 'multer';
 import { v2 } from 'cloudinary';
-import { HttpError } from './errors.middleware.js';
 
 jest.mock('multer');
 jest.mock('cloudinary');
@@ -23,7 +21,7 @@ describe('Given a instance of the class FilesInterceptor', () => {
     const mockMiddleware = jest.fn();
 
     multer.diskStorage = jest.fn().mockImplementation(({ filename }) =>
-      // eslint-disable-next-line @typescript-eslint/no-unsafe-return
+      // eslint-disable-next-line @typescript-eslint/no-unsafe-return, @typescript-eslint/no-unsafe-call
       filename('', '', () => {
         //
       })
@@ -44,21 +42,37 @@ describe('Given a instance of the class FilesInterceptor', () => {
       upload: jest.fn().mockResolvedValue({}),
     } as unknown as typeof v2.uploader;
 
+    describe('And file is not valid', () => {
+      test('Then it should call next with an error', async () => {
+        req.file = undefined;
+        await interceptor.cloudUpload(req, res, next);
+        expect(next).toHaveBeenCalledWith(
+          expect.objectContaining({
+            message: 'No file uploaded',
+          })
+        );
+      });
+    });
+
     describe('And file is valid', () => {
       test('Then it should call next', async () => {
         req.file = {} as unknown as Express.Multer.File;
-        await interceptor.upload(req, res, next);
+        await interceptor.cloudUpload(req, res, next);
         expect(v2.uploader.upload).toHaveBeenCalled();
         expect(next).toHaveBeenCalled();
       });
     });
-    describe('And file is not valid', () => {
-      test('Then it should call next', async () => {
-        req.file = undefined;
-        await interceptor.upload(req, res, next);
-        expect(v2.uploader.upload).toHaveBeenCalled();
+
+    describe('And upload fails', () => {
+      test('Then it should call next with an error', async () => {
+        req.file = {} as unknown as Express.Multer.File;
+        const error = new Error('Upload failed');
+        v2.uploader.upload = jest.fn().mockRejectedValue(error);
+        await interceptor.cloudUpload(req, res, next);
         expect(next).toHaveBeenCalledWith(
-          new HttpError(400, 'Bad Request', 'No file uploaded')
+          expect.objectContaining({
+            message: error.message,
+          })
         );
       });
     });

src/middleware/files.interceptor.ts

@@ -1,11 +1,8 @@
-/* eslint-disable @typescript-eslint/no-unsafe-assignment */
 import { type NextFunction, type Request, type Response } from 'express';
-import createDebug from 'debug';
-
 import { v2 as cloudinary } from 'cloudinary';
-import { HttpError } from './errors.middleware.js';
+import createDebug from 'debug';
 import multer from 'multer';
-
+import { HttpError } from './errors.middleware.js';
 const debug = createDebug('BOOKS:files:interceptor');
 
 export class FilesInterceptor {
@@ -14,58 +11,53 @@ export class FilesInterceptor {
   }
 
   singleFile(fieldName = 'avatar') {
+    debug('Creating single file middleware');
     const storage = multer.diskStorage({
       destination: 'uploads/',
-      filename(
-        _req: Request,
-        file: Express.Multer.File,
-        callback: (error: Error, filename: string) => void
-      ) {
-        callback(new Error(), Date.now() + '_' + file.originalname);
+      filename(_req, file, callback) {
+        callback(null, Date.now() + '_' + file.originalname);
       },
     });
 
     const upload = multer({ storage });
     const middleware = upload.single(fieldName);
 
     return (req: Request, res: Response, next: NextFunction) => {
-      const previousBody = { ...req.body };
-      middleware(req, res, (err: any) => {
-        if (err instanceof multer.MulterError) {
-          next(new HttpError(400, 'Bad Request', err.message));
-          return;
-        }
-
-        if (err) {
-          next(
-            new HttpError(500, 'Internal Server Error', 'File upload error')
-          );
-          return;
-        }
-
-        req.body = { ...previousBody, ...req.body };
-        next();
-      });
+      debug('Uploading single file');
+      const previousBody = req.body as Record<string, unknown>;
+      middleware(req, res, next);
+      req.body = { ...previousBody, ...req.body } as unknown;
     };
   }
 
-  async upload(req: Request, res: Response, next: NextFunction) {
+  async cloudUpload(req: Request, res: Response, next: NextFunction) {
+    debug('Uploading file to cloudinary');
     const options = {
-      useFilename: true,
-      uniqueFilename: false,
+      folder: 'avatars',
+      // eslint-disable-next-line @typescript-eslint/naming-convention
+      use_filename: true,
+      // eslint-disable-next-line @typescript-eslint/naming-convention
+      unique_filename: false,
       overwrite: true,
     };
+
     if (!req.file) {
-      next(new HttpError(400, 'Bad Request', 'No file uploaded'));
+      next(new HttpError(400, 'Bad request', 'No file uploaded'));
       return;
     }
 
+    const finalPath = req.file.destination + '/' + req.file.filename;
+
     try {
-      const result = await cloudinary.uploader.upload(req.file.path, options);
-      req.body.cloudinary = result;
+      const result = await cloudinary.uploader.upload(finalPath, options);
+
+      req.body.avatar = result.secure_url;
+
       next();
     } catch (error) {
-      next(new HttpError(500, 'Internal Server Error', 'Error uploading file'));
+      next(
+        new HttpError(500, 'Internal server error', (error as Error).message)
+      );
     }
   }
 }

src/repositories/users.sql.repo.ts

@@ -78,10 +78,6 @@ export class UsersSqlRepo implements WithLoginRepo<User, UserCreateDto> {
   }
 
   async create(data: UserCreateDto): Promise<User> {
-    if (data.password !== data.repeatPassword) {
-      throw new HttpError(400, 'Bad Request', 'Passwords do not match');
-    }
-
     try {
       const newUser = await this.prisma.user.create({ data, select });
       return newUser as User;

src/routers/files.router.spec.ts

@@ -1,18 +1,20 @@
+import { FilesRouter } from './files.router.js';
 import { type FilesController } from '../controllers/files.controller.js';
-import { type AuthInterceptor } from '../middleware/auth.interceptor.js';
 import { type FilesInterceptor } from '../middleware/files.interceptor.js';
-import { FilesRouter } from './files.router.js';
 
-describe('Given a instance of the class FilesRouter', () => {
+describe('Given an instance of the class FilesRouter', () => {
   const controller = {
     fileHandler: jest.fn(),
   } as unknown as FilesController;
+
   const interceptor = {
-    singleFile: jest.fn().mockReturnValue(jest.fn()),
-    upload: jest.fn(),
+    singleFile: jest.fn(() => jest.fn()),
+    cloudUpload: jest.fn(),
   } as unknown as FilesInterceptor;
+
   const router = new FilesRouter(controller, interceptor);
-  test('Then it should be instance of the class', () => {
+
+  test('Then it should be an instance of the class', () => {
     expect(router).toBeInstanceOf(FilesRouter);
   });
 });

src/routers/files.router.ts

@@ -1,7 +1,7 @@
 import { Router as createRouter } from 'express';
 import createDebug from 'debug';
-import { type FilesController } from '../controllers/files.controller.js';
-import { type FilesInterceptor } from '../middleware/files.interceptor.js';
+import { type FilesController } from '../controllers/files.controller';
+import { type FilesInterceptor } from '../middleware/files.interceptor';
 
 const debug = createDebug('BOOKS:files:router');
 export class FilesRouter {
@@ -16,7 +16,7 @@ export class FilesRouter {
     this.router.post(
       '/',
       interceptor.singleFile('avatar').bind(interceptor),
-      interceptor.upload.bind(interceptor),
+      interceptor.cloudUpload.bind(interceptor),
       controller.fileHandler.bind(controller)
     );
   }

src/routers/users.router.spec.ts

@@ -17,7 +17,7 @@ describe('Given a instance of the class UsersRouter', () => {
   } as unknown as AuthInterceptor;
   const fileInterceptor: FilesInterceptor = {
     singleFile: jest.fn().mockReturnValue(jest.fn()),
-    upload: jest.fn(),
+    cloudUpload: jest.fn(),
   } as unknown as FilesInterceptor;
   const router = new UsersRouter(controller, authInterceptor, fileInterceptor);
   test('Then it should be instance of the class', () => {

src/routers/users.router.ts

@@ -19,7 +19,7 @@ export class UsersRouter {
     this.router.post(
       '/signup',
       filesInterceptor.singleFile('avatar'),
-      filesInterceptor.upload.bind(filesInterceptor),
+      filesInterceptor.cloudUpload.bind(filesInterceptor),
       controller.create.bind(controller)
     );
     this.router.post('/login', controller.login.bind(controller));