fix: upsert attachmemts only when attachment exist and handle FileList for react native (#1539)

The goal of the PR is to upsert attachments only when they exist. This
is especially important because on React Native, we allow cancelling
uploading by calling removeAttachments, but when the upload still
happens, the upsert proceeds. We will only update you if the attachment
is present.

The other fix is that the `FileList` is not supported on RN. Thanks to
@MartinCupela for a prompt suggestion on this issue.

Author: khushal87

src/messageComposer/attachmentManager.ts

@@ -207,35 +207,55 @@ export class AttachmentManager {
     return this.attachments.indexOf(attachmentsById[localId]);
   };
 
-  upsertAttachments = (attachmentsToUpsert: LocalAttachment[]) => {
-    if (!attachmentsToUpsert.length) return;
+  private prepareAttachmentUpdate = (attachmentToUpdate: LocalAttachment) => {
+    const stateAttachments = this.attachments;
+    const attachments = [...this.attachments];
+    const attachmentIndex = this.getAttachmentIndex(attachmentToUpdate.localMetadata.id);
+    if (attachmentIndex === -1) return null;
+    // do not re-organize newAttachments array otherwise indexing would no longer work
+    // replace in place only with the attachments with the same id's
+    const merged = mergeWithDiff<LocalAttachment>(
+      stateAttachments[attachmentIndex],
+      attachmentToUpdate,
+    );
+    const updatesOnMerge = merged.diff && Object.keys(merged.diff.children).length;
+    if (updatesOnMerge) {
+      const localAttachment = ensureIsLocalAttachment(merged.result);
+      if (localAttachment) {
+        attachments.splice(attachmentIndex, 1, localAttachment);
+        return attachments;
+      }
+    }
+    return null;
+  };
 
-    const currentAttachments = this.attachments;
-    const newAttachments = [...currentAttachments];
+  updateAttachment = (attachmentToUpdate: LocalAttachment) => {
+    const updatedAttachments = this.prepareAttachmentUpdate(attachmentToUpdate);
+    if (updatedAttachments) {
+      this.state.partialNext({ attachments: updatedAttachments });
+    }
+  };
 
+  upsertAttachments = (attachmentsToUpsert: LocalAttachment[]) => {
+    if (!attachmentsToUpsert.length) return;
+    let attachments = [...this.attachments];
+    let hasUpdates = false;
     attachmentsToUpsert.forEach((attachment) => {
-      const targetAttachmentIndex = this.getAttachmentIndex(attachment.localMetadata?.id);
-
-      if (targetAttachmentIndex < 0) {
-        const localAttachment = ensureIsLocalAttachment(attachment);
-        if (localAttachment) newAttachments.push(localAttachment);
+      const updatedAttachments = this.prepareAttachmentUpdate(attachment);
+      if (updatedAttachments) {
+        attachments = updatedAttachments;
+        hasUpdates = true;
       } else {
-        // do not re-organize newAttachments array otherwise indexing would no longer work
-        // replace in place only with the attachments with the same id's
-        const merged = mergeWithDiff<LocalAttachment>(
-          currentAttachments[targetAttachmentIndex],
-          attachment,
-        );
-        const updatesOnMerge = merged.diff && Object.keys(merged.diff.children).length;
-        if (updatesOnMerge) {
-          const localAttachment = ensureIsLocalAttachment(merged.result);
-          if (localAttachment)
-            newAttachments.splice(targetAttachmentIndex, 1, localAttachment);
+        const localAttachment = ensureIsLocalAttachment(attachment);
+        if (localAttachment) {
+          attachments.push(localAttachment);
+          hasUpdates = true;
         }
       }
     });
-
-    this.state.partialNext({ attachments: newAttachments });
+    if (hasUpdates) {
+      this.state.partialNext({ attachments });
+    }
   };
 
   removeAttachments = (localAttachmentIds: string[]) => {
@@ -476,7 +496,7 @@ export class AttachmentManager {
         },
       };
 
-      this.upsertAttachments([failedAttachment]);
+      this.updateAttachment(failedAttachment);
       return failedAttachment;
     }
 
@@ -510,7 +530,7 @@ export class AttachmentManager {
       (uploadedAttachment as LocalNotImageAttachment).thumb_url = response.thumb_url;
     }
 
-    this.upsertAttachments([uploadedAttachment]);
+    this.updateAttachment(uploadedAttachment);
 
     return uploadedAttachment;
   };

src/messageComposer/fileUtils.ts

@@ -16,7 +16,7 @@ export const isFileList = (obj: unknown): obj is FileList => {
   if (typeof obj !== 'object') return false;
 
   return (
-    (obj as object) instanceof FileList ||
+    (typeof FileList !== 'undefined' && (obj as object) instanceof FileList) ||
     ('item' in obj && 'length' in obj && !Array.isArray(obj))
   );
 };

test/unit/MessageComposer/attachmentManager.test.ts

@@ -503,6 +503,57 @@ describe('AttachmentManager', () => {
     });
   });
 
+  describe('updateAttachment', () => {
+    it('should update an attachment by id', () => {
+      const {
+        messageComposer: { attachmentManager },
+      } = setup();
+
+      const newAttachments = [
+        { localMetadata: { id: 'test-id-1' }, type: 'image' },
+        { localMetadata: { id: 'test-id-2' }, type: 'video' },
+      ];
+
+      attachmentManager.upsertAttachments(newAttachments);
+
+      const updatedAttachment = {
+        id: 'test-id-1',
+        localMetadata: { id: 'test-id-1' },
+        type: 'audio',
+      };
+
+      attachmentManager.updateAttachment(updatedAttachment);
+
+      expect(attachmentManager.attachments).toEqual([
+        updatedAttachment,
+        newAttachments[1],
+      ]);
+    });
+
+    it('should not update an attachment if id is not found', () => {
+      const {
+        messageComposer: { attachmentManager },
+      } = setup();
+
+      const newAttachments = [
+        { localMetadata: { id: 'test-id-1' }, type: 'image' },
+        { localMetadata: { id: 'test-id-2' }, type: 'video' },
+      ];
+
+      attachmentManager.upsertAttachments(newAttachments);
+
+      const updatedAttachment = {
+        id: 'non-existent-id',
+        localMetadata: { id: 'non-existent-id' },
+        type: 'audio',
+      };
+
+      attachmentManager.updateAttachment(updatedAttachment);
+
+      expect(attachmentManager.attachments).toEqual(newAttachments);
+    });
+  });
+
   describe('removeAttachments', () => {
     it('should remove attachments by id', () => {
       const {