FIX: Uploading multiple files to chat could cause canellations (#20605)

When we introduced `existingUploads` as an arg to the
ChatComposerUploads component, we also introduced a bug where
if multiple uploads were being done at once, and the draft
was saved, then because of didReceiveAttrs we would cancel
the currently uploading files because the draft uploads became
the existingUploads.

To work around this, since we do want to keep this on didReceiveAttrs
for cases when the user opens a draft or edits another message,
the easiest thing to do is to just not save uploads into the chat
draft if there are still uploads in progress. That way only when
all uploads are complete do we make them a part of the draft.

There is a small risk that the user could do something to lose
their uploads in the draft, but it's a better gamble to have
that happen rather than in progress uploads to be cancelled
while the user is waiting for them to be done because of the
draft.

Also changes the uploads system spec back to the old way of
attaching multiple files since that is why it was failing.

plugins/chat/assets/javascripts/discourse/components/chat-composer-uploads.js

@@ -26,7 +26,7 @@ export default Component.extend(UppyUploadMixin, {
 
   didReceiveAttrs() {
     this._super(...arguments);
-    if (this._uppyInstance?.getState?.()?.totalProgress > 0) {
+    if (this.inProgressUploads?.length > 0) {
       this._uppyInstance?.cancelAll();
     }
 
@@ -53,7 +53,7 @@ export default Component.extend(UppyUploadMixin, {
 
   uploadDone(upload) {
     this.uploads.pushObject(upload);
-    this.onUploadChanged(this.uploads);
+    this._triggerUploadsChanged();
   },
 
   @discourseComputed("uploads.length", "inProgressUploads.length")
@@ -67,13 +67,13 @@ export default Component.extend(UppyUploadMixin, {
       fileId: upload.id,
     });
     this.uploads.removeObject(upload);
-    this.onUploadChanged(this.uploads);
+    this._triggerUploadsChanged();
   },
 
   @action
   removeUpload(upload) {
     this.uploads.removeObject(upload);
-    this.onUploadChanged(this.uploads);
+    this._triggerUploadsChanged();
   },
 
   _uploadDropTargetOptions() {
@@ -134,4 +134,10 @@ export default Component.extend(UppyUploadMixin, {
       this._addFiles([...event.clipboardData.files], { pasted: true });
     }
   },
+
+  _triggerUploadsChanged() {
+    this.onUploadChanged(this.uploads, {
+      inProgressUploadsCount: this.inProgressUploads?.length,
+    });
+  },
 });

plugins/chat/assets/javascripts/discourse/components/chat-composer.js

@@ -719,9 +719,9 @@ export default Component.extend(TextareaTextManipulation, {
   },
 
   @action
-  uploadsChanged(uploads) {
+  uploadsChanged(uploads, { inProgressUploadsCount }) {
     this.set("_uploads", cloneJSON(uploads));
-    this.onValueChange?.({ uploads: this._uploads });
+    this.onValueChange?.({ uploads: this._uploads, inProgressUploadsCount });
   },
 
   @action

plugins/chat/assets/javascripts/discourse/components/chat-live-pane.js

@@ -1005,14 +1005,24 @@ export default class ChatLivePane extends Component {
   }
 
   @action
-  composerValueChanged({ value, uploads, replyToMsg }) {
+  composerValueChanged({ value, uploads, replyToMsg, inProgressUploadsCount }) {
     if (!this.editingMessage && !this.args.channel.isDraft) {
       if (typeof value !== "undefined") {
         this.args.channel.draft.message = value;
       }
-      if (typeof uploads !== "undefined") {
+
+      // only save the uploads to the draft if we are not still uploading other
+      // ones, otherwise we get into a cycle where we pass the draft uploads as
+      // existingUploads back to the upload component and cause in progress ones
+      // to be cancelled
+      if (
+        typeof uploads !== "undefined" &&
+        inProgressUploadsCount !== "undefined" &&
+        inProgressUploadsCount === 0
+      ) {
         this.args.channel.draft.uploads = uploads;
       }
+
       if (typeof replyToMsg !== "undefined") {
         this.args.channel.draft.replyToMsg = replyToMsg;
       }

plugins/chat/spec/system/uploads_spec.rb

@@ -38,17 +38,10 @@ describe "Uploading files in chat messages", type: :system, js: true do
       chat.visit_channel(channel_1)
 
       file_path_1 = file_from_fixtures("logo.png", "images").path
-      attach_file([file_path_1]) do
-        channel.open_action_menu
-        channel.click_action_button("chat-upload-btn")
-        find(".chat-composer-input").click
-      end
-
       file_path_2 = file_from_fixtures("logo.jpg", "images").path
-      attach_file([file_path_2]) do
+      attach_file([file_path_1, file_path_2]) do
         channel.open_action_menu
         channel.click_action_button("chat-upload-btn")
-        find(".chat-composer-input").click
       end
 
       expect(page).to have_css(".chat-composer-upload .preview .preview-img", count: 2)