diff --git a/server/src/database.ts b/server/src/database.ts index dd979fdea6..cf9ff5677a 100644 --- a/server/src/database.ts +++ b/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; diff --git a/server/src/queries/asset.job.repository.sql b/server/src/queries/asset.job.repository.sql index 50f2c193fc..a2e131f041 100644 --- a/server/src/queries/asset.job.repository.sql +++ b/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 diff --git a/server/src/repositories/asset-job.repository.ts b/server/src/repositories/asset-job.repository.ts index 1608f7b6f6..677c0349f8 100644 --- a/server/src/repositories/asset-job.repository.ts +++ b/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('array_agg', [eb.table('stacked')]).as('assets')) - .where('stacked.deletedAt', 'is not', null) - .where('stacked.visibility', '=', AssetVisibility.Timeline) + .select((eb) => eb.fn('array_agg', [eb.table('stacked')]).as('assets')) .whereRef('stacked.stackId', '=', 'stack.id') .groupBy('stack.id') .as('stacked_assets'), diff --git a/server/src/services/asset.service.spec.ts b/server/src/services/asset.service.spec.ts index eca49bc14e..5188c8e6d6 100755 --- a/server/src/services/asset.service.spec.ts +++ b/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({ diff --git a/server/src/services/asset.service.ts b/server/src/services/asset.service.ts index 066084ed45..9780c8b969 100644 --- a/server/src/services/asset.service.ts +++ b/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, { diff --git a/server/test/medium/specs/services/asset.service.spec.ts b/server/test/medium/specs/services/asset.service.spec.ts index d0949c153c..aad54464f2 100644 --- a/server/test/medium/specs/services/asset.service.spec.ts +++ b/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();