fix: admin/emoji/update で不正なエラーが発生する (#14750)

* fix emoji updating bug

* update changelog

* type fix

* " -> '

* conprehensiveness check

* lint

* undefined -> null

CHANGELOG.md

@@ -4,6 +4,9 @@
 - Enhance: l10nの更新
 - Fix: メールアドレス不要でCaptchaが有効な場合にアカウント登録完了後自動でのログインに失敗する問題を修正
 
+### Server
+- Fix: `admin/emoji/update`エンドポイントのidのみ指定した時不正なエラーが発生するバグを修正
+
 ## 2024.10.0
 
 ### Note

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<void> {
+	}, moderator?: MiUser): Promise<
-		const emoji = await this.emojisRepository.findOneByOrFail({ id: id });
+		null
-		const sameNameEmoji = await this.emojisRepository.findOneBy({ name: data.name, host: IsNull() });
+		| 'NO_SUCH_EMOJI'
-		if (sameNameEmoji != null && sameNameEmoji.id !== id) throw new Error('name already exists');
+		| '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],
 			});
@@ -157,6 +171,7 @@ export class CustomEmojiService implements OnApplicationShutdown {
 				after: updated,
 			});
 		}
+		return null;
 	}
 
 	@bindThis

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,25 +78,14 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
 				if (driveFile == null) throw new ApiError(meta.errors.noSuchFile);
 			}
 
-			let emojiId;
+			// JSON schemeのanyOfの型変換がうまくいっていないらしい
-			if (ps.id) {
+			const required = { id: ps.id, name: ps.name } as 
-				emojiId = ps.id;
+				| { id: MiEmoji['id']; name?: string }
-				const emoji = await this.customEmojiService.getEmojiById(ps.id);
+				| { id?: MiEmoji['id']; name: string };
-				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, {
+			const error = await this.customEmojiService.update({
+				...required,
 				driveFile,
-				name: ps.name,
 				category: ps.category,
 				aliases: ps.aliases,
 				license: ps.license,
@@ -104,6 +93,14 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
 				localOnly: ps.localOnly,
 				roleIdsThatCanBeUsedThisEmojiAsReaction: ps.roleIdsThatCanBeUsedThisEmojiAsReaction,
 			}, me);
+
+			switch (error) {
+				case null: return;
+				case 'NO_SUCH_EMOJI': throw new ApiError(meta.errors.noSuchEmoji);
+				case 'SAME_NAME_EMOJI_EXISTS': throw new ApiError(meta.errors.sameNameEmojiExists);
+			}
+			// 網羅性チェック
+			const mustBeNever: never = error;
 		});
 	}
 }