FEATURE: move all uploads to a single endpoint + defer upload creation in a background thread

This commit is contained in:
Régis Hanol 2015-05-20 01:39:58 +02:00
parent 7d23826cee
commit 8d967d9065
21 changed files with 212 additions and 557 deletions

View File

@ -1,33 +1,29 @@
import UploadMixin from 'discourse/mixins/upload'; import UploadMixin from "discourse/mixins/upload";
export default Em.Component.extend(UploadMixin, { export default Em.Component.extend(UploadMixin, {
type: 'avatar', type: "avatar",
tagName: 'span', tagName: "span",
imageIsNotASquare: false, imageIsNotASquare: false,
uploadUrl: Discourse.computed.url('username', '/users/%@/preferences/user_image'),
uploadButtonText: function() { uploadButtonText: function() {
return this.get("uploading") ? return this.get("uploading") ? I18n.t("uploading") : I18n.t("user.change_avatar.upload_picture");
I18n.t("uploading") :
I18n.t("user.change_avatar.upload_picture");
}.property("uploading"), }.property("uploading"),
uploadDone(data) { uploadDone(upload) {
// display a warning whenever the image is not a square // 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 // 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 // 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 // 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 // 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); this.set("uploadedAvatarTemplate", avatarTemplate);
// indicates the users is using an uploaded avatar (must happen after cropping, otherwise // 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 // we will attempt to load an invalid avatar and cache a redirect to old one, uploadedAvatarTemplate
// trumps over custom avatar upload id) // 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 // the upload is now done

View File

@ -1,4 +1,4 @@
import UploadMixin from 'discourse/mixins/upload'; import UploadMixin from "discourse/mixins/upload";
export default Em.Component.extend(UploadMixin, { export default Em.Component.extend(UploadMixin, {
type: "emoji", type: "emoji",
@ -11,9 +11,9 @@ export default Em.Component.extend(UploadMixin, {
return Ember.isBlank(this.get("name")) ? {} : { name: this.get("name") }; return Ember.isBlank(this.get("name")) ? {} : { name: this.get("name") };
}.property("name"), }.property("name"),
uploadDone: function (data) { uploadDone(upload) {
this.set("name", null); this.set("name", null);
this.sendAction("done", data.result); this.sendAction("done", upload);
} }
}); });

View File

@ -1,32 +1,21 @@
import UploadMixin from 'discourse/mixins/upload'; import UploadMixin from "discourse/mixins/upload";
export default Em.Component.extend(UploadMixin, { export default Em.Component.extend(UploadMixin, {
classNames: ['image-uploader'], classNames: ["image-uploader"],
backgroundStyle: function() { backgroundStyle: function() {
const imageUrl = this.get('imageUrl'); const imageUrl = this.get("imageUrl");
if (Em.isNone(imageUrl)) { return; } if (Em.isNone(imageUrl)) { return; }
return ("background-image: url(" + imageUrl + ")").htmlSafe(); return ("background-image: url(" + imageUrl + ")").htmlSafe();
}.property('imageUrl'), }.property("imageUrl"),
uploadDone: function(data) { uploadDone(upload) {
this.set('imageUrl', data.result.url); this.set("imageUrl", upload.url);
}, },
actions: { actions: {
trash() { trash() {
this.set('imageUrl', null); 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'));
});
}
} }
} }
}); });

View File

@ -5,7 +5,7 @@ import { categoryBadgeHTML } from 'discourse/helpers/category-link';
// Modal for editing / creating a category // Modal for editing / creating a category
export default ObjectController.extend(ModalFunctionality, { export default ObjectController.extend(ModalFunctionality, {
foregroundColors: ['FFFFFF', '000000'], foregroundColors: ['FFFFFF', '000000'],
categoryUploadUrl: '/category/uploads', categoryUploadUrl: '/uploads',
editingPermissions: false, editingPermissions: false,
selectedTab: null, selectedTab: null,
saving: false, saving: false,

View File

@ -91,7 +91,6 @@ export default ObjectController.extend(CanCheckEmails, {
}.property('model.isSaving'), }.property('model.isSaving'),
passwordProgress: null, passwordProgress: null,
imageUploadUrl: Discourse.computed.url('model.username', '/users/%@/preferences/user_image'),
actions: { actions: {

View File

@ -45,6 +45,6 @@ export default {
inject(app, 'currentUser', 'component', 'route', 'controller'); inject(app, 'currentUser', 'component', 'route', 'controller');
app.register('message-bus:main', window.MessageBus, { instantiate: false }); app.register('message-bus:main', window.MessageBus, { instantiate: false });
inject(app, 'messageBus', 'route', 'controller', 'view'); inject(app, 'messageBus', 'route', 'controller', 'view', 'component');
} }
}; };

View File

@ -296,6 +296,8 @@ Discourse.Utilities = {
} }
return; return;
} }
} else if (data.errors) {
bootbox.alert(data.errors.join("\n"));
} }
// otherwise, display a generic error message // otherwise, display a generic error message
bootbox.alert(I18n.t('post.errors.upload')); bootbox.alert(I18n.t('post.errors.upload'));

View File

@ -2,27 +2,35 @@ export default Em.Mixin.create({
uploading: false, uploading: false,
uploadProgress: 0, uploadProgress: 0,
uploadDone: function() { uploadDone() {
Em.warn("You should implement `uploadDone`"); Em.warn("You should implement `uploadDone`");
}, },
deleteDone: function() { deleteDone() {
Em.warn("You should implement `deleteDone`"); Em.warn("You should implement `deleteDone`");
}, },
_initializeUploader: function() { _initialize: function() {
var $upload = this.$(), const $upload = this.$(),
self = this, csrf = Discourse.Session.currentProp("csrfToken"),
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({ $upload.fileupload({
url: this.get('uploadUrl') + ".json?authenticity_token=" + encodeURIComponent(csrf), url: uploadUrl + ".json?authenticity_token=" + encodeURIComponent(csrf),
dataType: "json", dataType: "json",
dropZone: $upload, dropZone: $upload,
pasteZone: $upload pasteZone: $upload
}); });
$upload.on("fileuploaddrop", function (e, data) { $upload.on("fileuploaddrop", (e, data) => {
if (data.files.length > 10) { if (data.files.length > 10) {
bootbox.alert(I18n.t("post.errors.too_many_dragged_and_dropped_files")); bootbox.alert(I18n.t("post.errors.too_many_dragged_and_dropped_files"));
return false; return false;
@ -31,51 +39,34 @@ export default Em.Mixin.create({
} }
}); });
$upload.on('fileuploadsubmit', function (e, data) { $upload.on("fileuploadsubmit", (e, data) => {
var isValid = Discourse.Utilities.validateUploadedFiles(data.files, true); const isValid = Discourse.Utilities.validateUploadedFiles(data.files, true);
var form = { image_type: self.get('type') }; let form = { type: this.get("type") };
if (self.get("data")) { form = $.extend(form, self.get("data")); } if (this.get("data")) { form = $.extend(form, this.get("data")); }
data.formData = form; data.formData = form;
self.setProperties({ uploadProgress: 0, uploading: isValid }); this.setProperties({ uploadProgress: 0, uploading: isValid });
return isValid; return isValid;
}); });
$upload.on("fileuploadprogressall", function(e, data) { $upload.on("fileuploadprogressall", (e, data) => {
var progress = parseInt(data.loaded / data.total * 100, 10); const progress = parseInt(data.loaded / data.total * 100, 10);
self.set("uploadProgress", progress); this.set("uploadProgress", progress);
}); });
$upload.on("fileuploaddone", function(e, data) { $upload.on("fileuploadfail", (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) {
Discourse.Utilities.displayErrorForUpload(data); Discourse.Utilities.displayErrorForUpload(data);
}); });
$upload.on("fileuploadalways", function() { $upload.on("fileuploadalways", () => {
self.setProperties({ uploading: false, uploadProgress: 0}); this.setProperties({ uploading: false, uploadProgress: 0});
}); });
}.on('didInsertElement'), }.on("didInsertElement"),
_destroyUploader: function() { _destroy: function() {
var $upload = this.$(); this.messageBus.unsubscribe("/uploads/" + this.get("type"));
try { $upload.fileupload('destroy'); } const $upload = this.$();
try { $upload.fileupload("destroy"); }
catch (e) { /* wasn't initialized yet */ } catch (e) { /* wasn't initialized yet */ }
$upload.off(); $upload.off();
}.on('willDestroyElement') }.on("willDestroyElement")
}); });

View File

@ -171,8 +171,9 @@ const User = RestModel.extend({
@returns {Promise} the result of the operation @returns {Promise} the result of the operation
**/ **/
save: function() { save: function() {
var self = this, const self = this,
data = this.getProperties('auto_track_topics_after_msecs', data = this.getProperties(
'auto_track_topics_after_msecs',
'bio_raw', 'bio_raw',
'website', 'website',
'location', 'location',
@ -191,7 +192,10 @@ const User = RestModel.extend({
'disable_jump_reply', 'disable_jump_reply',
'custom_fields', 'custom_fields',
'user_fields', 'user_fields',
'muted_usernames'); 'muted_usernames',
'profile_background',
'card_background'
);
['muted','watched','tracked'].forEach(function(s){ ['muted','watched','tracked'].forEach(function(s){
var cats = self.get(s + 'Categories').map(function(c){ return c.get('id')}); var cats = self.get(s + 'Categories').map(function(c){ return c.get('id')});

View File

@ -1,18 +1,6 @@
{{#if visible}} {{#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
<div class='composer-loading'>
{{loading-spinner}}
</div>
--}}
<div class='contents'> <div class='contents'>
{{render "composer-messages"}} {{render "composer-messages"}}
<div class='control'> <div class='control'>
<a href class='toggler' {{action "toggle" bubbles=false}} title='{{i18n 'composer.toggler'}}'></a> <a href class='toggler' {{action "toggle" bubbles=false}} title='{{i18n 'composer.toggler'}}'></a>

View File

@ -1,9 +1,9 @@
<section class='field'> <section class='field'>
<label>{{i18n 'category.logo'}}</label> <label>{{i18n 'category.logo'}}</label>
{{image-uploader uploadUrl=categoryUploadUrl imageUrl=model.logo_url type="logo"}} {{image-uploader imageUrl=model.logo_url type="category_logo"}}
</section> </section>
<section class='field'> <section class='field'>
<label>{{i18n 'category.background_image'}}</label> <label>{{i18n 'category.background_image'}}</label>
{{image-uploader uploadUrl=categoryUploadUrl imageUrl=model.background_url type="background"}} {{image-uploader imageUrl=model.background_url type="category_background"}}
</section> </section>

View File

@ -104,10 +104,7 @@
<div class="control-group pref-profile-bg"> <div class="control-group pref-profile-bg">
<label class="control-label">{{i18n 'user.change_profile_background.title'}}</label> <label class="control-label">{{i18n 'user.change_profile_background.title'}}</label>
<div class="controls"> <div class="controls">
{{image-uploader uploadUrl=imageUploadUrl {{image-uploader imageUrl=model.profile_background type="profile_background"}}
imageUrl=model.profile_background
instantDelete="true"
type="profile_background"}}
</div> </div>
<div class='instructions'> <div class='instructions'>
{{i18n 'user.change_profile_background.instructions'}} {{i18n 'user.change_profile_background.instructions'}}
@ -117,10 +114,7 @@
<div class="control-group pref-profile-bg"> <div class="control-group pref-profile-bg">
<label class="control-label">{{i18n 'user.change_card_background.title'}}</label> <label class="control-label">{{i18n 'user.change_card_background.title'}}</label>
<div class="controls"> <div class="controls">
{{image-uploader uploadUrl=imageUploadUrl {{image-uploader imageUrl=model.card_background type="card_background"}}
imageUrl=model.card_background
instantDelete="true"
type="card_background"}}
</div> </div>
<div class='instructions'> <div class='instructions'>
{{i18n 'user.change_card_background.instructions'}} {{i18n 'user.change_card_background.instructions'}}

View File

@ -306,82 +306,76 @@ const ComposerView = Discourse.View.extend(Ember.Evented, {
// in case it's still bound somehow // in case it's still bound somehow
this._unbindUploadTarget(); this._unbindUploadTarget();
const $uploadTarget = $('#reply-control'), const $uploadTarget = $("#reply-control"),
csrf = Discourse.Session.currentProp('csrfToken'); csrf = Discourse.Session.currentProp("csrfToken"),
let cancelledByTheUser; 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({ $uploadTarget.fileupload({
url: Discourse.getURL('/uploads.json?authenticity_token=' + encodeURIComponent(csrf)), url: Discourse.getURL("/uploads.json?authenticity_token=" + encodeURIComponent(csrf)),
dataType: 'json', dataType: "json",
pasteZone: $uploadTarget pasteZone: $uploadTarget,
}); });
// submit - this event is triggered for each upload $uploadTarget.on("fileuploadsubmit", (e, data) => {
$uploadTarget.on('fileuploadsubmit', function (e, data) { const isValid = Discourse.Utilities.validateUploadedFiles(data.files);
const result = Discourse.Utilities.validateUploadedFiles(data.files); data.formData = { type: "composer" };
// reset upload status when everything is ok this.setProperties({ uploadProgress: 0, isUploading: isValid });
if (result) self.setProperties({ uploadProgress: 0, isUploading: true }); return isValid;
return result;
}); });
// send - this event is triggered when the upload request is about to start $uploadTarget.on("fileuploadsend", (e, data) => {
$uploadTarget.on('fileuploadsend', function (e, data) {
cancelledByTheUser = false;
// hide the "file selector" modal // hide the "file selector" modal
self.get('controller').send('closeModal'); this.get("controller").send("closeModal");
// NOTE: IE9 doesn't support XHR // deal with cancellation
cancelledByTheUser = false;
if (data["xhr"]) { if (data["xhr"]) {
const jqHXR = data.xhr(); const jqHXR = data.xhr();
if (jqHXR) { if (jqHXR) {
// need to wait for the link to show up in the DOM // need to wait for the link to show up in the DOM
Em.run.schedule('afterRender', function() { Em.run.schedule("afterRender", () => {
// bind on the click event on the cancel link const $cancel = $("#cancel-file-upload");
$('#cancel-file-upload').on('click', function() { $cancel.on("click", () => {
// cancel the upload if (jqHXR) {
self.set('isUploading', false); cancelledByTheUser = true;
// NOTE: this might trigger a 'fileuploadfail' event with status = 0 // might trigger a "fileuploadfail" event with status = 0
if (jqHXR) { cancelledByTheUser = true; jqHXR.abort(); } jqHXR.abort();
// doesn't trigger the "fileuploadalways" event
reset();
}
// unbind // unbind
$(this).off('click'); $cancel.off("click");
}); });
}); });
} }
} }
}); });
// progress all $uploadTarget.on("fileuploadprogressall", (e, data) => {
$uploadTarget.on('fileuploadprogressall', function (e, data) {
const progress = parseInt(data.loaded / data.total * 100, 10); const progress = parseInt(data.loaded / data.total * 100, 10);
self.set('uploadProgress', progress); this.set("uploadProgress", progress);
}); });
// done $uploadTarget.on("fileuploadfail", (e, data) => {
$uploadTarget.on('fileuploaddone', function (e, data) {
if (!cancelledByTheUser) { 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); Discourse.Utilities.displayErrorForUpload(data);
} }
}); });
$uploadTarget.on("fileuploadalways", reset);
// contenteditable div hack for getting image paste to upload working in // contenteditable div hack for getting image paste to upload working in
// Firefox. This is pretty dangerous because it can potentially break // Firefox. This is pretty dangerous because it can potentially break
// Ctrl+v to paste so we should be conservative about what browsers this runs // 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() { titleValidation: function() {
const titleLength = this.get('model.titleLength'), const titleLength = this.get('model.titleLength'),
missingChars = this.get('model.missingTitleCharacters'); missingChars = this.get('model.missingTitleCharacters');
@ -580,13 +582,6 @@ const ComposerView = Discourse.View.extend(Ember.Evented, {
return Discourse.InputValidation.create({ failed: true, reason }); return Discourse.InputValidation.create({ failed: true, reason });
} }
}.property('model.reply', 'model.replyLength', 'model.missingReplyCharacters', 'model.minimumPostLength'), }.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); RSVP.EventTarget.mixin(ComposerView);

