diff --git a/app/assets/javascripts/discourse/components/avatar-uploader.js.es6 b/app/assets/javascripts/discourse/components/avatar-uploader.js.es6 index f00f6742806..d5e983ccf8c 100644 --- a/app/assets/javascripts/discourse/components/avatar-uploader.js.es6 +++ b/app/assets/javascripts/discourse/components/avatar-uploader.js.es6 @@ -1,33 +1,29 @@ -import UploadMixin from 'discourse/mixins/upload'; +import UploadMixin from "discourse/mixins/upload"; export default Em.Component.extend(UploadMixin, { - type: 'avatar', - tagName: 'span', + type: "avatar", + tagName: "span", imageIsNotASquare: false, - uploadUrl: Discourse.computed.url('username', '/users/%@/preferences/user_image'), - uploadButtonText: function() { - return this.get("uploading") ? - I18n.t("uploading") : - I18n.t("user.change_avatar.upload_picture"); + return this.get("uploading") ? I18n.t("uploading") : I18n.t("user.change_avatar.upload_picture"); }.property("uploading"), - uploadDone(data) { + uploadDone(upload) { // display a warning whenever the image is not a square - this.set("imageIsNotASquare", data.result.width !== data.result.height); + this.set("imageIsNotASquare", upload.width !== upload.height); // in order to be as much responsive as possible, we're cheating a bit here // indeed, the server gives us back the url to the file we've just uploaded // often, this file is not a square, so we need to crop it properly // this will also capture the first frame of animated avatars when they're not allowed - Discourse.Utilities.cropAvatar(data.result.url, data.files[0].type).then(avatarTemplate => { + Discourse.Utilities.cropAvatar(upload.url, upload.original_filename).then(avatarTemplate => { this.set("uploadedAvatarTemplate", avatarTemplate); // indicates the users is using an uploaded avatar (must happen after cropping, otherwise // we will attempt to load an invalid avatar and cache a redirect to old one, uploadedAvatarTemplate // trumps over custom avatar upload id) - this.set("custom_avatar_upload_id", data.result.upload_id); + this.set("custom_avatar_upload_id", upload.id); }); // the upload is now done diff --git a/app/assets/javascripts/discourse/components/emoji-uploader.js.es6 b/app/assets/javascripts/discourse/components/emoji-uploader.js.es6 index 59f65b3d556..b43bea04103 100644 --- a/app/assets/javascripts/discourse/components/emoji-uploader.js.es6 +++ b/app/assets/javascripts/discourse/components/emoji-uploader.js.es6 @@ -1,4 +1,4 @@ -import UploadMixin from 'discourse/mixins/upload'; +import UploadMixin from "discourse/mixins/upload"; export default Em.Component.extend(UploadMixin, { type: "emoji", @@ -11,9 +11,9 @@ export default Em.Component.extend(UploadMixin, { return Ember.isBlank(this.get("name")) ? {} : { name: this.get("name") }; }.property("name"), - uploadDone: function (data) { + uploadDone(upload) { this.set("name", null); - this.sendAction("done", data.result); + this.sendAction("done", upload); } }); diff --git a/app/assets/javascripts/discourse/components/image-uploader.js.es6 b/app/assets/javascripts/discourse/components/image-uploader.js.es6 index 2301242fbd7..4ca6bfdab22 100644 --- a/app/assets/javascripts/discourse/components/image-uploader.js.es6 +++ b/app/assets/javascripts/discourse/components/image-uploader.js.es6 @@ -1,32 +1,21 @@ -import UploadMixin from 'discourse/mixins/upload'; +import UploadMixin from "discourse/mixins/upload"; export default Em.Component.extend(UploadMixin, { - classNames: ['image-uploader'], + classNames: ["image-uploader"], backgroundStyle: function() { - const imageUrl = this.get('imageUrl'); + const imageUrl = this.get("imageUrl"); if (Em.isNone(imageUrl)) { return; } - return ("background-image: url(" + imageUrl + ")").htmlSafe(); - }.property('imageUrl'), + }.property("imageUrl"), - uploadDone: function(data) { - this.set('imageUrl', data.result.url); + uploadDone(upload) { + this.set("imageUrl", upload.url); }, actions: { trash() { - this.set('imageUrl', null); - - // Do we want to signal the delete to the server right away? - if (this.get('instantDelete')) { - Discourse.ajax(this.get('uploadUrl'), { - type: 'DELETE', - data: { image_type: this.get('type') } - }).then(null, function() { - bootbox.alert(I18n.t('generic_error')); - }); - } + this.set("imageUrl", null); } } }); diff --git a/app/assets/javascripts/discourse/controllers/edit-category.js.es6 b/app/assets/javascripts/discourse/controllers/edit-category.js.es6 index e667d447d56..ca15eda2496 100644 --- a/app/assets/javascripts/discourse/controllers/edit-category.js.es6 +++ b/app/assets/javascripts/discourse/controllers/edit-category.js.es6 @@ -5,7 +5,7 @@ import { categoryBadgeHTML } from 'discourse/helpers/category-link'; // Modal for editing / creating a category export default ObjectController.extend(ModalFunctionality, { foregroundColors: ['FFFFFF', '000000'], - categoryUploadUrl: '/category/uploads', + categoryUploadUrl: '/uploads', editingPermissions: false, selectedTab: null, saving: false, diff --git a/app/assets/javascripts/discourse/controllers/preferences.js.es6 b/app/assets/javascripts/discourse/controllers/preferences.js.es6 index ecd9033de8c..0cc11e05e95 100644 --- a/app/assets/javascripts/discourse/controllers/preferences.js.es6 +++ b/app/assets/javascripts/discourse/controllers/preferences.js.es6 @@ -91,7 +91,6 @@ export default ObjectController.extend(CanCheckEmails, { }.property('model.isSaving'), passwordProgress: null, - imageUploadUrl: Discourse.computed.url('model.username', '/users/%@/preferences/user_image'), actions: { diff --git a/app/assets/javascripts/discourse/initializers/inject-objects.js.es6 b/app/assets/javascripts/discourse/initializers/inject-objects.js.es6 index 2a3eded1dd8..16e4a4b2c43 100644 --- a/app/assets/javascripts/discourse/initializers/inject-objects.js.es6 +++ b/app/assets/javascripts/discourse/initializers/inject-objects.js.es6 @@ -45,6 +45,6 @@ export default { inject(app, 'currentUser', 'component', 'route', 'controller'); app.register('message-bus:main', window.MessageBus, { instantiate: false }); - inject(app, 'messageBus', 'route', 'controller', 'view'); + inject(app, 'messageBus', 'route', 'controller', 'view', 'component'); } }; diff --git a/app/assets/javascripts/discourse/lib/utilities.js b/app/assets/javascripts/discourse/lib/utilities.js index f51fd4d46f3..d85e474132e 100644 --- a/app/assets/javascripts/discourse/lib/utilities.js +++ b/app/assets/javascripts/discourse/lib/utilities.js @@ -296,6 +296,8 @@ Discourse.Utilities = { } return; } + } else if (data.errors) { + bootbox.alert(data.errors.join("\n")); } // otherwise, display a generic error message bootbox.alert(I18n.t('post.errors.upload')); diff --git a/app/assets/javascripts/discourse/mixins/upload.js.es6 b/app/assets/javascripts/discourse/mixins/upload.js.es6 index ac91a056de1..2914d72849f 100644 --- a/app/assets/javascripts/discourse/mixins/upload.js.es6 +++ b/app/assets/javascripts/discourse/mixins/upload.js.es6 @@ -2,27 +2,35 @@ export default Em.Mixin.create({ uploading: false, uploadProgress: 0, - uploadDone: function() { + uploadDone() { Em.warn("You should implement `uploadDone`"); }, - deleteDone: function() { + deleteDone() { Em.warn("You should implement `deleteDone`"); }, - _initializeUploader: function() { - var $upload = this.$(), - self = this, - csrf = Discourse.Session.currentProp("csrfToken"); + _initialize: function() { + const $upload = this.$(), + csrf = Discourse.Session.currentProp("csrfToken"), + uploadUrl = this.getWithDefault("uploadUrl", "/uploads"); + + this.messageBus.subscribe("/uploads/" + this.get("type"), upload => { + if (upload && upload.url) { + this.uploadDone(upload); + } else { + Discourse.Utilities.displayErrorForUpload(upload); + } + }); $upload.fileupload({ - url: this.get('uploadUrl') + ".json?authenticity_token=" + encodeURIComponent(csrf), + url: uploadUrl + ".json?authenticity_token=" + encodeURIComponent(csrf), dataType: "json", dropZone: $upload, pasteZone: $upload }); - $upload.on("fileuploaddrop", function (e, data) { + $upload.on("fileuploaddrop", (e, data) => { if (data.files.length > 10) { bootbox.alert(I18n.t("post.errors.too_many_dragged_and_dropped_files")); return false; @@ -31,51 +39,34 @@ export default Em.Mixin.create({ } }); - $upload.on('fileuploadsubmit', function (e, data) { - var isValid = Discourse.Utilities.validateUploadedFiles(data.files, true); - var form = { image_type: self.get('type') }; - if (self.get("data")) { form = $.extend(form, self.get("data")); } + $upload.on("fileuploadsubmit", (e, data) => { + const isValid = Discourse.Utilities.validateUploadedFiles(data.files, true); + let form = { type: this.get("type") }; + if (this.get("data")) { form = $.extend(form, this.get("data")); } data.formData = form; - self.setProperties({ uploadProgress: 0, uploading: isValid }); + this.setProperties({ uploadProgress: 0, uploading: isValid }); return isValid; }); - $upload.on("fileuploadprogressall", function(e, data) { - var progress = parseInt(data.loaded / data.total * 100, 10); - self.set("uploadProgress", progress); + $upload.on("fileuploadprogressall", (e, data) => { + const progress = parseInt(data.loaded / data.total * 100, 10); + this.set("uploadProgress", progress); }); - $upload.on("fileuploaddone", function(e, data) { - if (data.result) { - if (data.result.url) { - self.uploadDone(data); - } else { - if (data.result.message) { - bootbox.alert(data.result.message); - } else if (data.result.length > 0) { - bootbox.alert(data.result.join("\n")); - } else { - bootbox.alert(I18n.t('post.errors.upload')); - } - } - } else { - bootbox.alert(I18n.t('post.errors.upload')); - } - }); - - $upload.on("fileuploadfail", function(e, data) { + $upload.on("fileuploadfail", (e, data) => { Discourse.Utilities.displayErrorForUpload(data); }); - $upload.on("fileuploadalways", function() { - self.setProperties({ uploading: false, uploadProgress: 0}); + $upload.on("fileuploadalways", () => { + this.setProperties({ uploading: false, uploadProgress: 0}); }); - }.on('didInsertElement'), + }.on("didInsertElement"), - _destroyUploader: function() { - var $upload = this.$(); - try { $upload.fileupload('destroy'); } + _destroy: function() { + this.messageBus.unsubscribe("/uploads/" + this.get("type")); + const $upload = this.$(); + try { $upload.fileupload("destroy"); } catch (e) { /* wasn't initialized yet */ } $upload.off(); - }.on('willDestroyElement') + }.on("willDestroyElement") }); diff --git a/app/assets/javascripts/discourse/models/user.js.es6 b/app/assets/javascripts/discourse/models/user.js.es6 index 8322097ec46..aecb9cbc4c5 100644 --- a/app/assets/javascripts/discourse/models/user.js.es6 +++ b/app/assets/javascripts/discourse/models/user.js.es6 @@ -171,27 +171,31 @@ const User = RestModel.extend({ @returns {Promise} the result of the operation **/ save: function() { - var self = this, - data = this.getProperties('auto_track_topics_after_msecs', - 'bio_raw', - 'website', - 'location', - 'name', - 'locale', - 'email_digests', - 'email_direct', - 'email_always', - 'email_private_messages', - 'dynamic_favicon', - 'digest_after_days', - 'new_topic_duration_minutes', - 'external_links_in_new_tab', - 'mailing_list_mode', - 'enable_quoting', - 'disable_jump_reply', - 'custom_fields', - 'user_fields', - 'muted_usernames'); + const self = this, + data = this.getProperties( + 'auto_track_topics_after_msecs', + 'bio_raw', + 'website', + 'location', + 'name', + 'locale', + 'email_digests', + 'email_direct', + 'email_always', + 'email_private_messages', + 'dynamic_favicon', + 'digest_after_days', + 'new_topic_duration_minutes', + 'external_links_in_new_tab', + 'mailing_list_mode', + 'enable_quoting', + 'disable_jump_reply', + 'custom_fields', + 'user_fields', + 'muted_usernames', + 'profile_background', + 'card_background' + ); ['muted','watched','tracked'].forEach(function(s){ var cats = self.get(s + 'Categories').map(function(c){ return c.get('id')}); diff --git a/app/assets/javascripts/discourse/templates/composer.hbs b/app/assets/javascripts/discourse/templates/composer.hbs index ed94f9f4507..f561b20f642 100644 --- a/app/assets/javascripts/discourse/templates/composer.hbs +++ b/app/assets/javascripts/discourse/templates/composer.hbs @@ -1,18 +1,6 @@ {{#if visible}} - -{{!-- -note this spinner is NEVER turned "off" when the composer is open -so I'm going to stop rendering it until we figure out what's up - -
- {{loading-spinner}} -
---}} -
- {{render "composer-messages"}} -
diff --git a/app/assets/javascripts/discourse/templates/modal/edit-category-images.hbs b/app/assets/javascripts/discourse/templates/modal/edit-category-images.hbs index e5f65a5b113..c6cac30f593 100644 --- a/app/assets/javascripts/discourse/templates/modal/edit-category-images.hbs +++ b/app/assets/javascripts/discourse/templates/modal/edit-category-images.hbs @@ -1,9 +1,9 @@
- {{image-uploader uploadUrl=categoryUploadUrl imageUrl=model.logo_url type="logo"}} + {{image-uploader imageUrl=model.logo_url type="category_logo"}}
- {{image-uploader uploadUrl=categoryUploadUrl imageUrl=model.background_url type="background"}} + {{image-uploader imageUrl=model.background_url type="category_background"}}
diff --git a/app/assets/javascripts/discourse/templates/user/preferences.hbs b/app/assets/javascripts/discourse/templates/user/preferences.hbs index 5e3e8ecf532..76f5446c9c0 100644 --- a/app/assets/javascripts/discourse/templates/user/preferences.hbs +++ b/app/assets/javascripts/discourse/templates/user/preferences.hbs @@ -104,10 +104,7 @@
- {{image-uploader uploadUrl=imageUploadUrl - imageUrl=model.profile_background - instantDelete="true" - type="profile_background"}} + {{image-uploader imageUrl=model.profile_background type="profile_background"}}
{{i18n 'user.change_profile_background.instructions'}} @@ -117,10 +114,7 @@
- {{image-uploader uploadUrl=imageUploadUrl - imageUrl=model.card_background - instantDelete="true" - type="card_background"}} + {{image-uploader imageUrl=model.card_background type="card_background"}}
{{i18n 'user.change_card_background.instructions'}} diff --git a/app/assets/javascripts/discourse/views/composer.js.es6 b/app/assets/javascripts/discourse/views/composer.js.es6 index 5ab6235850c..40548fbd6a9 100644 --- a/app/assets/javascripts/discourse/views/composer.js.es6 +++ b/app/assets/javascripts/discourse/views/composer.js.es6 @@ -306,82 +306,76 @@ const ComposerView = Discourse.View.extend(Ember.Evented, { // in case it's still bound somehow this._unbindUploadTarget(); - const $uploadTarget = $('#reply-control'), - csrf = Discourse.Session.currentProp('csrfToken'); - let cancelledByTheUser; + const $uploadTarget = $("#reply-control"), + csrf = Discourse.Session.currentProp("csrfToken"), + reset = () => this.setProperties({ uploadProgress: 0, isUploading: false }); + + var cancelledByTheUser; + + this.messageBus.subscribe("/uploads/composer", upload => { + if (!cancelledByTheUser) { + if (upload && upload.url) { + const markdown = Discourse.Utilities.getUploadMarkdown(upload); + this.addMarkdown(markdown + " "); + } else { + Discourse.Utilities.displayErrorForUpload(upload); + } + } + }); - // NOTE: we need both the .json extension and the CSRF token as a query parameter for IE9 $uploadTarget.fileupload({ - url: Discourse.getURL('/uploads.json?authenticity_token=' + encodeURIComponent(csrf)), - dataType: 'json', - pasteZone: $uploadTarget + url: Discourse.getURL("/uploads.json?authenticity_token=" + encodeURIComponent(csrf)), + dataType: "json", + pasteZone: $uploadTarget, }); - // submit - this event is triggered for each upload - $uploadTarget.on('fileuploadsubmit', function (e, data) { - const result = Discourse.Utilities.validateUploadedFiles(data.files); - // reset upload status when everything is ok - if (result) self.setProperties({ uploadProgress: 0, isUploading: true }); - return result; + $uploadTarget.on("fileuploadsubmit", (e, data) => { + const isValid = Discourse.Utilities.validateUploadedFiles(data.files); + data.formData = { type: "composer" }; + this.setProperties({ uploadProgress: 0, isUploading: isValid }); + return isValid; }); - // send - this event is triggered when the upload request is about to start - $uploadTarget.on('fileuploadsend', function (e, data) { - cancelledByTheUser = false; + $uploadTarget.on("fileuploadsend", (e, data) => { // hide the "file selector" modal - self.get('controller').send('closeModal'); - // NOTE: IE9 doesn't support XHR + this.get("controller").send("closeModal"); + // deal with cancellation + cancelledByTheUser = false; if (data["xhr"]) { const jqHXR = data.xhr(); if (jqHXR) { // need to wait for the link to show up in the DOM - Em.run.schedule('afterRender', function() { - // bind on the click event on the cancel link - $('#cancel-file-upload').on('click', function() { - // cancel the upload - self.set('isUploading', false); - // NOTE: this might trigger a 'fileuploadfail' event with status = 0 - if (jqHXR) { cancelledByTheUser = true; jqHXR.abort(); } + Em.run.schedule("afterRender", () => { + const $cancel = $("#cancel-file-upload"); + $cancel.on("click", () => { + if (jqHXR) { + cancelledByTheUser = true; + // might trigger a "fileuploadfail" event with status = 0 + jqHXR.abort(); + // doesn't trigger the "fileuploadalways" event + reset(); + } // unbind - $(this).off('click'); + $cancel.off("click"); }); }); } } }); - // progress all - $uploadTarget.on('fileuploadprogressall', function (e, data) { + $uploadTarget.on("fileuploadprogressall", (e, data) => { const progress = parseInt(data.loaded / data.total * 100, 10); - self.set('uploadProgress', progress); + this.set("uploadProgress", progress); }); - // done - $uploadTarget.on('fileuploaddone', function (e, data) { + $uploadTarget.on("fileuploadfail", (e, data) => { if (!cancelledByTheUser) { - // make sure we have a url - if (data.result.url) { - const markdown = Discourse.Utilities.getUploadMarkdown(data.result); - // appends a space at the end of the inserted markdown - self.addMarkdown(markdown + " "); - self.set('isUploading', false); - } else { - // display the error message sent by the server - bootbox.alert(data.result.join("\n")); - } - } - }); - - // fail - $uploadTarget.on('fileuploadfail', function (e, data) { - // hide upload status - self.set('isUploading', false); - if (!cancelledByTheUser) { - // display an error message Discourse.Utilities.displayErrorForUpload(data); } }); + $uploadTarget.on("fileuploadalways", reset); + // contenteditable div hack for getting image paste to upload working in // Firefox. This is pretty dangerous because it can potentially break // Ctrl+v to paste so we should be conservative about what browsers this runs @@ -538,6 +532,14 @@ const ComposerView = Discourse.View.extend(Ember.Evented, { }); }, + _unbindUploadTarget() { + this.messageBus.unsubscribe("/uploads/composer"); + const $uploadTarget = $("#reply-controler"); + try { $uploadTarget.fileupload("destroy"); } + catch (e) { /* wasn't initialized yet */ } + $uploadTarget.off(); + }, + titleValidation: function() { const titleLength = this.get('model.titleLength'), missingChars = this.get('model.missingTitleCharacters'); @@ -580,13 +582,6 @@ const ComposerView = Discourse.View.extend(Ember.Evented, { return Discourse.InputValidation.create({ failed: true, reason }); } }.property('model.reply', 'model.replyLength', 'model.missingReplyCharacters', 'model.minimumPostLength'), - - _unbindUploadTarget() { - const $uploadTarget = $('#reply-control'); - try { $uploadTarget.fileupload('destroy'); } - catch (e) { /* wasn't initialized yet */ } - $uploadTarget.off(); - } }); RSVP.EventTarget.mixin(ComposerView); diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb index 9de2c23383e..0fa958c8ee5 100644 --- a/app/controllers/categories_controller.rb +++ b/app/controllers/categories_controller.rb @@ -31,19 +31,6 @@ class CategoriesController < ApplicationController end end - def upload - params.require(:image_type) - guardian.ensure_can_create!(Category) - - file = params[:file] || params[:files].first - upload = Upload.create_for(current_user.id, file.tempfile, file.original_filename, file.tempfile.size) - if upload.errors.blank? - render json: { url: upload.url, width: upload.width, height: upload.height } - else - render status: 422, text: upload.errors.full_messages - end - end - def move guardian.ensure_can_create!(Category) diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 195c8142357..03f0319c9df 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -3,23 +3,35 @@ class UploadsController < ApplicationController skip_before_filter :preload_json, :check_xhr, only: [:show] def create + type = params.require(:type) file = params[:file] || params[:files].first - filesize = file.tempfile.size - upload = Upload.create_for(current_user.id, file.tempfile, file.original_filename, filesize, { content_type: file.content_type }) + url = params[:url] - if upload.errors.empty? && current_user.admin? - retain_hours = params[:retain_hours].to_i - upload.update_columns(retain_hours: retain_hours) if retain_hours > 0 + # TODO: support for API providing a URL (cf. AvatarUploadService) + + Scheduler::Defer.later("Create Upload") do + upload = Upload.create_for( + current_user.id, + file.tempfile, + file.original_filename, + file.tempfile.size, + content_type: file.content_type + ) + + if upload.errors.empty? && current_user.admin? + retain_hours = params[:retain_hours].to_i + upload.update_columns(retain_hours: retain_hours) if retain_hours > 0 + end + + data = upload.errors.empty? ? upload : { errors: upload.errors.values.flatten } + + MessageBus.publish("/uploads/#{type}", data.as_json, user_ids: [current_user.id]) end # HACK FOR IE9 to prevent the "download dialog" response.headers["Content-Type"] = "text/plain" if request.user_agent =~ /MSIE 9/ - if upload.errors.empty? - render_serialized(upload, UploadSerializer, root: false) - else - render status: 422, text: upload.errors.full_messages - end + render json: success_json end def show diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index dc168a565d5..bf57b00e010 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -346,7 +346,6 @@ class UsersController < ApplicationController end def admin_login - unless SiteSetting.enable_sso && !current_user return redirect_to path("/") end @@ -465,7 +464,6 @@ class UsersController < ApplicationController end def send_activation_email - RateLimiter.new(nil, "activate-hr-#{request.remote_ip}", 30, 1.hour).performed! RateLimiter.new(nil, "activate-min-#{request.remote_ip}", 6, 1.minute).performed! @@ -503,38 +501,6 @@ class UsersController < ApplicationController render json: to_render end - def upload_user_image - params.require(:image_type) - user = fetch_user_from_params - guardian.ensure_can_edit!(user) - - file = params[:file] || params[:files].first - - # HACK FOR IE9 to prevent the "download dialog" - response.headers["Content-Type"] = "text/plain" if request.user_agent =~ /MSIE 9/ - - begin - image = build_user_image_from(file) - rescue Discourse::InvalidParameters - return render status: 422, text: I18n.t("upload.images.unknown_image_type") - end - - upload = Upload.create_for(user.id, image.file, image.filename, image.filesize) - - if upload.errors.empty? - case params[:image_type] - when "avatar" - upload_avatar_for(user, upload) - when "profile_background" - upload_profile_background_for(user.user_profile, upload) - when "card_background" - upload_card_background_for(user.user_profile, upload) - end - else - render status: 422, text: upload.errors.full_messages - end - end - def pick_avatar user = fetch_user_from_params guardian.ensure_can_edit!(user) @@ -555,16 +521,16 @@ class UsersController < ApplicationController user = fetch_user_from_params guardian.ensure_can_edit!(user) - image_type = params.require(:image_type) - if image_type == 'profile_background' + case params.require(:type) + when "profile_background" user.user_profile.clear_profile_background - elsif image_type == 'card_background' + when "card_background" user.user_profile.clear_card_background else - raise Discourse::InvalidParameters.new(:image_type) + raise Discourse::InvalidParameters.new(:type) end - render nothing: true + render json: success_json end def destroy @@ -614,51 +580,23 @@ class UsersController < ApplicationController challenge end - def build_user_image_from(file) - source = if file.is_a?(String) - is_api? ? :url : (raise Discourse::InvalidParameters) - else - :image - end - - AvatarUploadService.new(file, source) - end - - def upload_avatar_for(user, upload) - render json: { upload_id: upload.id, url: upload.url, width: upload.width, height: upload.height } - end - - def upload_profile_background_for(user_profile, upload) - user_profile.upload_profile_background(upload) - render json: { url: upload.url, width: upload.width, height: upload.height } - end - - def upload_card_background_for(user_profile, upload) - user_profile.upload_card_background(upload) - render json: { url: upload.url, width: upload.width, height: upload.height } - end - def respond_to_suspicious_request if suspicious?(params) - render( - json: { - success: true, - active: false, - message: I18n.t("login.activate_email", email: params[:email]) - } - ) + render json: { + success: true, + active: false, + message: I18n.t("login.activate_email", email: params[:email]) + } end end def suspicious?(params) return false if current_user && is_api? && current_user.admin? - honeypot_or_challenge_fails?(params) || SiteSetting.invite_only? end def honeypot_or_challenge_fails?(params) return false if is_api? - params[:password_confirmation] != honeypot_value || params[:challenge] != challenge_value.try(:reverse) end diff --git a/app/services/user_updater.rb b/app/services/user_updater.rb index c8741f867b2..24fc1f27334 100644 --- a/app/services/user_updater.rb +++ b/app/services/user_updater.rb @@ -26,8 +26,12 @@ class UserUpdater def update(attributes = {}) user_profile = user.user_profile + user_profile.location = attributes[:location] + user_profile.dismissed_banner_key = attributes[:dismissed_banner_key] if attributes[:dismissed_banner_key].present? user_profile.website = format_url(attributes.fetch(:website) { user_profile.website }) user_profile.bio_raw = attributes.fetch(:bio_raw) { user_profile.bio_raw } + user_profile.profile_background = attributes.fetch(:profile_background) { user_profile.profile_background } + user_profile.card_background = attributes.fetch(:card_background) { user_profile.card_background } user.name = attributes.fetch(:name) { user.name } user.locale = attributes.fetch(:locale) { user.locale } @@ -57,16 +61,12 @@ class UserUpdater end end - user_profile.location = attributes[:location] - user_profile.dismissed_banner_key = attributes[:dismissed_banner_key] if attributes[:dismissed_banner_key].present? - fields = attributes[:custom_fields] if fields.present? user.custom_fields = user.custom_fields.merge(fields) end User.transaction do - if attributes.key?(:muted_usernames) update_muted_users(attributes[:muted_usernames]) end @@ -103,10 +103,6 @@ class UserUpdater attr_reader :user, :guardian def format_url(website) - if website =~ /^http/ - website - else - "http://#{website}" - end + website =~ /^http/ ? website : "http://#{website}" end end diff --git a/config/routes.rb b/config/routes.rb index ff2a2edec46..911662521ad 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -261,7 +261,6 @@ Discourse::Application.routes.draw do put "users/:username/preferences/badge_title" => "users#badge_title", constraints: {username: USERNAME_ROUTE_FORMAT} get "users/:username/preferences/username" => "users#preferences", constraints: {username: USERNAME_ROUTE_FORMAT} put "users/:username/preferences/username" => "users#username", constraints: {username: USERNAME_ROUTE_FORMAT} - post "users/:username/preferences/user_image" => "users#upload_user_image", constraints: {username: USERNAME_ROUTE_FORMAT} delete "users/:username/preferences/user_image" => "users#destroy_user_image", constraints: {username: USERNAME_ROUTE_FORMAT} put "users/:username/preferences/avatar/pick" => "users#pick_avatar", constraints: {username: USERNAME_ROUTE_FORMAT} get "users/:username/preferences/card-badge" => "users#card_badge", constraints: {username: USERNAME_ROUTE_FORMAT} @@ -364,7 +363,6 @@ Discourse::Application.routes.draw do get '/c', to: redirect('/categories') resources :categories, :except => :show - post "category/uploads" => "categories#upload" post "category/:category_id/move" => "categories#move" post "category/:category_id/notifications" => "categories#set_notifications" put "category/:category_id/slug" => "categories#update_slug" diff --git a/spec/controllers/categories_controller_spec.rb b/spec/controllers/categories_controller_spec.rb index 48b32dac65e..3aa0d31e76b 100644 --- a/spec/controllers/categories_controller_spec.rb +++ b/spec/controllers/categories_controller_spec.rb @@ -95,37 +95,6 @@ describe CategoriesController do end - describe "upload" do - it "requires the user to be logged in" do - expect { xhr :post, :upload, image_type: 'logo'}.to raise_error(Discourse::NotLoggedIn) - end - - describe "logged in" do - let!(:user) { log_in(:admin) } - - let(:logo) { file_from_fixtures("logo.png") } - let(:upload) do - ActionDispatch::Http::UploadedFile.new({ filename: 'logo.png', tempfile: logo }) - end - - it "raises an error when you don't have permission to upload" do - Guardian.any_instance.expects(:can_create?).with(Category).returns(false) - xhr :post, :upload, image_type: 'logo', file: upload - expect(response).to be_forbidden - end - - it "requires the `image_type` param" do - expect { xhr :post, :upload }.to raise_error(ActionController::ParameterMissing) - end - - it "calls Upload.create_for" do - Upload.expects(:create_for).returns(Upload.new) - xhr :post, :upload, image_type: 'logo', file: upload - expect(response).to be_success - end - end - end - describe "update" do it "requires the user to be logged in" do diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb index cf777cfd2e2..860d2d1dcf9 100644 --- a/spec/controllers/uploads_controller_spec.rb +++ b/spec/controllers/uploads_controller_spec.rb @@ -19,13 +19,6 @@ describe UploadsController do }) end - let(:logo_dev) do - ActionDispatch::Http::UploadedFile.new({ - filename: 'logo-dev.png', - tempfile: file_from_fixtures("logo-dev.png") - }) - end - let(:text_file) do ActionDispatch::Http::UploadedFile.new({ filename: 'LICENSE.TXT', @@ -33,85 +26,47 @@ describe UploadsController do }) end - let(:files) { [ logo_dev, logo ] } + it 'is successful with an image' do + message = MessageBus.track_publish do + xhr :post, :create, file: logo, type: "composer" + end.first - context 'with a file' do - - context 'when authorized' do - - before { SiteSetting.stubs(:authorized_extensions).returns(".PNG|.txt") } - - it 'is successful with an image' do - xhr :post, :create, file: logo - expect(response.status).to eq 200 - end - - it 'is successful with an attachment' do - xhr :post, :create, file: text_file - expect(response.status).to eq 200 - end - - it 'correctly sets retain_hours for admins' do - log_in :admin - xhr :post, :create, file: logo, retain_hours: 100 - id = JSON.parse(response.body)["id"] - expect(Upload.find(id).retain_hours).to eq(100) - end - - context 'with a big file' do - - before { SiteSetting.stubs(:max_attachment_size_kb).returns(1) } - - it 'rejects the upload' do - xhr :post, :create, file: text_file - expect(response.status).to eq 422 - end - - end - - end - - context 'when not authorized' do - - before { SiteSetting.stubs(:authorized_extensions).returns(".png") } - - it 'rejects the upload' do - xhr :post, :create, file: text_file - expect(response.status).to eq 422 - end - - end - - context 'when everything is authorized' do - - before { SiteSetting.stubs(:authorized_extensions).returns("*") } - - it 'is successful with an image' do - xhr :post, :create, file: logo - expect(response.status).to eq 200 - end - - it 'is successful with an attachment' do - xhr :post, :create, file: text_file - expect(response.status).to eq 200 - end - - end + expect(response.status).to eq 200 + expect(message.channel).to eq("/uploads/composer") + expect(message.data).to be end - context 'with some files' do + it 'is successful with an attachment' do + message = MessageBus.track_publish do + xhr :post, :create, file: text_file, type: "avatar" + end.first - it 'is successful' do - xhr :post, :create, files: files - expect(response).to be_success - end + expect(response.status).to eq 200 + expect(message.channel).to eq("/uploads/avatar") + expect(message.data).to be + end - it 'takes the first file' do - xhr :post, :create, files: files - expect(response.body).to match /logo-dev.png/ - end + it 'correctly sets retain_hours for admins' do + log_in :admin + message = MessageBus.track_publish do + xhr :post, :create, file: logo, retain_hours: 100, type: "profile_background" + end.first + + id = message.data["id"] + expect(Upload.find(id).retain_hours).to eq(100) + end + + it 'properly returns errors' do + SiteSetting.stubs(:max_attachment_size_kb).returns(1) + + message = MessageBus.track_publish do + xhr :post, :create, file: text_file, type: "avatar" + end.first + + expect(response.status).to eq 200 + expect(message.data["errors"]).to be end end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 401cb481ddb..2735ef44134 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -1304,164 +1304,6 @@ describe UsersController do end end - describe '.upload_user_image' do - - it 'raises an error when not logged in' do - expect { xhr :put, :upload_user_image, username: 'asdf' }.to raise_error(Discourse::NotLoggedIn) - end - - context 'while logged in' do - - let!(:user) { log_in } - - let(:logo) { file_from_fixtures("logo.png") } - - let(:user_image) do - ActionDispatch::Http::UploadedFile.new({ filename: 'logo.png', tempfile: logo }) - end - - it 'raises an error without a image_type param' do - expect { xhr :put, :upload_user_image, username: user.username }.to raise_error(ActionController::ParameterMissing) - end - - describe "with uploaded file" do - - it 'raises an error when you don\'t have permission to upload an user image' do - Guardian.any_instance.expects(:can_edit?).with(user).returns(false) - xhr :post, :upload_user_image, username: user.username, image_type: "avatar" - expect(response).to be_forbidden - end - - it 'rejects large images' do - SiteSetting.stubs(:max_image_size_kb).returns(1) - xhr :post, :upload_user_image, username: user.username, file: user_image, image_type: "avatar" - expect(response.status).to eq 422 - end - - it 'rejects unauthorized images' do - SiteSetting.stubs(:authorized_extensions).returns(".txt") - xhr :post, :upload_user_image, username: user.username, file: user_image, image_type: "avatar" - expect(response.status).to eq 422 - end - - it 'is successful for avatars' do - upload = Fabricate(:upload) - Upload.expects(:create_for).returns(upload) - # enqueues the user_image generator job - xhr :post, :upload_user_image, username: user.username, file: user_image, image_type: "avatar" - # returns the url, width and height of the uploaded image - json = JSON.parse(response.body) - expect(json['url']).to eq("/uploads/default/1/1234567890123456.png") - expect(json['width']).to eq(100) - expect(json['height']).to eq(200) - expect(json['upload_id']).to eq(upload.id) - end - - it 'is successful for profile backgrounds' do - upload = Fabricate(:upload) - Upload.expects(:create_for).returns(upload) - xhr :post, :upload_user_image, username: user.username, file: user_image, image_type: "profile_background" - user.reload - - expect(user.user_profile.profile_background).to eq("/uploads/default/1/1234567890123456.png") - - # returns the url, width and height of the uploaded image - json = JSON.parse(response.body) - expect(json['url']).to eq("/uploads/default/1/1234567890123456.png") - expect(json['width']).to eq(100) - expect(json['height']).to eq(200) - end - - it 'is successful for card backgrounds' do - upload = Fabricate(:upload) - Upload.expects(:create_for).returns(upload) - xhr :post, :upload_user_image, username: user.username, file: user_image, image_type: "card_background" - user.reload - - expect(user.user_profile.card_background).to eq("/uploads/default/1/1234567890123456.png") - - # returns the url, width and height of the uploaded image - json = JSON.parse(response.body) - expect(json['url']).to eq("/uploads/default/1/1234567890123456.png") - expect(json['width']).to eq(100) - expect(json['height']).to eq(200) - end - - end - - describe "with url" do - let(:user_image_url) { "http://cdn.discourse.org/assets/logo.png" } - - before { UsersController.any_instance.stubs(:is_api?).returns(true) } - - describe "correct urls" do - - before { FileHelper.stubs(:download).returns(logo) } - - it 'rejects large images' do - SiteSetting.stubs(:max_image_size_kb).returns(1) - xhr :post, :upload_user_image, username: user.username, file: user_image_url, image_type: "profile_background" - expect(response.status).to eq 422 - end - - it 'rejects unauthorized images' do - SiteSetting.stubs(:authorized_extensions).returns(".txt") - xhr :post, :upload_user_image, username: user.username, file: user_image_url, image_type: "profile_background" - expect(response.status).to eq 422 - end - - it 'is successful for avatars' do - upload = Fabricate(:upload) - Upload.expects(:create_for).returns(upload) - # enqueues the user_image generator job - xhr :post, :upload_user_image, username: user.username, file: user_image_url, image_type: "avatar" - json = JSON.parse(response.body) - expect(json['url']).to eq("/uploads/default/1/1234567890123456.png") - expect(json['width']).to eq(100) - expect(json['height']).to eq(200) - expect(json['upload_id']).to eq(upload.id) - end - - it 'is successful for profile backgrounds' do - upload = Fabricate(:upload) - Upload.expects(:create_for).returns(upload) - xhr :post, :upload_user_image, username: user.username, file: user_image_url, image_type: "profile_background" - user.reload - expect(user.user_profile.profile_background).to eq("/uploads/default/1/1234567890123456.png") - - # returns the url, width and height of the uploaded image - json = JSON.parse(response.body) - expect(json['url']).to eq("/uploads/default/1/1234567890123456.png") - expect(json['width']).to eq(100) - expect(json['height']).to eq(200) - end - - it 'is successful for card backgrounds' do - upload = Fabricate(:upload) - Upload.expects(:create_for).returns(upload) - xhr :post, :upload_user_image, username: user.username, file: user_image_url, image_type: "card_background" - user.reload - expect(user.user_profile.card_background).to eq("/uploads/default/1/1234567890123456.png") - - # returns the url, width and height of the uploaded image - json = JSON.parse(response.body) - expect(json['url']).to eq("/uploads/default/1/1234567890123456.png") - expect(json['width']).to eq(100) - expect(json['height']).to eq(200) - end - end - - it "should handle malformed urls" do - xhr :post, :upload_user_image, username: user.username, file: "foobar", image_type: "profile_background" - expect(response.status).to eq 422 - end - - end - - end - - end - describe '.pick_avatar' do it 'raises an error when not logged in' do @@ -1511,20 +1353,20 @@ describe UsersController do it 'raises an error when you don\'t have permission to clear the profile background' do Guardian.any_instance.expects(:can_edit?).with(user).returns(false) - xhr :delete, :destroy_user_image, username: user.username, image_type: 'profile_background' + xhr :delete, :destroy_user_image, username: user.username, type: 'profile_background' expect(response).to be_forbidden end - it "requires the `image_type` param" do + it "requires the `type` param" do expect { xhr :delete, :destroy_user_image, username: user.username }.to raise_error(ActionController::ParameterMissing) end - it "only allows certain `image_types`" do - expect { xhr :delete, :destroy_user_image, username: user.username, image_type: 'wat' }.to raise_error(Discourse::InvalidParameters) + it "only allows certain `types`" do + expect { xhr :delete, :destroy_user_image, username: user.username, type: 'wat' }.to raise_error(Discourse::InvalidParameters) end it 'can clear the profile background' do - xhr :delete, :destroy_user_image, image_type: 'profile_background', username: user.username + xhr :delete, :destroy_user_image, type: 'profile_background', username: user.username expect(user.reload.user_profile.profile_background).to eq("") expect(response).to be_success end