refactor(server,web): add/remove album users (#2681)

* refactor(server,web): add/remove album users

* fix(web): bug fixes for multiple users

* fix: linting

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

@@ -1,18 +1,15 @@
-import { AlbumEntity, AssetEntity, UserEntity } from '@app/infra/entities';
+import { AlbumEntity, AssetEntity } from '@app/infra/entities';
 import { dataSource } from '@app/infra/database.config';
 import { Injectable } from '@nestjs/common';
 import { InjectRepository } from '@nestjs/typeorm';
 import { Repository } from 'typeorm';
 import { AddAssetsDto } from './dto/add-assets.dto';
-import { AddUsersDto } from './dto/add-users.dto';
 import { RemoveAssetsDto } from './dto/remove-assets.dto';
 import { AlbumCountResponseDto } from './response-dto/album-count-response.dto';
 import { AddAssetsResponseDto } from './response-dto/add-assets-response.dto';
 
 export interface IAlbumRepository {
   get(albumId: string): Promise<AlbumEntity | null>;
-  addSharedUsers(album: AlbumEntity, addUsersDto: AddUsersDto): Promise<AlbumEntity>;
-  removeUser(album: AlbumEntity, userId: string): Promise<void>;
   removeAssets(album: AlbumEntity, removeAssets: RemoveAssetsDto): Promise<number>;
   addAssets(album: AlbumEntity, addAssetsDto: AddAssetsDto): Promise<AddAssetsResponseDto>;
   updateThumbnails(): Promise<number | undefined>;
@@ -25,11 +22,8 @@ export const IAlbumRepository = 'IAlbumRepository';
 @Injectable()
 export class AlbumRepository implements IAlbumRepository {
   constructor(
-    @InjectRepository(AlbumEntity)
+    @InjectRepository(AlbumEntity) private albumRepository: Repository<AlbumEntity>,
-    private albumRepository: Repository<AlbumEntity>,
+    @InjectRepository(AssetEntity) private assetRepository: Repository<AssetEntity>,
-
-    @InjectRepository(AssetEntity)
-    private assetRepository: Repository<AssetEntity>,
   ) {}
 
   async getCountByUserId(userId: string): Promise<AlbumCountResponseDto> {
@@ -59,22 +53,6 @@ export class AlbumRepository implements IAlbumRepository {
     });
   }
 
-  async addSharedUsers(album: AlbumEntity, addUsersDto: AddUsersDto): Promise<AlbumEntity> {
-    album.sharedUsers.push(...addUsersDto.sharedUserIds.map((id) => ({ id } as UserEntity)));
-    album.updatedAt = new Date();
-
-    await this.albumRepository.save(album);
-
-    // need to re-load the shared user relation
-    return this.get(album.id) as Promise<AlbumEntity>;
-  }
-
-  async removeUser(album: AlbumEntity, userId: string): Promise<void> {
-    album.sharedUsers = album.sharedUsers.filter((user) => user.id !== userId);
-    album.updatedAt = new Date();
-    await this.albumRepository.save(album);
-  }
-
   async removeAssets(album: AlbumEntity, removeAssetsDto: RemoveAssetsDto): Promise<number> {
     const assetCount = album.assets.length;
 

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

@@ -1,10 +1,8 @@
 import { Controller, Get, Post, Body, Param, Delete, Put, Query, Response } from '@nestjs/common';
-import { ParseMeUUIDPipe } from '../validation/parse-me-uuid-pipe';
 import { AlbumService } from './album.service';
 import { Authenticated, SharedLinkRoute } from '../../decorators/authenticated.decorator';
 import { AuthUserDto, GetAuthUser } from '../../decorators/auth-user.decorator';
 import { AddAssetsDto } from './dto/add-assets.dto';
-import { AddUsersDto } from './dto/add-users.dto';
 import { RemoveAssetsDto } from './dto/remove-assets.dto';
 import { ApiOkResponse, ApiTags } from '@nestjs/swagger';
 import { AlbumResponseDto } from '@app/domain';
@@ -29,12 +27,6 @@ export class AlbumController {
     return this.service.getCountByUserId(authUser);
   }
 
-  @Put(':id/users')
-  addUsersToAlbum(@GetAuthUser() authUser: AuthUserDto, @Param() { id }: UUIDParamDto, @Body() dto: AddUsersDto) {
-    // TODO: Handle nonexistent sharedUserIds.
-    return this.service.addUsers(authUser, id, dto);
-  }
-
   @SharedLinkRoute()
   @Put(':id/assets')
   addAssetsToAlbum(
@@ -62,15 +54,6 @@ export class AlbumController {
     return this.service.removeAssets(authUser, id, dto);
   }
 
-  @Delete(':id/user/:userId')
-  removeUserFromAlbum(
-    @GetAuthUser() authUser: AuthUserDto,
-    @Param() { id }: UUIDParamDto,
-    @Param('userId', new ParseMeUUIDPipe({ version: '4' })) userId: string,
-  ) {
-    return this.service.removeUser(authUser, id, userId);
-  }
-
   @SharedLinkRoute()
   @Get(':id/download')
   @ApiOkResponse({ content: { 'application/zip': { schema: { type: 'string', format: 'binary' } } } })

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

@@ -1,6 +1,6 @@
 import { AlbumService } from './album.service';
 import { AuthUserDto } from '../../decorators/auth-user.decorator';
-import { BadRequestException, NotFoundException, ForbiddenException } from '@nestjs/common';
+import { NotFoundException, ForbiddenException } from '@nestjs/common';
 import { AlbumEntity, UserEntity } from '@app/infra/entities';
 import { AlbumResponseDto, ICryptoRepository, mapUser } from '@app/domain';
 import { AddAssetsResponseDto } from './response-dto/add-assets-response.dto';
@@ -39,7 +39,6 @@ describe('Album service', () => {
   const albumId = 'f19ab956-4761-41ea-a5d6-bae948308d58';
   const sharedAlbumOwnerId = '2222';
   const sharedAlbumSharedAlsoWithId = '3333';
-  const ownedAlbumSharedWithId = '4444';
 
   const _getOwnedAlbum = () => {
     const albumEntity = new AlbumEntity();
@@ -56,25 +55,6 @@ describe('Album service', () => {
     return albumEntity;
   };
 
-  const _getOwnedSharedAlbum = () => {
-    const albumEntity = new AlbumEntity();
-    albumEntity.ownerId = albumOwner.id;
-    albumEntity.owner = albumOwner;
-    albumEntity.id = albumId;
-    albumEntity.albumName = 'name';
-    albumEntity.createdAt = new Date('2022-06-19T23:41:36.910Z');
-    albumEntity.assets = [];
-    albumEntity.albumThumbnailAssetId = null;
-    albumEntity.sharedUsers = [
-      {
-        ...userEntityStub.user1,
-        id: ownedAlbumSharedWithId,
-      },
-    ];
-
-    return albumEntity;
-  };
-
   const _getSharedWithAuthUserAlbum = () => {
     const albumEntity = new AlbumEntity();
     albumEntity.ownerId = sharedAlbumOwnerId;
@@ -115,10 +95,8 @@ describe('Album service', () => {
   beforeAll(() => {
     albumRepositoryMock = {
       addAssets: jest.fn(),
-      addSharedUsers: jest.fn(),
       get: jest.fn(),
       removeAssets: jest.fn(),
-      removeUser: jest.fn(),
       updateThumbnails: jest.fn(),
       getCountByUserId: jest.fn(),
       getSharedWithUserAlbumCount: jest.fn(),
@@ -188,53 +166,6 @@ describe('Album service', () => {
     await expect(sut.get(authUser, '0002')).rejects.toBeInstanceOf(NotFoundException);
   });
 
-  it('removes a shared user from an owned album', async () => {
-    const albumEntity = _getOwnedSharedAlbum();
-    albumRepositoryMock.get.mockImplementation(() => Promise.resolve<AlbumEntity>(albumEntity));
-    albumRepositoryMock.removeUser.mockImplementation(() => Promise.resolve());
-    await expect(sut.removeUser(authUser, albumEntity.id, ownedAlbumSharedWithId)).resolves.toBeUndefined();
-    expect(albumRepositoryMock.removeUser).toHaveBeenCalledTimes(1);
-    expect(albumRepositoryMock.removeUser).toHaveBeenCalledWith(albumEntity, ownedAlbumSharedWithId);
-  });
-
-  it('prevents removing a shared user from a not owned album (shared with auth user)', async () => {
-    const albumEntity = _getSharedWithAuthUserAlbum();
-    const albumId = albumEntity.id;
-    const userIdToRemove = sharedAlbumSharedAlsoWithId;
-
-    albumRepositoryMock.get.mockImplementation(() => Promise.resolve<AlbumEntity>(albumEntity));
-
-    await expect(sut.removeUser(authUser, albumId, userIdToRemove)).rejects.toBeInstanceOf(ForbiddenException);
-    expect(albumRepositoryMock.removeUser).not.toHaveBeenCalled();
-  });
-
-  it('removes itself from a shared album', async () => {
-    const albumEntity = _getSharedWithAuthUserAlbum();
-    albumRepositoryMock.get.mockImplementation(() => Promise.resolve<AlbumEntity>(albumEntity));
-    albumRepositoryMock.removeUser.mockImplementation(() => Promise.resolve());
-
-    await sut.removeUser(authUser, albumEntity.id, authUser.id);
-    expect(albumRepositoryMock.removeUser).toHaveReturnedTimes(1);
-    expect(albumRepositoryMock.removeUser).toHaveBeenCalledWith(albumEntity, authUser.id);
-  });
-
-  it('removes itself from a shared album using "me" as id', async () => {
-    const albumEntity = _getSharedWithAuthUserAlbum();
-    albumRepositoryMock.get.mockImplementation(() => Promise.resolve<AlbumEntity>(albumEntity));
-    albumRepositoryMock.removeUser.mockImplementation(() => Promise.resolve());
-
-    await sut.removeUser(authUser, albumEntity.id, 'me');
-    expect(albumRepositoryMock.removeUser).toHaveReturnedTimes(1);
-    expect(albumRepositoryMock.removeUser).toHaveBeenCalledWith(albumEntity, authUser.id);
-  });
-
-  it('prevents removing itself from a owned album', async () => {
-    const albumEntity = _getOwnedAlbum();
-    albumRepositoryMock.get.mockImplementation(() => Promise.resolve<AlbumEntity>(albumEntity));
-
-    await expect(sut.removeUser(authUser, albumEntity.id, authUser.id)).rejects.toBeInstanceOf(BadRequestException);
-  });
-
   it('adds assets to owned album', async () => {
     const albumEntity = _getOwnedAlbum();
 

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

@@ -1,7 +1,6 @@
 import { BadRequestException, Inject, Injectable, NotFoundException, ForbiddenException, Logger } from '@nestjs/common';
 import { AuthUserDto } from '../../decorators/auth-user.decorator';
 import { AlbumEntity, SharedLinkType } from '@app/infra/entities';
-import { AddUsersDto } from './dto/add-users.dto';
 import { RemoveAssetsDto } from './dto/remove-assets.dto';
 import { AlbumResponseDto, mapAlbum } from '@app/domain';
 import { IAlbumRepository } from './album-repository';
@@ -63,24 +62,6 @@ export class AlbumService {
     return mapAlbum(album);
   }
 
-  async addUsers(authUser: AuthUserDto, albumId: string, dto: AddUsersDto): Promise<AlbumResponseDto> {
-    const album = await this._getAlbum({ authUser, albumId });
-    const updatedAlbum = await this.albumRepository.addSharedUsers(album, dto);
-    return mapAlbum(updatedAlbum);
-  }
-
-  async removeUser(authUser: AuthUserDto, albumId: string, userId: string | 'me'): Promise<void> {
-    const sharedUserId = userId == 'me' ? authUser.id : userId;
-    const album = await this._getAlbum({ authUser, albumId, validateIsOwner: false });
-    if (album.ownerId != authUser.id && authUser.id != sharedUserId) {
-      throw new ForbiddenException('Cannot remove a user from a album that is not owned');
-    }
-    if (album.ownerId == sharedUserId) {
-      throw new BadRequestException('The owner of the album cannot be removed');
-    }
-    await this.albumRepository.removeUser(album, sharedUserId);
-  }
-
   async removeAssets(authUser: AuthUserDto, albumId: string, dto: RemoveAssetsDto): Promise<AlbumResponseDto> {
     const album = await this._getAlbum({ authUser, albumId });
     const deletedCount = await this.albumRepository.removeAssets(album, dto);

server/apps/immich/src/api-v1/album/dto/add-users.dto.ts

@@ -1,4 +1,4 @@
-import { ValidateUUID } from 'apps/immich/src/decorators/validate-uuid.decorator';
+import { ValidateUUID } from '../../../../../../apps/immich/src/decorators/validate-uuid.decorator';
 
 export class AddUsersDto {
   @ValidateUUID({ each: true })

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

@@ -1,7 +1,8 @@
-/*  */ import { AlbumService, AuthUserDto, CreateAlbumDto, UpdateAlbumDto } from '@app/domain';
+import { AddUsersDto, AlbumService, AuthUserDto, CreateAlbumDto, UpdateAlbumDto } from '@app/domain';
 import { GetAlbumsDto } from '@app/domain/album/dto/get-albums.dto';
-import { Body, Controller, Delete, Get, Param, Patch, Post, Query } from '@nestjs/common';
+import { Body, Controller, Delete, Get, Param, Patch, Post, Put, Query } from '@nestjs/common';
 import { ApiTags } from '@nestjs/swagger';
+import { ParseMeUUIDPipe } from '../api-v1/validation/parse-me-uuid-pipe';
 import { GetAuthUser } from '../decorators/auth-user.decorator';
 import { Authenticated } from '../decorators/authenticated.decorator';
 import { UseValidation } from '../decorators/use-validation.decorator';
@@ -33,4 +34,18 @@ export class AlbumController {
   deleteAlbum(@GetAuthUser() authUser: AuthUserDto, @Param() { id }: UUIDParamDto) {
     return this.service.delete(authUser, id);
   }
+
+  @Put(':id/users')
+  addUsersToAlbum(@GetAuthUser() authUser: AuthUserDto, @Param() { id }: UUIDParamDto, @Body() dto: AddUsersDto) {
+    return this.service.addUsers(authUser, id, dto);
+  }
+
+  @Delete(':id/user/:userId')
+  removeUserFromAlbum(
+    @GetAuthUser() authUser: AuthUserDto,
+    @Param() { id }: UUIDParamDto,
+    @Param('userId', new ParseMeUUIDPipe({ version: '4' })) userId: string,
+  ) {
+    return this.service.removeUser(authUser, id, userId);
+  }
 }

server/immich-openapi-specs.json

@@ -228,6 +228,101 @@
         ]
       }
     },
+    "/album/{id}/users": {
+      "put": {
+        "operationId": "addUsersToAlbum",
+        "parameters": [
+          {
+            "name": "id",
+            "required": true,
+            "in": "path",
+            "schema": {
+              "format": "uuid",
+              "type": "string"
+            }
+          }
+        ],
+        "requestBody": {
+          "required": true,
+          "content": {
+            "application/json": {
+              "schema": {
+                "$ref": "#/components/schemas/AddUsersDto"
+              }
+            }
+          }
+        },
+        "responses": {
+          "200": {
+            "description": "",
+            "content": {
+              "application/json": {
+                "schema": {
+                  "$ref": "#/components/schemas/AlbumResponseDto"
+                }
+              }
+            }
+          }
+        },
+        "tags": [
+          "Album"
+        ],
+        "security": [
+          {
+            "bearer": []
+          },
+          {
+            "cookie": []
+          },
+          {
+            "api_key": []
+          }
+        ]
+      }
+    },
+    "/album/{id}/user/{userId}": {
+      "delete": {
+        "operationId": "removeUserFromAlbum",
+        "parameters": [
+          {
+            "name": "id",
+            "required": true,
+            "in": "path",
+            "schema": {
+              "format": "uuid",
+              "type": "string"
+            }
+          },
+          {
+            "name": "userId",
+            "required": true,
+            "in": "path",
+            "schema": {
+              "type": "string"
+            }
+          }
+        ],
+        "responses": {
+          "200": {
+            "description": ""
+          }
+        },
+        "tags": [
+          "Album"
+        ],
+        "security": [
+          {
+            "bearer": []
+          },
+          {
+            "cookie": []
+          },
+          {
+            "api_key": []
+          }
+        ]
+      }
+    },
     "/api-key": {
       "post": {
         "operationId": "createKey",
@@ -3990,58 +4085,6 @@
         ]
       }
     },
-    "/album/{id}/users": {
-      "put": {
-        "operationId": "addUsersToAlbum",
-        "parameters": [
-          {
-            "name": "id",
-            "required": true,
-            "in": "path",
-            "schema": {
-              "format": "uuid",
-              "type": "string"
-            }
-          }
-        ],
-        "requestBody": {
-          "required": true,
-          "content": {
-            "application/json": {
-              "schema": {
-                "$ref": "#/components/schemas/AddUsersDto"
-              }
-            }
-          }
-        },
-        "responses": {
-          "200": {
-            "description": "",
-            "content": {
-              "application/json": {
-                "schema": {
-                  "$ref": "#/components/schemas/AlbumResponseDto"
-                }
-              }
-            }
-          }
-        },
-        "tags": [
-          "Album"
-        ],
-        "security": [
-          {
-            "bearer": []
-          },
-          {
-            "cookie": []
-          },
-          {
-            "api_key": []
-          }
-        ]
-      }
-    },
     "/album/{id}/assets": {
       "put": {
         "operationId": "addAssetsToAlbum",
@@ -4152,49 +4195,6 @@
         ]
       }
     },
-    "/album/{id}/user/{userId}": {
-      "delete": {
-        "operationId": "removeUserFromAlbum",
-        "parameters": [
-          {
-            "name": "id",
-            "required": true,
-            "in": "path",
-            "schema": {
-              "format": "uuid",
-              "type": "string"
-            }
-          },
-          {
-            "name": "userId",
-            "required": true,
-            "in": "path",
-            "schema": {
-              "type": "string"
-            }
-          }
-        ],
-        "responses": {
-          "200": {
-            "description": ""
-          }
-        },
-        "tags": [
-          "Album"
-        ],
-        "security": [
-          {
-            "bearer": []
-          },
-          {
-            "cookie": []
-          },
-          {
-            "api_key": []
-          }
-        ]
-      }
-    },
     "/album/{id}/download": {
       "get": {
         "operationId": "downloadArchive",
@@ -4778,6 +4778,21 @@
           }
         }
       },
+      "AddUsersDto": {
+        "type": "object",
+        "properties": {
+          "sharedUserIds": {
+            "type": "array",
+            "items": {
+              "type": "string",
+              "format": "uuid"
+            }
+          }
+        },
+        "required": [
+          "sharedUserIds"
+        ]
+      },
       "APIKeyCreateDto": {
         "type": "object",
         "properties": {
@@ -6620,21 +6635,6 @@
           "sharing"
         ]
       },
-      "AddUsersDto": {
-        "type": "object",
-        "properties": {
-          "sharedUserIds": {
-            "type": "array",
-            "items": {
-              "type": "string",
-              "format": "uuid"
-            }
-          }
-        },
-        "required": [
-          "sharedUserIds"
-        ]
-      },
       "AddAssetsResponseDto": {
         "type": "object",
         "properties": {

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

@@ -1,7 +1,17 @@
 import { BadRequestException, ForbiddenException } from '@nestjs/common';
-import { albumStub, authStub, newAlbumRepositoryMock, newAssetRepositoryMock, newJobRepositoryMock } from '../../test';
+import _ from 'lodash';
+import {
+  albumStub,
+  authStub,
+  newAlbumRepositoryMock,
+  newAssetRepositoryMock,
+  newJobRepositoryMock,
+  newUserRepositoryMock,
+  userEntityStub,
+} from '../../test';
 import { IAssetRepository } from '../asset';
 import { IJobRepository, JobName } from '../job';
+import { IUserRepository } from '../user';
 import { IAlbumRepository } from './album.repository';
 import { AlbumService } from './album.service';
 
@@ -10,13 +20,15 @@ describe(AlbumService.name, () => {
   let albumMock: jest.Mocked<IAlbumRepository>;
   let assetMock: jest.Mocked<IAssetRepository>;
   let jobMock: jest.Mocked<IJobRepository>;
+  let userMock: jest.Mocked<IUserRepository>;
 
   beforeEach(async () => {
     albumMock = newAlbumRepositoryMock();
     assetMock = newAssetRepositoryMock();
     jobMock = newJobRepositoryMock();
+    userMock = newUserRepositoryMock();
 
-    sut = new AlbumService(albumMock, assetMock, jobMock);
+    sut = new AlbumService(albumMock, assetMock, jobMock, userMock);
   });
 
   it('should work', () => {
@@ -152,6 +164,18 @@ describe(AlbumService.name, () => {
         data: { ids: [albumStub.empty.id] },
       });
     });
+
+    it('should require valid userIds', async () => {
+      userMock.get.mockResolvedValue(null);
+      await expect(
+        sut.create(authStub.admin, {
+          albumName: 'Empty album',
+          sharedWithUserIds: ['user-3'],
+        }),
+      ).rejects.toBeInstanceOf(BadRequestException);
+      expect(userMock.get).toHaveBeenCalledWith('user-3');
+      expect(albumMock.create).not.toHaveBeenCalled();
+    });
   });
 
   describe('update', () => {
@@ -240,4 +264,130 @@ describe(AlbumService.name, () => {
       expect(albumMock.delete).toHaveBeenCalledWith(albumStub.empty);
     });
   });
+
+  describe('addUsers', () => {
+    it('should require a valid album id', async () => {
+      albumMock.getByIds.mockResolvedValue([]);
+      await expect(sut.addUsers(authStub.admin, 'album-1', { sharedUserIds: ['user-1'] })).rejects.toBeInstanceOf(
+        BadRequestException,
+      );
+      expect(albumMock.update).not.toHaveBeenCalled();
+    });
+
+    it('should require the user to be the owner', async () => {
+      albumMock.getByIds.mockResolvedValue([albumStub.sharedWithAdmin]);
+      await expect(
+        sut.addUsers(authStub.admin, albumStub.sharedWithAdmin.id, { sharedUserIds: ['user-1'] }),
+      ).rejects.toBeInstanceOf(ForbiddenException);
+      expect(albumMock.update).not.toHaveBeenCalled();
+    });
+
+    it('should throw an error if the userId is already added', async () => {
+      albumMock.getByIds.mockResolvedValue([albumStub.sharedWithAdmin]);
+      await expect(
+        sut.addUsers(authStub.user1, albumStub.sharedWithAdmin.id, { sharedUserIds: [authStub.admin.id] }),
+      ).rejects.toBeInstanceOf(BadRequestException);
+      expect(albumMock.update).not.toHaveBeenCalled();
+    });
+
+    it('should throw an error if the userId does not exist', async () => {
+      albumMock.getByIds.mockResolvedValue([albumStub.sharedWithAdmin]);
+      userMock.get.mockResolvedValue(null);
+      await expect(
+        sut.addUsers(authStub.user1, albumStub.sharedWithAdmin.id, { sharedUserIds: ['user-3'] }),
+      ).rejects.toBeInstanceOf(BadRequestException);
+      expect(albumMock.update).not.toHaveBeenCalled();
+    });
+
+    it('should add valid shared users', async () => {
+      albumMock.getByIds.mockResolvedValue([_.cloneDeep(albumStub.sharedWithAdmin)]);
+      albumMock.update.mockResolvedValue(albumStub.sharedWithAdmin);
+      userMock.get.mockResolvedValue(userEntityStub.user2);
+      await sut.addUsers(authStub.user1, albumStub.sharedWithAdmin.id, { sharedUserIds: [authStub.user2.id] });
+      expect(albumMock.update).toHaveBeenCalledWith({
+        id: albumStub.sharedWithAdmin.id,
+        updatedAt: expect.any(Date),
+        sharedUsers: [userEntityStub.admin, { id: authStub.user2.id }],
+      });
+    });
+  });
+
+  describe('removeUser', () => {
+    it('should require a valid album id', async () => {
+      albumMock.getByIds.mockResolvedValue([]);
+      await expect(sut.removeUser(authStub.admin, 'album-1', 'user-1')).rejects.toBeInstanceOf(BadRequestException);
+      expect(albumMock.update).not.toHaveBeenCalled();
+    });
+
+    it('should remove a shared user from an owned album', async () => {
+      albumMock.getByIds.mockResolvedValue([albumStub.sharedWithUser]);
+
+      await expect(
+        sut.removeUser(authStub.admin, albumStub.sharedWithUser.id, userEntityStub.user1.id),
+      ).resolves.toBeUndefined();
+
+      expect(albumMock.update).toHaveBeenCalledTimes(1);
+      expect(albumMock.update).toHaveBeenCalledWith({
+        id: albumStub.sharedWithUser.id,
+        updatedAt: expect.any(Date),
+        sharedUsers: [],
+      });
+    });
+
+    it('should prevent removing a shared user from a not-owned album (shared with auth user)', async () => {
+      albumMock.getByIds.mockResolvedValue([albumStub.sharedWithMultiple]);
+
+      await expect(
+        sut.removeUser(authStub.user1, albumStub.sharedWithMultiple.id, authStub.user2.id),
+      ).rejects.toBeInstanceOf(ForbiddenException);
+
+      expect(albumMock.update).not.toHaveBeenCalled();
+    });
+
+    it('should allow a shared user to remove themselves', async () => {
+      albumMock.getByIds.mockResolvedValue([albumStub.sharedWithUser]);
+
+      await sut.removeUser(authStub.user1, albumStub.sharedWithUser.id, authStub.user1.id);
+
+      expect(albumMock.update).toHaveBeenCalledTimes(1);
+      expect(albumMock.update).toHaveBeenCalledWith({
+        id: albumStub.sharedWithUser.id,
+        updatedAt: expect.any(Date),
+        sharedUsers: [],
+      });
+    });
+
+    it('should allow a shared user to remove themselves using "me"', async () => {
+      albumMock.getByIds.mockResolvedValue([albumStub.sharedWithUser]);
+
+      await sut.removeUser(authStub.user1, albumStub.sharedWithUser.id, 'me');
+
+      expect(albumMock.update).toHaveBeenCalledTimes(1);
+      expect(albumMock.update).toHaveBeenCalledWith({
+        id: albumStub.sharedWithUser.id,
+        updatedAt: expect.any(Date),
+        sharedUsers: [],
+      });
+    });
+
+    it('should not allow the owner to be removed', async () => {
+      albumMock.getByIds.mockResolvedValue([albumStub.empty]);
+
+      await expect(sut.removeUser(authStub.admin, albumStub.empty.id, authStub.admin.id)).rejects.toBeInstanceOf(
+        BadRequestException,
+      );
+
+      expect(albumMock.update).not.toHaveBeenCalled();
+    });
+
+    it('should throw an error for a user not in the album', async () => {
+      albumMock.getByIds.mockResolvedValue([albumStub.empty]);
+
+      await expect(sut.removeUser(authStub.admin, albumStub.empty.id, 'user-3')).rejects.toBeInstanceOf(
+        BadRequestException,
+      );
+
+      expect(albumMock.update).not.toHaveBeenCalled();
+    });
+  });
 });

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

@@ -3,8 +3,9 @@ import { BadRequestException, ForbiddenException, Inject, Injectable } from '@ne
 import { IAssetRepository, mapAsset } from '../asset';
 import { AuthUserDto } from '../auth';
 import { IJobRepository, JobName } from '../job';
+import { IUserRepository } from '../user';
 import { IAlbumRepository } from './album.repository';
-import { CreateAlbumDto, GetAlbumsDto, UpdateAlbumDto } from './dto';
+import { AddUsersDto, CreateAlbumDto, GetAlbumsDto, UpdateAlbumDto } from './dto';
 import { AlbumResponseDto, mapAlbum } from './response-dto';
 
 @Injectable()
@@ -13,6 +14,7 @@ export class AlbumService {
     @Inject(IAlbumRepository) private albumRepository: IAlbumRepository,
     @Inject(IAssetRepository) private assetRepository: IAssetRepository,
     @Inject(IJobRepository) private jobRepository: IJobRepository,
+    @Inject(IUserRepository) private userRepository: IUserRepository,
   ) {}
 
   async getAll({ id: ownerId }: AuthUserDto, { assetId, shared }: GetAlbumsDto): Promise<AlbumResponseDto[]> {
@@ -48,7 +50,7 @@ export class AlbumService {
     });
   }
 
-  async updateInvalidThumbnails(): Promise<number> {
+  private async updateInvalidThumbnails(): Promise<number> {
     const invalidAlbumIds = await this.albumRepository.getInvalidThumbnail();
 
     for (const albumId of invalidAlbumIds) {
@@ -60,7 +62,13 @@ export class AlbumService {
   }
 
   async create(authUser: AuthUserDto, dto: CreateAlbumDto): Promise<AlbumResponseDto> {
-    // TODO: Handle nonexistent sharedWithUserIds and assetIds.
+    for (const userId of dto.sharedWithUserIds || []) {
+      const exists = await this.userRepository.get(userId);
+      if (!exists) {
+        throw new BadRequestException('User not found');
+      }
+    }
+
     const album = await this.albumRepository.create({
       ownerId: authUser.id,
       albumName: dto.albumName,
@@ -68,19 +76,14 @@ export class AlbumService {
       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);
   }
 
   async update(authUser: AuthUserDto, id: string, dto: UpdateAlbumDto): Promise<AlbumResponseDto> {
-    const [album] = await this.albumRepository.getByIds([id]);
+    const album = await this.get(id);
-    if (!album) {
+    this.assertOwner(authUser, album);
-      throw new BadRequestException('Album not found');
-    }
-
-    if (album.ownerId !== authUser.id) {
-      throw new ForbiddenException('Album not owned by user');
-    }
 
     if (dto.albumThumbnailAssetId) {
       const valid = await this.albumRepository.hasAsset(id, dto.albumThumbnailAssetId);
@@ -113,4 +116,73 @@ export class AlbumService {
     await this.albumRepository.delete(album);
     await this.jobRepository.queue({ name: JobName.SEARCH_REMOVE_ALBUM, data: { ids: [id] } });
   }
+
+  async addUsers(authUser: AuthUserDto, id: string, dto: AddUsersDto) {
+    const album = await this.get(id);
+    this.assertOwner(authUser, album);
+
+    for (const userId of dto.sharedUserIds) {
+      const exists = album.sharedUsers.find((user) => user.id === userId);
+      if (exists) {
+        throw new BadRequestException('User already added');
+      }
+
+      const user = await this.userRepository.get(userId);
+      if (!user) {
+        throw new BadRequestException('User not found');
+      }
+
+      album.sharedUsers.push({ id: userId } as UserEntity);
+    }
+
+    return this.albumRepository
+      .update({
+        id: album.id,
+        updatedAt: new Date(),
+        sharedUsers: album.sharedUsers,
+      })
+      .then(mapAlbum);
+  }
+
+  async removeUser(authUser: AuthUserDto, id: string, userId: string | 'me'): Promise<void> {
+    if (userId === 'me') {
+      userId = authUser.id;
+    }
+
+    const album = await this.get(id);
+
+    if (album.ownerId === userId) {
+      throw new BadRequestException('Cannot remove album owner');
+    }
+
+    const exists = album.sharedUsers.find((user) => user.id === userId);
+    if (!exists) {
+      throw new BadRequestException('Album not shared with user');
+    }
+
+    // non-admin can remove themselves
+    if (authUser.id !== userId) {
+      this.assertOwner(authUser, album);
+    }
+
+    await this.albumRepository.update({
+      id: album.id,
+      updatedAt: new Date(),
+      sharedUsers: album.sharedUsers.filter((user) => user.id !== userId),
+    });
+  }
+
+  private async get(id: string) {
+    const [album] = await this.albumRepository.getByIds([id]);
+    if (!album) {
+      throw new BadRequestException('Album not found');
+    }
+    return album;
+  }
+
+  private assertOwner(authUser: AuthUserDto, album: AlbumEntity) {
+    if (album.ownerId !== authUser.id) {
+      throw new ForbiddenException('Album not owned by user');
+    }
+  }
 }

server/libs/domain/src/album/dto/album-add-users.dto.ts

@@ -0,0 +1,8 @@
+import { ArrayNotEmpty } from 'class-validator';
+import { ValidateUUID } from '../../../../../apps/immich/src/decorators/validate-uuid.decorator';
+
+export class AddUsersDto {
+  @ValidateUUID({ each: true })
+  @ArrayNotEmpty()
+  sharedUserIds!: string[];
+}

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

@@ -1,3 +1,4 @@
+export * from './album-add-users.dto';
 export * from './album-create.dto';
 export * from './album-update.dto';
 export * from './get-albums.dto';

server/libs/domain/test/fixtures.ts

@@ -61,6 +61,16 @@ export const authStub = {
     isShowExif: true,
     accessTokenId: 'token-id',
   }),
+  user2: Object.freeze<AuthUserDto>({
+    id: 'user-2',
+    email: 'user2@immich.app',
+    isAdmin: false,
+    isPublicUser: false,
+    isAllowUpload: true,
+    isAllowDownload: true,
+    isShowExif: true,
+    accessTokenId: 'token-id',
+  }),
   adminSharedLink: Object.freeze<AuthUserDto>({
     id: 'admin_id',
     email: 'admin@test.com',
@@ -125,6 +135,21 @@ export const userEntityStub = {
     tags: [],
     assets: [],
   }),
+  user2: Object.freeze<UserEntity>({
+    ...authStub.user2,
+    password: 'immich_password',
+    firstName: 'immich_first_name',
+    lastName: 'immich_last_name',
+    storageLabel: null,
+    oauthId: '',
+    shouldChangePassword: false,
+    profileImagePath: '',
+    createdAt: new Date('2021-01-01'),
+    deletedAt: null,
+    updatedAt: new Date('2021-01-01'),
+    tags: [],
+    assets: [],
+  }),
   storageLabel: Object.freeze<UserEntity>({
     ...authStub.user1,
     password: 'immich_password',
@@ -357,6 +382,19 @@ export const albumStub = {
     sharedLinks: [],
     sharedUsers: [userEntityStub.user1],
   }),
+  sharedWithMultiple: Object.freeze<AlbumEntity>({
+    id: 'album-3',
+    albumName: 'Empty album shared with users',
+    ownerId: authStub.admin.id,
+    owner: userEntityStub.admin,
+    assets: [],
+    albumThumbnailAsset: null,
+    albumThumbnailAssetId: null,
+    createdAt: new Date(),
+    updatedAt: new Date(),
+    sharedLinks: [],
+    sharedUsers: [userEntityStub.user1, userEntityStub.user2],
+  }),
   sharedWithAdmin: Object.freeze<AlbumEntity>({
     id: 'album-3',
     albumName: 'Empty album shared with admin',

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

@@ -16,6 +16,7 @@ export class AlbumRepository implements IAlbumRepository {
       },
       relations: {
         owner: true,
+        sharedUsers: true,
       },
     });
   }
@@ -153,6 +154,12 @@ export class AlbumRepository implements IAlbumRepository {
 
   private async save(album: Partial<AlbumEntity>) {
     const { id } = await this.repository.save(album);
-    return this.repository.findOneOrFail({ where: { id }, relations: { owner: true } });
+    return this.repository.findOneOrFail({
+      where: { id },
+      relations: {
+        owner: true,
+        sharedUsers: true,
+      },
+    });
   }
 }

web/src/lib/components/album-page/album-viewer.svelte

@@ -42,6 +42,7 @@
 	import ShareInfoModal from './share-info-modal.svelte';
 	import ThumbnailSelection from './thumbnail-selection.svelte';
 	import UserSelectionModal from './user-selection-modal.svelte';
+	import { handleError } from '../../utils/handle-error';
 
 	export let album: AlbumResponseDto;
 	export let sharedLink: SharedLinkResponseDto | undefined = undefined;
@@ -195,19 +196,16 @@
 		if (userId == 'me') {
 			isShowShareInfoModal = false;
 			goto(backUrl);
+			return;
 		}
 
 		try {
 			const { data } = await api.albumApi.getAlbumInfo({ id: album.id });
 
 			album = data;
-			isShowShareInfoModal = false;
+			isShowShareInfoModal = data.sharedUsers.length >= 1;
 		} catch (e) {
-			console.error('Error [sharedUserDeletedHandler] ', e);
+			handleError(e, 'Error deleting share users');
-			notificationController.show({
-				type: NotificationType.Error,
-				message: 'Error deleting share users, check console for more details'
-			});
 		}
 	};
 

web/src/lib/components/album-page/share-info-modal.svelte

@@ -1,7 +1,6 @@
 <script lang="ts">
 	import { createEventDispatcher, onMount } from 'svelte';
 	import { AlbumResponseDto, api, UserResponseDto } from '@api';
-	import { clickOutside } from '$lib/utils/click-outside';
 	import BaseModal from '../shared-components/base-modal.svelte';
 	import UserAvatar from '../shared-components/user-avatar.svelte';
 	import DotsVertical from 'svelte-material-icons/DotsVertical.svelte';
@@ -12,15 +11,18 @@
 		notificationController,
 		NotificationType
 	} from '../shared-components/notification/notification';
+	import { handleError } from '../../utils/handle-error';
+	import ConfirmDialogue from '../shared-components/confirm-dialogue.svelte';
 
 	export let album: AlbumResponseDto;
 
 	const dispatch = createEventDispatcher();
 
 	let currentUser: UserResponseDto;
-	let isShowMenu = false;
 	let position = { x: 0, y: 0 };
-	let targetUserId: string;
+	let selectedMenuUser: UserResponseDto | null = null;
+	let selectedRemoveUser: UserResponseDto | null = null;
+
 	$: isOwned = currentUser?.id == album.ownerId;
 
 	onMount(async () => {
@@ -28,16 +30,12 @@
 			const { data } = await api.userApi.getMyUserInfo();
 			currentUser = data;
 		} catch (e) {
-			console.error('Error [share-info-modal] [getAllUsers]', e);
+			handleError(e, 'Unable to refresh user');
-			notificationController.show({
-				message: 'Error getting user info, check console for more details',
-				type: NotificationType.Error
-			});
 		}
 	});
 
-	const showContextMenu = (userId: string) => {
+	const showContextMenu = (user: UserResponseDto) => {
-		const iconButton = document.getElementById('icon-' + userId);
+		const iconButton = document.getElementById('icon-' + user.id);
 
 		if (iconButton) {
 			position = {
@@ -46,69 +44,101 @@
 			};
 		}
 
-		targetUserId = userId;
+		selectedMenuUser = user;
-		isShowMenu = !isShowMenu;
+		selectedRemoveUser = null;
 	};
 
-	const removeUser = async (userId: string) => {
+	const handleMenuRemove = () => {
-		if (window.confirm('Do you want to remove selected user from the album?')) {
+		selectedRemoveUser = selectedMenuUser;
-			try {
+		selectedMenuUser = null;
-				await api.albumApi.removeUserFromAlbum({ id: album.id, userId });
+	};
-				dispatch('user-deleted', { userId });
+
-			} catch (e) {
+	const handleRemoveUser = async () => {
-				console.error('Error [share-info-modal] [removeUser]', e);
+		if (!selectedRemoveUser) {
-				notificationController.show({
+			return;
-					message: 'Error removing user, check console for more details',
+		}
-					type: NotificationType.Error
+
-				});
+		const userId = selectedRemoveUser.id === currentUser?.id ? 'me' : selectedRemoveUser.id;
-			}
+
+		try {
+			await api.albumApi.removeUserFromAlbum({ id: album.id, userId });
+			dispatch('user-deleted', { userId });
+			const message =
+				userId === 'me' ? `Left ${album.albumName}` : `Removed ${selectedRemoveUser.firstName}`;
+			notificationController.show({ type: NotificationType.Info, message });
+		} catch (e) {
+			handleError(e, 'Unable to remove user');
+		} finally {
+			selectedRemoveUser = null;
 		}
 	};
 </script>
 
-<BaseModal on:close={() => dispatch('close')}>
+{#if !selectedRemoveUser}
-	<svelte:fragment slot="title">
+	<BaseModal on:close={() => dispatch('close')}>
-		<span class="flex gap-2 place-items-center">
+		<svelte:fragment slot="title">
-			<p class="font-medium text-immich-fg dark:text-immich-dark-fg">Options</p>
+			<span class="flex gap-2 place-items-center">
-		</span>
+				<p class="font-medium text-immich-fg dark:text-immich-dark-fg">Options</p>
-	</svelte:fragment>
+			</span>
+		</svelte:fragment>
 
-	<section class="max-h-[400px] overflow-y-auto immich-scrollbar pb-4">
+		<section class="max-h-[400px] overflow-y-auto immich-scrollbar pb-4">
-		{#each album.sharedUsers as user}
+			{#each album.sharedUsers as user}
-			<div
+				<div
-				class="flex gap-4 p-5 place-items-center justify-between w-full transition-colors hover:bg-gray-50 dark:hover:bg-gray-700"
+					class="flex gap-4 p-5 place-items-center justify-between w-full transition-colors hover:bg-gray-50 dark:hover:bg-gray-700"
-			>
+				>
-				<div class="flex gap-4 place-items-center">
+					<div class="flex gap-4 place-items-center">
-					<UserAvatar {user} size="md" autoColor />
+						<UserAvatar {user} size="md" autoColor />
-					<p class="font-medium text-sm">{user.firstName} {user.lastName}</p>
+						<p class="font-medium text-sm">{user.firstName} {user.lastName}</p>
-				</div>
+					</div>
 
-				<div id={`icon-${user.id}`} class="flex place-items-center">
+					<div id={`icon-${user.id}`} class="flex place-items-center">
-					{#if isOwned}
+						{#if isOwned}
-						<div use:clickOutside on:outclick={() => (isShowMenu = false)}>
+							<div>
-							<CircleIconButton
+								<CircleIconButton
-								on:click={() => showContextMenu(user.id)}
+									on:click={() => showContextMenu(user)}
-								logo={DotsVertical}
+									logo={DotsVertical}
-								backgroundColor={'transparent'}
+									backgroundColor="transparent"
-								hoverColor={'#e2e7e9'}
+									hoverColor="#e2e7e9"
-								size={'20'}
+									size="20"
-							>
+								/>
-								{#if isShowMenu}
+
-									<ContextMenu {...position}>
+								{#if selectedMenuUser === user}
-										<MenuOption on:click={() => removeUser(targetUserId)} text="Remove" />
+									<ContextMenu {...position} on:outclick={() => (selectedMenuUser = null)}>
+										<MenuOption on:click={handleMenuRemove} text="Remove" />
 									</ContextMenu>
 								{/if}
-							</CircleIconButton>
+							</div>
-						</div>
+						{:else if user.id == currentUser?.id}
-					{:else if user.id == currentUser?.id}
+							<button
-						<button
+								on:click={() => (selectedRemoveUser = user)}
-							on:click={() => removeUser('me')}
+								class="text-sm text-immich-primary dark:text-immich-dark-primary font-medium transition-colors hover:text-immich-primary/75"
-							class="text-sm text-immich-primary dark:text-immich-dark-primary font-medium transition-colors hover:text-immich-primary/75"
+								>Leave</button
-							>Leave</button
+							>
-						>
+						{/if}
-					{/if}
+					</div>
 				</div>
-			</div>
+			{/each}
-		{/each}
+		</section>
-	</section>
+	</BaseModal>
-</BaseModal>
+{/if}
+
+{#if selectedRemoveUser && selectedRemoveUser?.id === currentUser?.id}
+	<ConfirmDialogue
+		title="Leave Album?"
+		prompt="Are you sure you want to leave {album.albumName}?"
+		confirmText="Leave"
+		on:confirm={handleRemoveUser}
+		on:cancel={() => (selectedRemoveUser = null)}
+	/>
+{/if}
+
+{#if selectedRemoveUser && selectedRemoveUser?.id !== currentUser?.id}
+	<ConfirmDialogue
+		title="Remove User?"
+		prompt="Are you sure you want to remove {selectedRemoveUser.firstName} {selectedRemoveUser.lastName}"
+		confirmText="Remove"
+		on:confirm={handleRemoveUser}
+		on:cancel={() => (selectedRemoveUser = null)}
+	/>
+{/if}