Fix user e2e tests (#194)

* WIP fix user e2e tests

The e2e tests still don't seem to work due to migrations not running.

Changes:
- update user.e2e tests to use new `userService.createUser` method
- fix server `typeorm` command to use ORM config
- update make test-e2e to re-create database volume every time
- add User DTO
- update auth.service and user.service to use User DTO
- update CreateUserDto making optional properties that are optional

* Fix migrations
- add missing `.ts` extension to migrations path
- update user e2e test for the new returned User resource

Makefile

@@ -11,7 +11,7 @@ stage:
 	docker-compose -f ./docker/docker-compose.staging.yml up --build -V --remove-orphans
 
 test-e2e:
-	docker-compose -f ./docker/docker-compose.test.yml --env-file ./docker/.env.test up --abort-on-container-exit --exit-code-from immich_server_test
+	docker-compose -f ./docker/docker-compose.test.yml --env-file ./docker/.env.test up --renew-anon-volumes --abort-on-container-exit --exit-code-from immich_server_test --remove-orphans
 
 prod:
 	docker-compose -f ./docker/docker-compose.yml up --build -V --remove-orphans

docker/.env.test

@@ -1,9 +1,12 @@
-DB_HOSTNAME=immich_postgres_test
 # Database
+DB_HOSTNAME=immich_postgres_test
 DB_USERNAME=postgres
 DB_PASSWORD=postgres
 DB_DATABASE_NAME=e2e_test
 
+# Redis
+REDIS_HOSTNAME=immich_redis_test
+
 # Upload File Config
 UPLOAD_LOCATION=./upload
 
@@ -13,4 +16,7 @@ JWT_SECRET=randomstringthatissolongandpowerfulthatnoonecanguess
 # MAPBOX
 ## ENABLE_MAPBOX is either true of false -> if true, you have to provide MAPBOX_KEY
 ENABLE_MAPBOX=false
+
+# WEB
 MAPBOX_KEY=
+VITE_SERVER_ENDPOINT=http://localhost:2283

docker/docker-compose.test.yml

@@ -40,7 +40,7 @@ services:
       POSTGRES_DB: ${DB_DATABASE_NAME}
       PG_DATA: /var/lib/postgresql/data
     volumes:
-      - pgdata-test:/var/lib/postgresql/data
+      - /var/lib/postgresql/data
     ports:
       - 5432:5432
     networks:
@@ -48,5 +48,3 @@ services:
 
 networks:
   immich_network_test:
-volumes:
-  pgdata-test:

server/package.json

@@ -19,7 +19,7 @@
     "test:cov": "jest --coverage",
     "test:debug": "node --inspect-brk -r tsconfig-paths/register -r ts-node/register node_modules/.bin/jest --runInBand",
     "test:e2e": "jest --config ./test/jest-e2e.json",
-    "typeorm": "node --require ts-node/register ./node_modules/typeorm/cli.js"
+    "typeorm": "node --require ts-node/register ./node_modules/typeorm/cli.js --config src/config/database.config.ts"
   },
   "dependencies": {
     "@mapbox/mapbox-sdk": "^0.13.3",

server/src/api-v1/auth/auth.service.ts

@@ -7,6 +7,7 @@ import { ImmichJwtService } from '../../modules/immich-jwt/immich-jwt.service';
 import { JwtPayloadDto } from './dto/jwt-payload.dto';
 import { SignUpDto } from './dto/sign-up.dto';
 import * as bcrypt from 'bcrypt';
+import { mapUser, User } from '../user/response-dto/user';
 
 @Injectable()
 export class AuthService {
@@ -14,12 +15,24 @@ export class AuthService {
     @InjectRepository(UserEntity)
     private userRepository: Repository<UserEntity>,
     private immichJwtService: ImmichJwtService,
-  ) { }
+  ) {}
 
   private async validateUser(loginCredential: LoginCredentialDto): Promise<UserEntity> {
     const user = await this.userRepository.findOne(
       { email: loginCredential.email },
-      { select: ['id', 'email', 'password', 'salt', 'firstName', 'lastName', 'isAdmin', 'profileImagePath', 'isFirstLoggedIn'] },
+      {
+        select: [
+          'id',
+          'email',
+          'password',
+          'salt',
+          'firstName',
+          'lastName',
+          'isAdmin',
+          'profileImagePath',
+          'isFirstLoggedIn',
+        ],
+      },
     );
 
     const isAuthenticated = await this.validatePassword(user.password, loginCredential.password, user.salt);
@@ -48,38 +61,29 @@ export class AuthService {
       lastName: validatedUser.lastName,
       isAdmin: validatedUser.isAdmin,
       profileImagePath: validatedUser.profileImagePath,
-      isFirstLogin: validatedUser.isFirstLoggedIn
+      isFirstLogin: validatedUser.isFirstLoggedIn,
     };
   }
 
-
-  public async adminSignUp(signUpCrendential: SignUpDto) {
+  public async adminSignUp(signUpCredential: SignUpDto): Promise<User> {
     const adminUser = await this.userRepository.findOne({ where: { isAdmin: true } });
 
     if (adminUser) {
-      throw new BadRequestException('The server already has an admin')
+      throw new BadRequestException('The server already has an admin');
     }
 
-
     const newAdminUser = new UserEntity();
-    newAdminUser.email = signUpCrendential.email;
+    newAdminUser.email = signUpCredential.email;
     newAdminUser.salt = await bcrypt.genSalt();
-    newAdminUser.password = await this.hashPassword(signUpCrendential.password, newAdminUser.salt);
-    newAdminUser.firstName = signUpCrendential.firstName;
-    newAdminUser.lastName = signUpCrendential.lastName;
+    newAdminUser.password = await this.hashPassword(signUpCredential.password, newAdminUser.salt);
+    newAdminUser.firstName = signUpCredential.firstName;
+    newAdminUser.lastName = signUpCredential.lastName;
     newAdminUser.isAdmin = true;
 
     try {
       const savedNewAdminUserUser = await this.userRepository.save(newAdminUser);
 
-      return {
-        id: savedNewAdminUserUser.id,
-        email: savedNewAdminUserUser.email,
-        firstName: savedNewAdminUserUser.firstName,
-        lastName: savedNewAdminUserUser.lastName,
-        createdAt: savedNewAdminUserUser.createdAt,
-      };
-
+      return mapUser(savedNewAdminUserUser);
     } catch (e) {
       Logger.error('e', 'signUp');
       throw new InternalServerErrorException('Failed to register new admin user');

server/src/api-v1/user/dto/create-user.dto.ts

@@ -14,14 +14,14 @@ export class CreateUserDto {
   lastName: string;
 
   @IsOptional()
-  profileImagePath: string;
+  profileImagePath?: string;
 
   @IsOptional()
-  isAdmin: boolean;
+  isAdmin?: boolean;
 
   @IsOptional()
-  isFirstLoggedIn: boolean;
+  isFirstLoggedIn?: boolean;
 
   @IsOptional()
-  id: string;
+  id?: string;
 }

server/src/api-v1/user/response-dto/user.ts

@@ -0,0 +1,19 @@
+import { UserEntity } from '../entities/user.entity';
+
+export interface User {
+  id: string;
+  email: string;
+  firstName: string;
+  lastName: string;
+  createdAt: string;
+}
+
+export function mapUser(entity: UserEntity): User {
+  return {
+    id: entity.id,
+    email: entity.email,
+    firstName: entity.firstName,
+    lastName: entity.lastName,
+    createdAt: entity.createdAt,
+  };
+}

server/src/api-v1/user/user.service.ts

@@ -6,19 +6,18 @@ import { CreateUserDto } from './dto/create-user.dto';
 import { UpdateUserDto } from './dto/update-user.dto';
 import { UserEntity } from './entities/user.entity';
 import * as bcrypt from 'bcrypt';
-import sharp from 'sharp';
-import { createReadStream, unlink, unlinkSync } from 'fs';
+import { createReadStream } from 'fs';
 import { Response as Res } from 'express';
+import { mapUser, User } from './response-dto/user';
 
 @Injectable()
 export class UserService {
   constructor(
     @InjectRepository(UserEntity)
     private userRepository: Repository<UserEntity>,
-  ) { }
+  ) {}
 
   async getAllUsers(authUser: AuthUserDto, isAll: boolean) {
-
     if (isAll) {
       return await this.userRepository.find();
     }
@@ -26,8 +25,8 @@ export class UserService {
     return await this.userRepository.find({
       where: { id: Not(authUser.id) },
       order: {
-        createdAt: 'DESC'
-      }
+        createdAt: 'DESC',
+      },
     });
   }
 
@@ -40,14 +39,12 @@ export class UserService {
       users = await this.userRepository.find();
     }
 
-
     return {
-      userCount: users.length
-    }
-
+      userCount: users.length,
+    };
   }
 
-  async createUser(createUserDto: CreateUserDto) {
+  async createUser(createUserDto: CreateUserDto): Promise<User> {
     const user = await this.userRepository.findOne({ where: { email: createUserDto.email } });
 
     if (user) {
@@ -62,18 +59,10 @@ export class UserService {
     newUser.lastName = createUserDto.lastName;
     newUser.isAdmin = false;
 
-
     try {
       const savedUser = await this.userRepository.save(newUser);
 
-      return {
-        id: savedUser.id,
-        email: savedUser.email,
-        firstName: savedUser.firstName,
-        lastName: savedUser.lastName,
-        createdAt: savedUser.createdAt,
-      };
-
+      return mapUser(savedUser);
     } catch (e) {
       Logger.error(e, 'Create new user');
       throw new InternalServerErrorException('Failed to register new user');
@@ -84,7 +73,6 @@ export class UserService {
     return bcrypt.hash(password, salt);
   }
 
-
   async updateUser(updateUserDto: UpdateUserDto) {
     const user = await this.userRepository.findOne(updateUserDto.id);
 
@@ -100,10 +88,10 @@ export class UserService {
     }
 
     if (updateUserDto.isAdmin) {
-      const adminUser = await this.userRepository.findOne({ where: { isAdmin: true } })
+      const adminUser = await this.userRepository.findOne({ where: { isAdmin: true } });
 
       if (adminUser) {
-        throw new BadRequestException("Admin user exists")
+        throw new BadRequestException('Admin user exists');
       }
 
       user.isAdmin = true;
@@ -120,7 +108,6 @@ export class UserService {
         isAdmin: updatedUser.isAdmin,
         profileImagePath: updatedUser.profileImagePath,
       };
-
     } catch (e) {
       Logger.error(e, 'Create new user');
       throw new InternalServerErrorException('Failed to register new user');
@@ -130,13 +117,12 @@ export class UserService {
   async createProfileImage(authUser: AuthUserDto, fileInfo: Express.Multer.File) {
     try {
       await this.userRepository.update(authUser.id, {
-        profileImagePath: fileInfo.path
-      })
-
+        profileImagePath: fileInfo.path,
+      });
 
       return {
         userId: authUser.id,
-        profileImagePath: fileInfo.path
+        profileImagePath: fileInfo.path,
       };
     } catch (e) {
       Logger.error(e, 'Create User Profile Image');
@@ -146,7 +132,7 @@ export class UserService {
 
   async getUserProfileImage(userId: string, res: Res) {
     try {
-      const user = await this.userRepository.findOne({ id: userId })
+      const user = await this.userRepository.findOne({ id: userId });
 
       if (!user.profileImagePath) {
         // throw new BadRequestException('User does not have a profile image');
@@ -157,11 +143,10 @@ export class UserService {
       res.set({
         'Content-Type': 'image/jpeg',
       });
-      const fileStream = createReadStream(user.profileImagePath)
+      const fileStream = createReadStream(user.profileImagePath);
       return new StreamableFile(fileStream);
     } catch (e) {
-      console.log("error getting user profile")
+      console.log('error getting user profile');
     }
-
   }
 }

server/src/config/database.config.ts

@@ -9,9 +9,11 @@ export const databaseConfig: TypeOrmModuleOptions = {
   database: process.env.DB_DATABASE_NAME,
   entities: [__dirname + '/../**/*.entity.{js,ts}'],
   synchronize: false,
-  migrations: [__dirname + '/../migration/*.js'],
+  migrations: [__dirname + '/../migration/*.{js,ts}'],
   cli: {
     migrationsDir: __dirname + '/../migration',
   },
   migrationsRun: true,
 };
+
+export default databaseConfig;

server/test/user.e2e-spec.ts

@@ -5,14 +5,13 @@ import request from 'supertest';
 import { clearDb, authCustom } from './test-utils';
 import { databaseConfig } from '../src/config/database.config';
 import { UserModule } from '../src/api-v1/user/user.module';
-import { AuthModule } from '../src/api-v1/auth/auth.module';
-import { AuthService } from '../src/api-v1/auth/auth.service';
 import { ImmichJwtModule } from '../src/modules/immich-jwt/immich-jwt.module';
-import { SignUpDto } from '../src/api-v1/auth/dto/sign-up.dto';
-import { AuthUserDto } from '../src/decorators/auth-user.decorator';
+import { UserService } from '../src/api-v1/user/user.service';
+import { CreateUserDto } from '../src/api-v1/user/dto/create-user.dto';
+import { User } from '../src/api-v1/user/response-dto/user';
 
-function _createUser(authService: AuthService, data: SignUpDto) {
-  return authService.signUp(data);
+function _createUser(userService: UserService, data: CreateUserDto) {
+  return userService.createUser(data);
 }
 
 describe('User', () => {
@@ -44,17 +43,17 @@ describe('User', () => {
   });
 
   describe('with auth', () => {
-    let authService: AuthService;
-    let authUser: AuthUserDto;
+    let userService: UserService;
+    let authUser: User;
 
     beforeAll(async () => {
       const builder = Test.createTestingModule({
-        imports: [UserModule, AuthModule, TypeOrmModule.forRoot(databaseConfig)],
+        imports: [UserModule, TypeOrmModule.forRoot(databaseConfig)],
       });
       const moduleFixture: TestingModule = await authCustom(builder, () => authUser).compile();
 
       app = moduleFixture.createNestApplication();
-      authService = app.get(AuthService);
+      userService = app.get(UserService);
       await app.init();
     });
 
@@ -65,9 +64,24 @@ describe('User', () => {
 
       beforeAll(async () => {
         await Promise.allSettled([
-          _createUser(authService, { email: authUserEmail, password: '1234' }).then((user) => (authUser = user)),
-          _createUser(authService, { email: userOneEmail, password: '1234' }),
-          _createUser(authService, { email: userTwoEmail, password: '1234' }),
+          _createUser(userService, {
+            firstName: 'auth-user',
+            lastName: 'test',
+            email: authUserEmail,
+            password: '1234',
+          }).then((user) => (authUser = user)),
+          _createUser(userService, {
+            firstName: 'one',
+            lastName: 'test',
+            email: userOneEmail,
+            password: '1234',
+          }),
+          _createUser(userService, {
+            firstName: 'two',
+            lastName: 'test',
+            email: userTwoEmail,
+            password: '1234',
+          }),
         ]);
       });
 
@@ -79,13 +93,23 @@ describe('User', () => {
           expect.arrayContaining([
             {
               email: userOneEmail,
+              firstName: 'one',
+              lastName: 'test',
               id: expect.anything(),
               createdAt: expect.anything(),
+              isAdmin: false,
+              isFirstLoggedIn: true,
+              profileImagePath: '',
             },
             {
               email: userTwoEmail,
+              firstName: 'two',
+              lastName: 'test',
               id: expect.anything(),
               createdAt: expect.anything(),
+              isAdmin: false,
+              isFirstLoggedIn: true,
+              profileImagePath: '',
             },
           ]),
         );