refactor: create album (#2555)

server/apps/immich/src/api-v1/album/album-repository.ts

@@ -5,14 +5,12 @@ import { InjectRepository } from '@nestjs/typeorm';
 import { Repository } from 'typeorm';
 import { AddAssetsDto } from './dto/add-assets.dto';
 import { AddUsersDto } from './dto/add-users.dto';
-import { CreateAlbumDto } from './dto/create-album.dto';
 import { RemoveAssetsDto } from './dto/remove-assets.dto';
 import { UpdateAlbumDto } from './dto/update-album.dto';
 import { AlbumCountResponseDto } from './response-dto/album-count-response.dto';
 import { AddAssetsResponseDto } from './response-dto/add-assets-response.dto';
 
 export interface IAlbumRepository {
-  create(ownerId: string, createAlbumDto: CreateAlbumDto): Promise<AlbumEntity>;
   get(albumId: string): Promise<AlbumEntity | null>;
   delete(album: AlbumEntity): Promise<void>;
   addSharedUsers(album: AlbumEntity, addUsersDto: AddUsersDto): Promise<AlbumEntity>;
@@ -45,19 +43,6 @@ export class AlbumRepository implements IAlbumRepository {
     return new AlbumCountResponseDto(ownedAlbums.length, sharedAlbums, sharedAlbumCount);
   }
 
-  async create(ownerId: string, dto: CreateAlbumDto): Promise<AlbumEntity> {
-    const album = await this.albumRepository.save({
-      ownerId,
-      albumName: dto.albumName,
-      sharedUsers: dto.sharedWithUserIds?.map((value) => ({ id: value } as UserEntity)) ?? [],
-      assets: dto.assetIds?.map((value) => ({ id: value } as AssetEntity)) ?? [],
-      albumThumbnailAssetId: dto.assetIds?.[0] || null,
-    });
-
-    // need to re-load the relations
-    return this.get(album.id) as Promise<AlbumEntity>;
-  }
-
   async get(albumId: string): Promise<AlbumEntity | null> {
     return this.albumRepository.findOne({
       where: { id: albumId },

server/apps/immich/src/api-v1/album/album.controller.ts

@@ -1,7 +1,6 @@
 import { Controller, Get, Post, Body, Patch, Param, Delete, Put, Query, Response } from '@nestjs/common';
 import { ParseMeUUIDPipe } from '../validation/parse-me-uuid-pipe';
 import { AlbumService } from './album.service';
-import { CreateAlbumDto } from './dto/create-album.dto';
 import { Authenticated } from '../../decorators/authenticated.decorator';
 import { AuthUserDto, GetAuthUser } from '../../decorators/auth-user.decorator';
 import { AddAssetsDto } from './dto/add-assets.dto';
@@ -44,13 +43,6 @@ export class AlbumController {
     return this.service.getCountByUserId(authUser);
   }
 
-  @Authenticated()
-  @Post()
-  createAlbum(@GetAuthUser() authUser: AuthUserDto, @Body() dto: CreateAlbumDto) {
-    // TODO: Handle nonexistent sharedWithUserIds and assetIds.
-    return this.service.create(authUser, dto);
-  }
-
   @Authenticated()
   @Put(':id/users')
   addUsersToAlbum(@GetAuthUser() authUser: AuthUserDto, @Param() { id }: UUIDParamDto, @Body() dto: AddUsersDto) {

server/apps/immich/src/api-v1/album/album.service.spec.ts

@@ -121,7 +121,6 @@ describe('Album service', () => {
     albumRepositoryMock = {
       addAssets: jest.fn(),
       addSharedUsers: jest.fn(),
-      create: jest.fn(),
       delete: jest.fn(),
       get: jest.fn(),
       removeAssets: jest.fn(),
@@ -150,19 +149,6 @@ describe('Album service', () => {
     );
   });
 
-  it('creates album', async () => {
-    const albumEntity = _getOwnedAlbum();
-    albumRepositoryMock.create.mockImplementation(() => Promise.resolve<AlbumEntity>(albumEntity));
-
-    const result = await sut.create(authUser, {
-      albumName: albumEntity.albumName,
-    });
-
-    expect(result.id).toEqual(albumEntity.id);
-    expect(result.albumName).toEqual(albumEntity.albumName);
-    expect(jobMock.queue).toHaveBeenCalledWith({ name: JobName.SEARCH_INDEX_ALBUM, data: { ids: [albumEntity.id] } });
-  });
-
   it('gets an owned album', async () => {
     const albumId = 'f19ab956-4761-41ea-a5d6-bae948308d58';
 

server/apps/immich/src/api-v1/album/album.service.ts

@@ -1,6 +1,5 @@
 import { BadRequestException, Inject, Injectable, NotFoundException, ForbiddenException, Logger } from '@nestjs/common';
 import { AuthUserDto } from '../../decorators/auth-user.decorator';
-import { CreateAlbumDto } from './dto/create-album.dto';
 import { AlbumEntity, SharedLinkType } from '@app/infra/entities';
 import { AddUsersDto } from './dto/add-users.dto';
 import { RemoveAssetsDto } from './dto/remove-assets.dto';
@@ -55,12 +54,6 @@ export class AlbumService {
     return album;
   }
 
-  async create(authUser: AuthUserDto, createAlbumDto: CreateAlbumDto): Promise<AlbumResponseDto> {
-    const albumEntity = await this.albumRepository.create(authUser.id, createAlbumDto);
-    await this.jobRepository.queue({ name: JobName.SEARCH_INDEX_ALBUM, data: { ids: [albumEntity.id] } });
-    return mapAlbum(albumEntity);
-  }
-
   async get(authUser: AuthUserDto, albumId: string): Promise<AlbumResponseDto> {
     const album = await this._getAlbum({ authUser, albumId, validateIsOwner: false });
     return mapAlbum(album);

server/apps/immich/src/controllers/album.controller.ts

@@ -1,6 +1,6 @@
-import { AlbumService, AuthUserDto } from '@app/domain';
+import { AlbumService, AuthUserDto, CreateAlbumDto } from '@app/domain';
 import { GetAlbumsDto } from '@app/domain/album/dto/get-albums.dto';
-import { Controller, Get, Query } from '@nestjs/common';
+import { Body, Controller, Get, Post, Query } from '@nestjs/common';
 import { ApiTags } from '@nestjs/swagger';
 import { GetAuthUser } from '../decorators/auth-user.decorator';
 import { Authenticated } from '../decorators/authenticated.decorator';
@@ -15,6 +15,11 @@ export class AlbumController {
 
   @Get()
   async getAllAlbums(@GetAuthUser() authUser: AuthUserDto, @Query() query: GetAlbumsDto) {
-    return this.service.getAllAlbums(authUser, query);
+    return this.service.getAll(authUser, query);
+  }
+
+  @Post()
+  createAlbum(@GetAuthUser() authUser: AuthUserDto, @Body() dto: CreateAlbumDto) {
+    return this.service.create(authUser, dto);
   }
 }

server/apps/immich/test/album.e2e-spec.ts

@@ -2,7 +2,7 @@ import { Test, TestingModule } from '@nestjs/testing';
 import { INestApplication } from '@nestjs/common';
 import request from 'supertest';
 import { clearDb, getAuthUser, authCustom } from './test-utils';
-import { CreateAlbumDto } from '../src/api-v1/album/dto/create-album.dto';
+import { CreateAlbumDto } from '@app/domain';
 import { CreateAlbumShareLinkDto } from '../src/api-v1/album/dto/create-album-shared-link.dto';
 import { AuthUserDto } from '../src/decorators/auth-user.decorator';
 import { AlbumResponseDto, AuthService, SharedLinkResponseDto, UserService } from '@app/domain';

server/immich-openapi-specs.json

@@ -4553,6 +4553,31 @@
           "owner"
         ]
       },
+      "CreateAlbumDto": {
+        "type": "object",
+        "properties": {
+          "albumName": {
+            "type": "string"
+          },
+          "sharedWithUserIds": {
+            "type": "array",
+            "items": {
+              "type": "string",
+              "format": "uuid"
+            }
+          },
+          "assetIds": {
+            "type": "array",
+            "items": {
+              "type": "string",
+              "format": "uuid"
+            }
+          }
+        },
+        "required": [
+          "albumName"
+        ]
+      },
       "APIKeyCreateDto": {
         "type": "object",
         "properties": {
@@ -6280,31 +6305,6 @@
           "sharing"
         ]
       },
-      "CreateAlbumDto": {
-        "type": "object",
-        "properties": {
-          "albumName": {
-            "type": "string"
-          },
-          "sharedWithUserIds": {
-            "type": "array",
-            "items": {
-              "type": "string",
-              "format": "uuid"
-            }
-          },
-          "assetIds": {
-            "type": "array",
-            "items": {
-              "type": "string",
-              "format": "uuid"
-            }
-          }
-        },
-        "required": [
-          "albumName"
-        ]
-      },
       "AddUsersDto": {
         "type": "object",
         "properties": {

server/libs/domain/src/album/album.repository.ts

@@ -17,5 +17,6 @@ export interface IAlbumRepository {
   getNotShared(ownerId: string): Promise<AlbumEntity[]>;
   deleteAll(userId: string): Promise<void>;
   getAll(): Promise<AlbumEntity[]>;
+  create(album: Partial<AlbumEntity>): Promise<AlbumEntity>;
   save(album: Partial<AlbumEntity>): Promise<AlbumEntity>;
 }

server/libs/domain/src/album/album.service.spec.ts

@@ -1,5 +1,6 @@
-import { albumStub, authStub, newAlbumRepositoryMock, newAssetRepositoryMock } from '../../test';
+import { albumStub, authStub, newAlbumRepositoryMock, newAssetRepositoryMock, newJobRepositoryMock } from '../../test';
 import { IAssetRepository } from '../asset';
+import { IJobRepository, JobName } from '../job';
 import { IAlbumRepository } from './album.repository';
 import { AlbumService } from './album.service';
 
@@ -7,19 +8,21 @@ describe(AlbumService.name, () => {
   let sut: AlbumService;
   let albumMock: jest.Mocked<IAlbumRepository>;
   let assetMock: jest.Mocked<IAssetRepository>;
+  let jobMock: jest.Mocked<IJobRepository>;
 
   beforeEach(async () => {
     albumMock = newAlbumRepositoryMock();
     assetMock = newAssetRepositoryMock();
+    jobMock = newJobRepositoryMock();
 
-    sut = new AlbumService(albumMock, assetMock);
+    sut = new AlbumService(albumMock, assetMock, jobMock);
   });
 
   it('should work', () => {
     expect(sut).toBeDefined();
   });
 
-  describe('get list of albums', () => {
+  describe('getAll', () => {
     it('gets list of albums for auth user', async () => {
       albumMock.getOwned.mockResolvedValue([albumStub.empty, albumStub.sharedWithUser]);
       albumMock.getAssetCountForIds.mockResolvedValue([
@@ -28,7 +31,7 @@ describe(AlbumService.name, () => {
       ]);
       albumMock.getInvalidThumbnail.mockResolvedValue([]);
 
-      const result = await sut.getAllAlbums(authStub.admin, {});
+      const result = await sut.getAll(authStub.admin, {});
       expect(result).toHaveLength(2);
       expect(result[0].id).toEqual(albumStub.empty.id);
       expect(result[1].id).toEqual(albumStub.sharedWithUser.id);
@@ -39,7 +42,7 @@ describe(AlbumService.name, () => {
       albumMock.getAssetCountForIds.mockResolvedValue([{ albumId: albumStub.oneAsset.id, assetCount: 1 }]);
       albumMock.getInvalidThumbnail.mockResolvedValue([]);
 
-      const result = await sut.getAllAlbums(authStub.admin, { assetId: albumStub.oneAsset.id });
+      const result = await sut.getAll(authStub.admin, { assetId: albumStub.oneAsset.id });
       expect(result).toHaveLength(1);
       expect(result[0].id).toEqual(albumStub.oneAsset.id);
       expect(albumMock.getByAssetId).toHaveBeenCalledTimes(1);
@@ -50,7 +53,7 @@ describe(AlbumService.name, () => {
       albumMock.getAssetCountForIds.mockResolvedValue([{ albumId: albumStub.sharedWithUser.id, assetCount: 0 }]);
       albumMock.getInvalidThumbnail.mockResolvedValue([]);
 
-      const result = await sut.getAllAlbums(authStub.admin, { shared: true });
+      const result = await sut.getAll(authStub.admin, { shared: true });
       expect(result).toHaveLength(1);
       expect(result[0].id).toEqual(albumStub.sharedWithUser.id);
       expect(albumMock.getShared).toHaveBeenCalledTimes(1);
@@ -61,7 +64,7 @@ describe(AlbumService.name, () => {
       albumMock.getAssetCountForIds.mockResolvedValue([{ albumId: albumStub.empty.id, assetCount: 0 }]);
       albumMock.getInvalidThumbnail.mockResolvedValue([]);
 
-      const result = await sut.getAllAlbums(authStub.admin, { shared: false });
+      const result = await sut.getAll(authStub.admin, { shared: false });
       expect(result).toHaveLength(1);
       expect(result[0].id).toEqual(albumStub.empty.id);
       expect(albumMock.getNotShared).toHaveBeenCalledTimes(1);
@@ -73,7 +76,7 @@ describe(AlbumService.name, () => {
     albumMock.getAssetCountForIds.mockResolvedValue([{ albumId: albumStub.oneAsset.id, assetCount: 1 }]);
     albumMock.getInvalidThumbnail.mockResolvedValue([]);
 
-    const result = await sut.getAllAlbums(authStub.admin, {});
+    const result = await sut.getAll(authStub.admin, {});
 
     expect(result).toHaveLength(1);
     expect(result[0].assetCount).toEqual(1);
@@ -89,7 +92,7 @@ describe(AlbumService.name, () => {
     albumMock.save.mockResolvedValue(albumStub.oneAssetValidThumbnail);
     assetMock.getFirstAssetForAlbumId.mockResolvedValue(albumStub.oneAssetInvalidThumbnail.assets[0]);
 
-    const result = await sut.getAllAlbums(authStub.admin, {});
+    const result = await sut.getAll(authStub.admin, {});
 
     expect(result).toHaveLength(1);
     expect(albumMock.getInvalidThumbnail).toHaveBeenCalledTimes(1);
@@ -105,10 +108,47 @@ describe(AlbumService.name, () => {
     albumMock.save.mockResolvedValue(albumStub.emptyWithValidThumbnail);
     assetMock.getFirstAssetForAlbumId.mockResolvedValue(null);
 
-    const result = await sut.getAllAlbums(authStub.admin, {});
+    const result = await sut.getAll(authStub.admin, {});
 
     expect(result).toHaveLength(1);
     expect(albumMock.getInvalidThumbnail).toHaveBeenCalledTimes(1);
     expect(albumMock.save).toHaveBeenCalledTimes(1);
   });
+
+  describe('create', () => {
+    it('creates album', async () => {
+      albumMock.create.mockResolvedValue(albumStub.empty);
+
+      await expect(sut.create(authStub.admin, { albumName: 'Empty album' })).resolves.toEqual({
+        albumName: 'Empty album',
+        albumThumbnailAssetId: null,
+        assetCount: 0,
+        assets: [],
+        createdAt: expect.anything(),
+        id: 'album-1',
+        owner: {
+          createdAt: '2021-01-01',
+          email: 'admin@test.com',
+          firstName: 'admin_first_name',
+          id: 'admin_id',
+          isAdmin: true,
+          lastName: 'admin_last_name',
+          oauthId: '',
+          profileImagePath: '',
+          shouldChangePassword: false,
+          storageLabel: 'admin',
+          updatedAt: '2021-01-01',
+        },
+        ownerId: 'admin_id',
+        shared: false,
+        sharedUsers: [],
+        updatedAt: expect.anything(),
+      });
+
+      expect(jobMock.queue).toHaveBeenCalledWith({
+        name: JobName.SEARCH_INDEX_ALBUM,
+        data: { ids: [albumStub.empty.id] },
+      });
+    });
+  });
 });

server/libs/domain/src/album/album.service.ts

@@ -1,19 +1,22 @@
-import { AlbumEntity } from '@app/infra/entities';
+import { AlbumEntity, AssetEntity, UserEntity } from '@app/infra/entities';
 import { Inject, Injectable } from '@nestjs/common';
 import { IAssetRepository } from '../asset';
 import { AuthUserDto } from '../auth';
+import { IJobRepository, JobName } from '../job';
 import { IAlbumRepository } from './album.repository';
+import { CreateAlbumDto } from './dto/album-create.dto';
 import { GetAlbumsDto } from './dto/get-albums.dto';
-import { AlbumResponseDto } from './response-dto';
+import { AlbumResponseDto, mapAlbum } from './response-dto';
 
 @Injectable()
 export class AlbumService {
   constructor(
     @Inject(IAlbumRepository) private albumRepository: IAlbumRepository,
     @Inject(IAssetRepository) private assetRepository: IAssetRepository,
+    @Inject(IJobRepository) private jobRepository: IJobRepository,
   ) {}
 
-  async getAllAlbums({ id: ownerId }: AuthUserDto, { assetId, shared }: GetAlbumsDto): Promise<AlbumResponseDto[]> {
+  async getAll({ id: ownerId }: AuthUserDto, { assetId, shared }: GetAlbumsDto): Promise<AlbumResponseDto[]> {
     await this.updateInvalidThumbnails();
 
     let albums: AlbumEntity[];
@@ -55,4 +58,17 @@ export class AlbumService {
 
     return invalidAlbumIds.length;
   }
+
+  async create(authUser: AuthUserDto, dto: CreateAlbumDto): Promise<AlbumResponseDto> {
+    // TODO: Handle nonexistent sharedWithUserIds and assetIds.
+    const album = await this.albumRepository.create({
+      ownerId: authUser.id,
+      albumName: dto.albumName,
+      sharedUsers: dto.sharedWithUserIds?.map((value) => ({ id: value } as UserEntity)) ?? [],
+      assets: (dto.assetIds || []).map((id) => ({ id } as AssetEntity)),
+      albumThumbnailAssetId: dto.assetIds?.[0] || null,
+    });
+    await this.jobRepository.queue({ name: JobName.SEARCH_INDEX_ALBUM, data: { ids: [album.id] } });
+    return mapAlbum(album);
+  }
 }

server/libs/domain/src/album/dto/album-create.dto.ts

@@ -1,5 +1,5 @@
 import { ApiProperty } from '@nestjs/swagger';
-import { ValidateUUID } from 'apps/immich/src/decorators/validate-uuid.decorator';
+import { ValidateUUID } from '../../../../../apps/immich/src/decorators/validate-uuid.decorator';
 import { IsNotEmpty, IsString } from 'class-validator';
 
 export class CreateAlbumDto {

server/libs/domain/src/album/dto/get-albums.dto.ts

@@ -1,8 +1,8 @@
+import { ApiProperty } from '@nestjs/swagger';
 import { Transform } from 'class-transformer';
 import { IsBoolean, IsOptional } from 'class-validator';
-import { toBoolean } from 'apps/immich/src/utils/transform.util';
+import { ValidateUUID } from '../../../../../apps/immich/src/decorators/validate-uuid.decorator';
-import { ApiProperty } from '@nestjs/swagger';
+import { toBoolean } from '../../../../../apps/immich/src/utils/transform.util';
-import { ValidateUUID } from 'apps/immich/src/decorators/validate-uuid.decorator';
 
 export class GetAlbumsDto {
   @IsOptional()

server/libs/domain/src/album/dto/index.ts

@@ -0,0 +1,2 @@
+export * from './album-create.dto';
+export * from './get-albums.dto';

server/libs/domain/src/album/index.ts

@@ -1,3 +1,4 @@
 export * from './album.repository';
 export * from './album.service';
+export * from './dto';
 export * from './response-dto';

server/libs/domain/test/album.repository.mock.ts

@@ -11,6 +11,7 @@ export const newAlbumRepositoryMock = (): jest.Mocked<IAlbumRepository> => {
     getNotShared: jest.fn(),
     deleteAll: jest.fn(),
     getAll: jest.fn(),
+    create: jest.fn(),
     save: jest.fn(),
   };
 };

server/libs/infra/src/repositories/album.repository.ts

@@ -123,8 +123,12 @@ export class AlbumRepository implements IAlbumRepository {
     });
   }
 
+  create(album: Partial<AlbumEntity>): Promise<AlbumEntity> {
+    return this.save(album);
+  }
+
   async save(album: Partial<AlbumEntity>) {
     const { id } = await this.repository.save(album);
-    return this.repository.findOneOrFail({ where: { id } });
+    return this.repository.findOneOrFail({ where: { id }, relations: { owner: true } });
   }
 }