diff --git a/src/messageComposer/attachmentManager.ts b/src/messageComposer/attachmentManager.ts index 0733e1176..37b42d761 100644 --- a/src/messageComposer/attachmentManager.ts +++ b/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( + 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( - 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; }; diff --git a/src/messageComposer/fileUtils.ts b/src/messageComposer/fileUtils.ts index fba2067f3..f6418604b 100644 --- a/src/messageComposer/fileUtils.ts +++ b/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)) ); }; diff --git a/test/unit/MessageComposer/attachmentManager.test.ts b/test/unit/MessageComposer/attachmentManager.test.ts index d3cdbb076..ff4158d59 100644 --- a/test/unit/MessageComposer/attachmentManager.test.ts +++ b/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 {