View File

@ -31,19 +31,6 @@ class CategoriesController < ApplicationController
end end
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 def move
guardian.ensure_can_create!(Category) guardian.ensure_can_create!(Category)

View File

@ -3,23 +3,35 @@ class UploadsController < ApplicationController
skip_before_filter :preload_json, :check_xhr, only: [:show] skip_before_filter :preload_json, :check_xhr, only: [:show]
def create def create
type = params.require(:type)
file = params[:file] || params[:files].first file = params[:file] || params[:files].first
filesize = file.tempfile.size url = params[:url]
upload = Upload.create_for(current_user.id, file.tempfile, file.original_filename, filesize, { content_type: file.content_type })
# 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? if upload.errors.empty? && current_user.admin?
retain_hours = params[:retain_hours].to_i retain_hours = params[:retain_hours].to_i
upload.update_columns(retain_hours: retain_hours) if retain_hours > 0 upload.update_columns(retain_hours: retain_hours) if retain_hours > 0
end 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" # HACK FOR IE9 to prevent the "download dialog"
response.headers["Content-Type"] = "text/plain" if request.user_agent =~ /MSIE 9/ response.headers["Content-Type"] = "text/plain" if request.user_agent =~ /MSIE 9/
if upload.errors.empty? render json: success_json
render_serialized(upload, UploadSerializer, root: false)
else
render status: 422, text: upload.errors.full_messages
end
end end
def show def show

