From ff612ad5dcd555ab61f991579ba848ce3dc8655d Mon Sep 17 00:00:00 2001 From: dakkar <dakkar@thenautilus.net> Date: Wed, 1 May 2024 11:03:46 +0100 Subject: [PATCH 1/2] really edit notes in more cases - fixes #424 Before this, changing visibility did not have any effect (if we don't want to allow editing visibility, we need to disable the drop-down in the editing window in the frontend!) Also, changing attachments did not have any effect, either. Adding a check that `oldnote.fileIds` is different from `data.files.map(file => file.id)` would not be enough, because editing the alt-text of an existing attachment would still not trigger the actual editing. Determining whether alt-text has been changed is a pain (e.g. if I edit the alt-text in the Drive, then edit the note that has that file attached, I would expect the note to be re-sent with the new alt-text). Therefore, we're publishing an edit event whenever a note has attachments, even if we can't see what changed. --- packages/backend/src/core/NoteEditService.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/backend/src/core/NoteEditService.ts b/packages/backend/src/core/NoteEditService.ts index 72fc01ae3b..e933e40e41 100644 --- a/packages/backend/src/core/NoteEditService.ts +++ b/packages/backend/src/core/NoteEditService.ts @@ -430,11 +430,18 @@ export class NoteEditService implements OnApplicationShutdown { update.hasPoll = !!data.poll; } + // technically we should check if the two sets of files are + // different, or if their descriptions have changed. In practice + // this is good enough. + const filesChanged = oldnote.fileIds?.length || data.files?.length; + + const visibilityChanged = oldnote.visibility !== data.visibility; + const poll = await this.pollsRepository.findOneBy({ noteId: oldnote.id }); const oldPoll = poll ? { choices: poll.choices, multiple: poll.multiple, expiresAt: poll.expiresAt } : null; - if (Object.keys(update).length > 0) { + if (Object.keys(update).length > 0 || filesChanged || visibilityChanged) { const exists = await this.noteEditRepository.findOneBy({ noteId: oldnote.id }); await this.noteEditRepository.insert({ -- GitLab From 1fe60e0815c7d7d811fa691191de90cebbd116b9 Mon Sep 17 00:00:00 2001 From: dakkar <dakkar@thenautilus.net> Date: Wed, 1 May 2024 15:19:42 +0100 Subject: [PATCH 2/2] can't really edit visibility MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit as @kopper remarked, other software explicitly prohibits them (e.g. https://iceshrimp.dev/iceshrimp/iceshrimp/commit/5e69fd791c3ef2effd542c5f2bc078b41ff8e87e ) thinking about it, they federate weirdly: if you narrow the visibility, some instances that received the Create, may not see the Update at all; even those who receive both the Create and the Update, will probably not hide the updated contents and show the old ones to the people covered by the old visibility and not the new one… if you widen visibility, more instances will get the Update than got the Create, so they'll ignore it… --- packages/backend/src/core/NoteEditService.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/backend/src/core/NoteEditService.ts b/packages/backend/src/core/NoteEditService.ts index e933e40e41..a01dfec664 100644 --- a/packages/backend/src/core/NoteEditService.ts +++ b/packages/backend/src/core/NoteEditService.ts @@ -435,13 +435,11 @@ export class NoteEditService implements OnApplicationShutdown { // this is good enough. const filesChanged = oldnote.fileIds?.length || data.files?.length; - const visibilityChanged = oldnote.visibility !== data.visibility; - const poll = await this.pollsRepository.findOneBy({ noteId: oldnote.id }); const oldPoll = poll ? { choices: poll.choices, multiple: poll.multiple, expiresAt: poll.expiresAt } : null; - if (Object.keys(update).length > 0 || filesChanged || visibilityChanged) { + if (Object.keys(update).length > 0 || filesChanged) { const exists = await this.noteEditRepository.findOneBy({ noteId: oldnote.id }); await this.noteEditRepository.insert({ -- GitLab