DEV: Add uploadHandler support to composer-upload-uppy mixin (#14692)

This commit adds uploadHandler support to composer uploads using
uppy. The only things we have that are using this are discourse-brightcove and
discourse-video, which both pop modal windows to handle the file upload and
completely leave out all the composer-type flows. This implementation simply
follows the existing one, where if a single file is uploaded and there
is a matching upload handler we take control away from uppy and hand
it off to the upload handler.

Trying to get this kind of thing working within uppy would require a few
changes because they have no way to restrict uploaders to certain file types
and with the way their uploaders are run it doesn't look like it would be easy
to add this either, so I don't think this is worth the work unless at some
point in the future we plan to have more upload handler integrations.

I also fixed an issue with `cleanUpComposerUploadHandler` which is used
in tests to reset the state of `uploadHandlers` in the composer. This
was doing `uploadHandlers = []` to clear that array, but that creates
a brand new array so anything else referencing the original array will
lose that reference. Better to set `uploadHandlers.length = 0` to
clear it. This was breaking the tests I added to see if upload handlers
were working.

app/assets/javascripts/discourse/app/components/composer-editor.js

@@ -48,7 +48,14 @@ export function addComposerUploadHandler(extensions, method) {
   });
 }
 export function cleanUpComposerUploadHandler() {
-  uploadHandlers = [];
+  // we cannot set this to uploadHandlers = [] because that messes with
+  // the references to the original array that the component has. this only
+  // really affects tests, but without doing this you could addComposerUploadHandler
+  // in a beforeEach function in a test but then it's not adding to the
+  // existing reference that the component has, because an earlier test ran
+  // cleanUpComposerUploadHandler and lost it. setting the length to 0 empties
+  // the array but keeps the reference
+  uploadHandlers.length = 0;
 }
 
 let uploadProcessorQueue = [];
@@ -688,6 +695,14 @@ export default Component.extend(ComposerUpload, {
     }
   },
 
+  _findMatchingUploadHandler(fileName) {
+    return this.uploadHandlers.find((handler) => {
+      const ext = handler.extensions.join("|");
+      const regex = new RegExp(`\\.(${ext})$`, "i");
+      return regex.test(fileName);
+    });
+  },
+
   actions: {
     importQuote(toolbarEvent) {
       this.importQuote(toolbarEvent);

app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js

@@ -67,6 +67,12 @@ export default Mixin.create(ExtendableUploader, {
     }
   },
 
+  _abortAndReset() {
+    this.appEvents.trigger(`${this.eventPrefix}:uploads-aborted`);
+    this._reset();
+    return false;
+  },
+
   _bindUploadTarget() {
     this.placeholders = {};
     this._inProgressUploads = 0;
@@ -118,6 +124,20 @@ export default Mixin.create(ExtendableUploader, {
         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];
+          const matchingHandler = this._findMatchingUploadHandler(file.name);
+          if (matchingHandler && !matchingHandler.method(file.data, this)) {
+            return this._abortAndReset();
+          }
+        }
+
         // Limit the number of simultaneous uploads
         if (maxFiles > 0 && fileCount > maxFiles) {
           bootbox.alert(
@@ -125,9 +145,7 @@ export default Mixin.create(ExtendableUploader, {
               count: maxFiles,
             })
           );
-          this.appEvents.trigger(`${this.eventPrefix}:uploads-aborted`);
-          this._reset();
-          return false;
+          return this._abortAndReset();
         }
       },
     });

app/assets/javascripts/discourse/app/mixins/composer-upload.js

