From f42ea6e96cb73dbea03dc84a8afe7aa466e0707b Mon Sep 17 00:00:00 2001 From: pacwoodson Date: Fri, 12 Dec 2025 21:18:26 +0100 Subject: [PATCH 1/3] feat(server): remove descendant tag when untagging pictures --- server/src/repositories/tag.repository.ts | 11 +++++++++++ server/src/services/tag.service.spec.ts | 23 +++++++++++++++++++++++ server/src/services/tag.service.ts | 7 +++++++ 3 files changed, 41 insertions(+) diff --git a/server/src/repositories/tag.repository.ts b/server/src/repositories/tag.repository.ts index d4572886af..6674e323a7 100644 --- a/server/src/repositories/tag.repository.ts +++ b/server/src/repositories/tag.repository.ts @@ -128,6 +128,17 @@ export class TagRepository { await this.db.deleteFrom('tag_asset').where('tagId', '=', tagId).where('assetId', 'in', assetIds).execute(); } + @GenerateSql({ params: [DummyValue.UUID] }) + async getDescendantIds(tagId: string): Promise { + const results = await this.db + .selectFrom('tag_closure') + .select('id_descendant') + .where('id_ancestor', '=', tagId) + .execute(); + + return results.map(({ id_descendant }) => id_descendant); + } + @GenerateSql({ params: [[{ assetId: DummyValue.UUID, tagIds: DummyValue.UUID }]] }) @Chunked() upsertAssetIds(items: Insertable[]) { diff --git a/server/src/services/tag.service.spec.ts b/server/src/services/tag.service.spec.ts index 6bb92abd8c..38bc0b2616 100644 --- a/server/src/services/tag.service.spec.ts +++ b/server/src/services/tag.service.spec.ts @@ -249,6 +249,7 @@ describe(TagService.name, () => { it('should throw an error for an invalid id', async () => { mocks.tag.getAssetIds.mockResolvedValue(new Set()); mocks.tag.removeAssetIds.mockResolvedValue(); + mocks.tag.getDescendantIds.mockResolvedValue(['tag-1']); await expect(sut.removeAssets(authStub.admin, 'tag-1', { ids: ['asset-1'] })).resolves.toEqual([ { id: 'asset-1', success: false, error: 'not_found' }, @@ -259,6 +260,7 @@ describe(TagService.name, () => { mocks.tag.get.mockResolvedValue(tagStub.tag); mocks.tag.getAssetIds.mockResolvedValue(new Set(['asset-1'])); mocks.tag.removeAssetIds.mockResolvedValue(); + mocks.tag.getDescendantIds.mockResolvedValue(['tag-1']); await expect( sut.removeAssets(authStub.admin, 'tag-1', { @@ -272,6 +274,27 @@ describe(TagService.name, () => { expect(mocks.tag.getAssetIds).toHaveBeenCalledWith('tag-1', ['asset-1', 'asset-2']); expect(mocks.tag.removeAssetIds).toHaveBeenCalledWith('tag-1', ['asset-1']); }); + + it('should remove assets from parent tag and all child tags', async () => { + mocks.tag.get.mockResolvedValue(tagStub.tag); + mocks.tag.getAssetIds.mockResolvedValue(new Set(['asset-1', 'asset-2'])); + mocks.tag.removeAssetIds.mockResolvedValue(); + mocks.tag.getDescendantIds.mockResolvedValue(['tag-1', 'tag-child-1', 'tag-child-2']); + + await expect( + sut.removeAssets(authStub.admin, 'tag-1', { + ids: ['asset-1', 'asset-2'], + }), + ).resolves.toEqual([ + { id: 'asset-1', success: true }, + { id: 'asset-2', success: true }, + ]); + + expect(mocks.tag.getAssetIds).toHaveBeenCalledWith('tag-1', ['asset-1', 'asset-2']); + expect(mocks.tag.removeAssetIds).toHaveBeenCalledWith('tag-1', ['asset-1', 'asset-2']); + expect(mocks.tag.removeAssetIds).toHaveBeenCalledWith('tag-child-1', ['asset-1', 'asset-2']); + expect(mocks.tag.removeAssetIds).toHaveBeenCalledWith('tag-child-2', ['asset-1', 'asset-2']); + }); }); describe('handleTagCleanup', () => { diff --git a/server/src/services/tag.service.ts b/server/src/services/tag.service.ts index 3ee5d29b75..1f18afd24f 100644 --- a/server/src/services/tag.service.ts +++ b/server/src/services/tag.service.ts @@ -123,6 +123,13 @@ export class TagService extends BaseService { { parentId: id, assetIds: dto.ids, canAlwaysRemove: Permission.TagDelete }, ); + const descendantTagIds = await this.tagRepository.getDescendantIds(id); + for (const descendantTagId of descendantTagIds) { + if (descendantTagId !== id) { + await this.tagRepository.removeAssetIds(descendantTagId, dto.ids); + } + } + for (const { id: assetId, success } of results) { if (success) { await this.eventRepository.emit('AssetUntag', { assetId }); From 04261c77a79df3868e68cd67029308f2c71811d1 Mon Sep 17 00:00:00 2001 From: pacwoodson Date: Sat, 13 Dec 2025 09:58:16 +0100 Subject: [PATCH 2/3] optional untag descendants --- open-api/immich-openapi-specs.json | 8 ++++++++ open-api/typescript-sdk/src/fetch-client.ts | 7 +++++-- server/src/controllers/tag.controller.ts | 6 ++++-- server/src/dtos/tag.dto.ts | 7 ++++++- server/src/services/tag.service.ts | 12 +++++++----- 5 files changed, 30 insertions(+), 10 deletions(-) diff --git a/open-api/immich-openapi-specs.json b/open-api/immich-openapi-specs.json index e21cf27beb..638dc21613 100644 --- a/open-api/immich-openapi-specs.json +++ b/open-api/immich-openapi-specs.json @@ -12455,6 +12455,14 @@ "format": "uuid", "type": "string" } + }, + { + "name": "untagDescendants", + "required": false, + "in": "query", + "schema": { + "type": "boolean" + } } ], "requestBody": { diff --git a/open-api/typescript-sdk/src/fetch-client.ts b/open-api/typescript-sdk/src/fetch-client.ts index 7afee42e2c..50423c661c 100644 --- a/open-api/typescript-sdk/src/fetch-client.ts +++ b/open-api/typescript-sdk/src/fetch-client.ts @@ -4695,14 +4695,17 @@ export function updateTag({ id, tagUpdateDto }: { /** * Untag assets */ -export function untagAssets({ id, bulkIdsDto }: { +export function untagAssets({ id, untagDescendants, bulkIdsDto }: { id: string; + untagDescendants?: boolean; bulkIdsDto: BulkIdsDto; }, opts?: Oazapfts.RequestOpts) { return oazapfts.ok(oazapfts.fetchJson<{ status: 200; data: BulkIdResponseDto[]; - }>(`/tags/${encodeURIComponent(id)}/assets`, oazapfts.json({ + }>(`/tags/${encodeURIComponent(id)}/assets${QS.query(QS.explode({ + untagDescendants + }))}`, oazapfts.json({ ...opts, method: "DELETE", body: bulkIdsDto diff --git a/server/src/controllers/tag.controller.ts b/server/src/controllers/tag.controller.ts index 101e89f3a5..a93832b728 100644 --- a/server/src/controllers/tag.controller.ts +++ b/server/src/controllers/tag.controller.ts @@ -1,4 +1,4 @@ -import { Body, Controller, Delete, Get, HttpCode, HttpStatus, Param, Post, Put } from '@nestjs/common'; +import { Body, Controller, Delete, Get, HttpCode, HttpStatus, Param, Query, Post, Put } from '@nestjs/common'; import { ApiTags } from '@nestjs/swagger'; import { Endpoint, HistoryBuilder } from 'src/decorators'; import { BulkIdResponseDto, BulkIdsDto } from 'src/dtos/asset-ids.response.dto'; @@ -10,6 +10,7 @@ import { TagResponseDto, TagUpdateDto, TagUpsertDto, + UntagAssetsOptionsDto, } from 'src/dtos/tag.dto'; import { ApiTag, Permission } from 'src/enum'; import { Auth, Authenticated } from 'src/middleware/auth.guard'; @@ -125,7 +126,8 @@ export class TagController { @Auth() auth: AuthDto, @Body() dto: BulkIdsDto, @Param() { id }: UUIDParamDto, + @Query() { untagDescendants }: UntagAssetsOptionsDto, ): Promise { - return this.service.removeAssets(auth, id, dto); + return this.service.removeAssets(auth, id, dto, untagDescendants); } } diff --git a/server/src/dtos/tag.dto.ts b/server/src/dtos/tag.dto.ts index a35801d07e..ab92632e20 100644 --- a/server/src/dtos/tag.dto.ts +++ b/server/src/dtos/tag.dto.ts @@ -1,5 +1,5 @@ import { ApiProperty } from '@nestjs/swagger'; -import { IsHexColor, IsNotEmpty, IsString } from 'class-validator'; +import { IsHexColor, IsNotEmpty, IsString, IsBoolean } from 'class-validator'; import { Tag } from 'src/database'; import { Optional, ValidateHexColor, ValidateUUID } from 'src/validation'; @@ -41,6 +41,11 @@ export class TagBulkAssetsResponseDto { count!: number; } +export class UntagAssetsOptionsDto { + @IsBoolean() + untagDescendants?: boolean; +} + export class TagResponseDto { id!: string; parentId?: string; diff --git a/server/src/services/tag.service.ts b/server/src/services/tag.service.ts index 1f18afd24f..86007896e9 100644 --- a/server/src/services/tag.service.ts +++ b/server/src/services/tag.service.ts @@ -114,7 +114,7 @@ export class TagService extends BaseService { return results; } - async removeAssets(auth: AuthDto, id: string, dto: BulkIdsDto): Promise { + async removeAssets(auth: AuthDto, id: string, dto: BulkIdsDto, untagDescendants?: boolean): Promise { await this.requireAccess({ auth, permission: Permission.TagAsset, ids: [id] }); const results = await removeAssets( @@ -123,10 +123,12 @@ export class TagService extends BaseService { { parentId: id, assetIds: dto.ids, canAlwaysRemove: Permission.TagDelete }, ); - const descendantTagIds = await this.tagRepository.getDescendantIds(id); - for (const descendantTagId of descendantTagIds) { - if (descendantTagId !== id) { - await this.tagRepository.removeAssetIds(descendantTagId, dto.ids); + if(untagDescendants) { + const descendantTagIds = await this.tagRepository.getDescendantIds(id); + for (const descendantTagId of descendantTagIds) { + if (descendantTagId !== id) { + await this.tagRepository.removeAssetIds(descendantTagId, dto.ids); + } } } From febe10dc937ed86312edcf30e0a809723d439fcb Mon Sep 17 00:00:00 2001 From: pacwoodson Date: Sat, 13 Dec 2025 10:03:07 +0100 Subject: [PATCH 3/3] tests --- server/src/services/tag.service.spec.ts | 32 ++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/server/src/services/tag.service.spec.ts b/server/src/services/tag.service.spec.ts index 38bc0b2616..8bdab4e4fc 100644 --- a/server/src/services/tag.service.spec.ts +++ b/server/src/services/tag.service.spec.ts @@ -249,7 +249,6 @@ describe(TagService.name, () => { it('should throw an error for an invalid id', async () => { mocks.tag.getAssetIds.mockResolvedValue(new Set()); mocks.tag.removeAssetIds.mockResolvedValue(); - mocks.tag.getDescendantIds.mockResolvedValue(['tag-1']); await expect(sut.removeAssets(authStub.admin, 'tag-1', { ids: ['asset-1'] })).resolves.toEqual([ { id: 'asset-1', success: false, error: 'not_found' }, @@ -260,7 +259,6 @@ describe(TagService.name, () => { mocks.tag.get.mockResolvedValue(tagStub.tag); mocks.tag.getAssetIds.mockResolvedValue(new Set(['asset-1'])); mocks.tag.removeAssetIds.mockResolvedValue(); - mocks.tag.getDescendantIds.mockResolvedValue(['tag-1']); await expect( sut.removeAssets(authStub.admin, 'tag-1', { @@ -275,7 +273,7 @@ describe(TagService.name, () => { expect(mocks.tag.removeAssetIds).toHaveBeenCalledWith('tag-1', ['asset-1']); }); - it('should remove assets from parent tag and all child tags', async () => { + it('should remove assets from parent tag but not child tags', async () => { mocks.tag.get.mockResolvedValue(tagStub.tag); mocks.tag.getAssetIds.mockResolvedValue(new Set(['asset-1', 'asset-2'])); mocks.tag.removeAssetIds.mockResolvedValue(); @@ -292,6 +290,34 @@ describe(TagService.name, () => { expect(mocks.tag.getAssetIds).toHaveBeenCalledWith('tag-1', ['asset-1', 'asset-2']); expect(mocks.tag.removeAssetIds).toHaveBeenCalledWith('tag-1', ['asset-1', 'asset-2']); + expect(mocks.tag.getDescendantIds).not.toHaveBeenCalled(); + expect(mocks.tag.removeAssetIds).not.toHaveBeenCalledWith('tag-child-1', ['asset-1', 'asset-2']); + expect(mocks.tag.removeAssetIds).not.toHaveBeenCalledWith('tag-child-2', ['asset-1', 'asset-2']); + }); + + it('should remove assets from parent tag and all child tags when asked', async () => { + mocks.tag.get.mockResolvedValue(tagStub.tag); + mocks.tag.getAssetIds.mockResolvedValue(new Set(['asset-1', 'asset-2'])); + mocks.tag.removeAssetIds.mockResolvedValue(); + mocks.tag.getDescendantIds.mockResolvedValue(['tag-1', 'tag-child-1', 'tag-child-2']); + + await expect( + sut.removeAssets( + authStub.admin, + 'tag-1', + { + ids: ['asset-1', 'asset-2'], + }, + true, + ), + ).resolves.toEqual([ + { id: 'asset-1', success: true }, + { id: 'asset-2', success: true }, + ]); + + expect(mocks.tag.getAssetIds).toHaveBeenCalledWith('tag-1', ['asset-1', 'asset-2']); + expect(mocks.tag.getDescendantIds).toHaveBeenCalledWith('tag-1'); + expect(mocks.tag.removeAssetIds).toHaveBeenCalledWith('tag-1', ['asset-1', 'asset-2']); expect(mocks.tag.removeAssetIds).toHaveBeenCalledWith('tag-child-1', ['asset-1', 'asset-2']); expect(mocks.tag.removeAssetIds).toHaveBeenCalledWith('tag-child-2', ['asset-1', 'asset-2']); });