fix: properly fix asset-viewer delete action, add tests (#25149)

Update timeline manager before nav, add e2e regression tests

e2e/src/mock-network/timeline-network.ts

@@ -1,3 +1,4 @@
+import { AssetResponseDto } from '@immich/sdk';
 import { BrowserContext, Page, Request, Route } from '@playwright/test';
 import { basename } from 'node:path';
 import {
@@ -63,15 +64,33 @@ export const setupTimelineMockApiRoutes = async (
   });
 
   await context.route('**/api/assets/*', async (route, request) => {
-    const url = new URL(request.url());
+    if (request.method() === 'GET') {
-    const pathname = url.pathname;
+      const url = new URL(request.url());
-    const assetId = basename(pathname);
+      const pathname = url.pathname;
-    const asset = getAsset(timelineRestData, assetId);
+      const assetId = basename(pathname);
-    return route.fulfill({
+      let asset = getAsset(timelineRestData, assetId);
-      status: 200,
+      if (changes.assetDeletions.includes(asset!.id)) {
-      contentType: 'application/json',
+        asset = {
-      json: asset,
+          ...asset,
-    });
+          isTrashed: true,
+        } as AssetResponseDto;
+      }
+      return route.fulfill({
+        status: 200,
+        contentType: 'application/json',
+        json: asset,
+      });
+    }
+    await route.fallback();
+  });
+
+  await context.route('**/api/assets', async (route, request) => {
+    if (request.method() === 'DELETE') {
+      return route.fulfill({
+        status: 204,
+      });
+    }
+    await route.fallback();
   });
 
   await context.route('**/api/assets/*/ocr', async (route) => {
@@ -117,17 +136,28 @@ export const setupTimelineMockApiRoutes = async (
   });
 
   await context.route('**/api/albums/**', async (route, request) => {
-    const pattern = /\/api\/albums\/(?<albumId>[^/?]+)/;
+    const albumsMatch = request.url().match(/\/api\/albums\/(?<albumId>[^/?]+)/);
-    const match = request.url().match(pattern);
+    if (albumsMatch) {
-    if (!match) {
+      const album = getAlbum(timelineRestData, testContext.adminId, albumsMatch.groups?.albumId, changes);
-      return route.continue();
+      return route.fulfill({
+        status: 200,
+        contentType: 'application/json',
+        json: album,
+      });
     }
-    const album = getAlbum(timelineRestData, testContext.adminId, match.groups?.albumId, changes);
+    return route.fallback();
-    return route.fulfill({
+  });
-      status: 200,
+
-      contentType: 'application/json',
+  await context.route('**/api/albums**', async (route, request) => {
-      json: album,
+    const allAlbums = request.url().match(/\/api\/albums\?assetId=(?<assetId>[^&]+)/);
-    });
+    if (allAlbums) {
+      return route.fulfill({
+        status: 200,
+        contentType: 'application/json',
+        json: [],
+      });
+    }
+    return route.fallback();
   });
 };
 

e2e/src/web/specs/asset-viewer/asset-viewer.parallel-e2e-spec.ts

@@ -0,0 +1,156 @@
+import { faker } from '@faker-js/faker';
+import { test } from '@playwright/test';
+import {
+  Changes,
+  createDefaultTimelineConfig,
+  generateTimelineData,
+  SeededRandom,
+  selectRandom,
+  TimelineAssetConfig,
+  TimelineData,
+} from 'src/generators/timeline';
+import { setupBaseMockApiRoutes } from 'src/mock-network/base-network';
+import { setupTimelineMockApiRoutes, TimelineTestContext } from 'src/mock-network/timeline-network';
+import { utils } from 'src/utils';
+import { assetViewerUtils, cancelAllPollers } from 'src/web/specs/timeline/utils';
+
+test.describe.configure({ mode: 'parallel' });
+test.describe('asset-viewer', () => {
+  const rng = new SeededRandom(529);
+  let adminUserId: string;
+  let timelineRestData: TimelineData;
+  const assets: TimelineAssetConfig[] = [];
+  const yearMonths: string[] = [];
+  const testContext = new TimelineTestContext();
+  const changes: Changes = {
+    albumAdditions: [],
+    assetDeletions: [],
+    assetArchivals: [],
+    assetFavorites: [],
+  };
+
+  test.beforeAll(async () => {
+    utils.initSdk();
+    adminUserId = faker.string.uuid();
+    testContext.adminId = adminUserId;
+    timelineRestData = generateTimelineData({ ...createDefaultTimelineConfig(), ownerId: adminUserId });
+    for (const timeBucket of timelineRestData.buckets.values()) {
+      assets.push(...timeBucket);
+    }
+    for (const yearMonth of timelineRestData.buckets.keys()) {
+      const [year, month] = yearMonth.split('-');
+      yearMonths.push(`${year}-${Number(month)}`);
+    }
+  });
+
+  test.beforeEach(async ({ context }) => {
+    await setupBaseMockApiRoutes(context, adminUserId);
+    await setupTimelineMockApiRoutes(context, timelineRestData, changes, testContext);
+  });
+
+  test.afterEach(() => {
+    cancelAllPollers();
+    testContext.slowBucket = false;
+    changes.albumAdditions = [];
+    changes.assetDeletions = [];
+    changes.assetArchivals = [];
+    changes.assetFavorites = [];
+  });
+
+  test.describe('/photos/:id', () => {
+    test('Delete photo advances to next', async ({ page }) => {
+      const asset = selectRandom(assets, rng);
+      await page.goto(`/photos/${asset.id}`);
+      await assetViewerUtils.waitForViewerLoad(page, asset);
+      await page.getByLabel('Delete').click();
+      const index = assets.indexOf(asset);
+      await assetViewerUtils.waitForViewerLoad(page, assets[index + 1]);
+    });
+    test('Delete photo advances to next (2x)', async ({ page }) => {
+      const asset = selectRandom(assets, rng);
+      await page.goto(`/photos/${asset.id}`);
+      await assetViewerUtils.waitForViewerLoad(page, asset);
+      await page.getByLabel('Delete').click();
+      const index = assets.indexOf(asset);
+      await assetViewerUtils.waitForViewerLoad(page, assets[index + 1]);
+      await page.getByLabel('Delete').click();
+      await assetViewerUtils.waitForViewerLoad(page, assets[index + 2]);
+    });
+    test('Delete last photo advances to prev', async ({ page }) => {
+      const asset = assets.at(-1)!;
+      await page.goto(`/photos/${asset.id}`);
+      await assetViewerUtils.waitForViewerLoad(page, asset);
+      await page.getByLabel('Delete').click();
+      const index = assets.indexOf(asset);
+      await assetViewerUtils.waitForViewerLoad(page, assets[index - 1]);
+    });
+    test('Delete last photo advances to prev (2x)', async ({ page }) => {
+      const asset = assets.at(-1)!;
+      await page.goto(`/photos/${asset.id}`);
+      await assetViewerUtils.waitForViewerLoad(page, asset);
+      await page.getByLabel('Delete').click();
+      const index = assets.indexOf(asset);
+      await assetViewerUtils.waitForViewerLoad(page, assets[index - 1]);
+      await page.getByLabel('Delete').click();
+      await assetViewerUtils.waitForViewerLoad(page, assets[index - 2]);
+    });
+  });
+  test.describe('/trash/photos/:id', () => {
+    test('Delete trashed photo advances to next', async ({ page }) => {
+      const asset = selectRandom(assets, rng);
+      const index = assets.indexOf(asset);
+      const deletedAssets = assets.slice(index - 10, index + 10).map((asset) => asset.id);
+      changes.assetDeletions.push(...deletedAssets);
+      await page.goto(`/trash/photos/${asset.id}`);
+      await assetViewerUtils.waitForViewerLoad(page, asset);
+      await page.getByLabel('Delete').click();
+      // confirm dialog
+      await page.getByRole('button').getByText('Delete').click();
+      await assetViewerUtils.waitForViewerLoad(page, assets[index + 1]);
+    });
+    test('Delete trashed photo advances to next 2x', async ({ page }) => {
+      const asset = selectRandom(assets, rng);
+      const index = assets.indexOf(asset);
+      const deletedAssets = assets.slice(index - 10, index + 10).map((asset) => asset.id);
+      changes.assetDeletions.push(...deletedAssets);
+      await page.goto(`/trash/photos/${asset.id}`);
+      await assetViewerUtils.waitForViewerLoad(page, asset);
+      await page.getByLabel('Delete').click();
+      // confirm dialog
+      await page.getByRole('button').getByText('Delete').click();
+      await assetViewerUtils.waitForViewerLoad(page, assets[index + 1]);
+      await page.getByLabel('Delete').click();
+      // confirm dialog
+      await page.getByRole('button').getByText('Delete').click();
+      await assetViewerUtils.waitForViewerLoad(page, assets[index + 2]);
+    });
+    test('Delete trashed photo advances to prev', async ({ page }) => {
+      const asset = selectRandom(assets, rng);
+      const index = assets.indexOf(asset);
+      const deletedAssets = assets.slice(index - 10, index + 10).map((asset) => asset.id);
+      changes.assetDeletions.push(...deletedAssets);
+      await page.goto(`/trash/photos/${assets[index + 9].id}`);
+      await assetViewerUtils.waitForViewerLoad(page, assets[index + 9]);
+      await page.getByLabel('Delete').click();
+      // confirm dialog
+      await page.getByRole('button').getByText('Delete').click();
+      await assetViewerUtils.waitForViewerLoad(page, assets[index + 8]);
+    });
+    test('Delete trashed photo advances to prev 2x', async ({ page }) => {
+      const asset = selectRandom(assets, rng);
+      const index = assets.indexOf(asset);
+      const deletedAssets = assets.slice(index - 10, index + 10).map((asset) => asset.id);
+      changes.assetDeletions.push(...deletedAssets);
+      await page.goto(`/trash/photos/${assets[index + 9].id}`);
+      await assetViewerUtils.waitForViewerLoad(page, assets[index + 9]);
+      await page.getByLabel('Delete').click();
+      // confirm dialog
+      await page.getByRole('button').getByText('Delete').click();
+      await assetViewerUtils.waitForViewerLoad(page, assets[index + 8]);
+      await page.getByLabel('Delete').click();
+      // confirm dialog
+      await page.getByRole('button').getByText('Delete').click();
+      await assetViewerUtils.waitForViewerLoad(page, assets[index + 7]);
+    });
+  });
+});

web/src/lib/components/timeline/TimelineAssetViewer.svelte

@@ -119,14 +119,15 @@
       case AssetAction.ARCHIVE:
       case AssetAction.SET_VISIBILITY_LOCKED:
       case AssetAction.SET_VISIBILITY_TIMELINE: {
+        // must update manager before performing any navigation
+        timelineManager.removeAssets([action.asset.id]);
+
         // find the next asset to show or close the viewer
         // eslint-disable-next-line @typescript-eslint/no-unused-expressions
         (await handleNavigateToAsset(assetCursor?.nextAsset)) ||
           (await handleNavigateToAsset(assetCursor?.previousAsset)) ||
           (await handleClose(action.asset));
 
-        // delete after find the next one
-        timelineManager.removeAssets([action.asset.id]);
         break;
       }
     }