From fac91854212cb1874f76bf3b610abc52c422e32a Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Thu, 4 Nov 2021 11:31:58 +1000 Subject: [PATCH] FEATURE: Move emoji uploader to use uppy (#14814) This commit changes the emoji uploader to use the UppyUploadMixin, and makes some minor changes to the emoji uploader (tightening the copy for drag and drop and adding a percentage for the upload). Since no other uppy upload mixin components have needed to upload multiple files so far, this necessitated adding a tracker for the in progress uploads so we know when to reset the uploader once all uploads are complete. At the moment, the emoji uploader cannot be used for direct S3 uploads because the admin emoji controller creates other records and does other magic with the emojis. At some point we need to factor this kind of thing into the ExternalUploadManager.transform! action to complete external uploads. --- .../app/components/emoji-uploader.js | 10 ++---- .../discourse/app/mixins/uppy-upload.js | 31 ++++++++++++++++--- .../templates/components/emoji-uploader.hbs | 3 +- app/controllers/admin/emojis_controller.rb | 3 ++ config/locales/client.en.yml | 2 +- 5 files changed, 36 insertions(+), 13 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/emoji-uploader.js b/app/assets/javascripts/discourse/app/components/emoji-uploader.js index 371137e4fa0..474aba66dc7 100644 --- a/app/assets/javascripts/discourse/app/components/emoji-uploader.js +++ b/app/assets/javascripts/discourse/app/components/emoji-uploader.js @@ -1,12 +1,12 @@ import Component from "@ember/component"; -import UploadMixin from "discourse/mixins/upload"; +import UppyUploadMixin from "discourse/mixins/uppy-upload"; import { action } from "@ember/object"; import discourseComputed from "discourse-common/utils/decorators"; import { notEmpty } from "@ember/object/computed"; const DEFAULT_GROUP = "default"; -export default Component.extend(UploadMixin, { +export default Component.extend(UppyUploadMixin, { type: "emoji", uploadUrl: "/admin/customize/emojis", hasName: notEmpty("name"), @@ -15,10 +15,10 @@ export default Component.extend(UploadMixin, { emojiGroups: null, newEmojiGroups: null, tagName: null, + preventDirectS3Uploads: true, didReceiveAttrs() { this._super(...arguments); - this.set("newEmojiGroups", this.emojiGroups); }, @@ -27,10 +27,6 @@ export default Component.extend(UploadMixin, { return !this.hasName || this.uploading; }, - uploadOptions() { - return { sequentialUploads: true }; - }, - @action createEmojiGroup(group) { this.setProperties({ diff --git a/app/assets/javascripts/discourse/app/mixins/uppy-upload.js b/app/assets/javascripts/discourse/app/mixins/uppy-upload.js index e6d282b988b..efa0b30710b 100644 --- a/app/assets/javascripts/discourse/app/mixins/uppy-upload.js +++ b/app/assets/javascripts/discourse/app/mixins/uppy-upload.js @@ -23,6 +23,7 @@ export default Mixin.create({ uploadProgress: 0, _uppyInstance: null, autoStartUploads: true, + _inProgressUploads: 0, id: null, // TODO (martin): currently used for backups to turn on auto upload and PUT/XML requests @@ -137,21 +138,34 @@ export default Mixin.create({ this.set("uploadProgress", progress); }); + this._uppyInstance.on("upload", (data) => { + this._inProgressUploads += data.fileIDs.length; + }); + this._uppyInstance.on("upload-success", (file, response) => { + this._inProgressUploads--; + if (this.usingS3Uploads) { this.setProperties({ uploading: false, processing: true }); this._completeExternalUpload(file) .then((completeResponse) => { this.uploadDone(completeResponse); - this._reset(); + + if (this._inProgressUploads === 0) { + this._reset(); + } }) .catch((errResponse) => { displayErrorForUpload(errResponse, this.siteSettings, file.name); - this._reset(); + if (this._inProgressUploads === 0) { + this._reset(); + } }); } else { this.uploadDone(response.body); - this._reset(); + if (this._inProgressUploads === 0) { + this._reset(); + } } }); @@ -160,7 +174,16 @@ export default Mixin.create({ this._reset(); }); - if (this.siteSettings.enable_direct_s3_uploads) { + // TODO (martin) preventDirectS3Uploads is necessary because some of + // the current upload mixin components, for example the emoji uploader, + // send the upload to custom endpoints that do fancy things in the rails + // controller with the upload or create additional data or records. we + // need a nice way to do this on complete-external-upload before we can + // allow these other uploaders to go direct to S3. + if ( + this.siteSettings.enable_direct_s3_uploads && + !this.preventDirectS3Uploads + ) { this._useS3Uploads(); } else { this._useXHRUploads(); diff --git a/app/assets/javascripts/discourse/app/templates/components/emoji-uploader.hbs b/app/assets/javascripts/discourse/app/templates/components/emoji-uploader.hbs index a32fe3772ad..4f4bdf782e7 100644 --- a/app/assets/javascripts/discourse/app/templates/components/emoji-uploader.hbs +++ b/app/assets/javascripts/discourse/app/templates/components/emoji-uploader.hbs @@ -35,7 +35,7 @@ diff --git a/app/controllers/admin/emojis_controller.rb b/app/controllers/admin/emojis_controller.rb index 72e1c308011..8320ea340e3 100644 --- a/app/controllers/admin/emojis_controller.rb +++ b/app/controllers/admin/emojis_controller.rb @@ -6,6 +6,9 @@ class Admin::EmojisController < Admin::AdminController render_serialized(Emoji.custom, EmojiSerializer, root: false) end + # TODO (martin) Figure out a way that this kind of custom logic can + # be run in the ExternalUploadManager when a direct S3 upload is completed, + # related to preventDirectS3Uploads in the UppyUploadMixin. def create file = params[:file] || params[:files].first name = params[:name] || File.basename(file.original_filename, ".*") diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 58e2adf1e3e..56b9810b5fe 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -5437,7 +5437,7 @@ en: emoji: title: "Emoji" - help: "Add new emoji that will be available to everyone. (PROTIP: drag & drop multiple files at once)" + help: "Add new emoji that will be available to everyone. Drag and drop multiple files at once without entering a name to create emojis using their file names." add: "Add New Emoji" uploading: "Uploading..." name: "Name"