@@ -199,9 +199,10 @@ export default Mixin.create({
 
     $element.on("fileuploadsubmit", (e, data) => {
       const max = this.siteSettings.simultaneous_uploads;
+      const fileCount = data.files.length;
 
       // Limit the number of simultaneous uploads
-      if (max > 0 && data.files.length > max) {
+      if (max > 0 && fileCount > max) {
         bootbox.alert(
           I18n.t("post.errors.too_many_dragged_and_dropped_files", {
             count: max,
@@ -211,15 +212,10 @@ export default Mixin.create({
       }
 
       // Look for a matching file upload handler contributed from a plugin
-      const matcher = (handler) => {
-        const ext = handler.extensions.join("|");
-        const regex = new RegExp(`\\.(${ext})$`, "i");
-        return regex.test(data.files[0].name);
-      };
-
-      const matchingHandler = this.uploadHandlers.find(matcher);
-      if (data.files.length === 1 && matchingHandler) {
-        if (!matchingHandler.method(data.files[0], this)) {
+      if (fileCount === 1) {
+        const file = data.files[0];
+        const matchingHandler = this._findMatchingUploadHandler(file.name);
+        if (matchingHandler && !matchingHandler.method(file, this)) {
           return false;
         }
       }

app/assets/javascripts/discourse/tests/acceptance/composer-attachment-test.js

@@ -4,7 +4,9 @@ import {
   query,
   queryAll,
 } from "discourse/tests/helpers/qunit-helpers";
+import { withPluginApi } from "discourse/lib/plugin-api";
 import { click, fillIn, visit } from "@ember/test-helpers";
+import bootbox from "bootbox";
 import { test } from "qunit";
 
 function pretender(server, helper) {
@@ -239,20 +241,51 @@ acceptance("Composer Attachment - Upload Placeholder", function (needs) {
       "![ima++ge|300x400](/images/avatar.png?4)\n"
     );
   });
+});
 
-  function createImage(name, url, width, height) {
-    const file = new Blob([""], { type: "image/png" });
-    file.name = name;
-    return {
-      files: [file],
-      result: {
-        original_filename: name,
-        thumbnail_width: width,
-        thumbnail_height: height,
-        url: url,
-      },
-    };
-  }
+function createImage(name, url, width, height) {
+  const file = new Blob([""], { type: "image/png" });
+  file.name = name;
+  return {
+    files: [file],
+    result: {
+      original_filename: name,
+      thumbnail_width: width,
+      thumbnail_height: height,
+      url,
+    },
+  };
+}
+
+acceptance("Composer Attachment - Upload Handler", function (needs) {
+  needs.user();
+  needs.hooks.beforeEach(() => {
+    withPluginApi("0.8.14", (api) => {
+      api.addComposerUploadHandler(["png"], (file) => {
+        bootbox.alert(`This is an upload handler test for ${file.name}`);
+      });
+    });
+  });
+
+  test("should handle a single file being uploaded with the extension handler", async function (assert) {
+    await visit("/");
+    await click("#create-topic");
+    const image = createImage(
+      "handlertest.png",
+      "/images/avatar.png?1",
+      200,
+      300
+    );
+    await fillIn(".d-editor-input", "This is a handler test.");
+
+    await queryAll(".wmd-controls").trigger("fileuploadsubmit", image);
+    assert.equal(
+      queryAll(".bootbox .modal-body").html(),
+      "This is an upload handler test for handlertest.png",
+      "it should show the bootbox triggered by the upload handler"
+    );
+    await click(".modal-footer .btn");
+  });
 });
 
 acceptance("Composer Attachment - File input", function (needs) {

app/assets/javascripts/discourse/tests/acceptance/composer-uploads-uppy-test.js

@@ -3,6 +3,8 @@ import {
   loggedInUser,
   queryAll,
 } from "discourse/tests/helpers/qunit-helpers";
+import { withPluginApi } from "discourse/lib/plugin-api";
+import bootbox from "bootbox";
 import { authorizedExtensions } from "discourse/lib/uploads";
 import { click, fillIn, visit } from "@ember/test-helpers";
 import I18n from "I18n";
@@ -222,3 +224,39 @@ acceptance("Uppy Composer Attachment - Upload Error", function (needs) {
     appEvents.trigger("composer:add-files", image);
   });
 });
+
+acceptance("Uppy Composer Attachment - Upload Handler", function (needs) {
+  needs.user();
+  needs.pretender(pretender);
+  needs.settings({
+    enable_experimental_composer_uploader: true,
+    simultaneous_uploads: 2,
+  });
+  needs.hooks.beforeEach(() => {
+    withPluginApi("0.8.14", (api) => {
+      api.addComposerUploadHandler(["png"], (file) => {
+        bootbox.alert(`This is an upload handler test for ${file.name}`);
+      });
+    });
+  });
+
+  test("should use upload handler if the matching extension is used and a single file is uploaded", async function (assert) {
+    await visit("/");
+    await click("#create-topic");
+    const image = createFile("handlertest.png");
+    const appEvents = loggedInUser().appEvents;
+    const done = assert.async();
+
+    appEvents.on("composer:uploads-aborted", async () => {
+      assert.equal(
+        queryAll(".bootbox .modal-body").html(),
+        "This is an upload handler test for handlertest.png",
+        "it should show the bootbox triggered by the upload handler"
+      );
+      await click(".modal-footer .btn");
+      done();
+    });
+
+    appEvents.trigger("composer:add-files", [image]);
+  });
+});