diff --git a/Changelog.md b/Changelog.md index e374c0195..3d0e9e6d4 100644 --- a/Changelog.md +++ b/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) diff --git a/app/models/profile.rb b/app/models/profile.rb index 8e7e3e4e5..b10f35deb 100644 --- a/app/models/profile.rb +++ b/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 - ActionController::Base.helpers.image_path('user/default.png') - else + if result 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 - return image_url if url == '' - if url.nil? || url.match(/^https?:\/\//) - super(url) - else - super(absolutify_local_url(url)) - end + def image_url=(url) + super(build_image_url(url)) end - def image_url_small= url - return image_url if url == '' - if url.nil? || url.match(/^https?:\/\//) - super(url) - else - super(absolutify_local_url(url)) - end + def image_url_small=(url) + super(build_image_url(url)) end - def image_url_medium= url - return image_url if url == '' - if url.nil? || url.match(/^https?:\/\//) - super(url) - else - super(absolutify_local_url(url)) - end + def image_url_medium=(url) + super(build_image_url(url)) 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 - "#{AppConfig.pod_uri.to_s.chomp("/")}#{url}" + def build_image_url(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 diff --git a/db/migrate/20150531005120_cleanup_default_avatars.rb b/db/migrate/20150531005120_cleanup_default_avatars.rb new file mode 100644 index 000000000..93010fb2a --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index 40ffa940f..08c88be2f 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: 20150404193023) do +ActiveRecord::Schema.define(version: 20150531005120) do create_table "account_deletions", force: :cascade do |t| t.string "diaspora_handle", limit: 255 diff --git a/spec/models/profile_spec.rb b/spec/models/profile_spec.rb index f62b43ce8..d1087a6e8 100644 --- a/spec/models/profile_spec.rb +++ b/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 - before do - @profile = FactoryGirl.build(:profile) - @profile.image_url = "http://tom.joindiaspora.com/images/user/tom.jpg" - @pod_url = AppConfig.pod_uri.to_s.chomp("/") - end + describe "image_url setters" do + %i(image_url image_url_small image_url_medium).each do |method| + describe "##{method}=" do + before do + @profile = FactoryGirl.build(:profile) + @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 - expect {@profile.image_url = ""}.not_to change(@profile, :image_url) - end + it "saves nil when setting nil" do + @profile.public_send("#{method}=", nil) + expect(@profile[method]).to be_nil + end - it 'makes relative urls absolute' do - @profile.image_url = "/relative/url" - expect(@profile.image_url).to eq("#{@pod_url}/relative/url") - end + it "saves nil when setting an empty string" do + @profile.public_send("#{method}=", "") + expect(@profile[method]).to be_nil + end - it "doesn't change absolute urls" do - @profile.image_url = "http://not/a/relative/url" - expect(@profile.image_url).to eq("http://not/a/relative/url") + it "makes relative urls absolute" do + @profile.public_send("#{method}=", "/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",