Fix specs for avatars

Implement avatar picker
Correct avatar related jobs
This commit is contained in:
Sam 2014-05-26 19:46:43 +10:00 committed by Sam Saffron
parent a864f8aefd
commit 504cfcff96
27 changed files with 228 additions and 158 deletions

View file

@ -9,6 +9,23 @@
**/ **/
export default Discourse.Controller.extend(Discourse.ModalFunctionality, { export default Discourse.Controller.extend(Discourse.ModalFunctionality, {
selectedUploadId: function(){
switch(this.get("selected")){
case "system":
return this.get("system_avatar_upload_id");
break;
case "gravatar":
return this.get("gravatar_avatar_upload_id");
break;
default:
return this.get("custom_avatar_upload_id");
}
}.property(
'selected',
'system_avatar_upload_id',
'gravatar_avatar_upload_id',
'custom_avatar_upload_id'),
actions: { actions: {
useUploadedAvatar: function() { useUploadedAvatar: function() {
this.set("selected", "uploaded"); this.set("selected", "uploaded");
@ -18,6 +35,16 @@ export default Discourse.Controller.extend(Discourse.ModalFunctionality, {
}, },
useSystem: function() { useSystem: function() {
this.set("selected", "system"); this.set("selected", "system");
},
refreshGravatar: function(){
var self = this;
self.set("gravatarRefreshDisabled", true);
Discourse
.ajax("/user_avatar/" + this.get("username") + "/refresh_gravatar", {method: 'POST'})
.then(function(result){
self.set("gravatarRefreshDisabled", false);
self.set("gravatar_avatar_upload_id", result.upload_id);
});
} }
} }
}); });

View file

@ -216,25 +216,35 @@ Handlebars.registerHelper('avatar', function(user, options) {
/** /**
Bound avatar helper. Bound avatar helper.
Will rerender whenever the "avatar_template" changes.
@method boundAvatar @method boundAvatar
@for Handlebars @for Handlebars
**/ **/
Ember.Handlebars.registerBoundHelper('boundAvatar', function(user, options) { Ember.Handlebars.registerBoundHelper('boundAvatar', function(user, size, uploadId) {
var username = Em.get(user, 'username'); var username = Em.get(user, 'username');
console.log(options.hash); if(arguments.length < 4){
uploadId = Em.get(user, 'uploaded_avatar_id');
}
var uploadId = (options.hash.uploadId && Em.get(user, options.hash.uploadId)) || Em.get(user, 'uploaded_avatar_id');
var avatarTemplate = Discourse.User.avatarTemplate(username,uploadId); var avatarTemplate = Discourse.User.avatarTemplate(username,uploadId);
return new Handlebars.SafeString(Discourse.Utilities.avatarImg({ return new Handlebars.SafeString(Discourse.Utilities.avatarImg({
size: options.hash.imageSize, size: size,
avatarTemplate: avatarTemplate avatarTemplate: avatarTemplate
})); }));
}, 'uploadId', 'username', 'uploaded_avatar_id'); }, 'uploaded_avatar_id');
/*
* Used when we only have a template
*/
Ember.Handlebars.registerBoundHelper('boundAvatarTemplate', function(avatarTemplate, size) {
return new Handlebars.SafeString(Discourse.Utilities.avatarImg({
size: size,
avatarTemplate: avatarTemplate
}));
});
/** /**
Nicely format a date without binding or returning HTML Nicely format a date without binding or returning HTML

View file

@ -332,15 +332,12 @@ Discourse.User = Discourse.Model.extend({
/* /*
Change avatar selection Change avatar selection
@method toggleAvatarSelection
@param {Boolean} useUploadedAvatar true if the user is using the uploaded avatar
@returns {Promise} the result of the toggle avatar selection
*/ */
toggleAvatarSelection: function(useUploadedAvatar) { pickAvatar: function(uploadId) {
return Discourse.ajax("/users/" + this.get("username_lower") + "/preferences/avatar/toggle", { this.set("uploaded_avatar_id", uploadId);
return Discourse.ajax("/users/" + this.get("username_lower") + "/preferences/avatar/pick", {
type: 'PUT', type: 'PUT',
data: { use_uploaded_avatar: useUploadedAvatar } data: { upload_id: uploadId }
}); });
}, },
@ -403,7 +400,7 @@ Discourse.User = Discourse.Model.extend({
return this.get('can_delete_account') && ((this.get('reply_count')||0) + (this.get('topic_count')||0)) <= 1; return this.get('can_delete_account') && ((this.get('reply_count')||0) + (this.get('topic_count')||0)) <= 1;
}.property('can_delete_account', 'reply_count', 'topic_count'), }.property('can_delete_account', 'reply_count', 'topic_count'),
delete: function() { "delete": function() {
if (this.get('can_delete_account')) { if (this.get('can_delete_account')) {
return Discourse.ajax("/users/" + this.get('username'), { return Discourse.ajax("/users/" + this.get('username'), {
type: 'DELETE', type: 'DELETE',
@ -419,7 +416,7 @@ Discourse.User = Discourse.Model.extend({
Discourse.User.reopenClass(Discourse.Singleton, { Discourse.User.reopenClass(Discourse.Singleton, {
avatarTemplate: function(username, uploadedAvatarId){ avatarTemplate: function(username, uploadedAvatarId){
return Discourse.getURL("/avatar/" + username.toLowerCase() + "/{size}/" + uploadedAvatarId + ".png"); return Discourse.getURL("/user_avatar/" + username.toLowerCase() + "/{size}/" + uploadedAvatarId + ".png");
}, },
/** /**

View file

@ -20,30 +20,46 @@ Discourse.PreferencesRoute = Discourse.RestrictedUserRoute.extend({
showAvatarSelector: function() { showAvatarSelector: function() {
Discourse.Route.showModal(this, 'avatar-selector'); Discourse.Route.showModal(this, 'avatar-selector');
// all the properties needed for displaying the avatar selector modal // all the properties needed for displaying the avatar selector modal
this.controllerFor('avatar-selector').setProperties(this.modelFor('user').getProperties( var controller = this.controllerFor('avatar-selector');
var user = this.modelFor('user');
var props = user.getProperties(
'username', 'email', 'username', 'email',
'uploaded_avatar_id',
'system_avatar_upload_id', 'system_avatar_upload_id',
'gravatr_avatar_upload_id', 'gravatar_avatar_upload_id',
'custom_avatar_upload_id' 'custom_avatar_upload_id'
)
); );
switch(props.uploaded_avatar_id){
case props.system_avatar_upload_id:
props.selected = "system";
break;
case props.gravatar_avatar_upload_id:
props.selected = "gravatar";
break;
default:
props.selected = "uploaded";
}
controller.setProperties(props);
}, },
saveAvatarSelection: function() { saveAvatarSelection: function() {
var user = this.modelFor('user'); var user = this.modelFor('user');
var avatarSelector = this.controllerFor('avatar-selector'); var avatarSelector = this.controllerFor('avatar-selector');
// sends the information to the server if it has changed // sends the information to the server if it has changed
if (avatarSelector.get('use_uploaded_avatar') !== user.get('use_uploaded_avatar')) { if (avatarSelector.get('selectedUploadId') !== user.get('uploaded_avatar_id')) {
user.toggleAvatarSelection(avatarSelector.get('use_uploaded_avatar')); user.pickAvatar(avatarSelector.get('selectedUploadId'));
} }
// saves the data back // saves the data back
user.setProperties(avatarSelector.getProperties( user.setProperties(avatarSelector.getProperties(
'has_uploaded_avatar', 'system_avatar_upload_id',
'use_uploaded_avatar', 'gravatar_avatar_upload_id',
'gravatar_template', 'custom_avatar_upload_id'
'uploaded_avatar_template'
)); ));
user.set('avatar_template', avatarSelector.get('avatarTemplate'));
avatarSelector.send('closeModal'); avatarSelector.send('closeModal');
}, },

View file

@ -104,7 +104,7 @@
href="#" href="#"
title='{{i18n user.avatar.title}}' title='{{i18n user.avatar.title}}'
id="current-user"> id="current-user">
{{boundAvatar currentUser imageSize="medium"}} {{boundAvatar currentUser "medium"}}
</a> </a>
</li> </li>
{{/if}} {{/if}}

View file

@ -2,18 +2,22 @@
<div> <div>
<div> <div>
<input type="radio" id="system-avatar" name="avatar" value="system" {{action useSystem}}> <input type="radio" id="system-avatar" name="avatar" value="system" {{action useSystem}}>
<label class="radio" for="system-avatar">{{boundAvatar controller imageSize="large" uploadId="system_avatar_upload_id"}} {{{i18n user.change_avatar.letter_based}}}</label> <label class="radio" for="system-avatar">{{boundAvatar controller "large" system_avatar_upload_id}} {{{i18n user.change_avatar.letter_based}}}</label>
</div> </div>
<div> <div>
<input type="radio" id="gravatar" name="avatar" value="gravatar" {{action useGravatar}}> <input type="radio" id="gravatar" name="avatar" value="gravatar" {{action useGravatar}}>
<label class="radio" for="gravatar">{{boundAvatar controller imageSize="large" uploadId="gravatar_avatar_upload_id"}} {{{i18n user.change_avatar.gravatar}}} {{email}}</label> <label class="radio" for="gravatar">{{boundAvatar controller "large" gravatar_avatar_upload_id}} {{{i18n user.change_avatar.gravatar}}} {{email}}</label>
<a href="#" {{action refreshGravatar}} title="{{i18n user.change_avatar.refresh_gravatar_title}}" class="btn no-text"><i class="fa fa-refresh"></i></a> <button href="#" {{action refreshGravatar}} title="{{i18n user.change_avatar.refresh_gravatar_title}}" {{bind-attr enabled="view.gravatarRefreshEnabled"}} class="btn no-text"><i class="fa fa-refresh"></i></button>
</div> </div>
<div> <div>
<input type="radio" id="uploaded_avatar" name="avatar" value="uploaded_avatar" {{action useUploadedAvatar}}> <input type="radio" id="uploaded_avatar" name="avatar" value="uploaded" {{action useUploadedAvatar}}>
<label class="radio" for="uploaded_avatar"> <label class="radio" for="uploaded_avatar">
{{#if custom_avatar_upload_id}} {{#if view.hasUploadedAvatar}}
{{boundAvatar controller imageSize="large" uploadId="custom_avatar_upload_id"}} {{i18n user.change_avatar.uploaded_avatar}} {{#if view.uploadedAvatarTemplate}}
{{boundAvatarTemplate view.uploadedAvatarTemplate "large"}}
{{else}}
{{boundAvatar controller "large" custom_avatar_upload_id}} {{i18n user.change_avatar.uploaded_avatar}}
{{/if}}
{{else}} {{else}}
{{i18n user.change_avatar.uploaded_avatar_empty}} {{i18n user.change_avatar.uploaded_avatar_empty}}
{{/if}} {{/if}}

View file

@ -17,7 +17,7 @@
</div> </div>
</div> </div>
<div id="revision-details"> <div id="revision-details">
{{i18n post.revisions.details.edited_by}} {{boundAvatar content imageSize="small"}} {{username}} <span class="date">{{date created_at}}</span> {{#if edit_reason}} &mdash; <span class="edit-reason">{{edit_reason}}</span>{{/if}} {{i18n post.revisions.details.edited_by}} {{boundAvatar content "small"}} {{username}} <span class="date">{{date created_at}}</span> {{#if edit_reason}} &mdash; <span class="edit-reason">{{edit_reason}}</span>{{/if}}
</div> </div>
<div id="revisions"> <div id="revisions">
{{#if title_changes}} {{#if title_changes}}
@ -32,7 +32,7 @@
{{/if}} {{/if}}
{{#if user_changes}} {{#if user_changes}}
<div class="row"> <div class="row">
{{boundAvatar user_changes.previous imageSize="small"}} {{user_changes.previous.username}}{{boundAvatar user_changes.current imageSize="small"}} {{user_changes.current.username}} {{boundAvatar user_changes.previous "small"}} {{user_changes.previous.username}}{{boundAvatar user_changes.current imageSize="small"}} {{user_changes.current.username}}
</div> </div>
{{/if}} {{/if}}
{{#if wiki_changes}} {{#if wiki_changes}}

View file

@ -1,5 +1,5 @@
{{#if model}} {{#if model}}
{{#link-to 'user' user}}{{boundAvatar model imageSize="huge"}}{{/link-to}} {{#link-to 'user' user}}{{boundAvatar model "huge"}}{{/link-to}}
<div class="names"> <div class="names">
<h1 {{bind-attr class="staff new_user"}}><a {{bind-attr href="usernameUrl"}}>{{username}}</a></h1> <h1 {{bind-attr class="staff new_user"}}><a {{bind-attr href="usernameUrl"}}>{{username}}</a></h1>

View file

@ -79,7 +79,7 @@
<div class="control-group"> <div class="control-group">
<label class="control-label">{{i18n user.avatar.title}}</label> <label class="control-label">{{i18n user.avatar.title}}</label>
<div class="controls"> <div class="controls">
{{boundAvatar model imageSize="large"}} {{boundAvatar model "large"}}
{{#if allowAvatarUpload}} {{#if allowAvatarUpload}}
<button {{action showAvatarSelector}} class="btn pad-left no-text"><i class="fa fa-pencil"></i></button> <button {{action showAvatarSelector}} class="btn pad-left no-text"><i class="fa fa-pencil"></i></button>
{{else}} {{else}}

View file

@ -64,7 +64,7 @@
<div class='details'> <div class='details'>
<div class='primary'> <div class='primary'>
{{boundAvatar model imageSize="huge"}} {{boundAvatar model "huge"}}
<div class="primary-textual"> <div class="primary-textual">
<h1>{{username}} {{{statusIcon}}}</h1> <h1>{{username}} {{{statusIcon}}}</h1>

View file

@ -12,11 +12,12 @@ Discourse.AvatarSelectorView = Discourse.ModalBodyView.extend({
title: I18n.t('user.change_avatar.title'), title: I18n.t('user.change_avatar.title'),
uploading: false, uploading: false,
uploadProgress: 0, uploadProgress: 0,
useGravatar: Em.computed.not("controller.use_uploaded_avatar"), saveDisabled: false,
canSaveAvatarSelection: Em.computed.or("useGravatar", "controller.has_uploaded_avatar"), gravatarRefreshEnabled: Em.computed.not('controller.gravatarRefreshDisabled'),
saveDisabled: Em.computed.not("canSaveAvatarSelection"),
imageIsNotASquare : false, imageIsNotASquare : false,
hasUploadedAvatar: Em.computed.or('uploadedAvatarTemplate', 'controller.custom_avatar_upload_id'),
didInsertElement: function() { didInsertElement: function() {
var self = this; var self = this;
var $upload = $("#avatar-input"); var $upload = $("#avatar-input");
@ -61,10 +62,8 @@ Discourse.AvatarSelectorView = Discourse.ModalBodyView.extend({
// make sure we have a url // make sure we have a url
if (data.result.url) { if (data.result.url) {
// indicates the users is using an uploaded avatar // indicates the users is using an uploaded avatar
self.get("controller").setProperties({ self.set("controller.custom_avatar_upload_id", data.result.upload_id);
has_uploaded_avatar: true,
use_uploaded_avatar: true
});
// display a warning whenever the image is not a square // display a warning whenever the image is not a square
self.set("imageIsNotASquare", data.result.width !== data.result.height); self.set("imageIsNotASquare", data.result.width !== data.result.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
@ -72,7 +71,7 @@ Discourse.AvatarSelectorView = Discourse.ModalBodyView.extend({
// 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(function(avatarTemplate) { Discourse.Utilities.cropAvatar(data.result.url, data.files[0].type).then(function(avatarTemplate) {
self.get("controller").set("uploaded_avatar_template", avatarTemplate); self.set("uploadedAvatarTemplate", avatarTemplate);
}); });
} else { } else {
bootbox.alert(I18n.t('post.errors.upload')); bootbox.alert(I18n.t('post.errors.upload'));
@ -95,14 +94,15 @@ Discourse.AvatarSelectorView = Discourse.ModalBodyView.extend({
$("#avatar-input").fileupload("destroy"); $("#avatar-input").fileupload("destroy");
}, },
// *HACK* used to select the proper radio button // *HACK* used to select the proper radio button, cause {{action}}
// stops the default behavior
selectedChanged: function() { selectedChanged: function() {
var self = this; var self = this;
Em.run.next(function() { Em.run.next(function() {
var value = self.get('controller.use_uploaded_avatar') ? 'uploaded_avatar' : 'gravatar'; var value = self.get('controller.selected');
$('input:radio[name="avatar"]').val([value]); $('input:radio[name="avatar"]').val([value]);
}); });
}.observes('controller.use_uploaded_avatar'), }.observes('controller.selected'),
uploadButtonText: function() { 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");

View file

@ -1,7 +1,23 @@
require_dependency 'letter_avatar' require_dependency 'letter_avatar'
class AvatarController < ApplicationController
skip_before_filter :check_xhr, :verify_authenticity_token class UserAvatarsController < ApplicationController
skip_before_filter :check_xhr, :verify_authenticity_token, only: :show
def refresh_gravatar
user = User.find_by(username_lower: params[:username].downcase)
guardian.ensure_can_edit!(user)
if user
user.create_user_avatar(user_id: user.id) unless user.user_avatar
user.user_avatar.update_gravatar!
render json: {upload_id: user.user_avatar.gravatar_upload_id}
else
raise Discourse::NotFound
end
end
def show def show
username = params[:username].to_s username = params[:username].to_s
@ -17,17 +33,17 @@ class AvatarController < ApplicationController
raise Discourse::NotFound unless version > 0 && user_avatar = user.user_avatar raise Discourse::NotFound unless version > 0 && user_avatar = user.user_avatar
upload = version if user_avatar.contains_upload?(version) upload = Upload.find(version) if user_avatar.contains_upload?(version)
upload ||= user.uploaded_avatar if user.uploaded_avatar_id == version upload ||= user.uploaded_avatar if user.uploaded_avatar_id == version
if user.uploaded_avatar && !upload if user.uploaded_avatar && !upload
return redirect_to "/avatar/#{user.username_lower}/#{size}/#{user.uploaded_avatar_id}.png" return redirect_to "/avatar/#{user.username_lower}/#{size}/#{user.uploaded_avatar_id}.png"
elsif upload elsif upload
# TODO broken with S3 (should retrun a permanent redirect) # TODO broken with S3 (should retrun a permanent redirect)
original = Discourse.store.path_for(user.uploaded_avatar) original = Discourse.store.path_for(upload)
if File.exists?(original) if File.exists?(original)
optimized = OptimizedImage.create_for( optimized = OptimizedImage.create_for(
user.uploaded_avatar, upload,
size, size,
size, size,
allow_animation: SiteSetting.allow_animated_avatars allow_animation: SiteSetting.allow_animated_avatars

View file

@ -7,7 +7,7 @@ class UsersController < ApplicationController
skip_before_filter :authorize_mini_profiler, only: [:avatar] skip_before_filter :authorize_mini_profiler, only: [:avatar]
skip_before_filter :check_xhr, only: [:show, :password_reset, :update, :activate_account, :authorize_email, :user_preferences_redirect, :avatar, :my_redirect] skip_before_filter :check_xhr, only: [:show, :password_reset, :update, :activate_account, :authorize_email, :user_preferences_redirect, :avatar, :my_redirect]
before_filter :ensure_logged_in, only: [:username, :update, :change_email, :user_preferences_redirect, :upload_user_image, :toggle_avatar, :clear_profile_background, :destroy] before_filter :ensure_logged_in, only: [:username, :update, :change_email, :user_preferences_redirect, :upload_user_image, :pick_avatar, :clear_profile_background, :destroy]
before_filter :respond_to_suspicious_request, only: [:create] before_filter :respond_to_suspicious_request, only: [:create]
# we need to allow account creation with bad CSRF tokens, if people are caching, the CSRF token on the # we need to allow account creation with bad CSRF tokens, if people are caching, the CSRF token on the
@ -364,12 +364,18 @@ class UsersController < ApplicationController
end end
end end
def toggle_avatar def pick_avatar
params.require(:use_uploaded_avatar) params.require(:upload_id)
user = fetch_user_from_params user = fetch_user_from_params
guardian.ensure_can_edit!(user) guardian.ensure_can_edit!(user)
upload_id = params[:upload_id]
user.use_uploaded_avatar = params[:use_uploaded_avatar] user.uploaded_avatar_id = upload_id
# ensure we associate the custom avatar properly
unless user.user_avatar.contains_upload?(upload_id)
user.user_avatar.custom_upload_id = upload_id
end
user.save! user.save!
render nothing: true render nothing: true
@ -421,8 +427,7 @@ class UsersController < ApplicationController
end end
def upload_avatar_for(user, upload) def upload_avatar_for(user, upload)
user.upload_avatar(upload) render json: { upload_id: upload.id, url: upload.url, width: upload.width, height: upload.height }
render json: { url: upload.url, width: upload.width, height: upload.height }
end end
def upload_profile_background_for(user, upload) def upload_profile_background_for(user, upload)

View file

@ -1,10 +0,0 @@
module Jobs
class CreateMissingAvatars < Jobs::Base
def execute(args)
User.find_each do |u|
u.refresh_avatar
u.save
end
end
end
end

View file

@ -6,14 +6,15 @@ module Jobs
def execute(args) def execute(args)
return unless SiteSetting.clean_up_uploads? return unless SiteSetting.clean_up_uploads?
uploads_used_in_posts = PostUpload.uniq.pluck(:upload_id)
uploads_used_as_avatars = User.uniq.where('uploaded_avatar_id IS NOT NULL').pluck(:uploaded_avatar_id)
uploads_used_as_profile_backgrounds = User.uniq.where("profile_background IS NOT NULL AND profile_background != ''").pluck(:profile_background) uploads_used_as_profile_backgrounds = User.uniq.where("profile_background IS NOT NULL AND profile_background != ''").pluck(:profile_background)
grace_period = [SiteSetting.clean_orphan_uploads_grace_period_hours, 1].max grace_period = [SiteSetting.clean_orphan_uploads_grace_period_hours, 1].max
Upload.where("created_at < ?", grace_period.hour.ago) Upload.where("created_at < ?", grace_period.hour.ago)
.where("id NOT IN (?)", uploads_used_in_posts + uploads_used_as_avatars) .where("id NOT IN (SELECT upload_id from post_uploads)")
.where("id NOT IN (SELECT system_upload_id from post_uploads)")
.where("id NOT IN (SELECT custom_upload_id from post_uploads)")
.where("id NOT IN (SELECT gravatar_upload_id from post_uploads)")
.where("url NOT IN (?)", uploads_used_as_profile_backgrounds) .where("url NOT IN (?)", uploads_used_as_profile_backgrounds)
.find_each do |upload| .find_each do |upload|
upload.destroy upload.destroy

View file

@ -0,0 +1,15 @@
module Jobs
class CreateMissingAvatars < Jobs::Scheduled
every 1.hour
def execute(args)
User.where(uploaded_avatar_id: nil).find_each do |u|
u.refresh_avatar
u.save
end
UserAvatar.where(system_upload_id: nil).find_each do |a|
a.update_system_avatar!
end
end
end
end

View file

@ -354,7 +354,7 @@ class User < ActiveRecord::Base
def self.avatar_template(username,uploaded_avatar_id) def self.avatar_template(username,uploaded_avatar_id)
id = uploaded_avatar_id || -1 id = uploaded_avatar_id || -1
username ||= "" username ||= ""
"/avatar/#{username.downcase}/{size}/#{id}.png" "/user_avatar/#{username.downcase}/{size}/#{id}.png"
end end
def avatar_template def avatar_template
@ -547,13 +547,9 @@ class User < ActiveRecord::Base
created_at > 1.day.ago created_at > 1.day.ago
end end
def upload_avatar(upload) # TODO this is a MESS
self.uploaded_avatar_template = nil # at most user table should have profile_background_upload_id
self.uploaded_avatar = upload # best case is to move this to another table
self.use_uploaded_avatar = true
self.save!
end
def upload_profile_background(upload) def upload_profile_background(upload)
self.profile_background = upload.url self.profile_background = upload.url
self.save! self.save!
@ -631,7 +627,11 @@ class User < ActiveRecord::Base
avatar.update_system_avatar! if !avatar.system_upload_id || username_changed? avatar.update_system_avatar! if !avatar.system_upload_id || username_changed?
end end
self.uploaded_avatar_id = (avatar.gravatar_upload_id || avatar.system_upload_id) unless uploaded_avatar_id desired_avatar_id = avatar.gravatar_upload_id || avatar.system_upload_id
if !self.uploaded_avatar_id && desired_avatar_id
self.update_column(:uploaded_avatar_id, desired_avatar_id)
end
end end
protected protected
@ -789,7 +789,6 @@ end
# dynamic_favicon :boolean default(FALSE), not null # dynamic_favicon :boolean default(FALSE), not null
# title :string(255) # title :string(255)
# use_uploaded_avatar :boolean default(FALSE) # use_uploaded_avatar :boolean default(FALSE)
# uploaded_avatar_template :string(255)
# uploaded_avatar_id :integer # uploaded_avatar_id :integer
# email_always :boolean default(FALSE), not null # email_always :boolean default(FALSE), not null
# mailing_list_mode :boolean default(FALSE), not null # mailing_list_mode :boolean default(FALSE), not null

View file

@ -27,10 +27,13 @@ class UserAvatar < ActiveRecord::Base
upload = Upload.create_for(user.id, tempfile, 'gravatar.png', File.size(tempfile.path)) upload = Upload.create_for(user.id, tempfile, 'gravatar.png', File.size(tempfile.path))
gravatar_upload.destroy! if gravatar_upload if gravatar_upload_id != upload.id
gravatar_upload.try(:destroy!)
self.gravatar_upload = upload self.gravatar_upload = upload
save! save!
rescue OpenURI::HTTPError else
gravatar_upload
end
save! save!
ensure ensure
tempfile.unlink if tempfile tempfile.unlink if tempfile

View file

@ -67,17 +67,4 @@ class UserActionSerializer < ApplicationSerializer
object.action_type == UserAction::EDIT object.action_type == UserAction::EDIT
end end
private
def avatar_for(user_id, email, use_uploaded_avatar, uploaded_avatar_template, uploaded_avatar_id)
# NOTE: id is required for cases where the template is blank (during initial population)
User.new(
id: user_id,
email: email,
use_uploaded_avatar: use_uploaded_avatar,
uploaded_avatar_template: uploaded_avatar_template,
uploaded_avatar_id: uploaded_avatar_id
).avatar_template
end
end end

View file

@ -202,7 +202,7 @@ Discourse::Application.routes.draw do
get "users/:username/avatar(/:size)" => "users#avatar", constraints: {username: USERNAME_ROUTE_FORMAT} # LEGACY ROUTE get "users/:username/avatar(/:size)" => "users#avatar", constraints: {username: USERNAME_ROUTE_FORMAT} # LEGACY ROUTE
post "users/:username/preferences/avatar" => "users#upload_avatar", constraints: {username: USERNAME_ROUTE_FORMAT} # LEGACY ROUTE post "users/:username/preferences/avatar" => "users#upload_avatar", constraints: {username: USERNAME_ROUTE_FORMAT} # LEGACY ROUTE
post "users/:username/preferences/user_image" => "users#upload_user_image", constraints: {username: USERNAME_ROUTE_FORMAT} post "users/:username/preferences/user_image" => "users#upload_user_image", constraints: {username: USERNAME_ROUTE_FORMAT}
put "users/:username/preferences/avatar/toggle" => "users#toggle_avatar", constraints: {username: USERNAME_ROUTE_FORMAT} put "users/:username/preferences/avatar/pick" => "users#pick_avatar", constraints: {username: USERNAME_ROUTE_FORMAT}
put "users/:username/preferences/profile_background/clear" => "users#clear_profile_background", constraints: {username: USERNAME_ROUTE_FORMAT} put "users/:username/preferences/profile_background/clear" => "users#clear_profile_background", constraints: {username: USERNAME_ROUTE_FORMAT}
get "users/:username/invited" => "users#invited", constraints: {username: USERNAME_ROUTE_FORMAT} get "users/:username/invited" => "users#invited", constraints: {username: USERNAME_ROUTE_FORMAT}
post "users/:username/send_activation_email" => "users#send_activation_email", constraints: {username: USERNAME_ROUTE_FORMAT} post "users/:username/send_activation_email" => "users#send_activation_email", constraints: {username: USERNAME_ROUTE_FORMAT}
@ -211,6 +211,10 @@ Discourse::Application.routes.draw do
get "users/:username/badges" => "users#show", constraints: {username: USERNAME_ROUTE_FORMAT} get "users/:username/badges" => "users#show", constraints: {username: USERNAME_ROUTE_FORMAT}
delete "users/:username" => "users#destroy", constraints: {username: USERNAME_ROUTE_FORMAT} delete "users/:username" => "users#destroy", constraints: {username: USERNAME_ROUTE_FORMAT}
post "user_avatar/:username/refresh_gravatar" => "user_avatars#refresh_gravatar"
get "user_avatar/:username/:size/:version.png" => "user_avatars#show", format: false
get "uploads/:site/:id/:sha.:extension" => "uploads#show", constraints: {site: /\w+/, id: /\d+/, sha: /[a-z0-9]{15,16}/i, extension: /\w{2,}/} get "uploads/:site/:id/:sha.:extension" => "uploads#show", constraints: {site: /\w+/, id: /\d+/, sha: /[a-z0-9]{15,16}/i, extension: /\w{2,}/}
post "uploads" => "uploads#create" post "uploads" => "uploads#create"
@ -366,8 +370,6 @@ Discourse::Application.routes.draw do
post "draft" => "draft#update" post "draft" => "draft#update"
delete "draft" => "draft#destroy" delete "draft" => "draft#destroy"
get "avatar/:username/:size/:version.png" => "avatar#show", format: false
get "cdn_asset/:site/*path" => "static#cdn_asset", format: false get "cdn_asset/:site/*path" => "static#cdn_asset", format: false
get "robots.txt" => "robots_txt#index" get "robots.txt" => "robots_txt#index"

View file

@ -23,8 +23,3 @@ User.seed do |u|
u.email_private_messages = false u.email_private_messages = false
u.trust_level = TrustLevel.levels[:elder] u.trust_level = TrustLevel.levels[:elder]
end end
# download avatars for existing users
if UserAvatar.count < User.count
Jobs.enqueue(:create_missing_avatars)
end

View file

@ -1,5 +1,5 @@
class AddUserAvatars < ActiveRecord::Migration class AddUserAvatars < ActiveRecord::Migration
def change def up
create_table :user_avatars do |t| create_table :user_avatars do |t|
t.integer :user_id, null: false t.integer :user_id, null: false
t.integer :system_upload_id t.integer :system_upload_id
@ -8,5 +8,17 @@ class AddUserAvatars < ActiveRecord::Migration
t.datetime :last_gravatar_download_attempt t.datetime :last_gravatar_download_attempt
t.timestamps t.timestamps
end end
add_index :user_avatars, [:user_id]
execute <<SQL
INSERT INTO user_avatars(user_id, custom_upload_id)
SELECT id, uploaded_avatar_id
FROM users
SQL
end
def down
drop_table :user_avatars
end end
end end

View file

@ -0,0 +1,5 @@
class RemoveUploadedAvatarTemplateFromUsers < ActiveRecord::Migration
def change
remove_column :users, :uploaded_avatar_template
end
end

View file

@ -89,7 +89,8 @@ describe CookedPostProcessor do
# optimized_image # optimized_image
FileUtils.stubs(:mkdir_p) FileUtils.stubs(:mkdir_p)
File.stubs(:open) File.stubs(:open)
ImageSorcery.any_instance.expects(:convert).returns(true) # hmmm this should be done in a cleaner way
OptimizedImage.any_instance.expects(:resize).returns(true)
end end
it "generates overlay information" do it "generates overlay information" do

View file

@ -1137,18 +1137,12 @@ describe UsersController do
Upload.expects(:create_for).returns(upload) Upload.expects(:create_for).returns(upload)
# enqueues the user_image generator job # enqueues the user_image generator job
xhr :post, :upload_user_image, username: user.username, file: user_image, user_image_type: "avatar" xhr :post, :upload_user_image, username: user.username, file: user_image, user_image_type: "avatar"
user.reload
# erase the previous template
user.uploaded_avatar_template.should == nil
# link to the right upload
user.uploaded_avatar.id.should == upload.id
# automatically set "use_uploaded_user_image"
user.use_uploaded_avatar.should == true
# returns the url, width and height of the uploaded image # returns the url, width and height of the uploaded image
json = JSON.parse(response.body) json = JSON.parse(response.body)
json['url'].should == "/uploads/default/1/1234567890123456.png" json['url'].should == "/uploads/default/1/1234567890123456.png"
json['width'].should == 100 json['width'].should == 100
json['height'].should == 200 json['height'].should == 200
json['upload_id'].should == upload.id
end end
it 'is successful for profile backgrounds' do it 'is successful for profile backgrounds' do
@ -1194,18 +1188,11 @@ describe UsersController do
Upload.expects(:create_for).returns(upload) Upload.expects(:create_for).returns(upload)
# enqueues the user_image generator job # enqueues the user_image generator job
xhr :post, :upload_avatar, username: user.username, file: user_image_url, user_image_type: "avatar" xhr :post, :upload_avatar, username: user.username, file: user_image_url, user_image_type: "avatar"
user.reload
# erase the previous template
user.uploaded_avatar_template.should == nil
# link to the right upload
user.uploaded_avatar.id.should == upload.id
# automatically set "use_uploaded_user_image"
user.use_uploaded_avatar.should == true
# returns the url, width and height of the uploaded image
json = JSON.parse(response.body) json = JSON.parse(response.body)
json['url'].should == "/uploads/default/1/1234567890123456.png" json['url'].should == "/uploads/default/1/1234567890123456.png"
json['width'].should == 100 json['width'].should == 100
json['height'].should == 200 json['height'].should == 200
json['upload_id'].should == upload.id
end end
it 'is successful for profile backgrounds' do it 'is successful for profile backgrounds' do
@ -1234,29 +1221,29 @@ describe UsersController do
end end
describe '.toggle_avatar' do describe '.pick_avatar' do
it 'raises an error when not logged in' do it 'raises an error when not logged in' do
lambda { xhr :put, :toggle_avatar, username: 'asdf' }.should raise_error(Discourse::NotLoggedIn) lambda { xhr :put, :pick_avatar, username: 'asdf', avatar_id: 1}.should raise_error(Discourse::NotLoggedIn)
end end
context 'while logged in' do context 'while logged in' do
let!(:user) { log_in } let!(:user) { log_in }
it 'raises an error without a use_uploaded_avatar param' do it 'raises an error without an avatar_id param' do
lambda { xhr :put, :toggle_avatar, username: user.username }.should raise_error(ActionController::ParameterMissing) lambda { xhr :put, :pick_avatar, username: user.username }.should raise_error(ActionController::ParameterMissing)
end end
it 'raises an error when you don\'t have permission to toggle the avatar' do it 'raises an error when you don\'t have permission to toggle the avatar' do
Guardian.any_instance.expects(:can_edit?).with(user).returns(false) another_user = Fabricate(:user)
xhr :put, :toggle_avatar, username: user.username, use_uploaded_avatar: "true" xhr :put, :pick_avatar, username: another_user.username, upload_id: 1
response.should be_forbidden response.should be_forbidden
end end
it 'it successful' do it 'it successful' do
xhr :put, :toggle_avatar, username: user.username, use_uploaded_avatar: "false" xhr :put, :pick_avatar, username: user.username, upload_id: 111
user.reload.use_uploaded_avatar.should == false user.reload.uploaded_avatar_id.should == 111
response.should be_success response.should be_success
end end

View file

@ -15,11 +15,10 @@ describe OptimizedImage do
let(:store) { FakeInternalStore.new } let(:store) { FakeInternalStore.new }
before { Discourse.stubs(:store).returns(store) } before { Discourse.stubs(:store).returns(store) }
context "when an error happened while generatign the thumbnail" do context "when an error happened while generating the thumbnail" do
before { ImageSorcery.any_instance.stubs(:convert).returns(false) }
it "returns nil" do it "returns nil" do
OptimizedImage.expects(:resize).returns(false)
OptimizedImage.create_for(upload, 100, 200).should be_nil OptimizedImage.create_for(upload, 100, 200).should be_nil
end end
@ -27,7 +26,9 @@ describe OptimizedImage do
context "when the thumbnail is properly generated" do context "when the thumbnail is properly generated" do
before { ImageSorcery.any_instance.stubs(:convert).returns(true) } before do
OptimizedImage.expects(:resize).returns(true)
end
it "does not download a copy of the original image" do it "does not download a copy of the original image" do
store.expects(:download).never store.expects(:download).never
@ -59,9 +60,8 @@ describe OptimizedImage do
context "when an error happened while generatign the thumbnail" do context "when an error happened while generatign the thumbnail" do
before { ImageSorcery.any_instance.stubs(:convert).returns(false) }
it "returns nil" do it "returns nil" do
OptimizedImage.expects(:resize).returns(false)
OptimizedImage.create_for(upload, 100, 200).should be_nil OptimizedImage.create_for(upload, 100, 200).should be_nil
end end
@ -69,7 +69,9 @@ describe OptimizedImage do
context "when the thumbnail is properly generated" do context "when the thumbnail is properly generated" do
before { ImageSorcery.any_instance.stubs(:convert).returns(true) } before do
OptimizedImage.expects(:resize).returns(true)
end
it "downloads a copy of the original image" do it "downloads a copy of the original image" do
Tempfile.any_instance.expects(:close!).twice Tempfile.any_instance.expects(:close!).twice

View file

@ -854,15 +854,6 @@ describe User do
end end
end end
describe "#upload_avatar" do
let(:upload) { Fabricate(:upload) }
let(:user) { Fabricate(:user) }
it "should update user's avatar" do
expect(user.upload_avatar(upload)).to be_true
end
end
describe 'api keys' do describe 'api keys' do
let(:admin) { Fabricate(:admin) } let(:admin) { Fabricate(:admin) }
let(:other_admin) { Fabricate(:admin) } let(:other_admin) { Fabricate(:admin) }
@ -987,7 +978,7 @@ describe User do
let(:user) { build(:user, username: 'Sam') } let(:user) { build(:user, username: 'Sam') }
it "returns a 45-pixel-wide avatar" do it "returns a 45-pixel-wide avatar" do
user.small_avatar_url.should == "//test.localhost/avatar/sam/45/-1.png" user.small_avatar_url.should == "//test.localhost/user_avatar/sam/45/-1.png"
end end
end end
@ -996,18 +987,13 @@ describe User do
let(:user) { build(:user, uploaded_avatar_id: 99, username: 'Sam') } let(:user) { build(:user, uploaded_avatar_id: 99, username: 'Sam') }
it "returns default when uploaded avatars are not allowed" do
SiteSetting.allow_uploaded_avatars = false
user.avatar_template_url.should == "//test.localhost/avatar/sam/{size}/-1.png"
end
it "returns a schemaless avatar template with correct id" do it "returns a schemaless avatar template with correct id" do
user.avatar_template_url.should == "//test.localhost/avatar/sam/{size}/99.png" user.avatar_template_url.should == "//test.localhost/user_avatar/sam/{size}/99.png"
end end
it "returns a schemaless cdn-based avatar template" do it "returns a schemaless cdn-based avatar template" do
Rails.configuration.action_controller.stubs(:asset_host).returns("http://my.cdn.com") Rails.configuration.action_controller.stubs(:asset_host).returns("http://my.cdn.com")
user.avatar_template_url.should == "//my.cdn.com/avatar/sam/{size}/99.png" user.avatar_template_url.should == "//my.cdn.com/user_avatar/sam/{size}/99.png"
end end
end end
@ -1148,6 +1134,16 @@ describe User do
end end
describe "automatic avatar creation" do
it "sets a system avatar for new users" do
SiteSetting.enable_system_avatars = true
u = User.create!(username: "bob", email: "bob@bob.com")
u.reload
u.user_avatar.system_upload_id.should == u.uploaded_avatar_id
u.uploaded_avatar_id.should_not == nil
end
end
describe "custom fields" do describe "custom fields" do
it "allows modification of custom fields" do it "allows modification of custom fields" do
user = Fabricate(:user) user = Fabricate(:user)