From 7d8be123308c3317c2101f491ced4f5980e876fa Mon Sep 17 00:00:00 2001 From: timonrieger Date: Sun, 3 May 2026 16:26:26 +0200 Subject: [PATCH] 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. --- e2e/src/specs/server/api/album.e2e-spec.ts | 66 +++++++++++++++++- mobile/openapi/lib/api/albums_api.dart | 23 +++++-- open-api/immich-openapi-specs.json | 13 +++- open-api/typescript-sdk/src/fetch-client.ts | 4 +- server/src/dtos/album.dto.ts | 7 +- server/src/repositories/album.repository.ts | 75 +++++++++++++++++++++ server/src/services/album.service.spec.ts | 54 ++++++++++++++- server/src/services/album.service.ts | 23 ++++++- 8 files changed, 246 insertions(+), 19 deletions(-) diff --git a/e2e/src/specs/server/api/album.e2e-spec.ts b/e2e/src/specs/server/api/album.e2e-spec.ts index e1e5178476..27e125cf84 100644 --- a/e2e/src/specs/server/api/album.e2e-spec.ts +++ b/e2e/src/specs/server/api/album.e2e-spec.ts @@ -188,7 +188,7 @@ describe('/albums', () => { it('should return the album collection including owned and shared', async () => { const { status, body } = await request(app).get('/albums').set('Authorization', `Bearer ${user1.accessToken}`); expect(status).toBe(200); - expect(body).toHaveLength(4); + expect(body).toHaveLength(5); expect(body).toEqual( expect.arrayContaining([ expect.objectContaining({ @@ -219,6 +219,13 @@ describe('/albums', () => { ]), shared: false, }), + expect.objectContaining({ + albumName: user2SharedUser, + albumUsers: expect.arrayContaining([ + { role: AlbumUserRole.Owner, user: expect.objectContaining({ id: user2.userId }) }, + ]), + shared: true, + }), ]), ); }); @@ -282,6 +289,63 @@ describe('/albums', () => { ); }); + it('should return only owned albums when filtered by owned=true', async () => { + const { status, body } = await request(app) + .get('/albums?owned=true') + .set('Authorization', `Bearer ${user1.accessToken}`); + expect(status).toBe(200); + expect(body).toHaveLength(4); + expect(body).toEqual( + expect.arrayContaining([ + expect.objectContaining({ albumName: user1SharedEditorUser }), + expect.objectContaining({ albumName: user1SharedViewerUser }), + expect.objectContaining({ albumName: user1SharedLink }), + expect.objectContaining({ albumName: user1NotShared }), + ]), + ); + }); + + it('should return only shared-with-me albums when filtered by owned=false', async () => { + const { status, body } = await request(app) + .get('/albums?owned=false') + .set('Authorization', `Bearer ${user1.accessToken}`); + expect(status).toBe(200); + expect(body).toHaveLength(1); + expect(body).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + albumName: user2SharedUser, + albumUsers: expect.arrayContaining([ + { role: AlbumUserRole.Owner, user: expect.objectContaining({ id: user2.userId }) }, + ]), + }), + ]), + ); + }); + + it('should return owned shared-out albums when filtered by owned=true&shared=true', async () => { + const { status, body } = await request(app) + .get('/albums?owned=true&shared=true') + .set('Authorization', `Bearer ${user1.accessToken}`); + expect(status).toBe(200); + expect(body).toHaveLength(3); + expect(body).toEqual( + expect.arrayContaining([ + expect.objectContaining({ albumName: user1SharedEditorUser }), + expect.objectContaining({ albumName: user1SharedViewerUser }), + expect.objectContaining({ albumName: user1SharedLink }), + ]), + ); + }); + + it('should return empty list when filtered by owned=false&shared=false', async () => { + const { status, body } = await request(app) + .get('/albums?owned=false&shared=false') + .set('Authorization', `Bearer ${user1.accessToken}`); + expect(status).toBe(200); + expect(body).toHaveLength(0); + }); + it('should return the album collection filtered by assetId', async () => { const { status, body } = await request(app) .get(`/albums?assetId=${user1Asset2.id}`) diff --git a/mobile/openapi/lib/api/albums_api.dart b/mobile/openapi/lib/api/albums_api.dart index d08d1cba9d..6d41b65518 100644 --- a/mobile/openapi/lib/api/albums_api.dart +++ b/mobile/openapi/lib/api/albums_api.dart @@ -506,11 +506,14 @@ class AlbumsApi { /// Parameters: /// /// * [String] assetId: - /// Filter albums containing this asset ID (ignores shared parameter) + /// Filter albums containing this asset ID (ignores other parameters) + /// + /// * [bool] owned: + /// Filter by ownership: true = only owned, false = only shared-with-me, undefined = no filter /// /// * [bool] shared: - /// Filter by shared status: true = only shared, false = not shared, undefined = all owned albums - Future getAllAlbumsWithHttpInfo({ String? assetId, bool? shared, }) async { + /// Filter by shared status: true = only shared, false = not shared, undefined = no filter + Future getAllAlbumsWithHttpInfo({ String? assetId, bool? owned, bool? shared, }) async { // ignore: prefer_const_declarations final apiPath = r'/albums'; @@ -524,6 +527,9 @@ class AlbumsApi { if (assetId != null) { queryParams.addAll(_queryParams('', 'assetId', assetId)); } + if (owned != null) { + queryParams.addAll(_queryParams('', 'owned', owned)); + } if (shared != null) { queryParams.addAll(_queryParams('', 'shared', shared)); } @@ -549,12 +555,15 @@ class AlbumsApi { /// Parameters: /// /// * [String] assetId: - /// Filter albums containing this asset ID (ignores shared parameter) + /// Filter albums containing this asset ID (ignores other parameters) + /// + /// * [bool] owned: + /// Filter by ownership: true = only owned, false = only shared-with-me, undefined = no filter /// /// * [bool] shared: - /// Filter by shared status: true = only shared, false = not shared, undefined = all owned albums - Future?> getAllAlbums({ String? assetId, bool? shared, }) async { - final response = await getAllAlbumsWithHttpInfo( assetId: assetId, shared: shared, ); + /// Filter by shared status: true = only shared, false = not shared, undefined = no filter + Future?> getAllAlbums({ String? assetId, bool? owned, bool? shared, }) async { + final response = await getAllAlbumsWithHttpInfo( assetId: assetId, owned: owned, shared: shared, ); if (response.statusCode >= HttpStatus.badRequest) { throw ApiException(response.statusCode, await _decodeBodyBytes(response)); } diff --git a/open-api/immich-openapi-specs.json b/open-api/immich-openapi-specs.json index 8dfa1cb2d1..3bd9400eb9 100644 --- a/open-api/immich-openapi-specs.json +++ b/open-api/immich-openapi-specs.json @@ -1641,18 +1641,27 @@ "name": "assetId", "required": false, "in": "query", - "description": "Filter albums containing this asset ID (ignores shared parameter)", + "description": "Filter albums containing this asset ID (ignores other parameters)", "schema": { "format": "uuid", "pattern": "^([0-9a-fA-F]{8}-[0-9a-fA-F]{4}-4[0-9a-fA-F]{3}-[89abAB][0-9a-fA-F]{3}-[0-9a-fA-F]{12})$", "type": "string" } }, + { + "name": "owned", + "required": false, + "in": "query", + "description": "Filter by ownership: true = only owned, false = only shared-with-me, undefined = no filter", + "schema": { + "type": "boolean" + } + }, { "name": "shared", "required": false, "in": "query", - "description": "Filter by shared status: true = only shared, false = not shared, undefined = all owned albums", + "description": "Filter by shared status: true = only shared, false = not shared, undefined = no filter", "schema": { "type": "boolean" } diff --git a/open-api/typescript-sdk/src/fetch-client.ts b/open-api/typescript-sdk/src/fetch-client.ts index bc304e483e..6462552c6e 100644 --- a/open-api/typescript-sdk/src/fetch-client.ts +++ b/open-api/typescript-sdk/src/fetch-client.ts @@ -3657,8 +3657,9 @@ export function getUserStatisticsAdmin({ id, isFavorite, isTrashed, visibility } /** * List all albums */ -export function getAllAlbums({ assetId, shared }: { +export function getAllAlbums({ assetId, owned, shared }: { assetId?: string; + owned?: boolean; shared?: boolean; }, opts?: Oazapfts.RequestOpts) { return oazapfts.ok(oazapfts.fetchJson<{ @@ -3666,6 +3667,7 @@ export function getAllAlbums({ assetId, shared }: { data: AlbumResponseDto[]; }>(`/albums${QS.query(QS.explode({ assetId, + owned, shared }))}`, { ...opts diff --git a/server/src/dtos/album.dto.ts b/server/src/dtos/album.dto.ts index 33870cd6fc..4d60ca657f 100644 --- a/server/src/dtos/album.dto.ts +++ b/server/src/dtos/album.dto.ts @@ -65,10 +65,13 @@ const UpdateAlbumSchema = z const GetAlbumsSchema = z .object({ + owned: stringToBool + .optional() + .describe('Filter by ownership: true = only owned, false = only shared-with-me, undefined = no filter'), shared: stringToBool .optional() - .describe('Filter by shared status: true = only shared, false = not shared, undefined = all owned albums'), - assetId: z.uuidv4().optional().describe('Filter albums containing this asset ID (ignores shared parameter)'), + .describe('Filter by shared status: true = only shared, false = not shared, undefined = no filter'), + assetId: z.uuidv4().optional().describe('Filter albums containing this asset ID (ignores other parameters)'), }) .meta({ id: 'GetAlbumsDto' }); diff --git a/server/src/repositories/album.repository.ts b/server/src/repositories/album.repository.ts index a910673c62..492ac9b1a9 100644 --- a/server/src/repositories/album.repository.ts +++ b/server/src/repositories/album.repository.ts @@ -201,6 +201,81 @@ export class AlbumRepository { .execute(); } + @GenerateSql({ params: [DummyValue.UUID] }) + getAllAccessible(ownerId: string) { + return this.db + .selectFrom('album') + .selectAll('album') + .innerJoin('album_user', (join) => + join.onRef('album_user.albumId', '=', 'album.id').on('album_user.userId', '=', ownerId), + ) + .where('album.deletedAt', 'is', null) + .where(({ exists, selectFrom }) => + exists( + selectFrom('album_user as au') + .whereRef('au.albumId', '=', 'album.id') + .where('au.role', '=', sql.lit(AlbumUserRole.Owner)), + ), + ) + .select(withAlbumUsers(ownerId)) + .select(withSharedLink) + .orderBy('album.createdAt', 'desc') + .execute(); + } + + @GenerateSql({ params: [DummyValue.UUID] }) + getSharedWithMe(ownerId: string) { + return this.db + .selectFrom('album') + .selectAll('album') + .innerJoin('album_user', (join) => + join + .onRef('album_user.albumId', '=', 'album.id') + .on('album_user.userId', '=', ownerId) + .on('album_user.role', '!=', sql.lit(AlbumUserRole.Owner)), + ) + .where('album.deletedAt', 'is', null) + .where(({ exists, selectFrom }) => + exists( + selectFrom('album_user as au') + .whereRef('au.albumId', '=', 'album.id') + .where('au.role', '=', sql.lit(AlbumUserRole.Owner)), + ), + ) + .select(withAlbumUsers(ownerId)) + .select(withSharedLink) + .orderBy('album.createdAt', 'desc') + .execute(); + } + + @GenerateSql({ params: [DummyValue.UUID] }) + getOwnedAndShared(ownerId: string) { + return this.db + .selectFrom('album') + .selectAll('album') + .innerJoin('album_user', (join) => + join + .onRef('album_user.albumId', '=', 'album.id') + .on('album_user.userId', '=', ownerId) + .on('album_user.role', '=', sql.lit(AlbumUserRole.Owner)), + ) + .where('album.deletedAt', 'is', null) + .where(({ or, exists, selectFrom }) => + or([ + exists( + selectFrom('album_user as au') + .whereRef('au.albumId', '=', 'album.id') + .where('au.role', '!=', sql.lit(AlbumUserRole.Owner)), + ), + exists(selectFrom('shared_link').whereRef('shared_link.albumId', '=', 'album.id')), + ]), + ) + .select(withAlbumUsers(ownerId)) + .select(withSharedLink) + .orderBy('album.createdAt', 'desc') + .execute(); + } + /** * Get albums shared with and shared by owner. */ diff --git a/server/src/services/album.service.spec.ts b/server/src/services/album.service.spec.ts index 24e28a9701..2ecd663ca4 100644 --- a/server/src/services/album.service.spec.ts +++ b/server/src/services/album.service.spec.ts @@ -46,7 +46,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.getAllAccessible.mockResolvedValue([getForAlbum(album), getForAlbum(sharedWithUserAlbum)]); mocks.album.getMetadataForIds.mockResolvedValue([ { albumId: album.id, @@ -134,12 +134,60 @@ describe(AlbumService.name, () => { expect(result[0].id).toEqual(album.id); expect(mocks.album.getNotShared).toHaveBeenCalledTimes(1); }); + + it('gets only owned albums when owned=true', async () => { + const album = AlbumFactory.create(); + const { user: owner } = album.albumUsers.find(({ role }) => role === AlbumUserRole.Owner)!; + mocks.album.getOwned.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), { owned: true }); + expect(result).toHaveLength(1); + expect(mocks.album.getOwned).toHaveBeenCalledTimes(1); + }); + + it('gets only shared-with-me albums when owned=false', async () => { + const album = AlbumFactory.create(); + const { user: owner } = album.albumUsers.find(({ role }) => role === AlbumUserRole.Owner)!; + mocks.album.getSharedWithMe.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), { owned: false }); + expect(result).toHaveLength(1); + expect(mocks.album.getSharedWithMe).toHaveBeenCalledTimes(1); + }); + + it('gets owned shared-out albums when owned=true and shared=true', async () => { + const album = AlbumFactory.create(); + const { user: owner } = album.albumUsers.find(({ role }) => role === AlbumUserRole.Owner)!; + mocks.album.getOwnedAndShared.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), { owned: true, shared: true }); + expect(result).toHaveLength(1); + expect(mocks.album.getOwnedAndShared).toHaveBeenCalledTimes(1); + }); + + it('returns empty list when owned=false and shared=false', async () => { + const album = AlbumFactory.create(); + const { user: owner } = album.albumUsers.find(({ role }) => role === AlbumUserRole.Owner)!; + + const result = await sut.getAll(AuthFactory.create(owner), { owned: false, shared: false }); + expect(result).toHaveLength(0); + expect(mocks.album.getMetadataForIds).not.toHaveBeenCalled(); + }); }); 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.getAllAccessible.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.getAllAccessible).toHaveBeenCalledTimes(1); }); describe('create', () => { diff --git a/server/src/services/album.service.ts b/server/src/services/album.service.ts index 7bfc0bdcc2..21bbcafda8 100644 --- a/server/src/services/album.service.ts +++ b/server/src/services/album.service.ts @@ -38,18 +38,35 @@ export class AlbumService extends BaseService { }; } - async getAll({ user: { id: ownerId } }: AuthDto, { assetId, shared }: GetAlbumsDto): Promise { + async getAll( + { user: { id: ownerId } }: AuthDto, + { assetId, owned, shared }: GetAlbumsDto, + ): Promise { await this.albumRepository.updateThumbnails(); let albums: MapAlbumDto[]; if (assetId) { albums = await this.albumRepository.getByAssetId(ownerId, assetId); + } else if (owned === false && shared === false) { + albums = []; + } else if (owned === false) { + albums = await this.albumRepository.getSharedWithMe(ownerId); } else if (shared === true) { - albums = await this.albumRepository.getShared(ownerId); + albums = + owned === true + ? await this.albumRepository.getOwnedAndShared(ownerId) + : await this.albumRepository.getShared(ownerId); } else if (shared === false) { albums = await this.albumRepository.getNotShared(ownerId); } else { - albums = await this.albumRepository.getOwned(ownerId); + albums = + owned === true + ? await this.albumRepository.getOwned(ownerId) + : await this.albumRepository.getAllAccessible(ownerId); + } + + if (albums.length === 0) { + return []; } // Get asset count for each album. Then map the result to an object: