Skip to content

Commit

Permalink
fix: admin/emoji/update で不正なエラーが発生する (#14750)
Browse files Browse the repository at this point in the history
* fix emoji updating bug

* update changelog

* type fix

* " -> '

* conprehensiveness check

* lint

* undefined -> null
  • Loading branch information
FineArchs authored Oct 11, 2024
1 parent d376aab commit 12bc671
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 25 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
- Enhance: l10nの更新
- Fix: メールアドレス不要でCaptchaが有効な場合にアカウント登録完了後自動でのログインに失敗する問題を修正

### Server
- Fix: `admin/emoji/update`エンドポイントのidのみ指定した時不正なエラーが発生するバグを修正

## 2024.10.0

### Note
Expand Down
29 changes: 22 additions & 7 deletions packages/backend/src/core/CustomEmojiService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
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<
null
| '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(),
Expand All @@ -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],
});
Expand All @@ -157,6 +171,7 @@ export class CustomEmojiService implements OnApplicationShutdown {
after: updated,
});
}
return null;
}

@bindThis
Expand Down
33 changes: 15 additions & 18 deletions packages/backend/src/server/api/endpoints/admin/emoji/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -78,32 +78,29 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // 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;
}
// JSON schemeのanyOfの型変換がうまくいっていないらしい
const required = { id: ps.id, name: ps.name } as
| { id: MiEmoji['id']; name?: string }
| { id?: MiEmoji['id']; name: string };

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,
isSensitive: ps.isSensitive,
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;
});
}
}

0 comments on commit 12bc671

Please sign in to comment.