fix(server): deleting stacked assets

.devcontainer/devcontainer.json

@@ -26,81 +26,7 @@
         "vitest.explorer",
         "ms-playwright.playwright",
         "ms-azuretools.vscode-docker"
-      ],
-      "settings": {
-        "tasks": {
-          "version": "2.0.0",
-          "tasks": [
-            {
-              "label": "Fix Permissions, Install Dependencies",
-              "type": "shell",
-              "command": "[ -f /immich-devcontainer/container-start.sh ] && /immich-devcontainer/container-start.sh || exit 0",
-              "isBackground": true,
-              "presentation": {
-                "echo": true,
-                "reveal": "always",
-                "focus": false,
-                "panel": "dedicated",
-                "showReuseMessage": true,
-                "clear": false,
-                "group": "Devcontainer tasks",
-                "close": true
-              },
-              "runOptions": {
-                "runOn": "default"
-              },
-              "problemMatcher": []
-            },
-            {
-              "label": "Immich API Server (Nest)",
-              "dependsOn": ["Fix Permissions, Install Dependencies"],
-              "type": "shell",
-              "command": "[ -f /immich-devcontainer/container-start-backend.sh ] && /immich-devcontainer/container-start-backend.sh || exit 0",
-              "isBackground": true,
-              "presentation": {
-                "echo": true,
-                "reveal": "always",
-                "focus": false,
-                "panel": "dedicated",
-                "showReuseMessage": true,
-                "clear": false,
-                "group": "Devcontainer tasks",
-                "close": true
-              },
-              "runOptions": {
-                "runOn": "folderOpen"
-              },
-              "problemMatcher": []
-            },
-            {
-              "label": "Immich Web Server (Vite)",
-              "dependsOn": ["Fix Permissions, Install Dependencies"],
-              "type": "shell",
-              "command": "[ -f /immich-devcontainer/container-start-frontend.sh ] && /immich-devcontainer/container-start-frontend.sh || exit 0",
-              "isBackground": true,
-              "presentation": {
-                "echo": true,
-                "reveal": "always",
-                "focus": false,
-                "panel": "dedicated",
-                "showReuseMessage": true,
-                "clear": false,
-                "group": "Devcontainer tasks",
-                "close": true
-              },
-              "runOptions": {
-                "runOn": "folderOpen"
-              },
-              "problemMatcher": []
-            },
-            {
-              "label": "Build Immich CLI",
-              "type": "shell",
-              "command": "pnpm --filter cli build:dev"
-            }
-          ]
-        }
-      }
+      ]
     }
   },
   "features": {

.vscode/tasks.json

@@ -0,0 +1,80 @@
+{
+  "version": "2.0.0",
+  "tasks": [
+    {
+      "label": "Fix Permissions, Install Dependencies",
+      "type": "shell",
+      "command": "[ -f /immich-devcontainer/container-start.sh ] && /immich-devcontainer/container-start.sh || exit 0",
+      "isBackground": true,
+      "presentation": {
+        "echo": true,
+        "reveal": "always",
+        "focus": false,
+        "panel": "dedicated",
+        "showReuseMessage": true,
+        "clear": false,
+        "group": "Devcontainer tasks",
+        "close": true
+      },
+      "runOptions": {
+        "runOn": "default"
+      },
+      "problemMatcher": []
+    },
+    {
+      "label": "Immich API Server (Nest)",
+      "dependsOn": ["Fix Permissions, Install Dependencies"],
+      "type": "shell",
+      "command": "[ -f /immich-devcontainer/container-start-backend.sh ] && /immich-devcontainer/container-start-backend.sh || exit 0",
+      "isBackground": true,
+      "presentation": {
+        "echo": true,
+        "reveal": "always",
+        "focus": false,
+        "panel": "dedicated",
+        "showReuseMessage": true,
+        "clear": false,
+        "group": "Devcontainer tasks",
+        "close": true
+      },
+      "runOptions": {
+        "runOn": "default"
+      },
+      "problemMatcher": []
+    },
+    {
+      "label": "Immich Web Server (Vite)",
+      "dependsOn": ["Fix Permissions, Install Dependencies"],
+      "type": "shell",
+      "command": "[ -f /immich-devcontainer/container-start-frontend.sh ] && /immich-devcontainer/container-start-frontend.sh || exit 0",
+      "isBackground": true,
+      "presentation": {
+        "echo": true,
+        "reveal": "always",
+        "focus": false,
+        "panel": "dedicated",
+        "showReuseMessage": true,
+        "clear": false,
+        "group": "Devcontainer tasks",
+        "close": true
+      },
+      "runOptions": {
+        "runOn": "default"
+      },
+      "problemMatcher": []
+    },
+    {
+      "label": "Immich Server and Web",
+      "dependsOn": ["Immich Web Server (Vite)", "Immich API Server (Nest)"],
+      "runOptions": {
+        "runOn": "folderOpen"
+      },
+      "problemMatcher": []
+    },
+    {
+      "label": "Build Immich CLI",
+      "type": "shell",
+      "command": "pnpm --filter cli build:dev"
+    }
+  ]
+}

server/src/database.ts

@@ -3,6 +3,7 @@ import { MapAsset } from 'src/dtos/asset-response.dto';
 import {
   AlbumUserRole,
   AssetFileType,
+  AssetStatus,
   AssetType,
   AssetVisibility,
   MemoryType,
@@ -126,6 +127,11 @@ export type Asset = {
   type: AssetType;
 };
 
+/** alternative type that include additional, non-standard columns (useful when using selectAll) */
+export type Asset2 = Asset & {
+  status: AssetStatus;
+};
+
 export type User = {
   id: string;
   name: string;

server/src/queries/asset.job.repository.sql

@@ -483,14 +483,12 @@ from
     from
       "asset" as "stacked"
     where
-      "stacked"."deletedAt" is not null
-      and "stacked"."visibility" = $1
-      and "stacked"."stackId" = "stack"."id"
+      "stacked"."stackId" = "stack"."id"
     group by
       "stack"."id"
   ) as "stacked_assets" on "stack"."id" is not null
 where
-  "asset"."id" = $2
+  "asset"."id" = $1
 
 -- AssetJobRepository.streamForVideoConversion
 select

server/src/repositories/asset-job.repository.ts

@@ -2,7 +2,7 @@ import { Injectable } from '@nestjs/common';
 import { Kysely } from 'kysely';
 import { jsonArrayFrom } from 'kysely/helpers/postgres';
 import { InjectKysely } from 'nestjs-kysely';
-import { Asset, columns } from 'src/database';
+import { Asset2, columns } from 'src/database';
 import { DummyValue, GenerateSql } from 'src/decorators';
 import { AssetFileType, AssetType, AssetVisibility } from 'src/enum';
 import { DB } from 'src/schema';
@@ -277,9 +277,7 @@ export class AssetJobRepository {
           eb
             .selectFrom('asset as stacked')
             .select(['stack.id', 'stack.primaryAssetId'])
-            .select((eb) => eb.fn<Asset[]>('array_agg', [eb.table('stacked')]).as('assets'))
-            .where('stacked.deletedAt', 'is not', null)
-            .where('stacked.visibility', '=', AssetVisibility.Timeline)
+            .select((eb) => eb.fn<Asset2[]>('array_agg', [eb.table('stacked')]).as('assets'))
             .whereRef('stacked.stackId', '=', 'stack.id')
             .groupBy('stack.id')
             .as('stacked_assets'),

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

@@ -591,18 +591,6 @@ describe(AssetService.name, () => {
       expect(mocks.asset.remove).toHaveBeenCalledWith(assetWithFace);
     });
 
-    it('should update stack primary asset if deleted asset was primary asset in a stack', async () => {
-      mocks.stack.update.mockResolvedValue(factory.stack() as any);
-      mocks.assetJob.getForAssetDeletion.mockResolvedValue(assetStub.primaryImage);
-
-      await sut.handleAssetDeletion({ id: assetStub.primaryImage.id, deleteOnDisk: true });
-
-      expect(mocks.stack.update).toHaveBeenCalledWith('stack-1', {
-        id: 'stack-1',
-        primaryAssetId: 'stack-child-asset-1',
-      });
-    });
-
     it('should delete the entire stack if deleted asset was the primary asset and the stack would only contain one asset afterwards', async () => {
       mocks.stack.delete.mockResolvedValue();
       mocks.assetJob.getForAssetDeletion.mockResolvedValue({

server/src/services/asset.service.ts

@@ -329,7 +329,14 @@ export class AssetService extends BaseService {
 
     // Replace the parent of the stack children with a new asset
     if (asset.stack?.primaryAssetId === id) {
-      const stackAssetIds = asset.stack?.assets.map((a) => a.id) ?? [];
+      console.log(asset.stack.assets[0].status);
+      const stackAssetIds =
+        asset.stack?.assets
+          // ignore assets about to be deleted
+          .filter((asset) => asset.status !== AssetStatus.Deleted)
+          // stacks are only on the timeline right now
+          .filter((asset) => asset.visibility === AssetVisibility.Timeline)
+          .map((a) => a.id) ?? [];
       if (stackAssetIds.length > 2) {
         const newPrimaryAssetId = stackAssetIds.find((a) => a !== id)!;
         await this.stackRepository.update(asset.stack.id, {

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

@@ -1,5 +1,5 @@
 import { Kysely } from 'kysely';
-import { AssetFileType, AssetMetadataKey, JobName, SharedLinkType } from 'src/enum';
+import { AssetFileType, AssetMetadataKey, AssetStatus, JobName, SharedLinkType } from 'src/enum';
 import { AccessRepository } from 'src/repositories/access.repository';
 import { AlbumRepository } from 'src/repositories/album.repository';
 import { AssetJobRepository } from 'src/repositories/asset-job.repository';
@@ -246,6 +246,71 @@ describe(AssetService.name, () => {
       });
     });
 
+    it('should delete a stacked primary asset (2 assets)', async () => {
+      const { sut, ctx } = setup();
+      ctx.getMock(EventRepository).emit.mockResolvedValue();
+      ctx.getMock(JobRepository).queue.mockResolvedValue();
+      const { user } = await ctx.newUser();
+      const { asset: asset1 } = await ctx.newAsset({ ownerId: user.id });
+      const { asset: asset2 } = await ctx.newAsset({ ownerId: user.id });
+      const { stack, result } = await ctx.newStack({ ownerId: user.id }, [asset1.id, asset2.id]);
+
+      const stackRepo = ctx.get(StackRepository);
+
+      expect(result).toMatchObject({ primaryAssetId: asset1.id });
+
+      await sut.handleAssetDeletion({ id: asset1.id, deleteOnDisk: true });
+
+      // stack is deleted as well
+      await expect(stackRepo.getById(stack.id)).resolves.toBe(undefined);
+    });
+
+    it('should delete a stacked primary asset (3 assets)', async () => {
+      const { sut, ctx } = setup();
+      ctx.getMock(EventRepository).emit.mockResolvedValue();
+      ctx.getMock(JobRepository).queue.mockResolvedValue();
+      const { user } = await ctx.newUser();
+      const { asset: asset1 } = await ctx.newAsset({ ownerId: user.id });
+      const { asset: asset2 } = await ctx.newAsset({ ownerId: user.id });
+      const { asset: asset3 } = await ctx.newAsset({ ownerId: user.id });
+      const { stack, result } = await ctx.newStack({ ownerId: user.id }, [asset1.id, asset2.id, asset3.id]);
+
+      const stackRepo = ctx.get(StackRepository);
+
+      expect(result).toMatchObject({ primaryAssetId: asset1.id });
+
+      await sut.handleAssetDeletion({ id: asset1.id, deleteOnDisk: true });
+
+      // new primary asset is picked
+      await expect(stackRepo.getById(stack.id)).resolves.toMatchObject({ primaryAssetId: asset2.id });
+    });
+
+    it('should delete a stacked primary asset (3 trashed assets)', async () => {
+      const { sut, ctx } = setup();
+      ctx.getMock(EventRepository).emit.mockResolvedValue();
+      ctx.getMock(JobRepository).queue.mockResolvedValue();
+      const { user } = await ctx.newUser();
+      const { asset: asset1 } = await ctx.newAsset({ ownerId: user.id });
+      const { asset: asset2 } = await ctx.newAsset({ ownerId: user.id });
+      const { asset: asset3 } = await ctx.newAsset({ ownerId: user.id });
+      const { stack, result } = await ctx.newStack({ ownerId: user.id }, [asset1.id, asset2.id, asset3.id]);
+
+      const assetRepo = ctx.get(AssetRepository);
+      const stackRepo = ctx.get(StackRepository);
+
+      await assetRepo.updateAll([asset1.id, asset2.id, asset3.id], {
+        deletedAt: new Date(),
+        status: AssetStatus.Deleted,
+      });
+
+      expect(result).toMatchObject({ primaryAssetId: asset1.id });
+
+      await sut.handleAssetDeletion({ id: asset1.id, deleteOnDisk: true });
+
+      // stack is deleted as well
+      await expect(stackRepo.getById(stack.id)).resolves.toBe(undefined);
+    });
+
     it('should not delete offline assets', async () => {
       const { sut, ctx } = setup();
       ctx.getMock(EventRepository).emit.mockResolvedValue();