merge: Autofill reply mentions based on the replies property instead of MFM text (resolves #1045) (!981)

View MR for information: https://activitypub.software/TransFem-org/Sharkey/-/merge_requests/981

Closes #1045

Approved-by: dakkar <dakkar@thenautilus.net>
Approved-by: Marie <github@yuugi.dev>

packages/backend/src/core/NoteCreateService.ts

@@ -539,7 +539,8 @@ export class NoteCreateService implements OnApplicationShutdown {
 				await this.notesRepository.insert(insert);
 			}
 
-			return insert;
+			// Re-fetch note to get the default values of null / unset fields.
+			return await this.notesRepository.findOneByOrFail({ id: insert.id });
 		} catch (e) {
 			// duplicate key error
 			if (isDuplicateKeyValueError(e)) {

packages/backend/src/core/NoteEditService.ts

@@ -574,12 +574,15 @@ export class NoteEditService implements OnApplicationShutdown {
 				await this.notesRepository.update(oldnote.id, note);
 			}
 
+			// Re-fetch note to get the default values of null / unset fields.
+			const edited = await this.notesRepository.findOneByOrFail({ id: note.id });
+
 			setImmediate('post edited', { signal: this.#shutdownController.signal }).then(
-				() => this.postNoteEdited(note, oldnote, user, data, silent, tags!, mentionedUsers!),
+				() => this.postNoteEdited(edited, oldnote, user, data, silent, tags!, mentionedUsers!),
 				() => { /* aborted, ignore this */ },
 			);
 
-			return note;
+			return edited;
 		} else {
 			return oldnote;
 		}

packages/backend/src/core/entities/NoteEntityService.ts

@@ -402,7 +402,8 @@ export class NoteEntityService implements OnModuleInit {
 				bufferedReactions: Map<MiNote['id'], { deltas: Record<string, number>; pairs: ([MiUser['id'], string])[] }> | null;
 				myReactions: Map<MiNote['id'], string | null>;
 				packedFiles: Map<MiNote['fileIds'][number], Packed<'DriveFile'> | null>;
-				packedUsers: Map<MiUser['id'], Packed<'UserLite'>>
+				packedUsers: Map<MiUser['id'], Packed<'UserLite'>>;
+				mentionHandles: Record<string, string | undefined>;
 			};
 		},
 	): Promise<Packed<'Note'>> {
@@ -476,7 +477,8 @@ export class NoteEntityService implements OnModuleInit {
 				allowRenoteToExternal: channel.allowRenoteToExternal,
 				userId: channel.userId,
 			} : undefined,
-			mentions: note.mentions && note.mentions.length > 0 ? note.mentions : undefined,
+			mentions: note.mentions.length > 0 ? note.mentions : undefined,
+			mentionHandles: note.mentions.length > 0 ? this.getUserHandles(note.mentions, options?._hint_?.mentionHandles) : undefined,
 			uri: note.uri ?? undefined,
 			url: note.url ?? undefined,
 			poll: note.hasPoll ? this.populatePoll(note, meId) : undefined,
@@ -529,6 +531,25 @@ export class NoteEntityService implements OnModuleInit {
 	) {
 		if (notes.length === 0) return [];
 
+		const targetNotes: MiNote[] = [];
+		for (const note of notes) {
+			if (isPureRenote(note)) {
+				// we may need to fetch 'my reaction' for renote target.
+				targetNotes.push(note.renote);
+				if (note.renote.reply) {
+					// idem if the renote is also a reply.
+					targetNotes.push(note.renote.reply);
+				}
+			} else {
+				if (note.reply) {
+					// idem for OP of a regular reply.
+					targetNotes.push(note.reply);
+				}
+
+				targetNotes.push(note);
+			}
+		}
+
 		const bufferedReactions = this.meta.enableReactionsBuffering ? await this.reactionsBufferingService.getMany([...getAppearNoteIds(notes)]) : null;
 
 		const meId = me ? me.id : null;
@@ -536,25 +557,6 @@ export class NoteEntityService implements OnModuleInit {
 		if (meId) {
 			const idsNeedFetchMyReaction = new Set<MiNote['id']>();
 
-			const targetNotes: MiNote[] = [];
-			for (const note of notes) {
-				if (isPureRenote(note)) {
-					// we may need to fetch 'my reaction' for renote target.
-					targetNotes.push(note.renote);
-					if (note.renote.reply) {
-						// idem if the renote is also a reply.
-						targetNotes.push(note.renote.reply);
-					}
-				} else {
-					if (note.reply) {
-						// idem for OP of a regular reply.
-						targetNotes.push(note.reply);
-					}
-
-					targetNotes.push(note);
-				}
-			}
-
 			for (const note of targetNotes) {
 				const reactionsCount = Object.values(this.reactionsBufferingService.mergeReactions(note.reactions, bufferedReactions?.get(note.id)?.deltas ?? {})).reduce((a, b) => a + b, 0);
 				if (reactionsCount === 0) {
@@ -594,6 +596,15 @@ export class NoteEntityService implements OnModuleInit {
 		const packedUsers = await this.userEntityService.packMany(users, me)
 			.then(users => new Map(users.map(u => [u.id, u])));
 
+		// Recursively add all mentioned users from all notes + replies + renotes
+		const allMentionedUsers = targetNotes.reduce((users, note) => {
+			for (const user of note.mentions) {
+				users.add(user);
+			}
+			return users;
+		}, new Set<string>());
+		const mentionHandles = await this.getUserHandles(Array.from(allMentionedUsers));
+
 		return await Promise.all(notes.map(n => this.pack(n, me, {
 			...options,
 			_hint_: {
@@ -601,6 +612,7 @@ export class NoteEntityService implements OnModuleInit {
 				myReactions: myReactionsMap,
 				packedFiles,
 				packedUsers,
+				mentionHandles,
 			},
 		})));
 	}
@@ -636,4 +648,36 @@ export class NoteEntityService implements OnModuleInit {
 			relations: ['user'],
 		});
 	}
+
+	private async getUserHandles(userIds: string[], hint?: Record<string, string | undefined>): Promise<Record<string, string | undefined>> {
+		if (userIds.length < 1) return {};
+
+		// Hint is provided by packMany to avoid N+1 queries.
+		// It should already include all existing mentioned users.
+		if (hint) {
+			const handles = {} as Record<string, string | undefined>;
+			for (const id of userIds) {
+				handles[id] = hint[id];
+			}
+			return handles;
+		}
+
+		const users = await this.usersRepository.find({
+			select: {
+				id: true,
+				username: true,
+				host: true,
+			},
+			where: {
+				id: In(userIds),
+			},
+		});
+
+		return users.reduce((map, user) => {
+			map[user.id] = user.host
+				? `@${user.username}@${user.host}`
+				: `@${user.username}`;
+			return map;
+		}, {} as Record<string, string | undefined>);
+	}
 }

packages/backend/src/models/json-schema/note.ts

@@ -85,6 +85,16 @@ export const packedNoteSchema = {
 				format: 'id',
 			},
 		},
+		mentionHandles: {
+			type: 'object',
+			optional: true, nullable: false,
+			additionalProperties: {
+				anyOf: [{
+					type: 'string',
+				}],
+				optional: true, nullable: false,
+			},
+		},
 		visibleUserIds: {
 			type: 'array',
 			optional: true, nullable: false,

packages/frontend/src/components/MkPostForm.vue

@@ -317,19 +317,10 @@ if (props.reply && (props.reply.user.username !== $i.username || (props.reply.us
 	text.value = `@${props.reply.user.username}${props.reply.user.host != null ? '@' + toASCII(props.reply.user.host) : ''} `;
 }
 
-if (props.reply && props.reply.text != null) {
+if (props.reply && props.reply.mentionHandles) {
-	const ast = mfm.parse(props.reply.text);
+	for (const [user, mention] of Object.entries(props.reply.mentionHandles)) {
-	const otherHost = props.reply.user.host;
+		// Don't mention ourself
-
+		if (user === $i.id) continue;
-	for (const x of extractMentions(ast)) {
-		const mention = x.host ?
-			`@${x.username}@${toASCII(x.host)}` :
-			(otherHost == null || otherHost === host) ?
-				`@${x.username}` :
-				`@${x.username}@${toASCII(otherHost)}`;
-
-		// 自分は除外
-		if ($i.username === x.username && (x.host == null || x.host === host)) continue;
 
 		// 重複は除外
 		if (text.value.includes(`${mention} `)) continue;
@@ -425,7 +416,7 @@ function checkMissingMention() {
 		const ast = mfm.parse(text.value);
 
 		for (const x of extractMentions(ast)) {
-			if (!visibleUsers.value.some(u => (u.username === x.username) && (u.host === x.host))) {
+			if (!visibleUsers.value.some(u => (u.username.toLowerCase() === x.username.toLowerCase()) && (u.host === x.host))) {
 				hasNotSpecifiedMentions.value = true;
 				return;
 			}
@@ -438,7 +429,7 @@ function addMissingMention() {
 	const ast = mfm.parse(text.value);
 
 	for (const x of extractMentions(ast)) {
-		if (!visibleUsers.value.some(u => (u.username === x.username) && (u.host === x.host))) {
+		if (!visibleUsers.value.some(u => (u.username.toLowerCase() === x.username.toLowerCase()) && (u.host === x.host))) {
 			misskeyApi('users/show', { username: x.username, host: x.host }).then(user => {
 				pushVisibleUser(user);
 			});
@@ -654,7 +645,7 @@ function showOtherSettings() {
 //#endregion
 
 function pushVisibleUser(user: Misskey.entities.UserDetailed) {
-	if (!visibleUsers.value.some(u => u.username === user.username && u.host === user.host)) {
+	if (!visibleUsers.value.some(u => u.username.toLowerCase() === user.username.toLowerCase() && u.host === user.host)) {
 		visibleUsers.value.push(user);
 	}
 }

packages/misskey-js/src/autogen/types.ts

@@ -4668,6 +4668,9 @@ export type components = {
       /** @enum {string} */
       visibility: 'public' | 'home' | 'followers' | 'specified';
       mentions?: string[];
+      mentionHandles?: {
+        [key: string]: string;
+      };
       visibleUserIds?: string[];
       fileIds?: string[];
       files?: components['schemas']['DriveFile'][];