From 02e4fb2e06f424c16ab25ea294a4af6490a5f7e3 Mon Sep 17 00:00:00 2001
From: Eugen Rochko <eugen@zeonfederated.com>
Date: Tue, 22 Mar 2016 21:05:23 +0100
Subject: [PATCH] Only re-download avatar if URL changed (fix #19)

---
 app/models/account.rb                         |  8 +--
 ...93748_add_avatar_remote_url_to_accounts.rb |  5 ++
 db/schema.rb                                  |  3 +-
 .../update_remote_profile_service_spec.rb     | 54 +++++++++++++++++++
 4 files changed, 66 insertions(+), 4 deletions(-)
 create mode 100644 db/migrate/20160322193748_add_avatar_remote_url_to_accounts.rb

diff --git a/app/models/account.rb b/app/models/account.rb
index 06f858c7c22..c686a47ed26 100644
--- a/app/models/account.rb
+++ b/app/models/account.rb
@@ -5,7 +5,6 @@ class Account < ActiveRecord::Base
   validates :username, uniqueness: { scope: :domain, case_sensitive: true },  unless: 'local?'
 
   # Avatar upload
-  attr_reader :avatar_remote_url
   has_attached_file :avatar, styles: { large: '300x300#', medium: '96x96#', small: '48x48#' }, default_url: 'avatars/missing.png'
   validates_attachment_content_type :avatar, content_type: /\Aimage\/.*\Z/
 
@@ -91,8 +90,11 @@ class Account < ActiveRecord::Base
   end
 
   def avatar_remote_url=(url)
-    self.avatar = URI.parse(url)
-    @avatar_remote_url = url
+    unless self[:avatar_remote_url] == url
+      self.avatar = URI.parse(url)
+    end
+
+    self[:avatar_remote_url] = url
   end
 
   def to_param
diff --git a/db/migrate/20160322193748_add_avatar_remote_url_to_accounts.rb b/db/migrate/20160322193748_add_avatar_remote_url_to_accounts.rb
new file mode 100644
index 00000000000..9dc4b027f02
--- /dev/null
+++ b/db/migrate/20160322193748_add_avatar_remote_url_to_accounts.rb
@@ -0,0 +1,5 @@
+class AddAvatarRemoteUrlToAccounts < ActiveRecord::Migration
+  def change
+    add_column :accounts, :avatar_remote_url, :string, null: true, default: nil
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 250b69f6d21..d6702b36e90 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20160316103650) do
+ActiveRecord::Schema.define(version: 20160322193748) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -40,6 +40,7 @@ ActiveRecord::Schema.define(version: 20160316103650) do
     t.string   "header_content_type"
     t.integer  "header_file_size"
     t.datetime "header_updated_at"
+    t.string   "avatar_remote_url"
   end
 
   add_index "accounts", ["username", "domain"], name: "index_accounts_on_username_and_domain", unique: true, using: :btree
diff --git a/spec/services/update_remote_profile_service_spec.rb b/spec/services/update_remote_profile_service_spec.rb
index f09f60adaf6..1ffcfbfac12 100644
--- a/spec/services/update_remote_profile_service_spec.rb
+++ b/spec/services/update_remote_profile_service_spec.rb
@@ -1,5 +1,59 @@
 require 'rails_helper'
 
 RSpec.describe UpdateRemoteProfileService do
+  let(:xml) { Nokogiri::XML(File.read(File.join(Rails.root, 'spec', 'fixtures', 'push', 'feed.atom'))).at_xpath('//xmlns:author') }
+
   subject { UpdateRemoteProfileService.new }
+
+  before do
+    stub_request(:get, 'https://quitter.no/avatar/7477-300-20160211190340.png').to_return(request_fixture('avatar.txt'))
+  end
+
+  context 'with updated details' do
+    let(:remote_account) { Fabricate(:account, username: 'bob', domain: 'example.com') }
+
+    before do
+      subject.(xml, remote_account)
+    end
+
+    it 'downloads new avatar' do
+      expect(a_request(:get, 'https://quitter.no/avatar/7477-300-20160211190340.png')).to have_been_made
+    end
+
+    it 'sets the avatar remote url' do
+      expect(remote_account.reload.avatar_remote_url).to eq 'https://quitter.no/avatar/7477-300-20160211190340.png'
+    end
+
+    it 'sets display name' do
+      expect(remote_account.reload.display_name).to eq 'ＤＩＧＩＴＡＬ ＣＡＴ'
+    end
+
+    it 'sets note' do
+      expect(remote_account.reload.note).to eq 'Software engineer, free time musician and ＤＩＧＩＴＡＬ ＳＰＯＲＴＳ enthusiast. Likes cats. Warning: May contain memes'
+    end
+  end
+
+  context 'with unchanged details' do
+    let(:remote_account) { Fabricate(:account, username: 'bob', domain: 'example.com',display_name: 'ＤＩＧＩＴＡＬ ＣＡＴ', note: 'Software engineer, free time musician and ＤＩＧＩＴＡＬ ＳＰＯＲＴＳ enthusiast. Likes cats. Warning: May contain memes', avatar_remote_url: 'https://quitter.no/avatar/7477-300-20160211190340.png') }
+
+    before do
+      subject.(xml, remote_account)
+    end
+
+    it 'does not re-download avatar' do
+      expect(a_request(:get, 'https://quitter.no/avatar/7477-300-20160211190340.png')).to have_been_made.once
+    end
+
+    it 'sets the avatar remote url' do
+      expect(remote_account.reload.avatar_remote_url).to eq 'https://quitter.no/avatar/7477-300-20160211190340.png'
+    end
+
+    it 'sets display name' do
+      expect(remote_account.reload.display_name).to eq 'ＤＩＧＩＴＡＬ ＣＡＴ'
+    end
+
+    it 'sets note' do
+      expect(remote_account.reload.note).to eq 'Software engineer, free time musician and ＤＩＧＩＴＡＬ ＳＰＯＲＴＳ enthusiast. Likes cats. Warning: May contain memes'
+    end
+  end
 end