From d9f51961c40dd466f9f59f5591e7f5da2bc1f066 Mon Sep 17 00:00:00 2001 From: Sam Date: Wed, 28 May 2014 16:54:21 +1000 Subject: [PATCH] BUGFIX: pick gravatar if it was just downloaded BUGFIX: don't go rebaking unless all avatars are downloaded --- app/jobs/scheduled/periodical_updates.rb | 9 +++++++-- app/models/user.rb | 15 ++++++++++----- app/models/user_avatar.rb | 3 +-- spec/models/user_spec.rb | 24 ++++++++++++++++++++++++ 4 files changed, 42 insertions(+), 9 deletions(-) diff --git a/app/jobs/scheduled/periodical_updates.rb b/app/jobs/scheduled/periodical_updates.rb index 88de72d79ca..77f40888d31 100644 --- a/app/jobs/scheduled/periodical_updates.rb +++ b/app/jobs/scheduled/periodical_updates.rb @@ -27,8 +27,13 @@ module Jobs # Automatically close stuff that we missed Topic.auto_close - # Forces rebake of old posts where needed - Post.rebake_old(250) + # Forces rebake of old posts where needed, as long as no system avatars need updating + unless UserAvatar + .where("last_gravatar_download_attempt IS NULL OR system_upload_id IS NULL OR system_avatar_version != ?", UserAvatar::SYSTEM_AVATAR_VERSION) + .limit(1) + .first + Post.rebake_old(250) + end end end diff --git a/app/models/user.rb b/app/models/user.rb index 20d57846518..267e4721655 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -619,21 +619,26 @@ class User < ActiveRecord::Base end def refresh_avatar - avatar = user_avatar || UserAvatar.create!(user_id: id) + avatar = user_avatar || create_user_avatar + gravatar_downloaded = false - if SiteSetting.automatically_download_gravatars? - avatar.update_gravatar! unless avatar.last_gravatar_download_attempt + if SiteSetting.automatically_download_gravatars? && !avatar.last_gravatar_download_attempt + avatar.update_gravatar! + gravatar_downloaded = avatar.gravatar_upload_id end if SiteSetting.enable_system_avatars? - avatar.update_system_avatar! if !avatar.system_upload_id || username_changed? + avatar.update_system_avatar! if !avatar.system_upload_id || + username_changed? || + avatar.system_avatar_version != UserAvatar::SYSTEM_AVATAR_VERSION end desired_avatar_id = avatar.gravatar_upload_id || avatar.system_upload_id - if !self.uploaded_avatar_id && desired_avatar_id + if (!self.uploaded_avatar_id || gravatar_downloaded) && desired_avatar_id self.update_column(:uploaded_avatar_id, desired_avatar_id) end + end protected diff --git a/app/models/user_avatar.rb b/app/models/user_avatar.rb index cf3739c7ad5..bfb3a3f26a5 100644 --- a/app/models/user_avatar.rb +++ b/app/models/user_avatar.rb @@ -26,8 +26,7 @@ class UserAvatar < ActiveRecord::Base self.system_avatar_version = SYSTEM_AVATAR_VERSION if old_id == user.uploaded_avatar_id - user.uploaded_avatar_id = system_upload_id - user.save! + user.update_column(:uploaded_avatar_id, system_upload_id) end save! diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 4dd25e8de73..7b0f34692eb 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1169,4 +1169,28 @@ describe User do end end + describe "refresh_avatar" do + it "picks gravatar if system avatar is picked and gravatar was just downloaded" do + + png = Base64.decode64("R0lGODlhAQABALMAAAAAAIAAAACAAICAAAAAgIAAgACAgMDAwICAgP8AAAD/AP//AAAA//8A/wD//wBiZCH5BAEAAA8ALAAAAAABAAEAAAQC8EUAOw==") + FakeWeb.register_uri( :get, + "http://www.gravatar.com/avatar/d10ca8d11301c2f4993ac2279ce4b930.png?s=500&d=404", + body: png ) + + user = User.create!(username: "bob", name: "bob", email: "a@a.com") + user.create_user_avatar + user.user_avatar.update_system_avatar! + user.save + user.reload + + SiteSetting.automatically_download_gravatars = true + SiteSetting.enable_system_avatars = true + + user.refresh_avatar + user.reload + + user.user_avatar.gravatar_upload_id.should == user.uploaded_avatar_id + end + end + end