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] }));
+    });
   });
 });

web/src/lib/components/shared-components/combobox.svelte

@@ -23,7 +23,7 @@
   import { focusOutside } from '$lib/actions/focus-outside';
   import { shortcuts } from '$lib/actions/shortcut';
   import { generateId } from '$lib/utils/generate-id';
-  import { Icon, IconButton } from '@immich/ui';
+  import { Icon, IconButton, Label } from '@immich/ui';
   import { mdiClose, mdiMagnify, mdiUnfoldMoreHorizontal } from '@mdi/js';
   import { onMount, tick } from 'svelte';
   import { t } from 'svelte-i18n';
@@ -251,7 +251,7 @@
 </script>
 
 <svelte:window onresize={onPositionChange} />
-<label class="immich-form-label" class:sr-only={hideLabel} for={inputId}>{label}</label>
+<Label class="block mb-1 {hideLabel ? 'sr-only' : ''}" for={inputId}>{label}</Label>
 <div
   class="relative w-full dark:text-gray-300 text-gray-700 text-base"
   use:focusOutside={{ onFocusOut: deactivate }}

web/src/lib/modals/ApiKeySecretModal.svelte

@@ -1,6 +1,6 @@
 <script lang="ts">
   import { copyToClipboard } from '$lib/utils';
-  import { Button, HStack, Modal, ModalBody, ModalFooter } from '@immich/ui';
+  import { Button, HStack, Modal, ModalBody, ModalFooter, Text, Textarea } from '@immich/ui';
   import { mdiKeyVariant } from '@mdi/js';
   import { t } from 'svelte-i18n';
 
@@ -14,16 +14,8 @@
 
 <Modal title={$t('api_key')} icon={mdiKeyVariant} {onClose} size="small">
   <ModalBody>
-    <div class="text-primary">
-      <p class="text-sm dark:text-immich-dark-fg">
-        {$t('api_key_description')}
-      </p>
-    </div>
-
-    <div class="my-4 flex flex-col gap-2">
-      <!-- <label class="immich-form-label" for="secret">{ $t("api_key") }</label> -->
-      <textarea class="immich-form-input" id="secret" name="secret" readonly={true} value={secret}></textarea>
-    </div>
+    <Text size="small" class="mb-4">{$t('api_key_description')}</Text>
+    <Textarea bind:value={secret} readonly />
   </ModalBody>
 
   <ModalFooter>

web/src/lib/modals/AssetChangeDateModal.svelte

@@ -5,7 +5,7 @@
   import { getPreferredTimeZone, getTimezones, toIsoDate } from '$lib/modals/timezone-utils';
   import { handleError } from '$lib/utils/handle-error';
   import { updateAsset } from '@immich/sdk';
-  import { Button, HStack, Modal, ModalBody, ModalFooter, VStack } from '@immich/ui';
+  import { Button, HStack, Label, Modal, ModalBody, ModalFooter } from '@immich/ui';
   import { mdiCalendarEdit } from '@mdi/js';
   import { DateTime } from 'luxon';
   import { t } from 'svelte-i18n';
@@ -51,29 +51,18 @@
 
 <Modal title={$t('edit_date_and_time')} icon={mdiCalendarEdit} onClose={() => onClose(false)} size="small">
   <ModalBody>
