From 07e7b07b63caff59c94bff3b43889009010a03bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Thu, 17 Sep 2015 19:42:44 +0200 Subject: [PATCH] FIX: refreshing gravatar wasn't working --- .../controllers/avatar-selector.js.es6 | 2 +- .../javascripts/discourse/models/user.js.es6 | 3 +- .../discourse/routes/preferences.js.es6 | 3 +- app/controllers/users_controller.rb | 17 +++++++--- app/models/user_avatar.rb | 10 +++--- config/locales/server.en.yml | 3 ++ spec/controllers/users_controller_spec.rb | 33 ++++++++++++------- 7 files changed, 47 insertions(+), 24 deletions(-) diff --git a/app/assets/javascripts/discourse/controllers/avatar-selector.js.es6 b/app/assets/javascripts/discourse/controllers/avatar-selector.js.es6 index 29aae8bf67d..32aa7b514d5 100644 --- a/app/assets/javascripts/discourse/controllers/avatar-selector.js.es6 +++ b/app/assets/javascripts/discourse/controllers/avatar-selector.js.es6 @@ -36,7 +36,7 @@ export default Ember.Controller.extend(ModalFunctionality, { .ajax(`/user_avatar/${this.get("username")}/refresh_gravatar.json`, { method: "POST" }) .then(result => this.setProperties({ gravatar_avatar_template: result.gravatar_avatar_template, - gravatar_upload_id: result.gravatar_upload_id, + gravatar_avatar_upload_id: result.gravatar_upload_id, })) .finally(() => this.set("gravatarRefreshDisabled", false)); } diff --git a/app/assets/javascripts/discourse/models/user.js.es6 b/app/assets/javascripts/discourse/models/user.js.es6 index bd0e36c5fab..d7727237177 100644 --- a/app/assets/javascripts/discourse/models/user.js.es6 +++ b/app/assets/javascripts/discourse/models/user.js.es6 @@ -4,8 +4,7 @@ import UserStream from 'discourse/models/user-stream'; import UserPostsStream from 'discourse/models/user-posts-stream'; import Singleton from 'discourse/mixins/singleton'; import { longDate } from 'discourse/lib/formatter'; -import computed from 'ember-addons/ember-computed-decorators'; -import { observes } from 'ember-addons/ember-computed-decorators'; +import { default as computed, observes } from 'ember-addons/ember-computed-decorators'; import Badge from 'discourse/models/badge'; import UserBadge from 'discourse/models/user-badge'; diff --git a/app/assets/javascripts/discourse/routes/preferences.js.es6 b/app/assets/javascripts/discourse/routes/preferences.js.es6 index 3ae6aed002b..b4f0e0baf82 100644 --- a/app/assets/javascripts/discourse/routes/preferences.js.es6 +++ b/app/assets/javascripts/discourse/routes/preferences.js.es6 @@ -1,5 +1,6 @@ import RestrictedUserRoute from "discourse/routes/restricted-user"; import showModal from 'discourse/lib/show-modal'; +import { popupAjaxError } from 'discourse/lib/ajax-error'; export default RestrictedUserRoute.extend({ model() { @@ -60,7 +61,7 @@ export default RestrictedUserRoute.extend({ 'custom_avatar_template' )); bootbox.alert(I18n.t("user.change_avatar.cache_notice")); - }); + }).catch(popupAjaxError); // saves the data back controller.send('closeModal'); diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index bb361de6fde..b5bb7c3e249 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -527,6 +527,8 @@ class UsersController < ApplicationController render json: to_render end + AVATAR_TYPES_WITH_UPLOAD ||= %w{uploaded custom gravatar} + def pick_avatar user = fetch_user_from_params guardian.ensure_can_edit!(user) @@ -536,10 +538,17 @@ class UsersController < ApplicationController user.uploaded_avatar_id = upload_id - if type == "uploaded" || type == "custom" - user.user_avatar.custom_upload_id = upload_id - elsif type == "gravatar" - user.user_avatar.gravatar_upload_id = upload_id + if AVATAR_TYPES_WITH_UPLOAD.include?(type) + # make sure the upload exists + unless Upload.where(id: upload_id).exists? + return render_json_error I18n.t("avatar.missing") + end + + if type == "gravatar" + user.user_avatar.gravatar_upload_id = upload_id + else + user.user_avatar.custom_upload_id = upload_id + end end user.save! diff --git a/app/models/user_avatar.rb b/app/models/user_avatar.rb index 8d57472c688..0669ea6be5a 100644 --- a/app/models/user_avatar.rb +++ b/app/models/user_avatar.rb @@ -10,20 +10,20 @@ class UserAvatar < ActiveRecord::Base end def update_gravatar! - DistributedMutex.synchronize("update_gravatar_#{user.id}") do + DistributedMutex.synchronize("update_gravatar_#{user_id}") do begin # special logic for our system user - email_hash = user.id == Discourse::SYSTEM_USER_ID ? User.email_hash("info@discourse.org") : user.email_hash + email_hash = user_id == Discourse::SYSTEM_USER_ID ? User.email_hash("info@discourse.org") : user.email_hash self.last_gravatar_download_attempt = Time.new max = Discourse.avatar_sizes.max gravatar_url = "http://www.gravatar.com/avatar/#{email_hash}.png?s=#{max}&d=404" tempfile = FileHelper.download(gravatar_url, SiteSetting.max_image_size_kb.kilobytes, "gravatar") - upload = Upload.create_for(user.id, tempfile, 'gravatar.png', File.size(tempfile.path), origin: gravatar_url, image_type: "avatar") + upload = Upload.create_for(user_id, tempfile, 'gravatar.png', File.size(tempfile.path), origin: gravatar_url, image_type: "avatar") if gravatar_upload_id != upload.id - gravatar_upload.try(:destroy!) + gravatar_upload.try(:destroy!) rescue nil self.gravatar_upload = upload save! end @@ -31,7 +31,7 @@ class UserAvatar < ActiveRecord::Base save! rescue SocketError # skip saving, we are not connected to the net - Rails.logger.warn "Failed to download gravatar, socket error - user id #{user.id}" + Rails.logger.warn "Failed to download gravatar, socket error - user id #{user_id}" ensure tempfile.try(:close!) end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index d256e5dceb0..e21f642ef97 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -2096,6 +2096,9 @@ en: too_large: "Sorry, the image you are trying to upload is too big (maximum size is %{max_size_kb}KB), please resize it and try again." size_not_found: "Sorry, but we couldn't determine the size of the image. Maybe your image is corrupted?" + avatar: + missing: "Sorry, but the avatar you have selected is not present on the server. Can you try uploading it again?" + flag_reason: sockpuppet: "A new user created a topic, and another new user at the same IP address replied. See the flag_sockpuppets site setting." spam_hosts: "This new user tried to create multiple posts with links to the same domain. See the newuser_spam_host_threshold site setting." diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 884f3f3ff20..fb258ec0937 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -1301,29 +1301,40 @@ describe UsersController do describe '.pick_avatar' do it 'raises an error when not logged in' do - expect { xhr :put, :pick_avatar, username: 'asdf', avatar_id: 1, type: "custom"}.to raise_error(Discourse::NotLoggedIn) + expect { + xhr :put, :pick_avatar, username: 'asdf', avatar_id: 1, type: "custom" + }.to raise_error(Discourse::NotLoggedIn) end context 'while logged in' do let!(:user) { log_in } + let(:upload) { Fabricate(:upload) } - 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 another_user = Fabricate(:user) - xhr :put, :pick_avatar, username: another_user.username, upload_id: 1, type: "custom" + xhr :put, :pick_avatar, username: another_user.username, upload_id: upload.id, type: "custom" expect(response).to be_forbidden end - it 'it successful' do - xhr :put, :pick_avatar, username: user.username, upload_id: 111, type: "custom" - expect(user.reload.uploaded_avatar_id).to eq(111) - expect(user.user_avatar.reload.custom_upload_id).to eq(111) - expect(response).to be_success - + it 'can successfully pick the system avatar' do xhr :put, :pick_avatar, username: user.username - expect(user.reload.uploaded_avatar_id).to eq(nil) - expect(user.user_avatar.reload.custom_upload_id).to eq(111) expect(response).to be_success + expect(user.reload.uploaded_avatar_id).to eq(nil) + end + + it 'can successfully pick a gravatar' do + xhr :put, :pick_avatar, username: user.username, upload_id: upload.id, type: "gravatar" + expect(response).to be_success + expect(user.reload.uploaded_avatar_id).to eq(upload.id) + expect(user.user_avatar.reload.gravatar_upload_id).to eq(upload.id) + end + + it 'can successfully pick a custom avatar' do + xhr :put, :pick_avatar, username: user.username, upload_id: upload.id, type: "custom" + expect(response).to be_success + expect(user.reload.uploaded_avatar_id).to eq(upload.id) + expect(user.user_avatar.reload.custom_upload_id).to eq(upload.id) end end