From 20b2a42f491c2633323a4db0c5b43c5b2f31324b Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Mon, 29 Nov 2021 11:19:02 +1000 Subject: [PATCH] DEV: Send multiple files in batches to composer upload handlers when using uppy (#15124) In jQuery file upload land, we were sending a single file through at a time to matching upload handlers. This in turn required plugin authors to marshal the files as they came through one by one if they wanted to group them together to do something with them. Now that we are using uppy, files come through in the groups they are added in (for example dropping multiple, selecting multiple from the system file dialogue). This commit changes the matching upload handlers to send through all matching files at once instead of piecemeal. --- .../app/mixins/composer-upload-uppy.js | 27 ++++++++++++------- .../acceptance/composer-uploads-uppy-test.js | 3 ++- 2 files changed, 20 insertions(+), 10 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 a79b742a717..c623a92388c 100644 --- a/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js +++ b/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js @@ -146,26 +146,35 @@ export default Mixin.create(ExtendableUploader, UppyS3Multipart, { // 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 = {}; + const handlerBuckets = {}; + for (const [fileId, file] of Object.entries(files)) { const matchingHandler = this._findMatchingUploadHandler(file.name); if (matchingHandler) { - uploadHandlerFiles.push([file, matchingHandler]); + // the function signature will be converted to a string for the + // object key, so we can send multiple files at once to each handler + if (handlerBuckets[matchingHandler.method]) { + handlerBuckets[matchingHandler.method].files.push(file); + } else { + handlerBuckets[matchingHandler.method] = { + fn: matchingHandler.method, + files: [file], + }; + } } else { unhandledFiles[fileId] = { ...files[fileId] }; } } - // 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)) { + // Send the collected array of files to each matching handler, + // rather than the old jQuery file uploader method of sending + // a single file at a time through to the handler. + for (const bucket of Object.values(handlerBuckets)) { + if (!bucket.fn(bucket.files, this)) { return this._abortAndReset(); } - }); + } // Limit the number of simultaneous uploads, for files which have // _not_ been handled by an upload handler. diff --git a/app/assets/javascripts/discourse/tests/acceptance/composer-uploads-uppy-test.js b/app/assets/javascripts/discourse/tests/acceptance/composer-uploads-uppy-test.js index d0a0847c710..6671bc8a925 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/composer-uploads-uppy-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/composer-uploads-uppy-test.js @@ -234,7 +234,8 @@ acceptance("Uppy Composer Attachment - Upload Handler", function (needs) { }); needs.hooks.beforeEach(() => { withPluginApi("0.8.14", (api) => { - api.addComposerUploadHandler(["png"], (file) => { + api.addComposerUploadHandler(["png"], (files) => { + const file = files[0]; bootbox.alert(`This is an upload handler test for ${file.name}`); }); });