Merge branch 'stable' into develop

Changelog.md

@@ -81,6 +81,7 @@ Ruby 2.0 is no longer officially supported.
 * Fix tag rendering in emails [#6009](https://github.com/diaspora/diaspora/pull/6009)
 * Fix the logo in emails [#6013](https://github.com/diaspora/diaspora/pull/6013)
 * Disable autocorrect for username on mobile sign in [#6028](https://github.com/diaspora/diaspora/pull/6028)
+* Fix broken default avatars in the database [#6014](https://github.com/diaspora/diaspora/pull/6014)
 
 ## Features
 * Hide post title of limited post in comment notification email [#5843](https://github.com/diaspora/diaspora/pull/5843)

app/models/profile.rb

@@ -68,7 +68,7 @@ class Profile < ActiveRecord::Base
     (self.person) ? self.person.diaspora_handle : self[:diaspora_handle]
   end
 
-  def image_url(size = :thumb_large)
+  def image_url(size=:thumb_large)
     result = if size == :thumb_medium && self[:image_url_medium]
                self[:image_url_medium]
              elsif size == :thumb_small && self[:image_url_small]
@@ -77,14 +77,14 @@ class Profile < ActiveRecord::Base
                self[:image_url]
              end
 
-    unless result
+    if result
-      ActionController::Base.helpers.image_path('user/default.png')
-    else
       if AppConfig.privacy.camo.proxy_remote_pod_images?
         Diaspora::Camo.image_url(result)
       else
         result
       end
+    else
+      ActionController::Base.helpers.image_path("user/default.png")
     end
   end
 
@@ -100,31 +100,16 @@ class Profile < ActiveRecord::Base
     self.attributes.merge(update_hash){|key, old, new| old.blank? ? new : old}
   end
 
-  def image_url= url
+  def image_url=(url)
-    return image_url if url == ''
+    super(build_image_url(url))
-    if url.nil? || url.match(/^https?:\/\//)
-      super(url)
-    else
-      super(absolutify_local_url(url))
-    end
   end
 
-  def image_url_small= url
+  def image_url_small=(url)
-    return image_url if url == ''
+    super(build_image_url(url))
-    if url.nil? || url.match(/^https?:\/\//)
-      super(url)
-    else
-      super(absolutify_local_url(url))
-    end
   end
 
-  def image_url_medium= url
+  def image_url_medium=(url)
-    return image_url if url == ''
+    super(build_image_url(url))
-    if url.nil? || url.match(/^https?:\/\//)
-      super(url)
-    else
-      super(absolutify_local_url(url))
-    end
   end
 
   def date= params
@@ -201,7 +186,9 @@ class Profile < ActiveRecord::Base
     self.attributes.keys - ["id", "created_at", "updated_at", "person_id"]
   end
 
-  def absolutify_local_url url
+  def build_image_url(url)
-    "#{AppConfig.pod_uri.to_s.chomp("/")}#{url}"
+    return nil if url.blank? || url.match(/user\/default/)
+    return url if url.match(/^https?:\/\//)
+    "#{AppConfig.pod_uri.to_s.chomp('/')}#{url}"
   end
 end

db/migrate/20150531005120_cleanup_default_avatars.rb

@@ -0,0 +1,10 @@
+class CleanupDefaultAvatars < ActiveRecord::Migration
+  def up
+    Profile.where("image_url LIKE ?", "%user/default%")
+      .update_all(image_url: nil, image_url_small: nil, image_url_medium: nil)
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

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: 20150404193023) do
+ActiveRecord::Schema.define(version: 20150531005120) do
 
   create_table "account_deletions", force: :cascade do |t|
     t.string   "diaspora_handle", limit: 255

spec/models/profile_spec.rb

@@ -46,7 +46,7 @@ describe Profile, :type => :model do
 
       it 'sets full name to first name' do
         @from_omniauth = {'name' => 'bob jones', 'description' => 'this is my bio', 'location' => 'sf', 'image' => 'http://cats.com/gif.gif'}
-        
+
         profile = Profile.new
         expect(profile.from_omniauth_hash(@from_omniauth)['first_name']).to eq('bob jones')
       end
@@ -117,32 +117,47 @@ describe Profile, :type => :model do
       profile = FactoryGirl.build(:profile, :location => "a"*255)
       expect(profile).to be_valid
     end
-   
+
     it "cannot be 256 characters" do
       profile = FactoryGirl.build(:profile, :location => "a"*256)
       expect(profile).not_to be_valid
     end
   end
 
-  describe '#image_url=' do
+  describe "image_url setters" do
-    before do
+    %i(image_url image_url_small image_url_medium).each do |method|
-      @profile = FactoryGirl.build(:profile)
+      describe "##{method}=" do
-      @profile.image_url = "http://tom.joindiaspora.com/images/user/tom.jpg"
+        before do
-      @pod_url = AppConfig.pod_uri.to_s.chomp("/")
+          @profile = FactoryGirl.build(:profile)
-    end
+          @profile.public_send("#{method}=", "http://tom.joindiaspora.com/images/user/tom.jpg")
+          @pod_url = AppConfig.pod_uri.to_s.chomp("/")
+        end
 
-    it 'ignores an empty string' do
+        it "saves nil when setting nil" do
-      expect {@profile.image_url = ""}.not_to change(@profile, :image_url)
+          @profile.public_send("#{method}=", nil)
-    end
+          expect(@profile[method]).to be_nil
+        end
 
-    it 'makes relative urls absolute' do
+        it "saves nil when setting an empty string" do
-      @profile.image_url = "/relative/url"
+          @profile.public_send("#{method}=", "")
-      expect(@profile.image_url).to eq("#{@pod_url}/relative/url")
+          expect(@profile[method]).to be_nil
-    end
+        end
 
-    it "doesn't change absolute urls" do
+        it "makes relative urls absolute" do
-      @profile.image_url = "http://not/a/relative/url"
+          @profile.public_send("#{method}=", "/relative/url")
-      expect(@profile.image_url).to eq("http://not/a/relative/url")
+          expect(@profile.public_send(method)).to eq("#{@pod_url}/relative/url")
+        end
+
+        it "doesn't change absolute urls" do
+          @profile.public_send("#{method}=", "http://not/a/relative/url")
+          expect(@profile.public_send(method)).to eq("http://not/a/relative/url")
+        end
+
+        it "saves the default-url as nil" do
+          @profile.public_send("#{method}=", "/assets/user/default.png")
+          expect(@profile[method]).to be_nil
+        end
+      end
     end
   end
 
@@ -157,7 +172,7 @@ describe Profile, :type => :model do
       expect(new_profile.tag_string).to include('#rafi')
     end
   end
-  
+
   describe 'serialization' do
     let(:person) {FactoryGirl.build(:person,:diaspora_handle => "foobar" )}
 
@@ -173,7 +188,7 @@ describe Profile, :type => :model do
       xml = person.profile.to_diaspora_xml
       expect(xml).to include "#one"
     end
-    
+
     it 'includes location' do
       person.profile.location = 'Dark Side, Moon'
       person.profile.save
@@ -339,7 +354,7 @@ describe Profile, :type => :model do
   describe "#clearable_fields" do
     it 'returns the current profile fields' do
       profile = FactoryGirl.build :profile
-      expect(profile.send(:clearable_fields).sort).to eq( 
+      expect(profile.send(:clearable_fields).sort).to eq(
       ["diaspora_handle",
       "first_name",
       "last_name",