From 3dc915ae898669453accc423e9279dc37fefdaaa Mon Sep 17 00:00:00 2001 From: FineArchs Date: Fri, 11 Oct 2024 13:41:16 +0900 Subject: [PATCH 1/7] fix emoji updating bug --- .../backend/src/core/CustomEmojiService.ts | 28 ++++++++++++++----- .../api/endpoints/admin/emoji/update.ts | 27 ++++++------------ 2 files changed, 30 insertions(+), 25 deletions(-) diff --git a/packages/backend/src/core/CustomEmojiService.ts b/packages/backend/src/core/CustomEmojiService.ts index 5db3c5b98035..1e31d39f11ea 100644 --- a/packages/backend/src/core/CustomEmojiService.ts +++ b/packages/backend/src/core/CustomEmojiService.ts @@ -103,19 +103,33 @@ export class CustomEmojiService implements OnApplicationShutdown { } @bindThis - public async update(id: MiEmoji['id'], data: { + public async update(data: ( + { id: MiEmoji['id'], name?: string; } | { name: string; id?: MiEmoji['id'], } + ) & { driveFile?: MiDriveFile; - name?: string; category?: string | null; aliases?: string[]; license?: string | null; isSensitive?: boolean; localOnly?: boolean; roleIdsThatCanBeUsedThisEmojiAsReaction?: MiRole['id'][]; - }, moderator?: MiUser): Promise { - const emoji = await this.emojisRepository.findOneByOrFail({ id: id }); - const sameNameEmoji = await this.emojisRepository.findOneBy({ name: data.name, host: IsNull() }); - if (sameNameEmoji != null && sameNameEmoji.id !== id) throw new Error('name already exists'); + }, moderator?: MiUser): Promise< + undefined + | "NO_SUCH_EMOJI" + | "SAME_NAME_EMOJI_EXISTS" + > { + const emoji = data.id + ? await this.getEmojiById(data.id) + : await this.getEmojiByName(data.name); + if (emoji === null) return "NO_SUCH_EMOJI"; + const id = emoji.id; + + // IDと絵文字名が両方指定されている場合は絵文字名の変更を行うため重複チェックが必要 + const doNameUpdate = data.id && data.name && (data.name !== emoji.name); + if (doNameUpdate) { + const isDuplicate = await this.checkDuplicate(data.name); + if (isDuplicate) return "SAME_NAME_EMOJI_EXISTS"; + } await this.emojisRepository.update(emoji.id, { updatedAt: new Date(), @@ -135,7 +149,7 @@ export class CustomEmojiService implements OnApplicationShutdown { const packed = await this.emojiEntityService.packDetailed(emoji.id); - if (emoji.name === data.name) { + if (!doNameUpdate) { this.globalEventService.publishBroadcastStream('emojiUpdated', { emojis: [packed], }); diff --git a/packages/backend/src/server/api/endpoints/admin/emoji/update.ts b/packages/backend/src/server/api/endpoints/admin/emoji/update.ts index 22609a16a39a..dca89c6578e7 100644 --- a/packages/backend/src/server/api/endpoints/admin/emoji/update.ts +++ b/packages/backend/src/server/api/endpoints/admin/emoji/update.ts @@ -78,25 +78,10 @@ export default class extends Endpoint { // eslint- if (driveFile == null) throw new ApiError(meta.errors.noSuchFile); } - let emojiId; - if (ps.id) { - emojiId = ps.id; - const emoji = await this.customEmojiService.getEmojiById(ps.id); - if (!emoji) throw new ApiError(meta.errors.noSuchEmoji); - if (ps.name && (ps.name !== emoji.name)) { - const isDuplicate = await this.customEmojiService.checkDuplicate(ps.name); - if (isDuplicate) throw new ApiError(meta.errors.sameNameEmojiExists); - } - } else { - if (!ps.name) throw new Error('Invalid Params unexpectedly passed. This is a BUG. Please report it to the development team.'); - const emoji = await this.customEmojiService.getEmojiByName(ps.name); - if (!emoji) throw new ApiError(meta.errors.noSuchEmoji); - emojiId = emoji.id; - } - - await this.customEmojiService.update(emojiId, { - driveFile, + const error = await this.customEmojiService.update({ + id: ps.id, name: ps.name, + driveFile, category: ps.category, aliases: ps.aliases, license: ps.license, @@ -104,6 +89,12 @@ export default class extends Endpoint { // eslint- localOnly: ps.localOnly, roleIdsThatCanBeUsedThisEmojiAsReaction: ps.roleIdsThatCanBeUsedThisEmojiAsReaction, }, me); + + switch (error) { + case undefined: return; + case "NO_SUCH_EMOJI": throw new ApiError(meta.errors.noSuchEmoji); + case "SAME_NAME_EMOJI_EXISTS": throw new ApiError(meta.errors.sameNameEmojiExists); + } }); } } From 77ae12bf1872b5e582b63232e3854d0953f50f3d Mon Sep 17 00:00:00 2001 From: FineArchs Date: Fri, 11 Oct 2024 13:46:39 +0900 Subject: [PATCH 2/7] update changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ee37bb3c6f29..b449a1b91e03 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ - Enhance: l10nの更新 - Fix: メールアドレス不要でCaptchaが有効な場合にアカウント登録完了後自動でのログインに失敗する問題を修正 +### Server +- Fix: `admin/emoji/update`エンドポイントのidのみ指定した時不正なエラーが発生するバグを修正 + ## 2024.10.0 ### Note From f965b35a2c652769d41dbe17882dbc2acf41985a Mon Sep 17 00:00:00 2001 From: FineArchs Date: Fri, 11 Oct 2024 14:15:44 +0900 Subject: [PATCH 3/7] type fix --- packages/backend/src/core/CustomEmojiService.ts | 6 +++--- .../src/server/api/endpoints/admin/emoji/update.ts | 10 +++++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/backend/src/core/CustomEmojiService.ts b/packages/backend/src/core/CustomEmojiService.ts index 1e31d39f11ea..078351327fdd 100644 --- a/packages/backend/src/core/CustomEmojiService.ts +++ b/packages/backend/src/core/CustomEmojiService.ts @@ -114,20 +114,20 @@ export class CustomEmojiService implements OnApplicationShutdown { localOnly?: boolean; roleIdsThatCanBeUsedThisEmojiAsReaction?: MiRole['id'][]; }, moderator?: MiUser): Promise< - undefined + void | "NO_SUCH_EMOJI" | "SAME_NAME_EMOJI_EXISTS" > { const emoji = data.id ? await this.getEmojiById(data.id) - : await this.getEmojiByName(data.name); + : await this.getEmojiByName(data.name!); if (emoji === null) return "NO_SUCH_EMOJI"; const id = emoji.id; // IDと絵文字名が両方指定されている場合は絵文字名の変更を行うため重複チェックが必要 const doNameUpdate = data.id && data.name && (data.name !== emoji.name); if (doNameUpdate) { - const isDuplicate = await this.checkDuplicate(data.name); + const isDuplicate = await this.checkDuplicate(data.name!); if (isDuplicate) return "SAME_NAME_EMOJI_EXISTS"; } diff --git a/packages/backend/src/server/api/endpoints/admin/emoji/update.ts b/packages/backend/src/server/api/endpoints/admin/emoji/update.ts index dca89c6578e7..2e6a662be24a 100644 --- a/packages/backend/src/server/api/endpoints/admin/emoji/update.ts +++ b/packages/backend/src/server/api/endpoints/admin/emoji/update.ts @@ -6,7 +6,7 @@ import { Inject, Injectable } from '@nestjs/common'; import { Endpoint } from '@/server/api/endpoint-base.js'; import { CustomEmojiService } from '@/core/CustomEmojiService.js'; -import type { DriveFilesRepository } from '@/models/_.js'; +import type { DriveFilesRepository, MiEmoji } from '@/models/_.js'; import { DI } from '@/di-symbols.js'; import { ApiError } from '../../../error.js'; @@ -78,9 +78,13 @@ export default class extends Endpoint { // eslint- if (driveFile == null) throw new ApiError(meta.errors.noSuchFile); } + // JSON schemeのanyOfの型変換がうまくいっていないらしい + const required = { id: ps.id, name: ps.name } as + | { id: MiEmoji['id']; name?: string } + | { id?: MiEmoji['id']; name: string }; + const error = await this.customEmojiService.update({ - id: ps.id, - name: ps.name, + ...required, driveFile, category: ps.category, aliases: ps.aliases, From 70bb365cd157f6f623dcc56667d9ebbc4410181f Mon Sep 17 00:00:00 2001 From: FineArchs Date: Fri, 11 Oct 2024 14:55:57 +0900 Subject: [PATCH 4/7] " -> ' --- packages/backend/src/core/CustomEmojiService.ts | 8 ++++---- .../src/server/api/endpoints/admin/emoji/update.ts | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/backend/src/core/CustomEmojiService.ts b/packages/backend/src/core/CustomEmojiService.ts index 078351327fdd..cb23a2f8c04f 100644 --- a/packages/backend/src/core/CustomEmojiService.ts +++ b/packages/backend/src/core/CustomEmojiService.ts @@ -115,20 +115,20 @@ export class CustomEmojiService implements OnApplicationShutdown { roleIdsThatCanBeUsedThisEmojiAsReaction?: MiRole['id'][]; }, moderator?: MiUser): Promise< void - | "NO_SUCH_EMOJI" - | "SAME_NAME_EMOJI_EXISTS" + | 'NO_SUCH_EMOJI' + | 'SAME_NAME_EMOJI_EXISTS' > { const emoji = data.id ? await this.getEmojiById(data.id) : await this.getEmojiByName(data.name!); - if (emoji === null) return "NO_SUCH_EMOJI"; + if (emoji === null) return 'NO_SUCH_EMOJI'; const id = emoji.id; // IDと絵文字名が両方指定されている場合は絵文字名の変更を行うため重複チェックが必要 const doNameUpdate = data.id && data.name && (data.name !== emoji.name); if (doNameUpdate) { const isDuplicate = await this.checkDuplicate(data.name!); - if (isDuplicate) return "SAME_NAME_EMOJI_EXISTS"; + if (isDuplicate) return 'SAME_NAME_EMOJI_EXISTS'; } await this.emojisRepository.update(emoji.id, { diff --git a/packages/backend/src/server/api/endpoints/admin/emoji/update.ts b/packages/backend/src/server/api/endpoints/admin/emoji/update.ts index 2e6a662be24a..95ee1d0810be 100644 --- a/packages/backend/src/server/api/endpoints/admin/emoji/update.ts +++ b/packages/backend/src/server/api/endpoints/admin/emoji/update.ts @@ -96,8 +96,8 @@ export default class extends Endpoint { // eslint- switch (error) { case undefined: return; - case "NO_SUCH_EMOJI": throw new ApiError(meta.errors.noSuchEmoji); - case "SAME_NAME_EMOJI_EXISTS": throw new ApiError(meta.errors.sameNameEmojiExists); + case 'NO_SUCH_EMOJI': throw new ApiError(meta.errors.noSuchEmoji); + case 'SAME_NAME_EMOJI_EXISTS': throw new ApiError(meta.errors.sameNameEmojiExists); } }); } From fcce5b98b19817aa342889943ef26e4d5c112422 Mon Sep 17 00:00:00 2001 From: FineArchs Date: Fri, 11 Oct 2024 15:17:01 +0900 Subject: [PATCH 5/7] conprehensiveness check --- packages/backend/src/core/CustomEmojiService.ts | 3 ++- .../backend/src/server/api/endpoints/admin/emoji/update.ts | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/backend/src/core/CustomEmojiService.ts b/packages/backend/src/core/CustomEmojiService.ts index cb23a2f8c04f..58d502bb2b7e 100644 --- a/packages/backend/src/core/CustomEmojiService.ts +++ b/packages/backend/src/core/CustomEmojiService.ts @@ -114,7 +114,7 @@ export class CustomEmojiService implements OnApplicationShutdown { localOnly?: boolean; roleIdsThatCanBeUsedThisEmojiAsReaction?: MiRole['id'][]; }, moderator?: MiUser): Promise< - void + undefined | 'NO_SUCH_EMOJI' | 'SAME_NAME_EMOJI_EXISTS' > { @@ -171,6 +171,7 @@ export class CustomEmojiService implements OnApplicationShutdown { after: updated, }); } + return undefined; } @bindThis diff --git a/packages/backend/src/server/api/endpoints/admin/emoji/update.ts b/packages/backend/src/server/api/endpoints/admin/emoji/update.ts index 95ee1d0810be..55f8f6a69c4b 100644 --- a/packages/backend/src/server/api/endpoints/admin/emoji/update.ts +++ b/packages/backend/src/server/api/endpoints/admin/emoji/update.ts @@ -98,6 +98,8 @@ export default class extends Endpoint { // eslint- case undefined: return; case 'NO_SUCH_EMOJI': throw new ApiError(meta.errors.noSuchEmoji); case 'SAME_NAME_EMOJI_EXISTS': throw new ApiError(meta.errors.sameNameEmojiExists); + // 網羅性チェック + default: const mustBeNever: never = error; } }); } From debf20f45c8b062182e41d0ce1aacf3890e38aba Mon Sep 17 00:00:00 2001 From: FineArchs Date: Fri, 11 Oct 2024 15:22:00 +0900 Subject: [PATCH 6/7] lint --- .../backend/src/server/api/endpoints/admin/emoji/update.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/server/api/endpoints/admin/emoji/update.ts b/packages/backend/src/server/api/endpoints/admin/emoji/update.ts index 55f8f6a69c4b..e1377d1ae386 100644 --- a/packages/backend/src/server/api/endpoints/admin/emoji/update.ts +++ b/packages/backend/src/server/api/endpoints/admin/emoji/update.ts @@ -98,9 +98,9 @@ export default class extends Endpoint { // eslint- case undefined: return; case 'NO_SUCH_EMOJI': throw new ApiError(meta.errors.noSuchEmoji); case 'SAME_NAME_EMOJI_EXISTS': throw new ApiError(meta.errors.sameNameEmojiExists); - // 網羅性チェック - default: const mustBeNever: never = error; } + // 網羅性チェック + const mustBeNever: never = error; }); } } From 1c72afae9efefda1b53a6cc3317b0c391bf7c248 Mon Sep 17 00:00:00 2001 From: FineArchs Date: Fri, 11 Oct 2024 16:57:31 +0900 Subject: [PATCH 7/7] undefined -> null --- packages/backend/src/core/CustomEmojiService.ts | 4 ++-- .../backend/src/server/api/endpoints/admin/emoji/update.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/backend/src/core/CustomEmojiService.ts b/packages/backend/src/core/CustomEmojiService.ts index 58d502bb2b7e..45661134491e 100644 --- a/packages/backend/src/core/CustomEmojiService.ts +++ b/packages/backend/src/core/CustomEmojiService.ts @@ -114,7 +114,7 @@ export class CustomEmojiService implements OnApplicationShutdown { localOnly?: boolean; roleIdsThatCanBeUsedThisEmojiAsReaction?: MiRole['id'][]; }, moderator?: MiUser): Promise< - undefined + null | 'NO_SUCH_EMOJI' | 'SAME_NAME_EMOJI_EXISTS' > { @@ -171,7 +171,7 @@ export class CustomEmojiService implements OnApplicationShutdown { after: updated, }); } - return undefined; + return null; } @bindThis diff --git a/packages/backend/src/server/api/endpoints/admin/emoji/update.ts b/packages/backend/src/server/api/endpoints/admin/emoji/update.ts index e1377d1ae386..212cba5c5dd9 100644 --- a/packages/backend/src/server/api/endpoints/admin/emoji/update.ts +++ b/packages/backend/src/server/api/endpoints/admin/emoji/update.ts @@ -95,7 +95,7 @@ export default class extends Endpoint { // eslint- }, me); switch (error) { - case undefined: return; + case null: return; case 'NO_SUCH_EMOJI': throw new ApiError(meta.errors.noSuchEmoji); case 'SAME_NAME_EMOJI_EXISTS': throw new ApiError(meta.errors.sameNameEmojiExists); }