feat(web): tag renaming v2 (#27909)

* implement updated rename tag feature

* Fix tag update handler

* Ensure toast displays updated tag name

* Ensure toast displays updated tag name

* Handle updates to tag closure table, prevent slash in API filename, test case updates

* delete child tags on parent delete

* Update frontend for better handling of nested tags

* pr checklist fixes

* fix unit test for tag controller

* Hide path for top-level tags

* regenerate SQL files

* name updates optional for tag update operations

* e2e tests for new tag api changes

* changes from code review, additional test coverage

* test fixes, web component refactor

* test assets

* test asset updates

* open-api rebuild

* update unit tests

* fix tag dto tests

* Update web/src/lib/modals/TagEditModal.svelte

Fix missing color bug

Co-authored-by: Ray <3608878+Pecacheu@users.noreply.github.com>

* fix tag e2e test

* remove closure table bug fixes

* remove nullish from tag name update schema

* regenerate openapi and sql

* fix test and e2e test

* simplify tag repository update process

* minor fix and sql regenerate

* fix tests, regen open-api, update dto

* Remove unused function

* Remove accidental test asset ref

* re-add test assets

* pin submodule to main branch commit

* Update test cases based on feedback

* Server fixes based on feedback

* regenerate sql

---------

Co-authored-by: Ray <3608878+Pecacheu@users.noreply.github.com>

i18n/en.json

@@ -807,7 +807,7 @@
   "create_shared_album_page_share_select_photos": "Select Photos",
   "create_shared_link": "Create shared link",
   "create_tag": "Create tag",
-  "create_tag_description": "Create a new tag. For nested tags, please enter the full path of the tag including forward slashes.",
+  "create_tag_description": "Create a new tag. For nested tags, the full path where your new tag will be placed is printed below.",
   "create_user": "Create user",
   "create_workflow": "Create workflow",
   "created": "Created",
@@ -2128,6 +2128,8 @@
   "tag_created": "Created tag: {tag}",
   "tag_face": "Tag face",
   "tag_feature_description": "Browsing photos and videos grouped by logical tag topics",
+  "tag_full_path": "Full path: {tag}",
+  "tag_not_found_question": "Cannot find a tag? <link>Create a new tag.</link>",
   "tag_people": "Tag People",
   "tag_plus_more_tags": "{tag} + {count, plural, one {# tag} other {# tags}}",
   "tag_updated": "Updated tag: {tag}",

open-api/immich-openapi-specs.json

@@ -28192,6 +28192,11 @@
             "nullable": true,
             "pattern": "^#?([0-9A-Fa-f]{3}|[0-9A-Fa-f]{4}|[0-9A-Fa-f]{6}|[0-9A-Fa-f]{8})$",
             "type": "string"
+          },
+          "name": {
+            "description": "Tag name",
+            "pattern": "^[^/]*$",
+            "type": "string"
           }
         },
         "type": "object"

packages/sdk/src/fetch-client.ts