-    <VStack fullWidth>
-      <HStack fullWidth>
-        <label class="immich-form-label" for="datetime">{$t('date_and_time')}</label>
-      </HStack>
-      <HStack fullWidth>
-        <DateInput
-          class="immich-form-input text-gray-700 w-full"
-          id="datetime"
-          type="datetime-local"
-          bind:value={selectedDate}
-        />
-      </HStack>
-      {#if timezoneInput}
-        <div class="w-full">
-          <Combobox
-            bind:selectedOption
-            label={$t('timezone')}
-            options={timezones}
-            placeholder={$t('search_timezone')}
-          />
-        </div>
-      {/if}
-    </VStack>
+    <Label for="datetime" class="block mb-1">{$t('date_and_time')}</Label>
+    <DateInput
+      class="immich-form-input text-gray-700 w-full mb-2"
+      id="datetime"
+      type="datetime-local"
+      bind:value={selectedDate}
+    />
+    {#if timezoneInput}
+      <div class="w-full">
+        <Combobox bind:selectedOption label={$t('timezone')} options={timezones} placeholder={$t('search_timezone')} />
+      </div>
+    {/if}
   </ModalBody>
   <ModalFooter>
     <HStack fullWidth>

web/src/lib/modals/AssetSelectionChangeDateModal.svelte

@@ -8,7 +8,7 @@
   import { getOwnedAssetsWithWarning } from '$lib/utils/asset-utils';
   import { handleError } from '$lib/utils/handle-error';
   import { updateAssets } from '@immich/sdk';
-  import { Button, Field, HStack, Modal, ModalBody, ModalFooter, Switch, VStack } from '@immich/ui';
+  import { Button, Field, HStack, Label, Modal, ModalBody, ModalFooter, Switch } from '@immich/ui';
   import { mdiCalendarEdit } from '@mdi/js';
   import { DateTime } from 'luxon';
   import { t } from 'svelte-i18n';
@@ -65,37 +65,30 @@
 
 <Modal title={$t('edit_date_and_time')} icon={mdiCalendarEdit} onClose={() => onClose(false)} size="small">
   <ModalBody>
-    <VStack fullWidth>
-      <HStack fullWidth>
-        <Field label={$t('edit_date_and_time_by_offset')}>
-          <Switch data-testid="edit-by-offset-switch" bind:checked={showRelative} />
-        </Field>
-      </HStack>
-      {#if showRelative}
-        <HStack fullWidth>
-          <label class="immich-form-label" for="relativedatetime">{$t('offset')}</label>
-        </HStack>
-        <HStack fullWidth>
-          <DurationInput class="immich-form-input text-gray-700" id="relativedatetime" bind:value={selectedDuration} />
-        </HStack>
-      {:else}
-        <HStack fullWidth>
-          <label class="immich-form-label" for="datetime">{$t('date_and_time')}</label>
-        </HStack>
-        <HStack fullWidth>
-          <DateInput class="immich-form-input" id="datetime" type="datetime-local" bind:value={selectedDate} />
-        </HStack>
-      {/if}
-      <div class="w-full">
-        <Combobox
-          bind:selectedOption
-          label={$t('timezone')}
-          options={timezones}
-          placeholder={$t('search_timezone')}
-          onSelect={(option) => (lastSelectedTimezone = option as ZoneOption)}
-        ></Combobox>
-      </div>
-      <!-- <Card color="secondary" class={!showRelative || !currentInterval ? 'invisible' : ''}>
+    <Field label={$t('edit_date_and_time_by_offset')}>
+      <Switch data-testid="edit-by-offset-switch" bind:checked={showRelative} class="mb-2" />
+    </Field>
+    {#if showRelative}
+      <Label for="relativedatetime" class="block mb-1">{$t('offset')}</Label>
+      <DurationInput
+        class="immich-form-input w-full text-gray-700 mb-2"
+        id="relativedatetime"
+        bind:value={selectedDuration}
+      />
+    {:else}
+      <Label for="datetime" class="block mb-1">{$t('date_and_time')}</Label>
+      <DateInput class="immich-form-input w-full mb-2" id="datetime" type="datetime-local" bind:value={selectedDate} />
+    {/if}
+    <div class="w-full">
+      <Combobox
+        bind:selectedOption
+        label={$t('timezone')}
+        options={timezones}
+        placeholder={$t('search_timezone')}
+        onSelect={(option) => (lastSelectedTimezone = option as ZoneOption)}
+      ></Combobox>
+    </div>
+    <!-- <Card color="secondary" class={!showRelative || !currentInterval ? 'invisible' : ''}>
         <CardBody class="p-2">
           <div class="grid grid-cols-[auto_1fr] gap-x-4 gap-y-3 items-center">
             <div class="col-span-2 immich-form-label" data-testid="interval-preview">Preview</div>
@@ -121,7 +114,6 @@
           </div>
         </CardBody>
       </Card> -->
-    </VStack>
   </ModalBody>
   <ModalFooter>
     <HStack fullWidth>