From 34a8c1a8a5aec643339b18e9c4df46b0adc89d50 Mon Sep 17 00:00:00 2001 From: Ian Macdonald Date: Wed, 15 Feb 2023 15:56:36 +0100 Subject: [PATCH 1/2] Dedupe attachments by file name, but allow any number of pasted images. Previously, the code simply deduped attachments on file name, using the internal name `image.png` for any pasted image. This effectively made it possible to paste only a single image into a message. Now, we ignore files called `image.png` when deduping. Deduping on file name is flawed anyway, because: 1. It's trivial to post multiple identical attachments, so long as each of them has a different file name. 2. It's **not** possible to post different attachments if they happen to have the same base file name (i.e. after the directory component is removed). So, we're actually getting both false positives and false negatives here. Fixes #2345, but introduces a new problem: When deleting a pasted attachment from a list of such attachments prior to sending, **all** pasted attachments are removed, presumably due to their all having the same internal file name. It may be better not to dedupe attachments at all, rather than use the crude mechanism of the file name. In that case, the following code would suffice: ``` - const uniqAttachments = _.uniqBy(allAttachments, m => m.fileName); - state.stagedAttachments[conversationKey] = uniqAttachments; + state.stagedAttachments[conversationKey] = allAttachments; ``` --- ts/state/ducks/stagedAttachments.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/ts/state/ducks/stagedAttachments.ts b/ts/state/ducks/stagedAttachments.ts index 727b3ab47..fd4f6921f 100644 --- a/ts/state/ducks/stagedAttachments.ts +++ b/ts/state/ducks/stagedAttachments.ts @@ -37,9 +37,12 @@ const stagedAttachmentsSlice = createSlice({ } const allAttachments = _.concat(currentStagedAttachments, newAttachments); - const uniqAttachments = _.uniqBy(allAttachments, m => m.fileName); + const pastedAttachments = allAttachments.filter(m => m.fileName === 'image.png'); + const allAttachmentsExceptPasted = allAttachments.filter(m => m.fileName !== 'image.png'); + const uniqAttachments = _.uniqBy(allAttachmentsExceptPasted, m => m.fileName); + const finalAttachments = _.concat(uniqAttachments, pastedAttachments); - state.stagedAttachments[conversationKey] = uniqAttachments; + state.stagedAttachments[conversationKey] = finalAttachments; return state; }, removeAllStagedAttachmentsInConversation( From 505007f1be695f1318755512971853c2d7ac5ba0 Mon Sep 17 00:00:00 2001 From: Ian Macdonald Date: Mon, 13 Mar 2023 11:26:02 +0100 Subject: [PATCH 2/2] Completely remove deduping of attachments at addition stage. --- ts/state/ducks/stagedAttachments.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/ts/state/ducks/stagedAttachments.ts b/ts/state/ducks/stagedAttachments.ts index fd4f6921f..496577fc9 100644 --- a/ts/state/ducks/stagedAttachments.ts +++ b/ts/state/ducks/stagedAttachments.ts @@ -37,12 +37,8 @@ const stagedAttachmentsSlice = createSlice({ } const allAttachments = _.concat(currentStagedAttachments, newAttachments); - const pastedAttachments = allAttachments.filter(m => m.fileName === 'image.png'); - const allAttachmentsExceptPasted = allAttachments.filter(m => m.fileName !== 'image.png'); - const uniqAttachments = _.uniqBy(allAttachmentsExceptPasted, m => m.fileName); - const finalAttachments = _.concat(uniqAttachments, pastedAttachments); - state.stagedAttachments[conversationKey] = finalAttachments; + state.stagedAttachments[conversationKey] = allAttachments; return state; }, removeAllStagedAttachmentsInConversation(