feat(server)!: add isOwned filter to albums API (#28213)

* feat(server)!: add owned filter to albums API

BREAKING CHANGE: GET /albums with no parameters now returns all accessible albums (owned + shared-with-me) instead of only owned albums.

* document tri-state matrix

* web impl

* collapse to single method and handover branching to sql

* dedupe

* verify that owned, shared, and notShared counts are mapped independently from their respective queries

* refactor(server): add select:['id'] overload to albumRepository.getAll

Avoid fetching full album rows (with albumUsers/sharedLinks subqueries) in map.service where only album IDs are needed.

* focus relevant test filters

* fmt

* Revert "verify that owned, shared, and notShared counts are mapped independently from their respective queries"

This reverts commit 47aab458192c766de4662aada5a6841b091d2a80.

* sync sql

* Revert "document tri-state matrix"

This reverts commit a5b2355d0c.

* address review comments

* inline shared condition and return as ternary

* sync sql

* use [...albums].sort

Array.toSorted() is not supported in Chrome 109

* use isShared and isOwned nomenclature

* fix e2e tests

* add params to sql query

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

@@ -26,18 +26,16 @@ describe(AlbumService.name, () => {
 
   describe('getStatistics', () => {
     it('should get the album count', async () => {
-      mocks.album.getOwned.mockResolvedValue([]);
-      mocks.album.getShared.mockResolvedValue([]);
-      mocks.album.getNotShared.mockResolvedValue([]);
+      mocks.album.getAll.mockResolvedValue([]);
       await expect(sut.getStatistics(authStub.admin)).resolves.toEqual({
         owned: 0,
         shared: 0,
         notShared: 0,
       });
 
-      expect(mocks.album.getOwned).toHaveBeenCalledWith(authStub.admin.user.id);
-      expect(mocks.album.getShared).toHaveBeenCalledWith(authStub.admin.user.id);
-      expect(mocks.album.getNotShared).toHaveBeenCalledWith(authStub.admin.user.id);
+      expect(mocks.album.getAll).toHaveBeenCalledWith(authStub.admin.user.id, { isOwned: true });
+      expect(mocks.album.getAll).toHaveBeenCalledWith(authStub.admin.user.id, { isShared: true });
+      expect(mocks.album.getAll).toHaveBeenCalledWith(authStub.admin.user.id, { isOwned: true, isShared: false });
     });
   });
 
@@ -46,7 +44,7 @@ describe(AlbumService.name, () => {
       const album = AlbumFactory.from().albumUser().build();
       const { user: owner } = album.albumUsers.find(({ role }) => role === AlbumUserRole.Owner)!;
       const sharedWithUserAlbum = AlbumFactory.from().owner(owner).albumUser().build();
-      mocks.album.getOwned.mockResolvedValue([getForAlbum(album), getForAlbum(sharedWithUserAlbum)]);
+      mocks.album.getAll.mockResolvedValue([getForAlbum(album), getForAlbum(sharedWithUserAlbum)]);
       mocks.album.getMetadataForIds.mockResolvedValue([
         {
           albumId: album.id,
@@ -68,6 +66,7 @@ describe(AlbumService.name, () => {
       expect(result).toHaveLength(2);
       expect(result[0].id).toEqual(album.id);
       expect(result[1].id).toEqual(sharedWithUserAlbum.id);
+      expect(mocks.album.getAll).toHaveBeenCalledWith(owner.id, { isOwned: undefined, isShared: undefined });
     });
 
     it('gets list of albums that have a specific asset', async () => {
@@ -98,7 +97,7 @@ describe(AlbumService.name, () => {
     it('gets list of albums that are shared', async () => {
       const album = AlbumFactory.from().albumUser().build();
       const { user: owner } = album.albumUsers.find(({ role }) => role === AlbumUserRole.Owner)!;
-      mocks.album.getShared.mockResolvedValue([getForAlbum(album)]);
+      mocks.album.getAll.mockResolvedValue([getForAlbum(album)]);
       mocks.album.getMetadataForIds.mockResolvedValue([
         {
           albumId: album.id,
@@ -109,16 +108,16 @@ describe(AlbumService.name, () => {
         },
       ]);
 
-      const result = await sut.getAll(AuthFactory.create(owner), { shared: true });
+      const result = await sut.getAll(AuthFactory.create(owner), { isShared: true });
       expect(result).toHaveLength(1);
       expect(result[0].id).toEqual(album.id);
-      expect(mocks.album.getShared).toHaveBeenCalledTimes(1);
+      expect(mocks.album.getAll).toHaveBeenCalledWith(owner.id, expect.objectContaining({ isShared: true }));
     });
 
     it('gets list of albums that are NOT shared', async () => {
       const album = AlbumFactory.create();
       const { user: owner } = album.albumUsers.find(({ role }) => role === AlbumUserRole.Owner)!;
-      mocks.album.getNotShared.mockResolvedValue([getForAlbum(album)]);
+      mocks.album.getAll.mockResolvedValue([getForAlbum(album)]);
       mocks.album.getMetadataForIds.mockResolvedValue([
         {
           albumId: album.id,
@@ -129,17 +128,66 @@ describe(AlbumService.name, () => {
         },
       ]);
 
-      const result = await sut.getAll(AuthFactory.create(owner), { shared: false });
+      const result = await sut.getAll(AuthFactory.create(owner), { isShared: false });
       expect(result).toHaveLength(1);
       expect(result[0].id).toEqual(album.id);
-      expect(mocks.album.getNotShared).toHaveBeenCalledTimes(1);
+      expect(mocks.album.getAll).toHaveBeenCalledWith(owner.id, expect.objectContaining({ isShared: false }));
+    });
+
+    it('gets only owned albums when isOwned=true', async () => {
+      const album = AlbumFactory.create();
+      const { user: owner } = album.albumUsers.find(({ role }) => role === AlbumUserRole.Owner)!;
+      mocks.album.getAll.mockResolvedValue([getForAlbum(album)]);
+      mocks.album.getMetadataForIds.mockResolvedValue([
+        { albumId: album.id, assetCount: 0, startDate: null, endDate: null, lastModifiedAssetTimestamp: null },
+      ]);
+
+      const result = await sut.getAll(AuthFactory.create(owner), { isOwned: true });
+      expect(result).toHaveLength(1);
+      expect(mocks.album.getAll).toHaveBeenCalledWith(owner.id, expect.objectContaining({ isOwned: true }));
+    });
+
+    it('gets only shared-with-me albums when isOwned=false', async () => {
+      const album = AlbumFactory.create();
+      const { user: owner } = album.albumUsers.find(({ role }) => role === AlbumUserRole.Owner)!;
+      mocks.album.getAll.mockResolvedValue([getForAlbum(album)]);
+      mocks.album.getMetadataForIds.mockResolvedValue([
+        { albumId: album.id, assetCount: 0, startDate: null, endDate: null, lastModifiedAssetTimestamp: null },
+      ]);
+
+      const result = await sut.getAll(AuthFactory.create(owner), { isOwned: false });
+      expect(result).toHaveLength(1);
+      expect(mocks.album.getAll).toHaveBeenCalledWith(owner.id, expect.objectContaining({ isOwned: false }));
+    });
+
+    it('gets owned shared-out albums when isOwned=true and isShared=true', async () => {
+      const album = AlbumFactory.create();
+      const { user: owner } = album.albumUsers.find(({ role }) => role === AlbumUserRole.Owner)!;
+      mocks.album.getAll.mockResolvedValue([getForAlbum(album)]);
+      mocks.album.getMetadataForIds.mockResolvedValue([
+        { albumId: album.id, assetCount: 0, startDate: null, endDate: null, lastModifiedAssetTimestamp: null },
+      ]);
+
+      const result = await sut.getAll(AuthFactory.create(owner), { isOwned: true, isShared: true });
+      expect(result).toHaveLength(1);
+      expect(mocks.album.getAll).toHaveBeenCalledWith(owner.id, { isOwned: true, isShared: true });
+    });
+
+    it('returns empty list when isOwned=false and isShared=false', async () => {
+      const album = AlbumFactory.create();
+      const { user: owner } = album.albumUsers.find(({ role }) => role === AlbumUserRole.Owner)!;
+      mocks.album.getAll.mockResolvedValue([]);
+
+      const result = await sut.getAll(AuthFactory.create(owner), { isOwned: false, isShared: false });
+      expect(result).toHaveLength(0);
+      expect(mocks.album.getAll).toHaveBeenCalledWith(owner.id, { isOwned: false, isShared: false });
     });
   });
 
   it('counts assets correctly', async () => {
     const album = AlbumFactory.create();
     const { user: owner } = album.albumUsers.find(({ role }) => role === AlbumUserRole.Owner)!;
-    mocks.album.getOwned.mockResolvedValue([getForAlbum(album)]);
+    mocks.album.getAll.mockResolvedValue([getForAlbum(album)]);
     mocks.album.getMetadataForIds.mockResolvedValue([
       {
         albumId: album.id,
@@ -153,7 +201,7 @@ describe(AlbumService.name, () => {
     const result = await sut.getAll(AuthFactory.create(owner), {});
     expect(result).toHaveLength(1);
     expect(result[0].assetCount).toEqual(1);
-    expect(mocks.album.getOwned).toHaveBeenCalledTimes(1);
+    expect(mocks.album.getAll).toHaveBeenCalledTimes(1);
   });
 
   describe('create', () => {

server/src/services/album.service.ts

@@ -8,7 +8,6 @@ import {
   CreateAlbumDto,
   GetAlbumsDto,
   mapAlbum,
-  MapAlbumDto,
   UpdateAlbumDto,
   UpdateAlbumUserDto,
 } from 'src/dtos/album.dto';
@@ -26,9 +25,9 @@ import { getPreferences } from 'src/utils/preferences';
 export class AlbumService extends BaseService {
   async getStatistics(auth: AuthDto): Promise<AlbumStatisticsResponseDto> {
     const [owned, shared, notShared] = await Promise.all([
-      this.albumRepository.getOwned(auth.user.id),
-      this.albumRepository.getShared(auth.user.id),
-      this.albumRepository.getNotShared(auth.user.id),
+      this.albumRepository.getAll(auth.user.id, { isOwned: true }),
+      this.albumRepository.getAll(auth.user.id, { isShared: true }),
+      this.albumRepository.getAll(auth.user.id, { isOwned: true, isShared: false }),
     ]);
 
     return {
@@ -38,18 +37,18 @@ export class AlbumService extends BaseService {
     };
   }
 
-  async getAll({ user: { id: ownerId } }: AuthDto, { assetId, shared }: GetAlbumsDto): Promise<AlbumResponseDto[]> {
+  async getAll(
+    { user: { id: ownerId } }: AuthDto,
+    { assetId, isOwned, isShared }: GetAlbumsDto,
+  ): Promise<AlbumResponseDto[]> {
     await this.albumRepository.updateThumbnails();
 
-    let albums: MapAlbumDto[];
-    if (assetId) {
-      albums = await this.albumRepository.getByAssetId(ownerId, assetId);
-    } else if (shared === true) {
-      albums = await this.albumRepository.getShared(ownerId);
-    } else if (shared === false) {
-      albums = await this.albumRepository.getNotShared(ownerId);
-    } else {
-      albums = await this.albumRepository.getOwned(ownerId);
+    const albums = assetId
+      ? await this.albumRepository.getByAssetId(ownerId, assetId)
+      : await this.albumRepository.getAll(ownerId, { isOwned, isShared });
+
+    if (albums.length === 0) {
+      return [];
     }
 
     // Get asset count for each album. Then map the result to an object:

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

@@ -4,7 +4,7 @@ import { AssetFactory } from 'test/factories/asset.factory';
 import { AuthFactory } from 'test/factories/auth.factory';
 import { PartnerFactory } from 'test/factories/partner.factory';
 import { userStub } from 'test/fixtures/user.stub';
-import { getForAlbum, getForPartner } from 'test/mappers';
+import { getForPartner } from 'test/mappers';
 import { newTestService, ServiceMocks } from 'test/utils';
 
 describe(MapService.name, () => {
@@ -82,15 +82,15 @@ describe(MapService.name, () => {
       };
       mocks.partner.getAll.mockResolvedValue([]);
       mocks.map.getMapMarkers.mockResolvedValue([marker]);
-      mocks.album.getOwned.mockResolvedValue([getForAlbum(AlbumFactory.create())]);
-      mocks.album.getShared.mockResolvedValue([
-        getForAlbum(AlbumFactory.from().albumUser({ userId: userStub.user1.id }).build()),
-      ]);
+      const album1 = AlbumFactory.create();
+      const album2 = AlbumFactory.from().albumUser({ userId: userStub.user1.id }).build();
+      mocks.album.getAllIds.mockResolvedValue([album1.id, album2.id]);
 
       const markers = await sut.getMapMarkers(auth, { withSharedAlbums: true });
 
       expect(markers).toHaveLength(1);
       expect(markers[0]).toEqual(marker);
+      expect(mocks.album.getAllIds).toHaveBeenCalledWith(auth.user.id);
     });
   });
 

server/src/services/map.service.ts

@@ -13,15 +13,7 @@ export class MapService extends BaseService {
       userIds.push(...partnerIds);
     }
 
-    // TODO convert to SQL join
-    const albumIds: string[] = [];
-    if (options.withSharedAlbums) {
-      const [ownedAlbums, sharedAlbums] = await Promise.all([
-        this.albumRepository.getOwned(auth.user.id),
-        this.albumRepository.getShared(auth.user.id),
-      ]);
-      albumIds.push(...ownedAlbums.map((album) => album.id), ...sharedAlbums.map((album) => album.id));
-    }
+    const albumIds = options.withSharedAlbums ? await this.albumRepository.getAllIds(auth.user.id) : [];
 
     return this.mapRepository.getMapMarkers(userIds, albumIds, options);
   }