diff --git a/app/assets/javascripts/admin/addon/components/site-settings-image-uploader.js b/app/assets/javascripts/admin/addon/components/site-settings-image-uploader.js deleted file mode 100644 index 3e19fa8f033..00000000000 --- a/app/assets/javascripts/admin/addon/components/site-settings-image-uploader.js +++ /dev/null @@ -1,6 +0,0 @@ -import ImageUploader from "discourse/components/image-uploader"; - -export default ImageUploader.extend({ - layoutName: "components/image-uploader", - uploadUrlParams: "&for_site_setting=true", -}); diff --git a/app/assets/javascripts/admin/addon/templates/badges-show.hbs b/app/assets/javascripts/admin/addon/templates/badges-show.hbs index 736d1b8b431..a1260d04816 100644 --- a/app/assets/javascripts/admin/addon/templates/badges-show.hbs +++ b/app/assets/javascripts/admin/addon/templates/badges-show.hbs @@ -40,12 +40,14 @@ {{#if imageUploaderSelected}} - {{image-uploader + {{uppy-image-uploader + id="badge-image-uploader" imageUrl=buffered.image_url + type="badge_image" onUploadDone=(action "setImage") onUploadDeleted=(action "removeImage") - type="badge_image" - class="no-repeat contain-image"}} + class="no-repeat contain-image" + }}

{{i18n "admin.badges.image_help"}}

diff --git a/app/assets/javascripts/admin/addon/templates/components/site-settings/upload.hbs b/app/assets/javascripts/admin/addon/templates/components/site-settings/upload.hbs index 652f773f362..78a64c25dd2 100644 --- a/app/assets/javascripts/admin/addon/templates/components/site-settings/upload.hbs +++ b/app/assets/javascripts/admin/addon/templates/components/site-settings/upload.hbs @@ -1,2 +1,8 @@ -{{site-settings-image-uploader imageUrl=value placeholderUrl=setting.placeholder type="site_setting"}} +{{uppy-image-uploader + imageUrl=value + placeholderUrl=setting.placeholder + additionalParams=(hash for_site_setting=true) + type="site_setting" + id=(concat "site-setting-image-uploader-" setting.setting) +}}
{{html-safe setting.description}}
diff --git a/app/assets/javascripts/discourse/app/components/image-uploader.js b/app/assets/javascripts/discourse/app/components/image-uploader.js index 441ec4b743c..9852497bca5 100644 --- a/app/assets/javascripts/discourse/app/components/image-uploader.js +++ b/app/assets/javascripts/discourse/app/components/image-uploader.js @@ -1,4 +1,5 @@ import Component from "@ember/component"; +import deprecated from "discourse-common/lib/deprecated"; import UploadMixin from "discourse/mixins/upload"; import { ajax } from "discourse/lib/ajax"; import discourseComputed from "discourse-common/utils/decorators"; @@ -14,6 +15,10 @@ export default Component.extend(UploadMixin, { init() { this._super(...arguments); + // TODO (martin) (2022-01-22) Remove this component. + deprecated( + "image-uploader will be removed in a future version, use uppy-image-uploader instead (the API is the same)" + ); this._applyLightbox(); }, diff --git a/app/assets/javascripts/discourse/app/components/uppy-image-uploader.js b/app/assets/javascripts/discourse/app/components/uppy-image-uploader.js index 7b416ca07c8..1d2175d51d9 100644 --- a/app/assets/javascripts/discourse/app/components/uppy-image-uploader.js +++ b/app/assets/javascripts/discourse/app/components/uppy-image-uploader.js @@ -69,8 +69,6 @@ export default Component.extend(UppyUploadMixin, { uploadDone(upload) { this.setProperties({ - imageUrl: upload.url, - imageId: upload.id, imageFilesize: upload.human_filesize, imageFilename: upload.original_filename, imageWidth: upload.width, @@ -79,8 +77,13 @@ export default Component.extend(UppyUploadMixin, { this._applyLightbox(); + // the value of the property used for imageUrl should be set + // in this callback. this should be done in cases where imageUrl + // is bound to a computed property of the parent component. if (this.onUploadDone) { this.onUploadDone(upload); + } else { + this.set("imageUrl", upload.url); } }, @@ -123,13 +126,16 @@ export default Component.extend(UppyUploadMixin, { }, trash() { - this.setProperties({ imageUrl: null, imageId: null }); - // uppy needs to be reset to allow for more uploads this._reset(); + // the value of the property used for imageUrl should be cleared + // in this callback. this should be done in cases where imageUrl + // is bound to a computed property of the parent component. if (this.onUploadDeleted) { this.onUploadDeleted(); + } else { + this.setProperties({ imageUrl: null }); } }, }, diff --git a/app/assets/javascripts/discourse/app/controllers/preferences/profile.js b/app/assets/javascripts/discourse/app/controllers/preferences/profile.js index 5356a8a0b9b..6e9403d2556 100644 --- a/app/assets/javascripts/discourse/app/controllers/preferences/profile.js +++ b/app/assets/javascripts/discourse/app/controllers/preferences/profile.js @@ -68,10 +68,6 @@ export default Controller.extend({ "model.can_upload_user_card_background" ), - experimentalUserCardImageUpload: readOnly( - "siteSettings.enable_experimental_image_uploader" - ), - actions: { showFeaturedTopicModal() { showModal("feature-topic-on-profile", { 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 3089a399b33..c5f1e08b670 100644 --- a/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js +++ b/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js @@ -136,7 +136,6 @@ export default Mixin.create(ExtendableUploader, { this._instrumentUploadTimings(); } - // hidden setting like enable_experimental_image_uploader if (this.siteSettings.enable_direct_s3_uploads) { this._useS3MultipartUploads(); } else { @@ -482,6 +481,8 @@ export default Mixin.create(ExtendableUploader, { data: JSON.stringify({ parts, unique_identifier: file.meta.unique_identifier, + pasted: file.meta.pasted, + for_private_message: file.meta.for_private_message, }), // uppy is inconsistent, an error here fires the upload-error event }).then((responseData) => { @@ -570,14 +571,14 @@ export default Mixin.create(ExtendableUploader, { } if (event && event.clipboardData && event.clipboardData.files) { - this._addFiles([...event.clipboardData.files]); + this._addFiles([...event.clipboardData.files], { pasted: true }); } }.bind(this); this.element.addEventListener("paste", this.pasteEventListener); }, - _addFiles(files) { + _addFiles(files, opts = {}) { files = Array.isArray(files) ? files : [files]; try { this._uppyInstance.addFiles( @@ -587,6 +588,7 @@ export default Mixin.create(ExtendableUploader, { name: file.name, type: file.type, data: file, + meta: { pasted: opts.pasted }, }; }) ); diff --git a/app/assets/javascripts/discourse/app/mixins/uppy-upload.js b/app/assets/javascripts/discourse/app/mixins/uppy-upload.js index ad8cd44084a..e6d282b988b 100644 --- a/app/assets/javascripts/discourse/app/mixins/uppy-upload.js +++ b/app/assets/javascripts/discourse/app/mixins/uppy-upload.js @@ -25,10 +25,6 @@ export default Mixin.create({ autoStartUploads: true, id: null, - // TODO (martin): this is only used in one place, consider just using - // form data/meta instead uploadUrlParams: "&for_site_setting=true", - uploadUrlParams: "", - // TODO (martin): currently used for backups to turn on auto upload and PUT/XML requests // and for emojis to do sequential uploads, when we get to replacing those // with uppy make sure this is used when initializing uppy @@ -83,7 +79,11 @@ export default Mixin.create({ // need to use upload_type because uppy overrides type with the // actual file type - meta: deepMerge({ upload_type: this.type }, this.data || {}), + meta: deepMerge( + { upload_type: this.type }, + this.additionalParams || {}, + this.data || {} + ), onBeforeFileAdded: (currentFile) => { const validationOpts = deepMerge( @@ -160,7 +160,6 @@ export default Mixin.create({ this._reset(); }); - // hidden setting like enable_experimental_image_uploader if (this.siteSettings.enable_direct_s3_uploads) { this._useS3Uploads(); } else { @@ -224,8 +223,7 @@ export default Mixin.create({ return ( getUrl(this.getWithDefault("uploadUrl", "/uploads")) + ".json?client_id=" + - (this.messageBus && this.messageBus.clientId) + - this.uploadUrlParams + this.messageBus?.clientId ); }, @@ -252,9 +250,10 @@ export default Mixin.create({ _completeExternalUpload(file) { return ajax(getUrl("/uploads/complete-external-upload"), { type: "POST", - data: { - unique_identifier: file.meta.uniqueUploadIdentifier, - }, + data: deepMerge( + { unique_identifier: file.meta.uniqueUploadIdentifier }, + this.additionalParams || {} + ), }); }, diff --git a/app/assets/javascripts/discourse/app/templates/components/edit-category-images.hbs b/app/assets/javascripts/discourse/app/templates/components/edit-category-images.hbs index b18e4385482..4b44458c03a 100644 --- a/app/assets/javascripts/discourse/app/templates/components/edit-category-images.hbs +++ b/app/assets/javascripts/discourse/app/templates/components/edit-category-images.hbs @@ -1,18 +1,22 @@
- {{image-uploader + {{uppy-image-uploader imageUrl=backgroundImageUrl onUploadDone=(action "backgroundUploadDone") onUploadDeleted=(action "backgroundUploadDeleted") - type="category_background"}} + type="category_background" + id="category-background-uploader" + }}
diff --git a/app/assets/javascripts/discourse/app/templates/components/group-flair-inputs.hbs b/app/assets/javascripts/discourse/app/templates/components/group-flair-inputs.hbs index 390f48ac059..1c41852fd84 100644 --- a/app/assets/javascripts/discourse/app/templates/components/group-flair-inputs.hbs +++ b/app/assets/javascripts/discourse/app/templates/components/group-flair-inputs.hbs @@ -21,11 +21,12 @@ onChange=(action (mut model.flair_icon)) }} {{else if flairPreviewImage}} - {{image-uploader + {{uppy-image-uploader imageUrl=flairImageUrl onUploadDone=(action "setFlairImage") onUploadDeleted=(action "removeFlairImage") type="group_flair" + id="group-flair-uploader" class="no-repeat contain-image"}}
{{i18n "groups.flair_upload_description"}} diff --git a/app/assets/javascripts/discourse/app/templates/preferences/profile.hbs b/app/assets/javascripts/discourse/app/templates/preferences/profile.hbs index a0f53843151..4ac86cea15d 100644 --- a/app/assets/javascripts/discourse/app/templates/preferences/profile.hbs +++ b/app/assets/javascripts/discourse/app/templates/preferences/profile.hbs @@ -47,8 +47,11 @@
- {{image-uploader imageUrl=model.profile_background_upload_url - type="profile_background"}} + {{uppy-image-uploader + imageUrl=model.profile_background_upload_url + type="profile_background" + id="profile-background-uploader" + }}
{{i18n "user.change_profile_background.instructions"}} @@ -59,15 +62,11 @@
- {{#if experimentalUserCardImageUpload}} - {{uppy-image-uploader - imageUrl=model.card_background_upload_url - type="card_background" - id="profile-card-background" - }} - {{else}} - {{image-uploader imageUrl=model.card_background_upload_url type="card_background"}} - {{/if}} + {{uppy-image-uploader + imageUrl=model.card_background_upload_url + type="card_background" + id="profile-card-background-uploader" + }}
{{i18n "user.change_card_background.instructions"}} diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index fa8fa177da1..142b6a00b8b 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -253,7 +253,13 @@ class UploadsController < ApplicationController end def complete_external_upload_via_manager(external_upload_stub) - external_upload_manager = ExternalUploadManager.new(external_upload_stub) + opts = { + for_private_message: params[:for_private_message]&.to_s == "true", + for_site_setting: params[:for_site_setting]&.to_s == "true", + pasted: params[:pasted]&.to_s == "true", + } + + external_upload_manager = ExternalUploadManager.new(external_upload_stub, opts) hijack do begin upload = external_upload_manager.promote_to_upload! diff --git a/app/services/external_upload_manager.rb b/app/services/external_upload_manager.rb index d08cee65fdf..e5c032b4682 100644 --- a/app/services/external_upload_manager.rb +++ b/app/services/external_upload_manager.rb @@ -20,8 +20,9 @@ class ExternalUploadManager Discourse.redis.get("#{BAN_USER_REDIS_PREFIX}#{user.id}") == "1" end - def initialize(external_upload_stub) + def initialize(external_upload_stub, upload_create_opts = {}) @external_upload_stub = external_upload_stub + @upload_create_opts = upload_create_opts end def can_promote? @@ -67,18 +68,13 @@ class ExternalUploadManager end # TODO (martin): See if these additional opts will be needed - # - # for_private_message: for_private_message, - # for_site_setting: for_site_setting, - # pasted: pasted, - # - # also check if retain_hours is needed + # - check if retain_hours is needed opts = { type: external_upload_stub.upload_type, existing_external_upload_key: external_upload_stub.key, external_upload_too_big: external_size > DOWNLOAD_LIMIT, filesize: external_size - } + }.merge(@upload_create_opts) UploadCreator.new(tempfile, external_upload_stub.original_filename, opts).create_for( external_upload_stub.created_by_id diff --git a/db/migrate/20211018234219_remove_enable_experimental_image_uploader_site_setting.rb b/db/migrate/20211018234219_remove_enable_experimental_image_uploader_site_setting.rb new file mode 100644 index 00000000000..2c5034053aa --- /dev/null +++ b/db/migrate/20211018234219_remove_enable_experimental_image_uploader_site_setting.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class RemoveEnableExperimentalImageUploaderSiteSetting < ActiveRecord::Migration[6.1] + def up + execute <<~SQL + DELETE FROM site_settings + WHERE name = 'enable_experimental_image_uploader' + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end