fix(server): only asset owner should see favorite status (#20654)

* fix: Any asset update disables isFavorite action in GUI. Only owner of asset in album should see favorited image.

* Fix unit tests

* Fix formatting

* better query, add medium test

* update sql

---------

Co-authored-by: mertalev <101130780+mertalev@users.noreply.github.com>

server/src/dtos/asset-response.dto.ts

@@ -212,7 +212,7 @@ export function mapAsset(entity: MapAsset, options: AssetMapOptions = {}): Asset
     fileModifiedAt: entity.fileModifiedAt,
     localDateTime: entity.localDateTime,
     updatedAt: entity.updatedAt,
-    isFavorite: options.auth?.user.id === entity.ownerId ? entity.isFavorite : false,
+    isFavorite: options.auth?.user.id === entity.ownerId && entity.isFavorite,
     isArchived: entity.visibility === AssetVisibility.Archive,
     isTrashed: !!entity.deletedAt,
     visibility: entity.visibility,

server/src/queries/asset.repository.sql

@@ -296,7 +296,8 @@ with
       "asset"."duration",
       "asset"."id",
       "asset"."visibility",
-      "asset"."isFavorite",
+      asset."isFavorite"
+      and asset."ownerId" = $1 as "isFavorite",
       asset.type = 'IMAGE' as "isImage",
       asset."deletedAt" is not null as "isTrashed",
       "asset"."livePhotoVideoId",
@@ -341,14 +342,14 @@ with
         where
           "stacked"."stackId" = "asset"."stackId"
           and "stacked"."deletedAt" is null
-          and "stacked"."visibility" = $1
+          and "stacked"."visibility" = $2
         group by
           "stacked"."stackId"
       ) as "stacked_assets" on true
     where
       "asset"."deletedAt" is null
       and "asset"."visibility" in ('archive', 'timeline')
-      and date_trunc('MONTH', "localDateTime" AT TIME ZONE 'UTC') AT TIME ZONE 'UTC' = $2
+      and date_trunc('MONTH', "localDateTime" AT TIME ZONE 'UTC') AT TIME ZONE 'UTC' = $3
       and not exists (
         select
         from

server/src/repositories/asset.repository.ts

@@ -4,6 +4,7 @@ import { isEmpty, isUndefined, omitBy } from 'lodash';
 import { InjectKysely } from 'nestjs-kysely';
 import { Stack } from 'src/database';
 import { Chunked, ChunkedArray, DummyValue, GenerateSql } from 'src/decorators';
+import { AuthDto } from 'src/dtos/auth.dto';
 import { AssetFileType, AssetMetadataKey, AssetOrder, AssetStatus, AssetType, AssetVisibility } from 'src/enum';
 import { DB } from 'src/schema';
 import { AssetExifTable } from 'src/schema/tables/asset-exif.table';
@@ -589,9 +590,9 @@ export class AssetRepository {
   }
 
   @GenerateSql({
-    params: [DummyValue.TIME_BUCKET, { withStacked: true }],
+    params: [DummyValue.TIME_BUCKET, { withStacked: true }, { user: { id: DummyValue.UUID } }],
   })
-  getTimeBucket(timeBucket: string, options: TimeBucketOptions) {
+  getTimeBucket(timeBucket: string, options: TimeBucketOptions, auth: AuthDto) {
     const query = this.db
       .with('cte', (qb) =>
         qb
@@ -601,7 +602,7 @@ export class AssetRepository {
             'asset.duration',
             'asset.id',
             'asset.visibility',
-            'asset.isFavorite',
+            sql`asset."isFavorite" and asset."ownerId" = ${auth.user.id}`.as('isFavorite'),
             sql`asset.type = 'IMAGE'`.as('isImage'),
             sql`asset."deletedAt" is not null`.as('isTrashed'),
             'asset.livePhotoVideoId',

server/src/services/notification.service.ts

@@ -162,7 +162,11 @@ export class NotificationService extends BaseService {
 
     const [asset] = await this.assetRepository.getByIdsWithAllRelationsButStacks([assetId]);
     if (asset) {
-      this.eventRepository.clientSend('on_asset_update', userId, mapAsset(asset));
+      this.eventRepository.clientSend(
+        'on_asset_update',
+        userId,
+        mapAsset(asset, { auth: { user: { id: userId } } as AuthDto }),
+      );
     }
   }
 

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

@@ -36,10 +36,14 @@ describe(TimelineService.name, () => {
       );
 
       expect(mocks.access.album.checkOwnerAccess).toHaveBeenCalledWith(authStub.admin.user.id, new Set(['album-id']));
-      expect(mocks.asset.getTimeBucket).toHaveBeenCalledWith('bucket', {
-        timeBucket: 'bucket',
-        albumId: 'album-id',
-      });
+      expect(mocks.asset.getTimeBucket).toHaveBeenCalledWith(
+        'bucket',
+        {
+          timeBucket: 'bucket',
+          albumId: 'album-id',
+        },
+        authStub.admin,
+      );
     });
 
     it('should return the assets for a archive time bucket if user has archive.read', async () => {
@@ -60,6 +64,7 @@ describe(TimelineService.name, () => {
           visibility: AssetVisibility.Archive,
           userIds: [authStub.admin.user.id],
         }),
+        authStub.admin,
       );
     });
 
@@ -76,12 +81,16 @@ describe(TimelineService.name, () => {
           withPartners: true,
         }),
       ).resolves.toEqual(json);
-      expect(mocks.asset.getTimeBucket).toHaveBeenCalledWith('bucket', {
-        timeBucket: 'bucket',
-        visibility: AssetVisibility.Timeline,
-        withPartners: true,
-        userIds: [authStub.admin.user.id],
-      });
+      expect(mocks.asset.getTimeBucket).toHaveBeenCalledWith(
+        'bucket',
+        {
+          timeBucket: 'bucket',
+          visibility: AssetVisibility.Timeline,
+          withPartners: true,
+          userIds: [authStub.admin.user.id],
+        },
+        authStub.admin,
+      );
     });
 
     it('should check permissions to read tag', async () => {
@@ -96,11 +105,15 @@ describe(TimelineService.name, () => {
           tagId: 'tag-123',
         }),
       ).resolves.toEqual(json);
-      expect(mocks.asset.getTimeBucket).toHaveBeenCalledWith('bucket', {
-        tagId: 'tag-123',
-        timeBucket: 'bucket',
-        userIds: [authStub.admin.user.id],
-      });
+      expect(mocks.asset.getTimeBucket).toHaveBeenCalledWith(
+        'bucket',
+        {
+          tagId: 'tag-123',
+          timeBucket: 'bucket',
+          userIds: [authStub.admin.user.id],
+        },
+        authStub.admin,
+      );
     });
 
     it('should return the assets for a library time bucket if user has library.read', async () => {
@@ -119,6 +132,7 @@ describe(TimelineService.name, () => {
           timeBucket: 'bucket',
           userIds: [authStub.admin.user.id],
         }),
+        authStub.admin,
       );
     });
 

