fix: allow same-name staged attachments

ts/components/conversation/SessionConversation.tsx

@@ -68,6 +68,7 @@ import {
 } from '../../state/selectors/selectedConversation';
 import { LUCIDE_ICONS_UNICODE } from '../icon/lucide';
 import { sleepFor } from '../../session/utils/Promise';
+import { uuidV4 } from '../../util/uuid';
 
 interface State {
   isDraggingFile: boolean;
@@ -478,6 +479,7 @@ export class SessionConversation extends Component<Props, State> {
         this.addAttachments([attachmentWithVideoPreview]);
       } else {
         const attachment: StagedAttachmentType = {
+          stagedAttachmentId: uuidV4(),
           file,
           size: file.size,
           contentType,
@@ -509,6 +511,7 @@ export class SessionConversation extends Component<Props, State> {
       );
       this.addAttachments([
         {
+          stagedAttachmentId: uuidV4(),
           file,
           size: file.size,
           contentType,
@@ -617,6 +620,7 @@ const renderVideoPreview = async (contentType: string, file: File, fileName: str
       type,
     });
     return {
+      stagedAttachmentId: uuidV4(),
       file,
       size: file.size,
       fileName,
@@ -642,6 +646,7 @@ const renderImagePreview = async (contentType: string, file: File, fileName: str
       throw new Error('Failed to create object url for image!');
     }
     return {
+      stagedAttachmentId: uuidV4(),
       file,
       size: file.size,
       fileName,
@@ -666,6 +671,7 @@ const renderImagePreview = async (contentType: string, file: File, fileName: str
   });
 
   return {
+    stagedAttachmentId: uuidV4(),
     file,
     size: file.size,
     fileName,

ts/components/conversation/StagedAttachmentList.tsx

@@ -19,9 +19,10 @@ import { AriaLabels } from '../../util/hardcodedAriaLabels';
 import { LUCIDE_ICONS_UNICODE } from '../icon/lucide';
 import { SessionLucideIconButton } from '../icon/SessionIconButton';
 import { THEME_GLOBALS } from '../../themes/globals';
+import type { StagedAttachmentType } from './composition/CompositionBox';
 
 type Props = {
-  attachments: Array<AttachmentType>;
+  attachments: Array<StagedAttachmentType>;
   onClickAttachment: (attachment: AttachmentType) => void;
   onAddAttachment: () => void;
 };
@@ -64,11 +65,11 @@ export const StagedAttachmentList = (props: Props) => {
     dispatch(removeAllStagedAttachmentsInConversation({ conversationId: conversationKey }));
   };
 
-  const onRemoveByFilename = (filename: string) => {
+  const onRemoveByStagedAttachmentId = (stagedAttachmentId: string) => {
     if (!conversationKey) {
       return;
     }
-    dispatch(removeStagedAttachmentInConversation({ conversationKey, filename }));
+    dispatch(removeStagedAttachmentInConversation({ conversationKey, stagedAttachmentId }));
   };
 
   if (!attachments.length) {
@@ -100,7 +101,8 @@ export const StagedAttachmentList = (props: Props) => {
       <StyledRail>
         {(attachments || []).map((attachment, index) => {
           const { contentType } = attachment;
-          const key = getUrl(attachment) || attachment.fileName || index;
+          const key =
+            attachment.stagedAttachmentId || getUrl(attachment) || attachment.fileName || index;
           if (isImageTypeSupported(contentType) || isVideoTypeSupported(contentType)) {
             return (
               <Image
@@ -116,7 +118,7 @@ export const StagedAttachmentList = (props: Props) => {
                 closeButton={true}
                 onClick={onClickAttachment}
                 onClickClose={() => {
-                  onRemoveByFilename(attachment.fileName);
+                  onRemoveByStagedAttachmentId(attachment.stagedAttachmentId);
                 }}
               />
             );
@@ -127,7 +129,7 @@ export const StagedAttachmentList = (props: Props) => {
               key={key}
               attachment={attachment}
               onClose={() => {
-                onRemoveByFilename(attachment.fileName);
+                onRemoveByStagedAttachmentId(attachment.stagedAttachmentId);
               }}
             />
           );

ts/components/conversation/composition/CompositionBox.tsx

@@ -103,6 +103,7 @@ export type StagedLinkPreviewData = {
 };
 
 export type StagedAttachmentType = AttachmentType & {
+  stagedAttachmentId: string;
   file: File;
   path?: string; // a bit hacky, but this is the only way to make our sending audio message be playable, this must be used only for those message
 };
@@ -807,15 +808,12 @@ class CompositionBoxInner extends Component<Props, State> {
       contentType: MIME.AUDIO_MP3,
     });
     // { ...savedAudioFile, path: savedAudioFile.path },
-    const audioAttachment: StagedAttachmentType = {
-      file: new File([], 'session-audio-message'), // this is just to emulate a file for the staged attachment type of that audio file
+    const audioAttachment: StagedAttachmentImportedType = {
       contentType: MIME.AUDIO_MP3,
       size: savedAudioFile.size,
-      fileSize: null,
       screenshot: null,
       fileName: 'session-audio-message',
       thumbnail: null,
-      url: '',
       isVoiceMessage: true,
       path: savedAudioFile.path,
     };

ts/state/ducks/stagedAttachments.ts

@@ -32,13 +32,22 @@ const stagedAttachmentsSlice = createSlice({
       }
       const currentStagedAttachments = state.stagedAttachments[conversationKey] || [];
 
-      if (newAttachments.some(a => a.isVoiceMessage) && currentStagedAttachments.length > 0) {
-        window?.log?.warn('A voice note cannot be sent with other attachments');
+      const hasCurrentVoiceMessage = currentStagedAttachments.some(a => a.isVoiceMessage);
+      const hasNewVoiceMessage = newAttachments.some(a => a.isVoiceMessage);
+
+      if (
+        (hasNewVoiceMessage &&
+          (currentStagedAttachments.length > 0 || newAttachments.length > 1)) ||
+        (hasCurrentVoiceMessage && newAttachments.length > 0)
+      ) {
+        window?.log?.warn(
+          'Only one voice note can be staged, and it cannot be mixed with other attachments'
+        );
         return state;
       }
 
       const allAttachments = _.concat(currentStagedAttachments, newAttachments);
-      const uniqAttachments = _.uniqBy(allAttachments, m => m.fileName);
+      const uniqAttachments = _.uniqBy(allAttachments, m => m.stagedAttachmentId);
 
       state.stagedAttachments[conversationKey] = uniqAttachments;
       return state;
@@ -67,16 +76,18 @@ const stagedAttachmentsSlice = createSlice({
     },
     removeStagedAttachmentInConversation(
       state: StagedAttachmentsStateType,
-      action: PayloadAction<{ conversationKey: string; filename: string }>
+      action: PayloadAction<{ conversationKey: string; stagedAttachmentId: string }>
     ) {
-      const { conversationKey, filename } = action.payload;
+      const { conversationKey, stagedAttachmentId } = action.payload;
 
       const currentStagedAttachments = state.stagedAttachments[conversationKey];
 
       if (!currentStagedAttachments || _.isEmpty(currentStagedAttachments)) {
         return state;
       }
-      const attachmentToRemove = currentStagedAttachments.find(m => m.fileName === filename);
+      const attachmentToRemove = currentStagedAttachments.find(
+        m => m.stagedAttachmentId === stagedAttachmentId
+      );
 
       if (!attachmentToRemove) {
         return state;
@@ -89,7 +100,7 @@ const stagedAttachmentsSlice = createSlice({
         URL.revokeObjectURL(attachmentToRemove.videoUrl);
       }
       state.stagedAttachments[conversationKey] = state.stagedAttachments[conversationKey].filter(
-        a => a.fileName !== filename
+        a => a.stagedAttachmentId !== stagedAttachmentId
       );
       return state;
     },

ts/test/session/unit/staged_attachments/StagedAttachments_test.ts

@@ -0,0 +1,215 @@
+import { expect } from 'chai';
+import Sinon from 'sinon';
+
+import {
+  addStagedAttachmentsInConversation,
+  getEmptyStagedAttachmentsState,
+  reducer,
+  removeStagedAttachmentInConversation,
+} from '../../../../state/ducks/stagedAttachments';
+import type { StagedAttachmentType } from '../../../../components/conversation/composition/CompositionBox';
+
+const conversationKey = 'conversation-key';
+
+function makeAttachment({
+  stagedAttachmentId,
+  fileName,
+  url = '',
+  videoUrl,
+  isVoiceMessage = false,
+}: {
+  stagedAttachmentId: string;
+  fileName: string;
+  url?: string;
+  videoUrl?: string;
+  isVoiceMessage?: boolean;
+}): StagedAttachmentType {
+  return {
+    stagedAttachmentId,
+    file: {} as File,
+    contentType: 'image/jpeg',
+    fileName,
+    url,
+    videoUrl,
+    fileSize: null,
+    isVoiceMessage,
+    screenshot: null,
+    thumbnail: null,
+  };
+}
+
+describe('state/ducks/stagedAttachments', () => {
+  beforeEach(() => {
+    (global as any).window = {
+      log: {
+        warn: Sinon.stub(),
+      },
+    };
+
+    if (!URL.revokeObjectURL) {
+      URL.revokeObjectURL = () => undefined;
+    }
+
+    Sinon.stub(URL, 'revokeObjectURL');
+  });
+
+  afterEach(() => {
+    Sinon.restore();
+    delete (global as any).window;
+  });
+
+  it('keeps staged attachments with the same filename when their staged ids differ', () => {
+    const first = makeAttachment({
+      stagedAttachmentId: 'first',
+      fileName: 'image.jpg',
+      url: 'blob:first',
+    });
+    const second = makeAttachment({
+      stagedAttachmentId: 'second',
+      fileName: 'image.jpg',
+      url: 'blob:second',
+    });
+
+    const state = reducer(
+      getEmptyStagedAttachmentsState(),
+      addStagedAttachmentsInConversation({
+        conversationKey,
+        newAttachments: [first, second],
+      })
+    );
+
+    expect(state.stagedAttachments[conversationKey].map(attachment => attachment.url)).to.deep.eq([
+      'blob:first',
+      'blob:second',
+    ]);
+  });
+
+  it('removes only the staged attachment matching the staged id', () => {
+    const first = makeAttachment({
+      stagedAttachmentId: 'first',
+      fileName: 'image.jpg',
+      url: 'blob:first',
+    });
+    const second = makeAttachment({
+      stagedAttachmentId: 'second',
+      fileName: 'image.jpg',
+      url: 'blob:second',
+      videoUrl: 'blob:second-video',
+    });
+
+    const stateWithAttachments = reducer(
+      getEmptyStagedAttachmentsState(),
+      addStagedAttachmentsInConversation({
+        conversationKey,
+        newAttachments: [first, second],
+      })
+    );
+
+    const state = reducer(
+      stateWithAttachments,
+      removeStagedAttachmentInConversation({
+        conversationKey,
+        stagedAttachmentId: 'second',
+      })
+    );
+
+    const revokedUrls = (URL.revokeObjectURL as Sinon.SinonStub)
+      .getCalls()
+      .map(call => call.args[0]);
+
+    expect(
+      state.stagedAttachments[conversationKey].map(attachment => attachment.stagedAttachmentId)
+    ).to.deep.eq(['first']);
+    expect(revokedUrls).to.deep.eq(['blob:second', 'blob:second-video']);
+  });
+
+  it('does not add a voice message with another staged attachment', () => {
+    const currentAttachment = makeAttachment({
+      stagedAttachmentId: 'current',
+      fileName: 'image.jpg',
+    });
+    const voiceAttachment = makeAttachment({
+      stagedAttachmentId: 'voice',
+      fileName: 'session-audio-message',
+      isVoiceMessage: true,
+    });
+
+    const stateWithAttachment = reducer(
+      getEmptyStagedAttachmentsState(),
+      addStagedAttachmentsInConversation({
+        conversationKey,
+        newAttachments: [currentAttachment],
+      })
+    );
+
+    const state = reducer(
+      stateWithAttachment,
+      addStagedAttachmentsInConversation({
+        conversationKey,
+        newAttachments: [voiceAttachment],
+      })
+    );
+
+    expect(
+      state.stagedAttachments[conversationKey].map(attachment => attachment.stagedAttachmentId)
+    ).to.deep.eq(['current']);
+    expect((global as any).window.log.warn.calledOnce).to.eq(true);
+  });
+
+  it('does not add multiple voice messages at once', () => {
+    const firstVoiceAttachment = makeAttachment({
+      stagedAttachmentId: 'first-voice',
+      fileName: 'session-audio-message',
+      isVoiceMessage: true,
+    });
+    const secondVoiceAttachment = makeAttachment({
+      stagedAttachmentId: 'second-voice',
+      fileName: 'session-audio-message',
+      isVoiceMessage: true,
+    });
+
+    const state = reducer(
+      getEmptyStagedAttachmentsState(),
+      addStagedAttachmentsInConversation({
+        conversationKey,
+        newAttachments: [firstVoiceAttachment, secondVoiceAttachment],
+      })
+    );
+
+    expect(state.stagedAttachments[conversationKey]).to.eq(undefined);
+    expect((global as any).window.log.warn.calledOnce).to.eq(true);
+  });
+
+  it('does not add another attachment when a voice message is already staged', () => {
+    const voiceAttachment = makeAttachment({
+      stagedAttachmentId: 'voice',
+      fileName: 'session-audio-message',
+      isVoiceMessage: true,
+    });
+    const nextAttachment = makeAttachment({
+      stagedAttachmentId: 'next',
+      fileName: 'image.jpg',
+    });
+
+    const stateWithVoiceMessage = reducer(
+      getEmptyStagedAttachmentsState(),
+      addStagedAttachmentsInConversation({
+        conversationKey,
+        newAttachments: [voiceAttachment],
+      })
+    );
+
+    const state = reducer(
+      stateWithVoiceMessage,
+      addStagedAttachmentsInConversation({
+        conversationKey,
+        newAttachments: [nextAttachment],
+      })
+    );
+
+    expect(
+      state.stagedAttachments[conversationKey].map(attachment => attachment.stagedAttachmentId)
+    ).to.deep.eq(['voice']);
+    expect((global as any).window.log.warn.calledOnce).to.eq(true);
+  });
+});