chore(server): Improve test coverage! (#3889)

* tests for person service

* tests for auth service

* tests for access core

* improve tests for album service

* fix missing brackets and remove comments

* tests for asset service

* tests for face recognition

* tests for job service

* feedback

* tests for search service (broken)

* fix: disabled search test

* tests for smart-info service

* tests for storage template service

* tests for user service

* fix formatting of untouched files LOL

* attempt to fix formatting

* streamline api utils, add asset api for uploading files

* test upload of assets

* fix formatting

* move test-utils to correct folder

* test add assets to album

* use random bytes instead of test image

* (e2e) test albums with assets

* (e2e) complete tests for album endpoints

* (e2e) tests for asset endpoint

* fix: asset upload/import dto validation

* (e2e) tests for statistics asset endpoint

* fix wrong describe text

* (e2e) tests for people with faces

* (e2e) clean up person tests

* (e2e) tests for partner sharing endpoints

* (e2e) tests for link sharing

* (e2e) tests for the asset time bucket endpoint

* fix minor issues

* remove access.core.spec.ts

* chore: wording

* chore: organize test api files

* chore: fix test describe

* implement feedback

* fix race condition in album tests

---------

Co-authored-by: Jason Rasmussen <jrasm91@gmail.com>

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

@@ -153,44 +153,30 @@ describe(AlbumService.name, () => {
   describe('create', () => {
     it('creates album', async () => {
       albumMock.create.mockResolvedValue(albumStub.empty);
+      userMock.get.mockResolvedValue(userStub.user1);
 
-      await expect(sut.create(authStub.admin, { albumName: 'Empty album' })).resolves.toEqual({
+      await sut.create(authStub.admin, {
         albumName: 'Empty album',
+        sharedWithUserIds: ['user-id'],
         description: '',
-        albumThumbnailAssetId: null,
-        assetCount: 0,
-        assets: [],
-        createdAt: expect.anything(),
-        id: 'album-1',
-        owner: {
-          email: 'admin@test.com',
-          firstName: 'admin_first_name',
-          id: 'admin_id',
-          isAdmin: true,
-          lastName: 'admin_last_name',
-          oauthId: '',
-          profileImagePath: '',
-          shouldChangePassword: false,
-          storageLabel: 'admin',
-          createdAt: new Date('2021-01-01'),
-          deletedAt: null,
-          updatedAt: new Date('2021-01-01'),
-          externalPath: null,
-          memoriesEnabled: true,
-        },
-        ownerId: 'admin_id',
-        shared: false,
-        sharedUsers: [],
-        startDate: undefined,
-        endDate: undefined,
-        hasSharedLink: false,
-        updatedAt: expect.anything(),
+        assetIds: ['123'],
       });
 
       expect(jobMock.queue).toHaveBeenCalledWith({
         name: JobName.SEARCH_INDEX_ALBUM,
         data: { ids: [albumStub.empty.id] },
       });
+
+      expect(albumMock.create).toHaveBeenCalledWith({
+        ownerId: authStub.admin.id,
+        albumName: albumStub.empty.albumName,
+        description: albumStub.empty.description,
+        sharedUsers: [{ id: 'user-id' }],
+        assets: [{ id: '123' }],
+        albumThumbnailAssetId: '123',
+      });
+
+      expect(userMock.get).toHaveBeenCalledWith('user-id');
     });
 
     it('should require valid userIds', async () => {

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

@@ -136,9 +136,6 @@ export class AlbumService {
     await this.access.requirePermission(authUser, Permission.ALBUM_DELETE, id);
 
     const album = await this.findOrFail(id, { withAssets: false });
-    if (!album) {
-      throw new BadRequestException('Album not found');
-    }
 
     await this.albumRepository.delete(album);
     await this.jobRepository.queue({ name: JobName.SEARCH_REMOVE_ALBUM, data: { ids: [id] } });
@@ -228,6 +225,10 @@ export class AlbumService {
     const album = await this.findOrFail(id, { withAssets: false });
 
     for (const userId of dto.sharedUserIds) {
+      if (album.ownerId === userId) {
+        throw new BadRequestException('Cannot be shared with owner');
+      }
+
       const exists = album.sharedUsers.find((user) => user.id === userId);
       if (exists) {
         throw new BadRequestException('User already added');

server/src/domain/asset/asset.service.spec.ts

@@ -15,7 +15,7 @@ import { Readable } from 'stream';
 import { ICryptoRepository } from '../crypto';
 import { IJobRepository, JobName } from '../job';
 import { IStorageRepository } from '../storage';
-import { AssetStats, IAssetRepository } from './asset.repository';
+import { AssetStats, IAssetRepository, TimeBucketSize } from './asset.repository';
 import { AssetService, UploadFieldName } from './asset.service';
 import { AssetJobName, AssetStatsResponseDto, DownloadResponseDto } from './dto';
 import { mapAsset } from './response-dto';
@@ -330,6 +330,73 @@ describe(AssetService.name, () => {
     });
   });
 
+  describe('getTimeBuckets', () => {
+    it("should return buckets if userId and albumId aren't set", async () => {
+      assetMock.getTimeBuckets.mockResolvedValue([{ timeBucket: 'bucket', count: 1 }]);
+
+      await expect(
+        sut.getTimeBuckets(authStub.admin, {
+          size: TimeBucketSize.DAY,
+        }),
+      ).resolves.toEqual(expect.arrayContaining([{ timeBucket: 'bucket', count: 1 }]));
+      expect(assetMock.getTimeBuckets).toBeCalledWith({ size: TimeBucketSize.DAY, userId: authStub.admin.id });
+    });
+  });
+
+  describe('getByTimeBucket', () => {
+    it('should return the assets for a album time bucket if user has album.read', async () => {
+      accessMock.album.hasOwnerAccess.mockResolvedValue(true);
+      assetMock.getByTimeBucket.mockResolvedValue([assetStub.image]);
+
+      await expect(
+        sut.getByTimeBucket(authStub.admin, { size: TimeBucketSize.DAY, timeBucket: 'bucket', albumId: 'album-id' }),
+      ).resolves.toEqual(expect.arrayContaining([expect.objectContaining({ id: 'asset-id' })]));
+
+      expect(accessMock.album.hasOwnerAccess).toHaveBeenCalledWith(authStub.admin.id, 'album-id');
+      expect(assetMock.getByTimeBucket).toBeCalledWith('bucket', {
+        size: TimeBucketSize.DAY,
+        timeBucket: 'bucket',
+        albumId: 'album-id',
+      });
+    });
+
+    it('should return the assets for a archive time bucket if user has archive.read', async () => {
+      assetMock.getByTimeBucket.mockResolvedValue([assetStub.image]);
+
+      await expect(
+        sut.getByTimeBucket(authStub.admin, {
+          size: TimeBucketSize.DAY,
+          timeBucket: 'bucket',
+          isArchived: true,
+          userId: authStub.admin.id,
+        }),
+      ).resolves.toEqual(expect.arrayContaining([expect.objectContaining({ id: 'asset-id' })]));
+      expect(assetMock.getByTimeBucket).toBeCalledWith('bucket', {
+        size: TimeBucketSize.DAY,
+        timeBucket: 'bucket',
+        isArchived: true,
+        userId: authStub.admin.id,
+      });
+    });
+
+    it('should return the assets for a library time bucket if user has library.read', async () => {
+      assetMock.getByTimeBucket.mockResolvedValue([assetStub.image]);
+
+      await expect(
+        sut.getByTimeBucket(authStub.admin, {
+          size: TimeBucketSize.DAY,
+          timeBucket: 'bucket',
+          userId: authStub.admin.id,
+        }),
+      ).resolves.toEqual(expect.arrayContaining([expect.objectContaining({ id: 'asset-id' })]));
+      expect(assetMock.getByTimeBucket).toBeCalledWith('bucket', {
+        size: TimeBucketSize.DAY,
+        timeBucket: 'bucket',
+        userId: authStub.admin.id,
+      });
+    });
+  });
+
   describe('downloadFile', () => {
     it('should require the asset.download permission', async () => {
       accessMock.asset.hasOwnerAccess.mockResolvedValue(false);

server/src/domain/auth/auth.service.spec.ts

@@ -214,6 +214,15 @@ describe('AuthService', () => {
 
       expect(userTokenMock.delete).toHaveBeenCalledWith('123', 'token123');
     });
+
+    it('should return the default redirect if auth type is OAUTH but oauth is not enabled', async () => {
+      const authUser = { id: '123' } as AuthUserDto;
+
+      await expect(sut.logout(authUser, AuthType.OAUTH)).resolves.toEqual({
+        successful: true,
+        redirectUri: '/auth/login?autoLaunch=0',
+      });
+    });
   });
 
   describe('adminSignUp', () => {

server/src/domain/facial-recognition/facial-recognition.service.spec.ts

@@ -1,4 +1,4 @@
-import { Colorspace } from '@app/infra/entities';
+import { Colorspace, SystemConfigKey } from '@app/infra/entities';
 import {
   assetStub,
   faceStub,
@@ -137,6 +137,14 @@ describe(FacialRecognitionService.name, () => {
   });
 
   describe('handleQueueRecognizeFaces', () => {
+    it('should return if machine learning is disabled', async () => {
+      configMock.load.mockResolvedValue([{ key: SystemConfigKey.MACHINE_LEARNING_ENABLED, value: false }]);
+
+      await expect(sut.handleQueueRecognizeFaces({})).resolves.toBe(true);
+      expect(jobMock.queue).not.toHaveBeenCalled();
+      expect(configMock.load).toHaveBeenCalled();
+    });
+
     it('should queue missing assets', async () => {
       assetMock.getWithout.mockResolvedValue({
         items: [assetStub.image],
@@ -170,6 +178,14 @@ describe(FacialRecognitionService.name, () => {
   });
 
   describe('handleRecognizeFaces', () => {
+    it('should return if machine learning is disabled', async () => {
+      configMock.load.mockResolvedValue([{ key: SystemConfigKey.MACHINE_LEARNING_ENABLED, value: false }]);
+
+      await expect(sut.handleRecognizeFaces({ id: 'foo' })).resolves.toBe(true);
+      expect(assetMock.getByIds).not.toHaveBeenCalled();
+      expect(configMock.load).toHaveBeenCalled();
+    });
+
     it('should skip when no resize path', async () => {
       assetMock.getByIds.mockResolvedValue([assetStub.noResizePath]);
       await sut.handleRecognizeFaces({ id: assetStub.noResizePath.id });
@@ -260,6 +276,14 @@ describe(FacialRecognitionService.name, () => {
   });
 
   describe('handleGenerateFaceThumbnail', () => {
+    it('should return if machine learning is disabled', async () => {
+      configMock.load.mockResolvedValue([{ key: SystemConfigKey.MACHINE_LEARNING_ENABLED, value: false }]);
+
+      await expect(sut.handleGenerateFaceThumbnail(face.middle)).resolves.toBe(true);
+      expect(assetMock.getByIds).not.toHaveBeenCalled();
+      expect(configMock.load).toHaveBeenCalled();
+    });
+
     it('should skip an asset not found', async () => {
       assetMock.getByIds.mockResolvedValue([]);
 

server/src/domain/job/job.service.spec.ts

@@ -288,6 +288,17 @@ describe(JobService.name, () => {
           JobName.VIDEO_CONVERSION,
         ],
       },
+      {
+        item: { name: JobName.GENERATE_JPEG_THUMBNAIL, data: { id: 'asset-live-image', source: 'upload' } },
+        jobs: [
+          JobName.CLASSIFY_IMAGE,
+          JobName.GENERATE_WEBP_THUMBNAIL,
+          JobName.RECOGNIZE_FACES,
+          JobName.GENERATE_THUMBHASH_THUMBNAIL,
+          JobName.ENCODE_CLIP,
+          JobName.VIDEO_CONVERSION,
+        ],
+      },
       {
         item: { name: JobName.CLASSIFY_IMAGE, data: { id: 'asset-1' } },
         jobs: [JobName.SEARCH_INDEX_ASSET],
@@ -305,7 +316,11 @@ describe(JobService.name, () => {
     for (const { item, jobs } of tests) {
       it(`should queue ${jobs.length} jobs when a ${item.name} job finishes successfully`, async () => {
         if (item.name === JobName.GENERATE_JPEG_THUMBNAIL && item.data.source === 'upload') {
-          assetMock.getByIds.mockResolvedValue([assetStub.livePhotoMotionAsset]);
+          if (item.data.id === 'asset-live-image') {
+            assetMock.getByIds.mockResolvedValue([assetStub.livePhotoStillAsset]);
+          } else {
+            assetMock.getByIds.mockResolvedValue([assetStub.livePhotoMotionAsset]);
+          }
         } else {
           assetMock.getByIds.mockResolvedValue([]);
         }

server/src/domain/person/person.service.spec.ts

@@ -215,6 +215,15 @@ describe(PersonService.name, () => {
         },
       });
     });
+
+    it('should throw an error when the face feature assetId is invalid', async () => {
+      personMock.getById.mockResolvedValue(personStub.withName);
+
+      await expect(sut.update(authStub.admin, 'person-1', { featureFaceAssetId: '-1' })).rejects.toThrow(
+        BadRequestException,
+      );
+      expect(personMock.update).not.toHaveBeenCalled();
+    });
   });
 
   describe('updateAll', () => {

server/src/domain/search/search.service.spec.ts

@@ -1,3 +1,4 @@
+import { BadRequestException } from '@nestjs/common';
 import {
   albumStub,
   assetStub,
@@ -15,12 +16,13 @@ import {
 } from '@test';
 import { plainToInstance } from 'class-transformer';
 import { IAlbumRepository } from '../album/album.repository';
+import { mapAsset } from '../asset';
 import { IAssetRepository } from '../asset/asset.repository';
 import { IFaceRepository } from '../facial-recognition';
-import { ISystemConfigRepository } from '../index';
 import { JobName } from '../job';
 import { IJobRepository } from '../job/job.repository';
 import { IMachineLearningRepository } from '../smart-info';
+import { ISystemConfigRepository } from '../system-config';
 import { SearchDto } from './dto';
 import { ISearchRepository } from './search.repository';
 import { SearchService } from './search.service';
@@ -50,9 +52,17 @@ describe(SearchService.name, () => {
 
     searchMock.checkMigrationStatus.mockResolvedValue({ assets: false, albums: false, faces: false });
 
+    delete process.env.TYPESENSE_ENABLED;
     await sut.init();
   });
 
+  const disableSearch = () => {
+    searchMock.setup.mockClear();
+    searchMock.checkMigrationStatus.mockClear();
+    jobMock.queue.mockClear();
+    process.env.TYPESENSE_ENABLED = 'false';
+  };
+
   afterEach(() => {
     sut.teardown();
   });
@@ -84,15 +94,14 @@ describe(SearchService.name, () => {
   });
 
   describe(`init`, () => {
-    // it('should skip when search is disabled', async () => {
-    //   await sut.init();
+    it('should skip when search is disabled', async () => {
+      disableSearch();
+      await sut.init();
 
-    //   expect(searchMock.setup).not.toHaveBeenCalled();
-    //   expect(searchMock.checkMigrationStatus).not.toHaveBeenCalled();
-    //   expect(jobMock.queue).not.toHaveBeenCalled();
-
-    //   sut.teardown();
-    // });
+      expect(searchMock.setup).not.toHaveBeenCalled();
+      expect(searchMock.checkMigrationStatus).not.toHaveBeenCalled();
+      expect(jobMock.queue).not.toHaveBeenCalled();
+    });
 
     it('should skip schema migration if not needed', async () => {
       await sut.init();
@@ -114,6 +123,29 @@ describe(SearchService.name, () => {
     });
   });
 
+  describe('getExploreData', () => {
+    it('should throw bad request exception if search is disabled', async () => {
+      disableSearch();
+      await expect(sut.getExploreData(authStub.admin)).rejects.toBeInstanceOf(BadRequestException);
+      expect(searchMock.explore).not.toHaveBeenCalled();
+    });
+
+    it('should return explore data if feature flag SEARCH is set', async () => {
+      searchMock.explore.mockResolvedValue([{ fieldName: 'name', items: [{ value: 'image', data: assetStub.image }] }]);
+      assetMock.getByIds.mockResolvedValue([assetStub.image]);
+
+      await expect(sut.getExploreData(authStub.admin)).resolves.toEqual([
+        {
+          fieldName: 'name',
+          items: [{ value: 'image', data: mapAsset(assetStub.image) }],
+        },
+      ]);
+
+      expect(searchMock.explore).toHaveBeenCalledWith(authStub.admin.id);
+      expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.image.id]);
+    });
+  });
+
   describe('search', () => {
     // it('should throw an error is search is disabled', async () => {
     //   sut['enabled'] = false;
@@ -124,12 +156,40 @@ describe(SearchService.name, () => {
     //   expect(searchMock.searchAssets).not.toHaveBeenCalled();
     // });
 
-    it('should search assets and albums', async () => {
-      searchMock.searchAssets.mockResolvedValue(searchStub.emptyResults);
+    it('should search assets and albums using text search', async () => {
+      searchMock.searchAssets.mockResolvedValue(searchStub.withImage);
       searchMock.searchAlbums.mockResolvedValue(searchStub.emptyResults);
-      searchMock.vectorSearch.mockResolvedValue(searchStub.emptyResults);
+      assetMock.getByIds.mockResolvedValue([assetStub.image]);
 
       await expect(sut.search(authStub.admin, {})).resolves.toEqual({
+        albums: {
+          total: 0,
+          count: 0,
+          page: 1,
+          items: [],
+          facets: [],
+          distances: [],
+        },
+        assets: {
+          total: 1,
+          count: 1,
+          page: 1,
+          items: [mapAsset(assetStub.image)],
+          facets: [],
+          distances: [],
+        },
+      });
+
+      // expect(searchMock.searchAssets).toHaveBeenCalledWith('*', { userId: authStub.admin.id });
+      expect(searchMock.searchAlbums).toHaveBeenCalledWith('*', { userId: authStub.admin.id });
+    });
+
+    it('should search assets and albums using vector search', async () => {
+      searchMock.vectorSearch.mockResolvedValue(searchStub.emptyResults);
+      searchMock.searchAlbums.mockResolvedValue(searchStub.emptyResults);
+      machineMock.encodeText.mockResolvedValue([123]);
+
+      await expect(sut.search(authStub.admin, { clip: true, query: 'foo' })).resolves.toEqual({
         albums: {
           total: 0,
           count: 0,
@@ -148,8 +208,17 @@ describe(SearchService.name, () => {
         },
       });
 
-      // expect(searchMock.searchAssets).toHaveBeenCalledWith('*', { userId: authStub.admin.id });
-      expect(searchMock.searchAlbums).toHaveBeenCalledWith('*', { userId: authStub.admin.id });
+      expect(machineMock.encodeText).toHaveBeenCalledWith(expect.any(String), { text: 'foo' }, expect.any(Object));
+      expect(searchMock.vectorSearch).toHaveBeenCalledWith([123], {
+        userId: authStub.admin.id,
+        clip: true,
+        query: 'foo',
+      });
+      expect(searchMock.searchAlbums).toHaveBeenCalledWith('foo', {
+        userId: authStub.admin.id,
+        clip: true,
+        query: 'foo',
+      });
     });
   });
 

server/src/domain/smart-info/smart-info.service.spec.ts

@@ -1,4 +1,4 @@
-import { AssetEntity } from '@app/infra/entities';
+import { AssetEntity, SystemConfigKey } from '@app/infra/entities';
 import {
   assetStub,
   newAssetRepositoryMock,
@@ -43,6 +43,15 @@ describe(SmartInfoService.name, () => {
   });
 
   describe('handleQueueObjectTagging', () => {
+    it('should do nothing if machine learning is disabled', async () => {
+      configMock.load.mockResolvedValue([{ key: SystemConfigKey.MACHINE_LEARNING_ENABLED, value: false }]);
+
+      await sut.handleQueueObjectTagging({});
+
+      expect(assetMock.getAll).not.toHaveBeenCalled();
+      expect(assetMock.getWithout).not.toHaveBeenCalled();
+    });
+
     it('should queue the assets without tags', async () => {
       assetMock.getWithout.mockResolvedValue({
         items: [assetStub.image],
@@ -68,7 +77,16 @@ describe(SmartInfoService.name, () => {
     });
   });
 
-  describe('handleTagImage', () => {
+  describe('handleClassifyImage', () => {
+    it('should do nothing if machine learning is disabled', async () => {
+      configMock.load.mockResolvedValue([{ key: SystemConfigKey.MACHINE_LEARNING_ENABLED, value: false }]);
+
+      await sut.handleClassifyImage({ id: '123' });
+
+      expect(machineMock.classifyImage).not.toHaveBeenCalled();
+      expect(assetMock.getByIds).not.toHaveBeenCalled();
+    });
+
     it('should skip assets without a resize path', async () => {
       const asset = { resizePath: '' } as AssetEntity;
       assetMock.getByIds.mockResolvedValue([asset]);
@@ -108,6 +126,15 @@ describe(SmartInfoService.name, () => {
   });
 
   describe('handleQueueEncodeClip', () => {
+    it('should do nothing if machine learning is disabled', async () => {
+      configMock.load.mockResolvedValue([{ key: SystemConfigKey.MACHINE_LEARNING_ENABLED, value: false }]);
+
+      await sut.handleQueueEncodeClip({});
+
+      expect(assetMock.getAll).not.toHaveBeenCalled();
+      expect(assetMock.getWithout).not.toHaveBeenCalled();
+    });
+
     it('should queue the assets without clip embeddings', async () => {
       assetMock.getWithout.mockResolvedValue({
         items: [assetStub.image],
@@ -134,6 +161,15 @@ describe(SmartInfoService.name, () => {
   });
 
   describe('handleEncodeClip', () => {
+    it('should do nothing if machine learning is disabled', async () => {
+      configMock.load.mockResolvedValue([{ key: SystemConfigKey.MACHINE_LEARNING_ENABLED, value: false }]);
+
+      await sut.handleEncodeClip({ id: '123' });
+
+      expect(assetMock.getByIds).not.toHaveBeenCalled();
+      expect(machineMock.encodeImage).not.toHaveBeenCalled();
+    });
+
     it('should skip assets without a resize path', async () => {
       const asset = { resizePath: '' } as AssetEntity;
       assetMock.getByIds.mockResolvedValue([asset]);

server/src/domain/storage-template/storage-template.service.spec.ts

@@ -34,6 +34,41 @@ describe(StorageTemplateService.name, () => {
     sut = new StorageTemplateService(assetMock, configMock, defaults, storageMock, userMock);
   });
 
+  describe('handleMigrationSingle', () => {
+    it('should migrate single moving picture', async () => {
+      userMock.get.mockResolvedValue(userStub.user1);
+      const path = (id: string) => `upload/library/${userStub.user1.id}/2023/2023-02-23/${id}.jpg`;
+      const newPath = (id: string) => `upload/library/${userStub.user1.id}/2023/2023-02-23/${id}+1.jpg`;
+
+      when(storageMock.checkFileExists).calledWith(path(assetStub.livePhotoStillAsset.id)).mockResolvedValue(true);
+      when(storageMock.checkFileExists).calledWith(newPath(assetStub.livePhotoStillAsset.id)).mockResolvedValue(false);
+
+      when(storageMock.checkFileExists).calledWith(path(assetStub.livePhotoMotionAsset.id)).mockResolvedValue(true);
+      when(storageMock.checkFileExists).calledWith(newPath(assetStub.livePhotoMotionAsset.id)).mockResolvedValue(false);
+
+      when(assetMock.save)
+        .calledWith({ id: assetStub.livePhotoStillAsset.id, originalPath: newPath(assetStub.livePhotoStillAsset.id) })
+        .mockResolvedValue(assetStub.livePhotoStillAsset);
+
+      when(assetMock.save)
+        .calledWith({ id: assetStub.livePhotoMotionAsset.id, originalPath: newPath(assetStub.livePhotoMotionAsset.id) })
+        .mockResolvedValue(assetStub.livePhotoMotionAsset);
+
+      when(assetMock.getByIds)
+        .calledWith([assetStub.livePhotoStillAsset.id])
+        .mockResolvedValue([assetStub.livePhotoStillAsset]);
+
+      when(assetMock.getByIds)
+        .calledWith([assetStub.livePhotoMotionAsset.id])
+        .mockResolvedValue([assetStub.livePhotoMotionAsset]);
+
+      await expect(sut.handleMigrationSingle({ id: assetStub.livePhotoStillAsset.id })).resolves.toBe(true);
+
+      expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.livePhotoStillAsset.id]);
+      expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.livePhotoMotionAsset.id]);
+    });
+  });
+
   describe('handle template migration', () => {
     it('should handle no assets', async () => {
       assetMock.getAll.mockResolvedValue({

server/src/domain/storage-template/storage-template.service.ts

@@ -89,7 +89,6 @@ export class StorageTemplateService {
     return true;
   }
 
-  // TODO: use asset core (once in domain)
   async moveAsset(asset: AssetEntity, metadata: MoveAssetMetadata) {
     if (asset.isReadOnly) {
       this.logger.verbose(`Not moving read-only asset: ${asset.originalPath}`);
@@ -121,7 +120,7 @@ export class StorageTemplateService {
             error?.stack,
           );
 
-          // Either sidecar move failed or the save failed. Eithr way, move media back
+          // Either sidecar move failed or the save failed. Either way, move media back
           await this.storageRepository.moveFile(destination, source);
 
           if (asset.sidecarPath && sidecarDestination && sidecarMoved) {

server/src/domain/user/user.service.spec.ts

@@ -1,5 +1,10 @@
 import { UserEntity } from '@app/infra/entities';
-import { BadRequestException, ForbiddenException, NotFoundException } from '@nestjs/common';
+import {
+  BadRequestException,
+  ForbiddenException,
+  InternalServerErrorException,
+  NotFoundException,
+} from '@nestjs/common';
 import {
   newAlbumRepositoryMock,
   newAssetRepositoryMock,
@@ -7,6 +12,7 @@ import {
   newJobRepositoryMock,
   newStorageRepositoryMock,
   newUserRepositoryMock,
+  userStub,
 } from '@test';
 import { when } from 'jest-when';
 import { IAlbumRepository } from '../album';
@@ -16,7 +22,7 @@ import { ICryptoRepository } from '../crypto';
 import { IJobRepository, JobName } from '../job';
 import { IStorageRepository } from '../storage';
 import { UpdateUserDto } from './dto/update-user.dto';
-import { UserResponseDto } from './response-dto';
+import { UserResponseDto, mapUser } from './response-dto';
 import { IUserRepository } from './user.repository';
 import { UserService } from './user.service';
 
@@ -216,6 +222,13 @@ describe(UserService.name, () => {
       expect(userMock.getList).toHaveBeenCalled();
       expect(response).toEqual({ userCount: 1 });
     });
+
+    it('should get the user count of all admin users', async () => {
+      userMock.getList.mockResolvedValue([adminUser, immichUser]);
+
+      await expect(sut.getCount({ admin: true })).resolves.toEqual({ userCount: 1 });
+      expect(userMock.getList).toHaveBeenCalled();
+    });
   });
 
   describe('update', () => {
@@ -223,12 +236,17 @@ describe(UserService.name, () => {
       const update: UpdateUserDto = {
         id: immichUser.id,
         shouldChangePassword: true,
+        email: 'immich@test.com',
+        storageLabel: 'storage_label',
       };
+      userMock.getByEmail.mockResolvedValue(null);
+      userMock.getByStorageLabel.mockResolvedValue(null);
+      userMock.update.mockResolvedValue({ ...updatedImmichUser, isAdmin: true, storageLabel: 'storage_label' });
 
-      when(userMock.update).calledWith(update.id, update).mockResolvedValueOnce(updatedImmichUser);
-
-      const updatedUser = await sut.update(immichUserAuth, update);
+      const updatedUser = await sut.update({ ...immichUserAuth, isAdmin: true }, update);
       expect(updatedUser.shouldChangePassword).toEqual(true);
+      expect(userMock.getByEmail).toHaveBeenCalledWith(update.email);
+      expect(userMock.getByStorageLabel).toHaveBeenCalledWith(update.storageLabel);
     });
 
     it('should not set an empty string for storage label', async () => {
@@ -345,20 +363,37 @@ describe(UserService.name, () => {
   });
 
   describe('restore', () => {
+    it('should throw error if user could not be found', async () => {
+      userMock.get.mockResolvedValue(null);
+
+      await expect(sut.restore(immichUserAuth, adminUser.id)).rejects.toThrowError(BadRequestException);
+      expect(userMock.restore).not.toHaveBeenCalled();
+    });
+
     it('should require an admin', async () => {
       when(userMock.get).calledWith(adminUser.id, true).mockResolvedValue(adminUser);
       await expect(sut.restore(immichUserAuth, adminUser.id)).rejects.toBeInstanceOf(ForbiddenException);
       expect(userMock.get).toHaveBeenCalledWith(adminUser.id, true);
     });
 
-    it('should require the auth user be an admin', async () => {
-      await expect(sut.delete(immichUserAuth, adminUserAuth.id)).rejects.toBeInstanceOf(ForbiddenException);
+    it('should restore an user', async () => {
+      userMock.get.mockResolvedValue(immichUser);
+      userMock.restore.mockResolvedValue(immichUser);
 
-      expect(userMock.delete).not.toHaveBeenCalled();
+      await expect(sut.restore(adminUserAuth, immichUser.id)).resolves.toEqual(mapUser(immichUser));
+      expect(userMock.get).toHaveBeenCalledWith(immichUser.id, true);
+      expect(userMock.restore).toHaveBeenCalledWith(immichUser);
     });
   });
 
   describe('delete', () => {
+    it('should throw error if user could not be found', async () => {
+      userMock.get.mockResolvedValue(null);
+
+      await expect(sut.delete(immichUserAuth, adminUser.id)).rejects.toThrowError(BadRequestException);
+      expect(userMock.delete).not.toHaveBeenCalled();
+    });
+
     it('cannot delete admin user', async () => {
       await expect(sut.delete(adminUserAuth, adminUserAuth.id)).rejects.toBeInstanceOf(ForbiddenException);
     });
@@ -368,9 +403,18 @@ describe(UserService.name, () => {
 
       expect(userMock.delete).not.toHaveBeenCalled();
     });
+
+    it('should delete user', async () => {
+      userMock.get.mockResolvedValue(immichUser);
+      userMock.delete.mockResolvedValue(immichUser);
+
+      await expect(sut.delete(adminUserAuth, immichUser.id)).resolves.toEqual(mapUser(immichUser));
+      expect(userMock.get).toHaveBeenCalledWith(immichUser.id, undefined);
+      expect(userMock.delete).toHaveBeenCalledWith(immichUser);
+    });
   });
 
-  describe('update', () => {
+  describe('create', () => {
     it('should not create a user if there is no local admin account', async () => {
       when(userMock.getAdmin).calledWith().mockResolvedValueOnce(null);
 
@@ -383,6 +427,30 @@ describe(UserService.name, () => {
         }),
       ).rejects.toBeInstanceOf(BadRequestException);
     });
+
+    it('should create user', async () => {
+      userMock.getAdmin.mockResolvedValue(userStub.admin);
+      userMock.create.mockResolvedValue(userStub.user1);
+
+      await expect(
+        sut.create({
+          email: userStub.user1.email,
+          firstName: userStub.user1.firstName,
+          lastName: userStub.user1.lastName,
+          password: 'password',
+          storageLabel: 'label',
+        }),
+      ).resolves.toEqual(mapUser(userStub.user1));
+
+      expect(userMock.getAdmin).toBeCalled();
+      expect(userMock.create).toBeCalledWith({
+        email: userStub.user1.email,
+        firstName: userStub.user1.firstName,
+        lastName: userStub.user1.lastName,
+        storageLabel: 'label',
+        password: expect.anything(),
+      });
+    });
   });
 
   describe('createProfileImage', () => {
@@ -394,6 +462,13 @@ describe(UserService.name, () => {
 
       expect(userMock.update).toHaveBeenCalledWith(adminUserAuth.id, { profileImagePath: file.path });
     });
+
+    it('should throw an error if the user profile could not be updated with the new image', async () => {
+      const file = { path: '/profile/path' } as Express.Multer.File;
+      userMock.update.mockRejectedValue(new InternalServerErrorException('mocked error'));
+
+      await expect(sut.createProfileImage(adminUserAuth, file)).rejects.toThrowError(InternalServerErrorException);
+    });
   });
 
   describe('getUserProfileImage', () => {