Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion i18n/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -1845,6 +1845,7 @@
"search": "Search",
"search_albums": "Search albums",
"search_by_context": "Search by context",
"search_by_context_example": "Describe the photo you want to find, e.g. \"Children jumping on a trampoline\"",
"search_by_description": "Search by description",
"search_by_description_example": "Hiking day in Sapa",
"search_by_filename": "Search by file name or extension",
Expand Down Expand Up @@ -2128,6 +2129,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}",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ class BaseActionButton extends StatelessWidget {
const SizedBox(height: 8),
Text(
label,
style: const TextStyle(fontSize: 14.0, fontWeight: FontWeight.w400),
style: const TextStyle(fontSize: 14.0, fontWeight: FontWeight.w500),
maxLines: 3,
textAlign: TextAlign.center,
softWrap: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class ViewerBottomBar extends ConsumerWidget {
? const SizedBox.shrink()
: Theme(
data: context.themeData.copyWith(
iconTheme: const IconThemeData(size: 22, color: Colors.white),
iconTheme: const IconThemeData(size: ImmichIconSize.md, color: Colors.white),
textTheme: context.themeData.textTheme.copyWith(
labelLarge: context.themeData.textTheme.labelLarge?.copyWith(color: Colors.white),
),
Expand Down
2 changes: 1 addition & 1 deletion mobile/packages/ui/lib/src/components/column_button.dart
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ class _ImmichColumnButtonState extends State<ImmichColumnButton> {
maxLines: 2,
textAlign: .center,
overflow: .ellipsis,
style: const .new(fontSize: ImmichTextSize.label, fontWeight: .w500),
style: const .new(fontSize: ImmichTextSize.body, fontWeight: .w500),
),
],
),
Expand Down
5 changes: 5 additions & 0 deletions open-api/immich-openapi-specs.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 2 additions & 0 deletions packages/sdk/src/fetch-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
41 changes: 41 additions & 0 deletions server/src/controllers/tag.controller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, () => {
Expand Down Expand Up @@ -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`);
Expand Down
7 changes: 6 additions & 1 deletion server/src/dtos/tag.dto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' });
Expand Down
13 changes: 11 additions & 2 deletions server/src/queries/tag.repository.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
65 changes: 62 additions & 3 deletions server/src/repositories/tag.repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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] })
Expand Down
17 changes: 14 additions & 3 deletions server/src/services/tag.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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' });
});
});

Expand Down
15 changes: 13 additions & 2 deletions server/src/services/tag.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
6 changes: 6 additions & 0 deletions server/test/medium.factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
74 changes: 74 additions & 0 deletions server/test/medium/specs/repositories/tag.repository.spec.ts
Original file line number Diff line number Diff line change
@@ -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 });
});
});
});
Loading
Loading