From e46495833309d010544ae98ca4ab4c2d24c84713 Mon Sep 17 00:00:00 2001
From: Martin Brennan <mjrbrennan@gmail.com>
Date: Mon, 29 Nov 2021 08:32:06 +1000
Subject: [PATCH] DEV: Fix uploadHandler impl. in composer-upload-uppy mixin
 (#15105)

In f6528afa019ded81012947efdf2835324488183b I added parity support
for composer upload handlers to the uppy-ized composer. However the
way I assumed that it was only possible to handle a single file
upload at a time was false; it only appeared this way in the old
jQuery file upload composer because jQuery file upload sent through
files one at a time even if multiple were added at once. This caused
issues in certain plugins and themes by third parties.

This commit fixes the issue by making the uppy upload handler work
the same as the old one, by capturing all of the added files that
have matching handlers then going through them one by one and passing
them to the handler function.
---
 .../app/mixins/composer-upload-uppy.js        | 43 +++++++++++++------
 1 file changed, 29 insertions(+), 14 deletions(-)

diff --git a/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js b/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js
index 316518f9dee..a79b742a717 100644
--- a/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js
+++ b/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js
@@ -140,24 +140,36 @@ export default Mixin.create(ExtendableUploader, UppyS3Multipart, {
       },
 
       onBeforeUpload: (files) => {
-        const fileCount = Object.keys(files).length;
         const maxFiles = this.siteSettings.simultaneous_uploads;
 
         // Look for a matching file upload handler contributed from a plugin.
-        // It is not ideal that this only works for single file uploads, but
-        // at this time it is all we need. In future we may want to devise a
-        // nicer way of doing this. Uppy plugins are out of the question because
-        // there is no way to define which uploader plugin handles which file
-        // extensions at this time.
-        if (fileCount === 1) {
-          const file = Object.values(files)[0];
+        // In future we may want to devise a nicer way of doing this.
+        // Uppy plugins are out of the question because there is no way to
+        // define which uploader plugin handles which file extensions at this time.
+        const uploadHandlerFiles = [];
+        const unhandledFiles = {};
+        for (const [fileId, file] of Object.entries(files)) {
           const matchingHandler = this._findMatchingUploadHandler(file.name);
-          if (matchingHandler && !matchingHandler.method(file.data, this)) {
-            return this._abortAndReset();
+          if (matchingHandler) {
+            uploadHandlerFiles.push([file, matchingHandler]);
+          } else {
+            unhandledFiles[fileId] = { ...files[fileId] };
           }
         }
 
-        // Limit the number of simultaneous uploads
+        // This replicates what the old composer upload handler did; because
+        // jQuery file uploader passed through a single file at a time to
+        // the upload handlers.
+        uploadHandlerFiles.forEach((fileWithHandler) => {
+          const [file, matchingHandler] = fileWithHandler;
+          if (matchingHandler && !matchingHandler.method(file.data, this)) {
+            return this._abortAndReset();
+          }
+        });
+
+        // Limit the number of simultaneous uploads, for files which have
+        // _not_ been handled by an upload handler.
+        const fileCount = Object.keys(unhandledFiles).length;
         if (maxFiles > 0 && fileCount > maxFiles) {
           bootbox.alert(
             I18n.t("post.errors.too_many_dragged_and_dropped_files", {
@@ -166,6 +178,9 @@ export default Mixin.create(ExtendableUploader, UppyS3Multipart, {
           );
           return this._abortAndReset();
         }
+
+        // uppy uses this new object to track progress of remaining files
+        return unhandledFiles;
       },
     });
 
@@ -196,14 +211,14 @@ export default Mixin.create(ExtendableUploader, UppyS3Multipart, {
     });
 
     this._uppyInstance.on("file-removed", (file, reason) => {
-      file.meta.cancelled = true;
-
       // we handle the cancel-all event specifically, so no need
-      // to do anything here
+      // to do anything here. this event is also fired when some files
+      // are handled by an upload handler
       if (reason === "cancel-all") {
         return;
       }
 
+      file.meta.cancelled = true;
       this._removeInProgressUpload(file.id);
       this._resetUpload(file, { removePlaceholder: true });
       if (this.inProgressUploads.length === 0) {