View File

@ -346,7 +346,6 @@ class UsersController < ApplicationController
end end
def admin_login def admin_login
unless SiteSetting.enable_sso && !current_user unless SiteSetting.enable_sso && !current_user
return redirect_to path("/") return redirect_to path("/")
end end
@ -465,7 +464,6 @@ class UsersController < ApplicationController
end end
def send_activation_email def send_activation_email
RateLimiter.new(nil, "activate-hr-#{request.remote_ip}", 30, 1.hour).performed! RateLimiter.new(nil, "activate-hr-#{request.remote_ip}", 30, 1.hour).performed!
RateLimiter.new(nil, "activate-min-#{request.remote_ip}", 6, 1.minute).performed! RateLimiter.new(nil, "activate-min-#{request.remote_ip}", 6, 1.minute).performed!
@ -503,38 +501,6 @@ class UsersController < ApplicationController
render json: to_render render json: to_render
end 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 def pick_avatar
user = fetch_user_from_params user = fetch_user_from_params
guardian.ensure_can_edit!(user) guardian.ensure_can_edit!(user)
@ -555,16 +521,16 @@ class UsersController < ApplicationController
user = fetch_user_from_params user = fetch_user_from_params
guardian.ensure_can_edit!(user) guardian.ensure_can_edit!(user)
image_type = params.require(:image_type) case params.require(:type)
if image_type == 'profile_background' when "profile_background"
user.user_profile.clear_profile_background user.user_profile.clear_profile_background
elsif image_type == 'card_background' when "card_background"
user.user_profile.clear_card_background user.user_profile.clear_card_background
else else
raise Discourse::InvalidParameters.new(:image_type) raise Discourse::InvalidParameters.new(:type)
end end
render nothing: true render json: success_json
end end
def destroy def destroy
@ -614,51 +580,23 @@ class UsersController < ApplicationController
challenge challenge
end 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 def respond_to_suspicious_request
if suspicious?(params) if suspicious?(params)
render( render json: {
json: {
success: true, success: true,
active: false, active: false,
message: I18n.t("login.activate_email", email: params[:email]) message: I18n.t("login.activate_email", email: params[:email])
} }
)
end end
end end
def suspicious?(params) def suspicious?(params)
return false if current_user && is_api? && current_user.admin? return false if current_user && is_api? && current_user.admin?
honeypot_or_challenge_fails?(params) || SiteSetting.invite_only? honeypot_or_challenge_fails?(params) || SiteSetting.invite_only?
end end
def honeypot_or_challenge_fails?(params) def honeypot_or_challenge_fails?(params)
return false if is_api? return false if is_api?
params[:password_confirmation] != honeypot_value || params[:password_confirmation] != honeypot_value ||
params[:challenge] != challenge_value.try(:reverse) params[:challenge] != challenge_value.try(:reverse)
end end

