diff --git a/e2e/src/specs/server/api/album.e2e-spec.ts b/e2e/src/specs/server/api/album.e2e-spec.ts index 1d3be1248c..0aa7b7ec03 100644 --- a/e2e/src/specs/server/api/album.e2e-spec.ts +++ b/e2e/src/specs/server/api/album.e2e-spec.ts @@ -730,8 +730,8 @@ describe('/albums', () => { .set('Authorization', `Bearer ${user1.accessToken}`) .send({ albumUsers: [{ userId: user1.userId, role: AlbumUserRole.Editor }] }); - expect(status).toBe(400); - expect(body).toEqual(errorDto.badRequest('User already added')); + expect(status).toBe(200); + expect(body.albumUsers.length).toEqual(1); }); it('should not be able to add existing user to shared album', async () => { @@ -745,8 +745,8 @@ describe('/albums', () => { .set('Authorization', `Bearer ${user1.accessToken}`) .send({ albumUsers: [{ userId: user2.userId, role: AlbumUserRole.Editor }] }); - expect(status).toBe(400); - expect(body).toEqual(errorDto.badRequest('User already added')); + expect(status).toBe(200); + expect(body.albumUsers.length).toEqual(2); }); }); diff --git a/server/src/services/album.service.spec.ts b/server/src/services/album.service.spec.ts index 374cd58a0e..f1d125993c 100644 --- a/server/src/services/album.service.spec.ts +++ b/server/src/services/album.service.spec.ts @@ -472,17 +472,19 @@ describe(AlbumService.name, () => { expect(mocks.album.update).not.toHaveBeenCalled(); }); - it('should throw an error if the userId is already added', async () => { + it('should skip if the userId is already added', async () => { const userId = newUuid(); const album = AlbumFactory.from().albumUser({ userId }).build(); const { user: owner } = album.albumUsers.find(({ role }) => role === AlbumUserRole.Owner)!; mocks.access.album.checkOwnerAccess.mockResolvedValue(new Set([album.id])); mocks.album.getById.mockResolvedValue(getForAlbum(album)); - await expect( - sut.addUsers(AuthFactory.create(owner), album.id, { albumUsers: [{ userId }] }), - ).rejects.toBeInstanceOf(BadRequestException); + await expect(sut.addUsers(AuthFactory.create(owner), album.id, { albumUsers: [{ userId }] })).resolves.toEqual( + expect.objectContaining({ id: album.id }), + ); expect(mocks.album.update).not.toHaveBeenCalled(); expect(mocks.user.get).not.toHaveBeenCalled(); + expect(mocks.albumUser.create).not.toHaveBeenCalled(); + expect(mocks.event.emit).not.toHaveBeenCalled(); }); it('should throw an error if the userId does not exist', async () => { @@ -498,7 +500,7 @@ describe(AlbumService.name, () => { expect(mocks.user.get).toHaveBeenCalledWith('unknown-user', {}); }); - it('should throw an error if the userId is the ownerId', async () => { + it('should skip if the userId is the ownerId', async () => { const album = AlbumFactory.create(); const { user: owner } = album.albumUsers.find(({ role }) => role === AlbumUserRole.Owner)!; mocks.access.album.checkOwnerAccess.mockResolvedValue(new Set([album.id])); @@ -507,9 +509,11 @@ describe(AlbumService.name, () => { sut.addUsers(AuthFactory.create(owner), album.id, { albumUsers: [{ userId: owner.id }], }), - ).rejects.toBeInstanceOf(BadRequestException); + ).resolves.toEqual(expect.objectContaining({ id: album.id })); expect(mocks.album.update).not.toHaveBeenCalled(); expect(mocks.user.get).not.toHaveBeenCalled(); + expect(mocks.albumUser.create).not.toHaveBeenCalled(); + expect(mocks.event.emit).not.toHaveBeenCalled(); }); it('should add valid shared users', async () => { @@ -534,6 +538,35 @@ describe(AlbumService.name, () => { senderName: owner.name, }); }); + + it('should add new users when already-added users are included', async () => { + const existingUserId = newUuid(); + const album = AlbumFactory.from().albumUser({ userId: existingUserId }).build(); + const { user: owner } = album.albumUsers.find(({ role }) => role === AlbumUserRole.Owner)!; + const user = UserFactory.create(); + mocks.access.album.checkOwnerAccess.mockResolvedValue(new Set([album.id])); + mocks.album.getById.mockResolvedValue(getForAlbum(album)); + mocks.user.get.mockResolvedValue(user); + mocks.albumUser.create.mockResolvedValue(AlbumUserFactory.from().album(album).user(user).build()); + + await sut.addUsers(AuthFactory.create(owner), album.id, { + albumUsers: [{ userId: existingUserId }, { userId: user.id }], + }); + + expect(mocks.user.get).toHaveBeenCalledTimes(1); + expect(mocks.user.get).toHaveBeenCalledWith(user.id, {}); + expect(mocks.albumUser.create).toHaveBeenCalledTimes(1); + expect(mocks.albumUser.create).toHaveBeenCalledWith({ + userId: user.id, + albumId: album.id, + }); + expect(mocks.event.emit).toHaveBeenCalledTimes(1); + expect(mocks.event.emit).toHaveBeenCalledWith('AlbumInvite', { + id: album.id, + userId: user.id, + senderName: owner.name, + }); + }); }); describe('removeUser', () => { diff --git a/server/src/services/album.service.ts b/server/src/services/album.service.ts index cc0b0d899c..5f4bc56d98 100644 --- a/server/src/services/album.service.ts +++ b/server/src/services/album.service.ts @@ -290,7 +290,7 @@ export class AlbumService extends BaseService { const exists = album.albumUsers.find(({ user: { id } }) => id === userId); if (exists) { - throw new BadRequestException('User already added'); + continue; } const user = await this.userRepository.get(userId, {});