chore: owner role for album_user

server/src/dtos/album.dto.ts

@@ -230,7 +230,7 @@ export const mapAlbum = (
   const assets = entity.assets || [];
 
   const hasSharedLink = !!entity.sharedLinks && entity.sharedLinks.length > 0;
-  const hasSharedUser = albumUsers.length > 0;
+  const hasSharedUser = albumUsers.some(({ role }) => role !== AlbumUserRole.Owner);
 
   let startDate = assets.at(0)?.localDateTime;
   let endDate = assets.at(-1)?.localDateTime;

server/src/enum.ts

@@ -56,6 +56,7 @@ export enum AssetFileType {
 export enum AlbumUserRole {
   Editor = 'editor',
   Viewer = 'viewer',
+  Owner = 'owner',
 }
 
 export enum AssetOrder {

server/src/repositories/access.repository.ts

@@ -91,7 +91,9 @@ class AlbumAccess {
     }
 
     const accessRole =
-      access === AlbumUserRole.Editor ? [AlbumUserRole.Editor] : [AlbumUserRole.Editor, AlbumUserRole.Viewer];
+      access === AlbumUserRole.Editor
+        ? [AlbumUserRole.Editor, AlbumUserRole.Owner]
+        : [AlbumUserRole.Editor, AlbumUserRole.Viewer, AlbumUserRole.Owner];
 
     return this.db
       .selectFrom('album')

server/src/repositories/album.repository.ts

@@ -14,6 +14,7 @@ import { InjectKysely } from 'nestjs-kysely';
 import { columns } from 'src/database';
 import { Chunked, ChunkedArray, ChunkedSet, DummyValue, GenerateSql } from 'src/decorators';
 import { AlbumUserCreateDto } from 'src/dtos/album.dto';
+import { AlbumUserRole } from 'src/enum';
 import { DB } from 'src/schema';
 import { AlbumTable } from 'src/schema/tables/album.table';
 import { AssetExifTable } from 'src/schema/tables/asset-exif.table';
@@ -213,12 +214,25 @@ export class AlbumRepository {
       .selectAll('album')
       .where((eb) =>
         eb.or([
+          // Albums I own that have non-owner shared users
+          eb.and([
+            eb('album.ownerId', '=', ownerId),
+            eb.exists(
+              eb
+                .selectFrom('album_user')
+                .whereRef('album_user.albumId', '=', 'album.id')
+                .where('album_user.role', '!=', AlbumUserRole.Owner),
+            ),
+          ]),
+          // Albums shared with me (I'm in album_user but not the owner)
           eb.exists(
             eb
               .selectFrom('album_user')
               .whereRef('album_user.albumId', '=', 'album.id')
-              .where((eb) => eb.or([eb('album.ownerId', '=', ownerId), eb('album_user.userId', '=', ownerId)])),
+              .where('album_user.userId', '=', ownerId)
+              .where('album_user.role', '!=', AlbumUserRole.Owner),
           ),
+          // Albums with shared links
           eb.exists(
             eb
               .selectFrom('shared_link')
@@ -245,7 +259,16 @@ export class AlbumRepository {
       .selectAll('album')
       .where('album.ownerId', '=', ownerId)
       .where('album.deletedAt', 'is', null)
-      .where((eb) => eb.not(eb.exists(eb.selectFrom('album_user').whereRef('album_user.albumId', '=', 'album.id'))))
+      .where((eb) =>
+        eb.not(
+          eb.exists(
+            eb
+              .selectFrom('album_user')
+              .whereRef('album_user.albumId', '=', 'album.id')
+              .where('album_user.role', '!=', AlbumUserRole.Owner),
+          ),
+        ),
+      )
       .where((eb) => eb.not(eb.exists(eb.selectFrom('shared_link').whereRef('shared_link.albumId', '=', 'album.id'))))
       .select(withOwner)
       .orderBy('album.createdAt', 'desc')
@@ -322,14 +345,13 @@ export class AlbumRepository {
         await this.addAssets(tx, newAlbum.id, assetIds);
       }
 
-      if (albumUsers.length > 0) {
-        await tx
-          .insertInto('album_user')
-          .values(
-            albumUsers.map((albumUser) => ({ albumId: newAlbum.id, userId: albumUser.userId, role: albumUser.role })),
-          )
-          .execute();
-      }
+      // Always insert the owner with Owner role in album_user
+      const allAlbumUsers = [
+        { albumId: newAlbum.id, userId: album.ownerId, role: AlbumUserRole.Owner },
+        ...albumUsers.map((albumUser) => ({ albumId: newAlbum.id, userId: albumUser.userId, role: albumUser.role })),
+      ];
+
+      await tx.insertInto('album_user').values(allAlbumUsers).execute();
 
       return tx
         .selectFrom('album')

server/src/schema/functions.ts

@@ -125,9 +125,6 @@ export const album_delete_audit = registerFunction({
   language: 'PLPGSQL',
   body: `
     BEGIN
-      INSERT INTO album_audit ("albumId", "userId")
-      SELECT "id", "ownerId"
-      FROM OLD;
       RETURN NULL;
     END`,
 });

server/src/schema/migrations/1774923680849-AddOwnerToAlbumUser.ts

@@ -0,0 +1,44 @@
+import { Kysely, sql } from 'kysely';
+
+export async function up(db: Kysely<any>): Promise<void> {
+  // Backfill: insert album owner into album_user with 'owner' role for all existing albums
+  await sql`
+    INSERT INTO "album_user" ("albumId", "userId", "role")
+    SELECT "id", "ownerId", 'owner' FROM "album"
+    ON CONFLICT ("albumId", "userId") DO NOTHING
+  `.execute(db);
+
+  // Make album_delete_audit a no-op since the owner is now in album_user
+  // and the CASCADE delete will trigger album_user_delete_audit instead
+  await sql`
+    CREATE OR REPLACE FUNCTION album_delete_audit()
+    RETURNS TRIGGER
+    LANGUAGE PLPGSQL AS $$
+    BEGIN
+      RETURN NULL;
+    END $$
+  `.execute(db);
+}
+
+export async function down(db: Kysely<any>): Promise<void> {
+  // Restore album_delete_audit to insert owner audit entries
+  await sql`
+    CREATE OR REPLACE FUNCTION album_delete_audit()
+    RETURNS TRIGGER
+    LANGUAGE PLPGSQL AS $$
+    BEGIN
+      INSERT INTO album_audit ("albumId", "userId")
+      SELECT "id", "ownerId"
+      FROM OLD;
+      RETURN NULL;
+    END $$
+  `.execute(db);
+
+  // Remove owner entries from album_user
+  await sql`
+    DELETE FROM "album_user"
+    WHERE ("albumId", "userId") IN (
+      SELECT "id", "ownerId" FROM "album"
+    )
+  `.execute(db);
+}

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

@@ -283,16 +283,19 @@ describe(AlbumService.name, () => {
       );
     });
 
-    it('should throw an error if the userId is the ownerId', async () => {
+    it('should silently filter the owner from albumUsers', async () => {
       const album = AlbumFactory.create();
-      mocks.user.get.mockResolvedValue(album.owner);
-      await expect(
-        sut.create(AuthFactory.create(album.owner), {
-          albumName: 'Empty album',
-          albumUsers: [{ userId: album.owner.id, role: AlbumUserRole.Editor }],
-        }),
-      ).rejects.toBeInstanceOf(BadRequestException);
-      expect(mocks.album.create).not.toHaveBeenCalled();
+      mocks.album.create.mockResolvedValue(getForAlbum(album));
+      mocks.user.getMetadata.mockResolvedValue([]);
+      await sut.create(AuthFactory.create(album.owner), {
+        albumName: 'Empty album',
+        albumUsers: [{ userId: album.owner.id, role: AlbumUserRole.Editor }],
+      });
+      expect(mocks.album.create).toHaveBeenCalledWith(
+        expect.objectContaining({ ownerId: album.owner.id }),
+        [],
+        [],
+      );
     });
   });
 
@@ -420,7 +423,7 @@ describe(AlbumService.name, () => {
         sut.addUsers(AuthFactory.create(album.owner), album.id, {
           albumUsers: [{ userId: album.owner.id }],
         }),
-      ).rejects.toBeInstanceOf(BadRequestException);
+      ).rejects.toThrow('User already added');
       expect(mocks.album.update).not.toHaveBeenCalled();
       expect(mocks.user.get).not.toHaveBeenCalled();
     });
@@ -537,6 +540,7 @@ describe(AlbumService.name, () => {
       const user = UserFactory.create();
       const album = AlbumFactory.from().albumUser({ userId: user.id }).build();
       mocks.access.album.checkOwnerAccess.mockResolvedValue(new Set([album.id]));
+      mocks.album.getById.mockResolvedValue(getForAlbum(album));
       mocks.albumUser.update.mockResolvedValue();
 
       await sut.updateUser(AuthFactory.create(album.owner), album.id, user.id, { role: AlbumUserRole.Viewer });
@@ -546,6 +550,16 @@ describe(AlbumService.name, () => {
         { role: AlbumUserRole.Viewer },
       );
     });
+
+    it('should not allow changing the album owner role', async () => {
+      const album = AlbumFactory.create();
+      mocks.access.album.checkOwnerAccess.mockResolvedValue(new Set([album.id]));
+      mocks.album.getById.mockResolvedValue(getForAlbum(album));
+
+      await expect(
+        sut.updateUser(AuthFactory.create(album.owner), album.id, album.owner.id, { role: AlbumUserRole.Viewer }),
+      ).rejects.toThrow('Cannot change album owner role');
+    });
   });
 
   describe('getAlbumInfo', () => {

server/src/services/album.service.ts

@@ -17,7 +17,7 @@ import {
 } from 'src/dtos/album.dto';
 import { BulkIdErrorReason, BulkIdResponseDto, BulkIdsDto } from 'src/dtos/asset-ids.response.dto';
 import { AuthDto } from 'src/dtos/auth.dto';
-import { Permission } from 'src/enum';
+import { AlbumUserRole, Permission } from 'src/enum';
 import { AlbumAssetCount, AlbumInfoOptions } from 'src/repositories/album.repository';
 import { BaseService } from 'src/services/base.service';
 import { addAssets, removeAssets } from 'src/utils/asset.util';
@@ -80,7 +80,8 @@ export class AlbumService extends BaseService {
     const album = await this.findOrFail(id, { withAssets });
     const [albumMetadataForIds] = await this.albumRepository.getMetadataForIds([album.id]);
 
-    const hasSharedUsers = album.albumUsers && album.albumUsers.length > 0;
+    const hasSharedUsers =
+      album.albumUsers && album.albumUsers.some(({ role }) => role !== AlbumUserRole.Owner);
     const hasSharedLink = album.sharedLinks && album.sharedLinks.length > 0;
     const isShared = hasSharedUsers || hasSharedLink;
 
@@ -98,16 +99,19 @@ export class AlbumService extends BaseService {
     const albumUsers = dto.albumUsers || [];
 
     for (const { userId } of albumUsers) {
+      if (userId == auth.user.id) {
+        continue;
+      }
+
       const exists = await this.userRepository.get(userId, {});
       if (!exists) {
         throw new BadRequestException('User not found');
       }
-
-      if (userId == auth.user.id) {
-        throw new BadRequestException('Cannot share album with owner');
-      }
     }
 
+    // Filter out the owner from albumUsers since they are always added by the repository
+    const filteredAlbumUsers = albumUsers.filter(({ userId }) => userId !== auth.user.id);
+
     const allowedAssetIdsSet = await this.checkAccess({
       auth,
       permission: Permission.AssetShare,
@@ -126,10 +130,10 @@ export class AlbumService extends BaseService {
         order: getPreferences(userMetadata).albums.defaultAssetOrder,
       },
       assetIds,
-      albumUsers,
+      filteredAlbumUsers,
     );
 
-    for (const { userId } of albumUsers) {
+    for (const { userId } of filteredAlbumUsers) {
       await this.eventRepository.emit('AlbumInvite', { id: album.id, userId });
     }
 
@@ -188,9 +192,9 @@ export class AlbumService extends BaseService {
         albumThumbnailAssetId: album.albumThumbnailAssetId ?? firstNewAssetId,
       });
 
-      const allUsersExceptUs = [...album.albumUsers.map(({ user }) => user.id), album.owner.id].filter(
-        (userId) => userId !== auth.user.id,
-      );
+      const allUsersExceptUs = album.albumUsers
+        .map(({ user }) => user.id)
+        .filter((userId) => userId !== auth.user.id);
 
       for (const recipientId of allUsersExceptUs) {
         await this.eventRepository.emit('AlbumUpdate', { id, recipientId });
@@ -248,9 +252,9 @@ export class AlbumService extends BaseService {
         updatedAt: new Date(),
         albumThumbnailAssetId: album.albumThumbnailAssetId ?? notPresentAssetIds[0],
       });
-      const allUsersExceptUs = [...album.albumUsers.map(({ user }) => user.id), album.owner.id].filter(
-        (userId) => userId !== auth.user.id,
-      );
+      const allUsersExceptUs = album.albumUsers
+        .map(({ user }) => user.id)
+        .filter((userId) => userId !== auth.user.id);
       events.push({ id: albumId, recipients: allUsersExceptUs });
     }
 
@@ -288,10 +292,6 @@ export class AlbumService extends BaseService {
     const album = await this.findOrFail(id, { withAssets: false });
 
     for (const { userId, role } of albumUsers) {
-      if (album.ownerId === userId) {
-        throw new BadRequestException('Cannot be shared with owner');
-      }
-
       const exists = album.albumUsers.find(({ user: { id } }) => id === userId);
       if (exists) {
         throw new BadRequestException('User already added');
@@ -316,13 +316,13 @@ export class AlbumService extends BaseService {
 
     const album = await this.findOrFail(id, { withAssets: false });
 
-    if (album.ownerId === userId) {
-      throw new BadRequestException('Cannot remove album owner');
+    const albumUser = album.albumUsers.find(({ user: { id } }) => id === userId);
+    if (!albumUser) {
+      throw new BadRequestException('Album not shared with user');
     }
 
-    const exists = album.albumUsers.find(({ user: { id } }) => id === userId);
-    if (!exists) {
-      throw new BadRequestException('Album not shared with user');
+    if (albumUser.role === AlbumUserRole.Owner) {
+      throw new BadRequestException('Cannot remove album owner');
     }
 
     // non-admin can remove themselves
@@ -335,6 +335,13 @@ export class AlbumService extends BaseService {
 
   async updateUser(auth: AuthDto, id: string, userId: string, dto: UpdateAlbumUserDto): Promise<void> {
     await this.requireAccess({ auth, permission: Permission.AlbumShare, ids: [id] });
+
+    const album = await this.findOrFail(id, { withAssets: false });
+    const albumUser = album.albumUsers.find(({ user: { id } }) => id === userId);
+    if (albumUser?.role === AlbumUserRole.Owner) {
+      throw new BadRequestException('Cannot change album owner role');
+    }
+
     await this.albumUserRepository.update({ albumId: id, userId }, { role: dto.role });
   }
 

server/test/factories/album.factory.ts

@@ -1,5 +1,5 @@
 import { Selectable } from 'kysely';
-import { AssetOrder } from 'src/enum';
+import { AlbumUserRole, AssetOrder } from 'src/enum';
 import { AlbumTable } from 'src/schema/tables/album.table';
 import { SharedLinkTable } from 'src/schema/tables/shared-link.table';
 import { AlbumUserFactory } from 'test/factories/album-user.factory';
@@ -76,11 +76,20 @@ export class AlbumFactory {
   }
 
   build() {
+    const owner = this.#owner.build();
+    const ownerAlbumUser = AlbumUserFactory.from({
+      albumId: this.value.id,
+      userId: owner.id,
+      role: AlbumUserRole.Owner,
+    })
+      .user(owner)
+      .build();
+
     return {
       ...this.value,
-      owner: this.#owner.build(),
+      owner,
       assets: this.#assets.map((asset) => asset.build()),
-      albumUsers: this.#albumUsers.map((albumUser) => albumUser.build()),
+      albumUsers: [ownerAlbumUser, ...this.#albumUsers.map((albumUser) => albumUser.build())],
       sharedLinks: this.#sharedLinks,
     };
   }

server/test/medium/specs/sync/sync-album-user.spec.ts

@@ -25,6 +25,15 @@ describe(SyncRequestType.AlbumUsersV1, () => {
     const { albumUser } = await ctx.newAlbumUser({ albumId: album.id, userId: user.id, role: AlbumUserRole.Editor });
 
     await expect(ctx.syncStream(auth, [SyncRequestType.AlbumUsersV1])).resolves.toEqual([
+      {
+        ack: expect.any(String),
+        data: expect.objectContaining({
+          albumId: album.id,
+          role: AlbumUserRole.Owner,
+          userId: auth.user.id,
+        }),
+        type: SyncEntityType.AlbumUserV1,
+      },
       {
         ack: expect.any(String),
         data: expect.objectContaining({
@@ -47,6 +56,10 @@ describe(SyncRequestType.AlbumUsersV1, () => {
 
       const response = await ctx.syncStream(auth, [SyncRequestType.AlbumUsersV1]);
       expect(response).toEqual([
+        expect.objectContaining({
+          data: expect.objectContaining({ albumId: album.id, userId: auth.user.id, role: AlbumUserRole.Owner }),
+          type: SyncEntityType.AlbumUserV1,
+        }),
         {
           ack: expect.any(String),
           data: expect.objectContaining({
@@ -136,6 +149,10 @@ describe(SyncRequestType.AlbumUsersV1, () => {
 
       const response = await ctx.syncStream(auth, [SyncRequestType.AlbumUsersV1]);
       expect(response).toEqual([
+        expect.objectContaining({
+          data: expect.objectContaining({ albumId: album.id, userId: user1.id, role: AlbumUserRole.Owner }),
+          type: SyncEntityType.AlbumUserV1,
+        }),
         {
           ack: expect.any(String),
           data: expect.objectContaining({
@@ -163,6 +180,7 @@ describe(SyncRequestType.AlbumUsersV1, () => {
 
       const response = await ctx.syncStream(auth, [SyncRequestType.AlbumUsersV1]);
       expect(response).toEqual([
+        expect.objectContaining({ type: SyncEntityType.AlbumUserV1 }),
         expect.objectContaining({ type: SyncEntityType.AlbumUserV1 }),
         expect.objectContaining({ type: SyncEntityType.AlbumUserV1 }),
         expect.objectContaining({ type: SyncEntityType.SyncCompleteV1 }),
@@ -201,6 +219,7 @@ describe(SyncRequestType.AlbumUsersV1, () => {
 
       const response = await ctx.syncStream(auth, [SyncRequestType.AlbumUsersV1]);
       expect(response).toEqual([
+        expect.objectContaining({ type: SyncEntityType.AlbumUserV1 }),
         expect.objectContaining({ type: SyncEntityType.AlbumUserV1 }),
         expect.objectContaining({ type: SyncEntityType.AlbumUserV1 }),
         expect.objectContaining({ type: SyncEntityType.SyncCompleteV1 }),
@@ -233,8 +252,7 @@ describe(SyncRequestType.AlbumUsersV1, () => {
       const { user: user2 } = await ctx.newUser();
       const { album: album1 } = await ctx.newAlbum({ ownerId: user1.id });
       const { album: album2 } = await ctx.newAlbum({ ownerId: user1.id });
-      // backfill album user
-      await ctx.newAlbumUser({ albumId: album1.id, userId: user1.id, role: AlbumUserRole.Editor });
+      // owner (user1) is already in album_user from album creation
       await wait(2);
       // initial album user
       await ctx.newAlbumUser({ albumId: album2.id, userId: auth.user.id, role: AlbumUserRole.Editor });
@@ -244,6 +262,10 @@ describe(SyncRequestType.AlbumUsersV1, () => {
 
       const response = await ctx.syncStream(auth, [SyncRequestType.AlbumUsersV1]);
       expect(response).toEqual([
+        expect.objectContaining({
+          data: expect.objectContaining({ albumId: album2.id, userId: user1.id, role: AlbumUserRole.Owner }),
+          type: SyncEntityType.AlbumUserV1,
+        }),
         {
           ack: expect.any(String),
           data: expect.objectContaining({
@@ -261,14 +283,14 @@ describe(SyncRequestType.AlbumUsersV1, () => {
       // get access to the backfill album user
       await ctx.newAlbumUser({ albumId: album1.id, userId: auth.user.id, role: AlbumUserRole.Editor });
 
-      // should backfill the album user
+      // should backfill the album user (owner user1 is already there from album creation)
       const newResponse = await ctx.syncStream(auth, [SyncRequestType.AlbumUsersV1]);
       expect(newResponse).toEqual([
         {
           ack: expect.any(String),
           data: expect.objectContaining({
             albumId: album1.id,
-            role: AlbumUserRole.Editor,
+            role: AlbumUserRole.Owner,
             userId: user1.id,
           }),
           type: SyncEntityType.AlbumUserBackfillV1,