@@ -2876,6 +2876,8 @@ export type TagBulkAssetsResponseDto = {
 export type TagUpdateDto = {
     /** Tag color (hex) */
     color?: string | null;
+    /** Tag name */
+    name?: string;
 };
 export type TimeBucketAssetResponseDto = {
     /** Array of city names extracted from EXIF GPS data */

server/src/controllers/tag.controller.spec.ts

@@ -2,6 +2,7 @@ import { TagController } from 'src/controllers/tag.controller';
 import { TagService } from 'src/services/tag.service';
 import request from 'supertest';
 import { errorDto } from 'test/medium/responses';
+import { factory } from 'test/small.factory';
 import { ControllerContext, controllerSetup, mockBaseService } from 'test/utils';
 
 describe(TagController.name, () => {
@@ -33,6 +34,46 @@ describe(TagController.name, () => {
     });
   });
 
+  describe('PUT /tags/:id', () => {
+    it('should require a valid uuid', async () => {
+      const { status, body } = await request(ctx.getHttpServer())
+        .put(`/tags/123`)
+        .send({ name: 'tag', color: '#000000' });
+      expect(status).toBe(400);
+      expect(body).toEqual(errorDto.validationError([{ path: ['id'], message: 'Invalid UUID' }]));
+    });
+    it('should require a valid color', async () => {
+      const { status, body } = await request(ctx.getHttpServer())
+        .put(`/tags/${factory.uuid()}`)
+        .send({ name: 'tag', color: 'invalid-color' });
+      expect(status).toBe(400);
+      expect(body).toEqual(
+        errorDto.validationError([
+          {
+            path: ['color'],
+            message:
+              'Invalid string: must match pattern /^#?([0-9A-Fa-f]{3}|[0-9A-Fa-f]{4}|[0-9A-Fa-f]{6}|[0-9A-Fa-f]{8})$/',
+          },
+        ]),
+      );
+    });
+    it('should throw an error if a slash is in tag name', async () => {
+      const { status, body } = await request(ctx.getHttpServer())
+        .put(`/tags/${factory.uuid()}`)
+        .send({ name: 'tagA/tagB', color: '#000000' });
+      expect(status).toBe(400);
+      expect(body).toEqual(
+        errorDto.validationError([{ path: ['name'], message: 'Tag name cannot contain slash characters ("/")' }]),
+      );
+    });
+    it('should accept a null color', async () => {
+      const { status } = await request(ctx.getHttpServer())
+        .put(`/tags/${factory.uuid()}`)
+        .send({ name: 'tagA', color: null });
+      expect(status).toBe(200);
+    });
+  });
+
   describe('DELETE /tags/:id', () => {
     it('should require a valid uuid', async () => {
       const { status, body } = await request(ctx.getHttpServer()).delete(`/tags/123`);

server/src/dtos/tag.dto.ts

@@ -13,8 +13,13 @@ const TagCreateSchema = z
   })
   .meta({ id: 'TagCreateDto' });
 
-const TagUpdateSchema = z
+export const TagUpdateSchema = z
   .object({
+    name: z
+      .string()
+      .regex(/^[^/]*$/, `Tag name cannot contain slash characters ("/")`)
+      .optional()
+      .describe('Tag name'),
     color: hexColor.nullable().optional().describe('Tag color (hex)'),
   })
   .meta({ id: 'TagUpdateDto' });

server/src/queries/tag.repository.sql

@@ -69,13 +69,22 @@ returning
   *
 
 -- TagRepository.update
+begin
+select
+  "value"
+from
+  "tag"
+where
+  "id" = $1
 update "tag"
 set
-  "color" = $1
+  "value" = $1,
+  "color" = $2
 where
-  "id" = $2
+  "id" = $3
 returning
   *
+rollback
 
 -- TagRepository.delete
 delete from "tag"

server/src/repositories/tag.repository.ts

@@ -78,9 +78,68 @@ export class TagRepository {
     return this.db.insertInto('tag').values(tag).returningAll().executeTakeFirstOrThrow();
   }
 
-  @GenerateSql({ params: [DummyValue.UUID, { color: DummyValue.STRING }] })
-  update(id: string, dto: Updateable<TagTable>) {
-    return this.db.updateTable('tag').set(dto).where('id', '=', id).returningAll().executeTakeFirstOrThrow();
+  @GenerateSql({ params: [DummyValue.UUID, { value: DummyValue.STRING, color: DummyValue.STRING }] })
+  async update(id: string, dto: Updateable<TagTable>) {
+    return this.db.transaction().execute(async (tx) => {
+      // Get previous tag value for reference if the current update contains a new value
+      const previousTag =
+        dto.value === undefined
+          ? undefined
+          : await tx.selectFrom('tag').select('value').where('id', '=', id).executeTakeFirst();
+
+      // Perform main tag update
+      const updated = await tx
+        .updateTable('tag')
+        .set(dto)
+        .where('id', '=', id)
+        .returningAll()
+        .executeTakeFirstOrThrow();
+
+      // Check if value has changed, trigger value updates on all children if so
+      if (previousTag && dto.value !== previousTag.value) {
+        await tx
+          // Use a recursive cte to get all levels of nested child tags that need to be updated
+          .withRecursive('descendants(id, value)', (qb) => {
+            const directChildren = qb
+              .selectFrom('tag as child')
+              .select((eb) => [
+                'child.id as id',
+                eb
+                  .fn<string>('concat', [
+                    eb.cast<string>(eb.val(updated.value), 'text'),
+                    eb.cast<string>(eb.val('/'), 'text'),
+                    eb.fn<string>('regexp_replace', ['child.value', eb.val('^.*/'), eb.val('')]),
+                  ])
+                  .as('value'),
+              ])
+              .where('child.parentId', '=', id);
+
+            const nestedChildren = qb
+              .selectFrom('tag as child')
+              .innerJoin('descendants as parent', 'parent.id', 'child.parentId')
+              .select((eb) => [
+                'child.id as id',
+                eb
+                  .fn<string>('concat', [
+                    'parent.value',
+                    eb.cast<string>(eb.val('/'), 'text'),
+                    eb.fn<string>('regexp_replace', ['child.value', eb.val('^.*/'), eb.val('')]),
+                  ])
+                  .as('value'),
+              ]);
+
+            return directChildren.unionAll(nestedChildren);
+          })
+          .updateTable('tag')
+          .from('descendants')
+          .set((eb) => ({
+            value: eb.ref('descendants.value'),
+          }))
+          .whereRef('tag.id', '=', 'descendants.id')
+          .execute();
+      }
+      return updated;
+    });
   }
 
   @GenerateSql({ params: [DummyValue.UUID] })

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

@@ -104,7 +104,15 @@ describe(TagService.name, () => {
   describe('update', () => {
     it('should throw an error for no update permission', async () => {
       mocks.access.tag.checkOwnerAccess.mockResolvedValue(new Set());
-      await expect(sut.update(authStub.admin, 'tag-1', { color: '#000000' })).rejects.toBeInstanceOf(
+      await expect(sut.update(authStub.admin, 'tag-1', { name: 'tag', color: '#000000' })).rejects.toBeInstanceOf(
+        BadRequestException,
+      );
+      expect(mocks.tag.update).not.toHaveBeenCalled();
+    });
+
+    it('should throw an error if updated tag name has a slash', async () => {
+      mocks.access.tag.checkOwnerAccess.mockResolvedValue(new Set(['tag-parent']));
+      await expect(sut.update(authStub.admin, 'tag-1', { name: 'tag/test2', color: '#000000' })).rejects.toBeInstanceOf(
         BadRequestException,
       );
       expect(mocks.tag.update).not.toHaveBeenCalled();
@@ -113,8 +121,11 @@ describe(TagService.name, () => {
     it('should update a tag', async () => {
       mocks.access.tag.checkOwnerAccess.mockResolvedValue(new Set(['tag-1']));
       mocks.tag.update.mockResolvedValue(tagStub.colorCreate);
-      await expect(sut.update(authStub.admin, 'tag-1', { color: '#000000' })).resolves.toEqual(tagResponseStub.color1);
-      expect(mocks.tag.update).toHaveBeenCalledWith('tag-1', { color: '#000000' });
+      mocks.tag.get.mockResolvedValue(tagStub.tag);
+      await expect(sut.update(authStub.admin, 'tag-1', { name: 'tag', color: '#000000' })).resolves.toEqual(
+        tagResponseStub.color1,
+      );
+      expect(mocks.tag.update).toHaveBeenCalledWith('tag-1', { value: 'tag', color: '#000000' });
     });
   });
 

server/src/services/tag.service.ts

@@ -59,8 +59,19 @@ export class TagService extends BaseService {
   async update(auth: AuthDto, id: string, dto: TagUpdateDto): Promise<TagResponseDto> {
     await this.requireAccess({ auth, permission: Permission.TagUpdate, ids: [id] });
 
-    const { color } = dto;
-    const tag = await this.tagRepository.update(id, { color });
+    const { name, color } = dto;
+    const existing = await this.findOrFail(id);
+
+    let value;
+    if (name) {
+      const parts = existing.value.split('/');
+      parts[parts.length - 1] = name;
+      value = parts.join('/');
+    } else {
+      value = existing.value;
+    }
+
+    const tag = await this.tagRepository.update(id, { value, color });
     return mapTag(tag);
   }
 

server/test/medium.factory.ts

@@ -316,6 +316,12 @@ export class MediumTestContext<S extends ClassConstructor<typeof BaseService> =
     };
   }
 
+  async newTag(dto: Insertable<TagTable>) {
+    const tag = mediumFactory.tagInsert(dto);
+    const result = await this.get(TagRepository).create(tag);
+    return { tag, result };
+  }
+
   async newTagAsset(tagBulkAssets: { tagIds: string[]; assetIds: string[] }) {
     const tagsAssets: Insertable<TagAssetTable>[] = [];
     for (const tagId of tagBulkAssets.tagIds) {

server/test/medium/specs/repositories/tag.repository.spec.ts

@@ -0,0 +1,74 @@
+import { Kysely } from 'kysely';
+import { LoggingRepository } from 'src/repositories/logging.repository';
+import { TagRepository } from 'src/repositories/tag.repository';
+import { DB } from 'src/schema';
+import { BaseService } from 'src/services/base.service';
+import { newMediumService } from 'test/medium.factory';
+import { getKyselyDB } from 'test/utils';
+
+let defaultDatabase: Kysely<DB>;
+
+const setup = (db?: Kysely<DB>) => {
+  const { ctx } = newMediumService(BaseService, {
+    database: db || defaultDatabase,
+    real: [],
+    mock: [LoggingRepository],
+  });
+  return { ctx, sut: ctx.get(TagRepository) };
+};
+
+beforeAll(async () => {
+  defaultDatabase = await getKyselyDB();
+});
+
+describe(TagRepository.name, () => {
+  afterEach(async () => {
+    const { ctx } = setup();
+    await ctx.database.deleteFrom('tag_closure').execute();
+    await ctx.database.deleteFrom('tag_asset').execute();
+    await ctx.database.deleteFrom('tag').execute();
+  });
+
+  describe('update', () => {
+    it('should update a tag color', async () => {
+      const { ctx, sut } = setup();
+      const { user } = await ctx.newUser();
+
+      const { tag } = await ctx.newTag({
+        userId: user.id,
+        value: 'tagA',
+        color: '#000000',
+      });
+
+      await sut.update(tag.id, { color: '#FFFFFF' });
+
+      await expect(
+        ctx.database
+          .selectFrom('tag')
+          .select(['userId', 'value', 'color', 'parentId'])
+          .where('id', '=', tag.id)
+          .executeTakeFirstOrThrow(),
+      ).resolves.toEqual({ userId: user.id, value: 'tagA', color: '#FFFFFF', parentId: null });
+    });
+    it('should update a top-level tag value', async () => {
+      const { ctx, sut } = setup();
+      const { user } = await ctx.newUser();
+
+      const { tag } = await ctx.newTag({
+        userId: user.id,
+        value: 'tagA',
+        color: '#000000',
+      });
+
+      await sut.update(tag.id, { value: 'updatedTagA' });
+
+      await expect(
+        ctx.database
+          .selectFrom('tag')
+          .select(['userId', 'value', 'color', 'parentId'])
+          .where('id', '=', tag.id)
+          .executeTakeFirstOrThrow(),
+      ).resolves.toEqual({ userId: user.id, value: 'updatedTagA', color: '#000000', parentId: null });
+    });
+  });
+});

web/src/lib/constants.ts

@@ -257,6 +257,7 @@ export enum SettingInputFieldType {
   NUMBER = 'number',
   PASSWORD = 'password',
   COLOR = 'color',
+  NAME = 'name',
 }
 
 export const AlbumPageViewMode = {

web/src/lib/modals/TagCreateModal.svelte

@@ -1,7 +1,9 @@
 <script lang="ts">
+  import SettingInputField from '$lib/components/shared-components/settings/SettingInputField.svelte';
+  import { SettingInputFieldType } from '$lib/constants';
   import { handleCreateTag } from '$lib/services/tag.service';
   import type { TreeNode } from '$lib/utils/tree-utils';
-  import { Field, FormModal, Input, Text } from '@immich/ui';
+  import { FormModal, Text } from '@immich/ui';
   import { mdiTag } from '@mdi/js';
   import { t } from 'svelte-i18n';
 
@@ -13,9 +15,10 @@
   const { onClose, baseTag }: Props = $props();
 
   let tagValue = $state(baseTag?.path ? `${baseTag.path}/` : '');
+  let tagName = $state('');
 
   const onSubmit = async () => {
-    const success = await handleCreateTag(tagValue);
+    const success = await handleCreateTag(tagValue + tagName);
     if (success) {
       onClose();
     }
@@ -23,8 +26,9 @@
 </script>
 
 <FormModal size="small" title={$t('create_tag')} submitText={$t('create')} icon={mdiTag} {onClose} {onSubmit}>
-  <Text size="small">{$t('create_tag_description')}</Text>
-  <Field label={$t('tag')} required>
-    <Input autofocus bind:value={tagValue} />
-  </Field>
+  <Text size="small" class="mb-4">{$t('create_tag_description')}</Text>
+  <SettingInputField inputType={SettingInputFieldType.TEXT} label={$t('tag')} bind:value={tagName} />
+  {#if tagValue !== ''}
+    <Text size="small">{$t('tag_full_path', { values: { tag: tagValue } })}{tagName}</Text>
+  {/if}
 </FormModal>

web/src/lib/modals/TagEditModal.svelte

@@ -3,7 +3,7 @@
   import { SettingInputFieldType } from '$lib/constants';
   import { handleUpdateTag } from '$lib/services/tag.service';
   import type { TreeNode } from '$lib/utils/tree-utils';
-  import { FormModal } from '@immich/ui';
+  import { FormModal, Text } from '@immich/ui';
   import { mdiTag } from '@mdi/js';
   import { t } from 'svelte-i18n';
 
@@ -15,9 +15,12 @@
   const { tag, onClose }: Props = $props();
 
   let tagColor = $state(tag.color ?? '');
+  let tagName = $state(tag.value ?? '');
+  let tagPath = $state(tag.path ?? '');
+  const tagPathDisplay = $state(tagPath.endsWith(tagName) ? tagPath.slice(0, -tagName.length) : tagPath);
 
   const onSubmit = async () => {
-    const success = await handleUpdateTag(tag, { color: tagColor });
+    const success = await handleUpdateTag(tag, { color: tagColor || null, name: tagName });
     if (success) {
       onClose();
     }
@@ -26,4 +29,8 @@
 
 <FormModal title={$t('edit_tag')} size="small" icon={mdiTag} {onClose} {onSubmit}>
   <SettingInputField inputType={SettingInputFieldType.COLOR} label={$t('color')} bind:value={tagColor} />
+  <SettingInputField inputType={SettingInputFieldType.TEXT} label={$t('name')} bind:value={tagName} />
+  {#if tagPathDisplay !== ''}
+    <Text size="small">{$t('tag_full_path', { values: { tag: tagPathDisplay } })}{tagName}</Text>
+  {/if}
 </FormModal>

web/src/lib/services/tag.service.ts

@@ -61,7 +61,7 @@ export const handleUpdateTag = async (tag: TreeNode, dto: TagUpdateDto) => {
   try {
     const response = await updateTag({ id: tag.id, tagUpdateDto: dto });
 
-    toastManager.primary($t('tag_updated', { values: { tag: tag.value } }));
+    toastManager.primary($t('tag_updated', { values: { tag: response?.value || tag.value } }));
     eventManager.emit('TagUpdate', response);
 
     return true;

web/src/routes/(user)/tags/[[photos=photos]]/[[assetId=id]]/+page.svelte

@@ -63,6 +63,14 @@
     tags = await getAllTags();
   };
 
+  const onTagUpdate = async (response: TagResponseDto) => {
+    if (response.value !== tag.path) {
+      await navigateToView(response.value || '');
+    }
+
+    await onRefresh();
+  };
+
   const onTagDelete = async (response: TreeNode) => {
     if (response.path === tag.path) {
       await navigateToView(tag.parent ? tag.parent.path : '');
@@ -74,7 +82,7 @@
   const { Create, Update, Delete } = $derived(getTagActions($t, tag));
 </script>
 
-<OnEvents onTagCreate={onRefresh} onTagUpdate={onRefresh} {onTagDelete} />
+<OnEvents onTagCreate={onRefresh} {onTagUpdate} {onTagDelete} />
 
 <UserPageLayout title={data.meta.title} actions={[Create, Update, Delete]}>
   {#snippet sidebar()}