refactor: use factory and kysely types for partner repository (#16812)

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

@@ -9,7 +9,6 @@ import { AssetService } from 'src/services/asset.service';
 import { assetStub } from 'test/fixtures/asset.stub';
 import { authStub } from 'test/fixtures/auth.stub';
 import { faceStub } from 'test/fixtures/face.stub';
-import { partnerStub } from 'test/fixtures/partner.stub';
 import { userStub } from 'test/fixtures/user.stub';
 import { factory } from 'test/small.factory';
 import { makeStream, newTestService, ServiceMocks } from 'test/utils';
@@ -88,13 +87,16 @@ describe(AssetService.name, () => {
     });
 
     it('should get memories with partners with inTimeline enabled', async () => {
-      mocks.partner.getAll.mockResolvedValue([partnerStub.user1ToAdmin1]);
+      const partner = factory.partner();
+      const auth = factory.auth({ id: partner.sharedWithId });
+
+      mocks.partner.getAll.mockResolvedValue([partner]);
       mocks.asset.getByDayOfYear.mockResolvedValue([]);
 
-      await sut.getMemoryLane(authStub.admin, { day: 15, month: 1 });
+      await sut.getMemoryLane(auth, { day: 15, month: 1 });
 
       expect(mocks.asset.getByDayOfYear.mock.calls).toEqual([
-        [[authStub.admin.user.id, userStub.user1.id], { day: 15, month: 1 }],
+        [[auth.user.id, partner.sharedById], { day: 15, month: 1 }],
       ]);
     });
   });
@@ -136,17 +138,27 @@ describe(AssetService.name, () => {
     });
 
     it('should not include partner assets if not in timeline', async () => {
+      const partner = factory.partner({ inTimeline: false });
+      const auth = factory.auth({ id: partner.sharedWithId });
+
       mocks.asset.getRandom.mockResolvedValue([assetStub.image]);
-      mocks.partner.getAll.mockResolvedValue([{ ...partnerStub.user1ToAdmin1, inTimeline: false }]);
-      await sut.getRandom(authStub.admin, 1);
-      expect(mocks.asset.getRandom).toHaveBeenCalledWith([authStub.admin.user.id], 1);
+      mocks.partner.getAll.mockResolvedValue([partner]);
+
+      await sut.getRandom(auth, 1);
+
+      expect(mocks.asset.getRandom).toHaveBeenCalledWith([auth.user.id], 1);
     });
 
     it('should include partner assets if in timeline', async () => {
+      const partner = factory.partner({ inTimeline: true });
+      const auth = factory.auth({ id: partner.sharedWithId });
+
       mocks.asset.getRandom.mockResolvedValue([assetStub.image]);
-      mocks.partner.getAll.mockResolvedValue([partnerStub.user1ToAdmin1]);
-      await sut.getRandom(authStub.admin, 1);
-      expect(mocks.asset.getRandom).toHaveBeenCalledWith([userStub.admin.id, userStub.user1.id], 1);
+      mocks.partner.getAll.mockResolvedValue([partner]);
+
+      await sut.getRandom(auth, 1);
+
+      expect(mocks.asset.getRandom).toHaveBeenCalledWith([auth.user.id, partner.sharedById], 1);
     });
   });
 