server/src/services/timeline.service.ts

@@ -21,7 +21,7 @@ export class TimelineService extends BaseService {
     const timeBucketOptions = await this.buildTimeBucketOptions(auth, { ...dto });
 
     // TODO: use id cursor for pagination
-    const bucket = await this.assetRepository.getTimeBucket(dto.timeBucket, timeBucketOptions);
+    const bucket = await this.assetRepository.getTimeBucket(dto.timeBucket, timeBucketOptions, auth);
     return bucket.assets;
   }
 

server/test/medium/specs/services/timeline.service.spec.ts

@@ -4,6 +4,7 @@ import { AssetVisibility } from 'src/enum';
 import { AccessRepository } from 'src/repositories/access.repository';
 import { AssetRepository } from 'src/repositories/asset.repository';
 import { LoggingRepository } from 'src/repositories/logging.repository';
+import { PartnerRepository } from 'src/repositories/partner.repository';
 import { DB } from 'src/schema';
 import { TimelineService } from 'src/services/timeline.service';
 import { newMediumService } from 'test/medium.factory';
@@ -15,7 +16,7 @@ let defaultDatabase: Kysely<DB>;
 const setup = (db?: Kysely<DB>) => {
   return newMediumService(TimelineService, {
     database: db || defaultDatabase,
-    real: [AssetRepository, AccessRepository],
+    real: [AssetRepository, AccessRepository, PartnerRepository],
     mock: [LoggingRepository],
   });
 };
@@ -155,5 +156,54 @@ describe(TimelineService.name, () => {
       const response = JSON.parse(rawResponse);
       expect(response).toEqual(expect.objectContaining({ isTrashed: [true] }));
     });
+
+    it('should return false for favorite status unless asset owner', async () => {
+      const { sut, ctx } = setup();
+      const [{ asset: asset1 }, { asset: asset2 }] = await Promise.all([
+        ctx.newUser().then(async ({ user }) => {
+          const result = await ctx.newAsset({
+            ownerId: user.id,
+            fileCreatedAt: new Date('1970-02-12'),
+            localDateTime: new Date('1970-02-12'),
+            isFavorite: true,
+          });
+          await ctx.newExif({ assetId: result.asset.id, make: 'Canon' });
+          return result;
+        }),
+        ctx.newUser().then(async ({ user }) => {
+          const result = await ctx.newAsset({
+            ownerId: user.id,
+            fileCreatedAt: new Date('1970-02-13'),
+            localDateTime: new Date('1970-02-13'),
+            isFavorite: true,
+          });
+          await ctx.newExif({ assetId: result.asset.id, make: 'Canon' });
+          return result;
+        }),
+      ]);
+
+      await Promise.all([
+        ctx.newPartner({ sharedById: asset1.ownerId, sharedWithId: asset2.ownerId }),
+        ctx.newPartner({ sharedById: asset2.ownerId, sharedWithId: asset1.ownerId }),
+      ]);
+
+      const auth1 = factory.auth({ user: { id: asset1.ownerId } });
+      const rawResponse1 = await sut.getTimeBucket(auth1, {
+        timeBucket: '1970-02-01',
+        withPartners: true,
+        visibility: AssetVisibility.Timeline,
+      });
+      const response1 = JSON.parse(rawResponse1);
+      expect(response1).toEqual(expect.objectContaining({ id: [asset2.id, asset1.id], isFavorite: [false, true] }));
+
+      const auth2 = factory.auth({ user: { id: asset2.ownerId } });
+      const rawResponse2 = await sut.getTimeBucket(auth2, {
+        timeBucket: '1970-02-01',
+        withPartners: true,
+        visibility: AssetVisibility.Timeline,
+      });
+      const response2 = JSON.parse(rawResponse2);
+      expect(response2).toEqual(expect.objectContaining({ id: [asset2.id, asset1.id], isFavorite: [true, false] }));
+    });
   });
 });