View File

@ -26,8 +26,12 @@ class UserUpdater
def update(attributes = {}) def update(attributes = {})
user_profile = user.user_profile 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.website = format_url(attributes.fetch(:website) { user_profile.website })
user_profile.bio_raw = attributes.fetch(:bio_raw) { user_profile.bio_raw } 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.name = attributes.fetch(:name) { user.name }
user.locale = attributes.fetch(:locale) { user.locale } user.locale = attributes.fetch(:locale) { user.locale }
@ -57,16 +61,12 @@ class UserUpdater
end end
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] fields = attributes[:custom_fields]
if fields.present? if fields.present?
user.custom_fields = user.custom_fields.merge(fields) user.custom_fields = user.custom_fields.merge(fields)
end end
User.transaction do User.transaction do
if attributes.key?(:muted_usernames) if attributes.key?(:muted_usernames)
update_muted_users(attributes[:muted_usernames]) update_muted_users(attributes[:muted_usernames])
end end
@ -103,10 +103,6 @@ class UserUpdater
attr_reader :user, :guardian attr_reader :user, :guardian
def format_url(website) def format_url(website)
if website =~ /^http/ website =~ /^http/ ? website : "http://#{website}"
website
else
"http://#{website}"
end
end end
end end

View File

@ -261,7 +261,6 @@ Discourse::Application.routes.draw do
put "users/:username/preferences/badge_title" => "users#badge_title", constraints: {username: USERNAME_ROUTE_FORMAT} 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} get "users/:username/preferences/username" => "users#preferences", constraints: {username: USERNAME_ROUTE_FORMAT}
put "users/:username/preferences/username" => "users#username", 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} 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} 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} 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') get '/c', to: redirect('/categories')
resources :categories, :except => :show resources :categories, :except => :show
post "category/uploads" => "categories#upload"
post "category/:category_id/move" => "categories#move" post "category/:category_id/move" => "categories#move"
post "category/:category_id/notifications" => "categories#set_notifications" post "category/:category_id/notifications" => "categories#set_notifications"
put "category/:category_id/slug" => "categories#update_slug" put "category/:category_id/slug" => "categories#update_slug"