@@ -154,7 +166,9 @@ describe(AssetService.name, () => {
     it('should allow owner access', async () => {
       mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set([assetStub.image.id]));
       mocks.asset.getById.mockResolvedValue(assetStub.image);
+
       await sut.get(authStub.admin, assetStub.image.id);
+
       expect(mocks.access.asset.checkOwnerAccess).toHaveBeenCalledWith(
         authStub.admin.user.id,
         new Set([assetStub.image.id]),
@@ -164,7 +178,9 @@ describe(AssetService.name, () => {
     it('should allow shared link access', async () => {
       mocks.access.asset.checkSharedLinkAccess.mockResolvedValue(new Set([assetStub.image.id]));
       mocks.asset.getById.mockResolvedValue(assetStub.image);
+
       await sut.get(authStub.adminSharedLink, assetStub.image.id);
+
       expect(mocks.access.asset.checkSharedLinkAccess).toHaveBeenCalledWith(
         authStub.adminSharedLink.sharedLink?.id,
         new Set([assetStub.image.id]),
@@ -191,7 +207,9 @@ describe(AssetService.name, () => {
     it('should allow partner sharing access', async () => {
       mocks.access.asset.checkPartnerAccess.mockResolvedValue(new Set([assetStub.image.id]));
       mocks.asset.getById.mockResolvedValue(assetStub.image);
+
       await sut.get(authStub.admin, assetStub.image.id);
+
       expect(mocks.access.asset.checkPartnerAccess).toHaveBeenCalledWith(
         authStub.admin.user.id,
         new Set([assetStub.image.id]),
@@ -201,7 +219,9 @@ describe(AssetService.name, () => {
     it('should allow shared album access', async () => {
       mocks.access.asset.checkAlbumAccess.mockResolvedValue(new Set([assetStub.image.id]));
       mocks.asset.getById.mockResolvedValue(assetStub.image);
+
       await sut.get(authStub.admin, assetStub.image.id);
+
       expect(mocks.access.asset.checkAlbumAccess).toHaveBeenCalledWith(
         authStub.admin.user.id,
         new Set([assetStub.image.id]),
@@ -210,17 +230,20 @@ describe(AssetService.name, () => {
 
     it('should throw an error for no access', async () => {
       await expect(sut.get(authStub.admin, assetStub.image.id)).rejects.toBeInstanceOf(BadRequestException);
+
       expect(mocks.asset.getById).not.toHaveBeenCalled();
     });
 
     it('should throw an error for an invalid shared link', async () => {
       await expect(sut.get(authStub.adminSharedLink, assetStub.image.id)).rejects.toBeInstanceOf(BadRequestException);
+
       expect(mocks.access.asset.checkOwnerAccess).not.toHaveBeenCalled();
       expect(mocks.asset.getById).not.toHaveBeenCalled();
     });
 
     it('should throw an error if the asset could not be found', async () => {
       mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set([assetStub.image.id]));
+
       await expect(sut.get(authStub.admin, assetStub.image.id)).rejects.toBeInstanceOf(BadRequestException);
     });
   });
@@ -230,6 +253,7 @@ describe(AssetService.name, () => {
       await expect(sut.update(authStub.admin, 'asset-1', { isArchived: false })).rejects.toBeInstanceOf(
         BadRequestException,
       );
+
       expect(mocks.asset.update).not.toHaveBeenCalled();
     });
 
@@ -259,6 +283,7 @@ describe(AssetService.name, () => {
       mocks.asset.update.mockResolvedValueOnce(assetStub.image);
 
       await sut.update(authStub.admin, 'asset-1', { rating: 3 });
+
       expect(mocks.asset.upsertExif).toHaveBeenCalledWith({ assetId: 'asset-1', rating: 3 });
     });
 
@@ -401,12 +426,15 @@ describe(AssetService.name, () => {
 
     it('should update all assets', async () => {
       mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set(['asset-1', 'asset-2']));
+
       await sut.updateAll(authStub.admin, { ids: ['asset-1', 'asset-2'], isArchived: true });
+
       expect(mocks.asset.updateAll).toHaveBeenCalledWith(['asset-1', 'asset-2'], { isArchived: true });
     });
 
     it('should not update Assets table if no relevant fields are provided', async () => {
       mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set(['asset-1']));
+
       await sut.updateAll(authStub.admin, {
         ids: ['asset-1'],
         latitude: 0,
@@ -421,6 +449,7 @@ describe(AssetService.name, () => {
 
     it('should update Assets table if isArchived field is provided', async () => {
       mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set(['asset-1']));
+
       await sut.updateAll(authStub.admin, {
         ids: ['asset-1'],
         latitude: 0,
@@ -624,25 +653,33 @@ describe(AssetService.name, () => {
   describe('run', () => {
     it('should run the refresh faces job', async () => {
       mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set(['asset-1']));
+
       await sut.run(authStub.admin, { assetIds: ['asset-1'], name: AssetJobName.REFRESH_FACES });
+
       expect(mocks.job.queueAll).toHaveBeenCalledWith([{ name: JobName.FACE_DETECTION, data: { id: 'asset-1' } }]);
     });
 
     it('should run the refresh metadata job', async () => {
       mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set(['asset-1']));
+
       await sut.run(authStub.admin, { assetIds: ['asset-1'], name: AssetJobName.REFRESH_METADATA });
+
       expect(mocks.job.queueAll).toHaveBeenCalledWith([{ name: JobName.METADATA_EXTRACTION, data: { id: 'asset-1' } }]);
     });
 
     it('should run the refresh thumbnails job', async () => {
       mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set(['asset-1']));
+
       await sut.run(authStub.admin, { assetIds: ['asset-1'], name: AssetJobName.REGENERATE_THUMBNAIL });
+
       expect(mocks.job.queueAll).toHaveBeenCalledWith([{ name: JobName.GENERATE_THUMBNAILS, data: { id: 'asset-1' } }]);
     });
 
     it('should run the transcode video', async () => {
       mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set(['asset-1']));
+
       await sut.run(authStub.admin, { assetIds: ['asset-1'], name: AssetJobName.TRANSCODE_VIDEO });
+
       expect(mocks.job.queueAll).toHaveBeenCalledWith([{ name: JobName.VIDEO_CONVERSION, data: { id: 'asset-1' } }]);
     });
   });

server/src/services/map.service.spec.ts

@@ -2,7 +2,7 @@ import { MapService } from 'src/services/map.service';
 import { albumStub } from 'test/fixtures/album.stub';
 import { assetStub } from 'test/fixtures/asset.stub';
 import { authStub } from 'test/fixtures/auth.stub';
-import { partnerStub } from 'test/fixtures/partner.stub';
+import { factory } from 'test/small.factory';
 import { newTestService, ServiceMocks } from 'test/utils';
 
 describe(MapService.name, () => {
@@ -34,6 +34,9 @@ describe(MapService.name, () => {
     });
 
     it('should include partner assets', async () => {
+      const partner = factory.partner();
+      const auth = factory.auth({ id: partner.sharedWithId });
+
       const asset = assetStub.withLocation;
       const marker = {
         id: asset.id,
@@ -43,13 +46,13 @@ describe(MapService.name, () => {
         state: asset.exifInfo!.state,
         country: asset.exifInfo!.country,
       };
-      mocks.partner.getAll.mockResolvedValue([partnerStub.adminToUser1]);
+      mocks.partner.getAll.mockResolvedValue([partner]);
       mocks.map.getMapMarkers.mockResolvedValue([marker]);
 
-      const markers = await sut.getMapMarkers(authStub.user1, { withPartners: true });
+      const markers = await sut.getMapMarkers(auth, { withPartners: true });
 
       expect(mocks.map.getMapMarkers).toHaveBeenCalledWith(
-        [authStub.user1.user.id, partnerStub.adminToUser1.sharedById],
+        [auth.user.id, partner.sharedById],
         expect.arrayContaining([]),
         { withPartners: true },
       );

server/src/services/partner.service.spec.ts

@@ -1,8 +1,7 @@
 import { BadRequestException } from '@nestjs/common';
 import { PartnerDirection } from 'src/repositories/partner.repository';
 import { PartnerService } from 'src/services/partner.service';
-import { authStub } from 'test/fixtures/auth.stub';
-import { partnerStub } from 'test/fixtures/partner.stub';
+import { factory } from 'test/small.factory';
 import { newTestService, ServiceMocks } from 'test/utils';
 
 describe(PartnerService.name, () => {
@@ -19,35 +18,58 @@ describe(PartnerService.name, () => {
 
   describe('search', () => {
     it("should return a list of partners with whom I've shared my library", async () => {
-      mocks.partner.getAll.mockResolvedValue([partnerStub.adminToUser1, partnerStub.user1ToAdmin1]);
-      await expect(sut.search(authStub.user1, { direction: PartnerDirection.SharedBy })).resolves.toBeDefined();
-      expect(mocks.partner.getAll).toHaveBeenCalledWith(authStub.user1.user.id);
+      const user1 = factory.user();
+      const user2 = factory.user();
+      const sharedWithUser2 = factory.partner({ sharedBy: user1, sharedWith: user2 });
+      const sharedWithUser1 = factory.partner({ sharedBy: user2, sharedWith: user1 });
+      const auth = factory.auth({ id: user1.id });
+
+      mocks.partner.getAll.mockResolvedValue([sharedWithUser1, sharedWithUser2]);
+
+      await expect(sut.search(auth, { direction: PartnerDirection.SharedBy })).resolves.toBeDefined();
+      expect(mocks.partner.getAll).toHaveBeenCalledWith(user1.id);
     });
 
     it('should return a list of partners who have shared their libraries with me', async () => {
-      mocks.partner.getAll.mockResolvedValue([partnerStub.adminToUser1, partnerStub.user1ToAdmin1]);
-      await expect(sut.search(authStub.user1, { direction: PartnerDirection.SharedWith })).resolves.toBeDefined();
-      expect(mocks.partner.getAll).toHaveBeenCalledWith(authStub.user1.user.id);
+      const user1 = factory.user();
+      const user2 = factory.user();
+      const sharedWithUser2 = factory.partner({ sharedBy: user1, sharedWith: user2 });
+      const sharedWithUser1 = factory.partner({ sharedBy: user2, sharedWith: user1 });
+      const auth = factory.auth({ id: user1.id });
+
+      mocks.partner.getAll.mockResolvedValue([sharedWithUser1, sharedWithUser2]);
+      await expect(sut.search(auth, { direction: PartnerDirection.SharedWith })).resolves.toBeDefined();
+      expect(mocks.partner.getAll).toHaveBeenCalledWith(user1.id);
     });
   });
 
   describe('create', () => {
     it('should create a new partner', async () => {
-      mocks.partner.get.mockResolvedValue(void 0);
-      mocks.partner.create.mockResolvedValue(partnerStub.adminToUser1);
+      const user1 = factory.user();
+      const user2 = factory.user();
+      const partner = factory.partner({ sharedBy: user1, sharedWith: user2 });
+      const auth = factory.auth({ id: user1.id });
 
-      await expect(sut.create(authStub.admin, authStub.user1.user.id)).resolves.toBeDefined();
+      mocks.partner.get.mockResolvedValue(void 0);
+      mocks.partner.create.mockResolvedValue(partner);
+
+      await expect(sut.create(auth, user2.id)).resolves.toBeDefined();
 
       expect(mocks.partner.create).toHaveBeenCalledWith({
-        sharedById: authStub.admin.user.id,
-        sharedWithId: authStub.user1.user.id,
+        sharedById: partner.sharedById,
+        sharedWithId: partner.sharedWithId,
       });
     });
 
     it('should throw an error when the partner already exists', async () => {
-      mocks.partner.get.mockResolvedValue(partnerStub.adminToUser1);
+      const user1 = factory.user();
+      const user2 = factory.user();
+      const partner = factory.partner({ sharedBy: user1, sharedWith: user2 });
+      const auth = factory.auth({ id: user1.id });
 
-      await expect(sut.create(authStub.admin, authStub.user1.user.id)).rejects.toBeInstanceOf(BadRequestException);
+      mocks.partner.get.mockResolvedValue(partner);
+
+      await expect(sut.create(auth, user2.id)).rejects.toBeInstanceOf(BadRequestException);
 
       expect(mocks.partner.create).not.toHaveBeenCalled();
     });
@@ -55,17 +77,25 @@ describe(PartnerService.name, () => {
 
   describe('remove', () => {
     it('should remove a partner', async () => {
-      mocks.partner.get.mockResolvedValue(partnerStub.adminToUser1);
+      const user1 = factory.user();
+      const user2 = factory.user();
+      const partner = factory.partner({ sharedBy: user1, sharedWith: user2 });
+      const auth = factory.auth({ id: user1.id });
 
-      await sut.remove(authStub.admin, authStub.user1.user.id);
+      mocks.partner.get.mockResolvedValue(partner);
 
-      expect(mocks.partner.remove).toHaveBeenCalledWith(partnerStub.adminToUser1);
+      await sut.remove(auth, user2.id);
+
+      expect(mocks.partner.remove).toHaveBeenCalledWith({ sharedById: user1.id, sharedWithId: user2.id });
     });
 
     it('should throw an error when the partner does not exist', async () => {
+      const user2 = factory.user();
+      const auth = factory.auth();
+
       mocks.partner.get.mockResolvedValue(void 0);
 
-      await expect(sut.remove(authStub.admin, authStub.user1.user.id)).rejects.toBeInstanceOf(BadRequestException);
+      await expect(sut.remove(auth, user2.id)).rejects.toBeInstanceOf(BadRequestException);
 
       expect(mocks.partner.remove).not.toHaveBeenCalled();
     });
@@ -73,18 +103,24 @@ describe(PartnerService.name, () => {
 
   describe('update', () => {
     it('should require access', async () => {
-      await expect(sut.update(authStub.admin, 'shared-by-id', { inTimeline: false })).rejects.toBeInstanceOf(
-        BadRequestException,
-      );
+      const user2 = factory.user();
+      const auth = factory.auth();
+
+      await expect(sut.update(auth, user2.id, { inTimeline: false })).rejects.toBeInstanceOf(BadRequestException);
     });
 
     it('should update partner', async () => {
-      mocks.access.partner.checkUpdateAccess.mockResolvedValue(new Set(['shared-by-id']));
-      mocks.partner.update.mockResolvedValue(partnerStub.adminToUser1);
+      const user1 = factory.user();
+      const user2 = factory.user();
+      const partner = factory.partner({ sharedBy: user1, sharedWith: user2 });
+      const auth = factory.auth({ id: user1.id });
 
-      await expect(sut.update(authStub.admin, 'shared-by-id', { inTimeline: true })).resolves.toBeDefined();
+      mocks.access.partner.checkUpdateAccess.mockResolvedValue(new Set([user2.id]));
+      mocks.partner.update.mockResolvedValue(partner);
+
+      await expect(sut.update(auth, user2.id, { inTimeline: true })).resolves.toBeDefined();
       expect(mocks.partner.update).toHaveBeenCalledWith(
-        { sharedById: 'shared-by-id', sharedWithId: authStub.admin.user.id },
+        { sharedById: user2.id, sharedWithId: user1.id },
         { inTimeline: true },
       );
     });

server/src/services/partner.service.ts

@@ -1,8 +1,8 @@
 import { BadRequestException, Injectable } from '@nestjs/common';
+import { Partner } from 'src/database';
 import { AuthDto } from 'src/dtos/auth.dto';
 import { PartnerResponseDto, PartnerSearchDto, UpdatePartnerDto } from 'src/dtos/partner.dto';
-import { mapUser } from 'src/dtos/user.dto';
-import { PartnerEntity } from 'src/entities/partner.entity';
+import { mapDatabaseUser } from 'src/dtos/user.dto';
 import { Permission } from 'src/enum';
 import { PartnerDirection, PartnerIds } from 'src/repositories/partner.repository';
 import { BaseService } from 'src/services/base.service';
@@ -27,14 +27,14 @@ export class PartnerService extends BaseService {
       throw new BadRequestException('Partner not found');
     }
 
-    await this.partnerRepository.remove(partner);
+    await this.partnerRepository.remove(partnerId);
   }
 
   async search(auth: AuthDto, { direction }: PartnerSearchDto): Promise<PartnerResponseDto[]> {
     const partners = await this.partnerRepository.getAll(auth.user.id);
     const key = direction === PartnerDirection.SharedBy ? 'sharedById' : 'sharedWithId';
     return partners
-      .filter((partner) => partner.sharedBy && partner.sharedWith) // Filter out soft deleted users
+      .filter((partner): partner is Partner => !!(partner.sharedBy && partner.sharedWith)) // Filter out soft deleted users
       .filter((partner) => partner[key] === auth.user.id)
       .map((partner) => this.mapPartner(partner, direction));
   }
@@ -47,14 +47,12 @@ export class PartnerService extends BaseService {
     return this.mapPartner(entity, PartnerDirection.SharedWith);
   }
 
-  private mapPartner(partner: PartnerEntity, direction: PartnerDirection): PartnerResponseDto {
+  private mapPartner(partner: Partner, direction: PartnerDirection): PartnerResponseDto {
     // this is opposite to return the non-me user of the "partner"
-    const user = mapUser(
+    const user = mapDatabaseUser(
       direction === PartnerDirection.SharedBy ? partner.sharedWith : partner.sharedBy,
     ) as PartnerResponseDto;
 
-    user.inTimeline = partner.inTimeline;
-
-    return user;
+    return { ...user, inTimeline: partner.inTimeline };
   }
 }

server/src/services/sync.service.spec.ts

@@ -3,7 +3,7 @@ import { AssetEntity } from 'src/entities/asset.entity';
 import { SyncService } from 'src/services/sync.service';
 import { assetStub } from 'test/fixtures/asset.stub';
 import { authStub } from 'test/fixtures/auth.stub';
-import { partnerStub } from 'test/fixtures/partner.stub';
+import { factory } from 'test/small.factory';
 import { newTestService, ServiceMocks } from 'test/utils';
 
 const untilDate = new Date(2024);
@@ -38,10 +38,15 @@ describe(SyncService.name, () => {
 
   describe('getChangesForDeltaSync', () => {
     it('should return a response requiring a full sync when partners are out of sync', async () => {
-      mocks.partner.getAll.mockResolvedValue([partnerStub.adminToUser1]);
+      const partner = factory.partner();
+      const auth = factory.auth({ id: partner.sharedWithId });
+
+      mocks.partner.getAll.mockResolvedValue([partner]);
+
       await expect(
-        sut.getDeltaSync(authStub.user1, { updatedAfter: new Date(), userIds: [authStub.user1.user.id] }),
+        sut.getDeltaSync(authStub.user1, { updatedAfter: new Date(), userIds: [auth.user.id] }),
       ).resolves.toEqual({ needsFullSync: true, upserted: [], deleted: [] });
+
       expect(mocks.asset.getChangedDeltaSync).toHaveBeenCalledTimes(0);
       expect(mocks.audit.getAfter).toHaveBeenCalledTimes(0);
     });