View File

@ -95,37 +95,6 @@ describe CategoriesController do
end 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 describe "update" do
it "requires the user to be logged in" do it "requires the user to be logged in" do

View File

@ -19,13 +19,6 @@ describe UploadsController do
}) })
end 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 let(:text_file) do
ActionDispatch::Http::UploadedFile.new({ ActionDispatch::Http::UploadedFile.new({
filename: 'LICENSE.TXT', filename: 'LICENSE.TXT',
@ -33,85 +26,47 @@ describe UploadsController do
}) })
end end
let(:files) { [ logo_dev, logo ] }
context 'with a file' do
context 'when authorized' do
before { SiteSetting.stubs(:authorized_extensions).returns(".PNG|.txt") }
it 'is successful with an image' do it 'is successful with an image' do
xhr :post, :create, file: logo message = MessageBus.track_publish do
xhr :post, :create, file: logo, type: "composer"
end.first
expect(response.status).to eq 200 expect(response.status).to eq 200
expect(message.channel).to eq("/uploads/composer")
expect(message.data).to be
end end
it 'is successful with an attachment' do it 'is successful with an attachment' do
xhr :post, :create, file: text_file message = MessageBus.track_publish do
xhr :post, :create, file: text_file, type: "avatar"
end.first
expect(response.status).to eq 200 expect(response.status).to eq 200
expect(message.channel).to eq("/uploads/avatar")
expect(message.data).to be
end end
it 'correctly sets retain_hours for admins' do it 'correctly sets retain_hours for admins' do
log_in :admin log_in :admin
xhr :post, :create, file: logo, retain_hours: 100
id = JSON.parse(response.body)["id"] 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) expect(Upload.find(id).retain_hours).to eq(100)
end end
context 'with a big file' do it 'properly returns errors' do
SiteSetting.stubs(:max_attachment_size_kb).returns(1)
before { SiteSetting.stubs(:max_attachment_size_kb).returns(1) } message = MessageBus.track_publish do
xhr :post, :create, file: text_file, type: "avatar"
end.first
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 expect(response.status).to eq 200
end expect(message.data["errors"]).to be
it 'is successful with an attachment' do
xhr :post, :create, file: text_file
expect(response.status).to eq 200
end
end
end
context 'with some files' do
it 'is successful' do
xhr :post, :create, files: files
expect(response).to be_success
end
it 'takes the first file' do
xhr :post, :create, files: files
expect(response.body).to match /logo-dev.png/
end
end end
end end

View File

@ -1304,164 +1304,6 @@ describe UsersController do
end end
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 describe '.pick_avatar' do
it 'raises an error when not logged in' 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 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) 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 expect(response).to be_forbidden
end 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) expect { xhr :delete, :destroy_user_image, username: user.username }.to raise_error(ActionController::ParameterMissing)
end end
it "only allows certain `image_types`" do it "only allows certain `types`" do
expect { xhr :delete, :destroy_user_image, username: user.username, image_type: 'wat' }.to raise_error(Discourse::InvalidParameters) expect { xhr :delete, :destroy_user_image, username: user.username, type: 'wat' }.to raise_error(Discourse::InvalidParameters)
end end
it 'can clear the profile background' do 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(user.reload.user_profile.profile_background).to eq("")
expect(response).to be_success expect(response).to be_success
end end