From: Tom Hughes <tom@compton.nu>
Date: Tue, 16 Jul 2019 17:45:41 +0000 (+0100)
Subject: Remove support for old style user images
X-Git-Tag: live~2462
X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/91a37235ac748df7dc241b215790d6cbecd652f3?ds=sidebyside

Remove support for old style user images
---

diff --git a/app/helpers/user_helper.rb b/app/helpers/user_helper.rb
index 86990f9fb..a35da210b 100644
--- a/app/helpers/user_helper.rb
+++ b/app/helpers/user_helper.rb
@@ -9,8 +9,6 @@ module UserHelper
       user_gravatar_tag(user, options)
     elsif user.avatar.attached?
       image_tag user.avatar.variant(:resize => "100x100>"), options
-    elsif user.image.file?
-      image_tag user.image.url(:large), options
     else
       image_tag "avatar_large.png", options
     end
@@ -24,8 +22,6 @@ module UserHelper
       user_gravatar_tag(user, options)
     elsif user.avatar.attached?
       image_tag user.avatar.variant(:resize => "50x50>"), options
-    elsif user.image.file?
-      image_tag user.image.url(:small), options
     else
       image_tag "avatar_small.png", options
     end
@@ -39,8 +35,6 @@ module UserHelper
       user_gravatar_tag(user, options)
     elsif user.avatar.attached?
       image_tag user.avatar.variant(:resize => "50x50>"), options
-    elsif user.image.file?
-      image_tag user.image.url(:small), options
     else
       image_tag "avatar_small.png", options
     end
@@ -51,8 +45,6 @@ module UserHelper
       user_gravatar_url(user, options)
     elsif user.avatar.attached?
       polymorphic_url(user.avatar.variant(:resize => "100x100>"))
-    elsif user.image.file?
-      image_url(user.image.url(:large))
     else
       image_url("avatar_large.png")
     end
diff --git a/app/mailers/notifier.rb b/app/mailers/notifier.rb
index bd2c83b56..4509f1011 100644
--- a/app/mailers/notifier.rb
+++ b/app/mailers/notifier.rb
@@ -187,16 +187,7 @@ class Notifier < ActionMailer::Base
     if avatar&.attached?
       return avatar.variant(:resize => "50x50>").blob.download
     else
-      return File.read(user_avatar_file_path(user))
-    end
-  end
-
-  def user_avatar_file_path(user)
-    image = user&.image
-    if image&.file?
-      return image.path(:small)
-    else
-      return Rails.root.join("app", "assets", "images", "avatar_small.png")
+      return File.read(Rails.root.join("app", "assets", "images", "avatar_small.png"))
     end
   end
 
diff --git a/app/models/user.rb b/app/models/user.rb
index 8e3ac17ca..ae68c3998 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -49,6 +49,8 @@
 class User < ActiveRecord::Base
   require "xml/libxml"
 
+  self.ignored_columns = %w[image_file_name image_fingerprint image_content_type]
+
   has_many :traces, -> { where(:visible => true) }
   has_many :diary_entries, -> { order(:created_at => :desc) }
   has_many :diary_comments, -> { order(:created_at => :desc) }
@@ -87,10 +89,6 @@ class User < ActiveRecord::Base
 
   has_one_attached :avatar
 
-  has_attached_file :image,
-                    :default_url => "/assets/:class/:attachment/:style.png",
-                    :styles => { :large => "100x100>", :small => "50x50>" }
-
   validates :display_name, :presence => true, :length => 3..255,
                            :exclusion => %w[new terms save confirm confirm-email go_public reset-password forgot-password suspended]
   validates :display_name, :if => proc { |u| u.display_name_changed? },
@@ -108,7 +106,6 @@ class User < ActiveRecord::Base
   validates :home_lon, :allow_nil => true, :numericality => true, :inclusion => { :in => -180..180 }
   validates :home_zoom, :allow_nil => true, :numericality => { :only_integer => true }
   validates :preferred_editor, :inclusion => Editors::ALL_EDITORS, :allow_nil => true
-  validates :image, :attachment_content_type => { :content_type => %r{\Aimage/.*\Z} }
   validates :auth_uid, :unless => proc { |u| u.auth_provider.nil? },
                        :uniqueness => { :scope => :auth_provider }
 
@@ -275,7 +272,6 @@ class User < ActiveRecord::Base
     self.description = ""
     self.home_lat = nil
     self.home_lon = nil
-    self.image = nil
     self.email_valid = false
     self.new_email = nil
     self.auth_provider = nil
diff --git a/app/views/api/users/_user.builder b/app/views/api/users/_user.builder
index 638e8583f..d8c6c1c6e 100644
--- a/app/views/api/users/_user.builder
+++ b/app/views/api/users/_user.builder
@@ -8,7 +8,7 @@ xml.tag! "user", :id => user.id,
   else
     xml.tag! "contributor-terms", :agreed => user.terms_agreed.present?
   end
-  xml.tag! "img", :href => user_image_url(user) if user.avatar.attached? || user.image.file? || user.image_use_gravatar
+  xml.tag! "img", :href => user_image_url(user) if user.avatar.attached? || user.image_use_gravatar
   xml.tag! "roles" do
     user.roles.each do |role|
       xml.tag! role.role
diff --git a/app/views/users/account.html.erb b/app/views/users/account.html.erb
index 7e8d533b0..7fd6929ca 100644
--- a/app/views/users/account.html.erb
+++ b/app/views/users/account.html.erb
@@ -104,19 +104,19 @@
       <label class="standard-label"><%= t ".image" %></label>
         <%= user_image current_user %>
         <ul class='form-list accountImage-options'>
-        <% if current_user.avatar.attached? || current_user.image.file? %>
+        <% if current_user.avatar.attached? %>
         <li>
           <%= radio_button_tag "avatar_action", "keep", !current_user.image_use_gravatar %>
           <label class='standard-label' for='avatar_action_keep'><%= t ".keep image" %></label>
         </li>
         <% end %>
-        <% if current_user.avatar.attached? || current_user.image.file? || current_user.image_use_gravatar? %>
+        <% if current_user.avatar.attached? || current_user.image_use_gravatar? %>
         <li>
           <%= radio_button_tag "avatar_action", "delete" %>
           <label class='standard-label' for='avatar_action_delete'><%= t ".delete image" %></label>
         </li>
         <% end %>
-        <% if current_user.avatar.attached? || current_user.image.file? %>
+        <% if current_user.avatar.attached? %>
           <li>
             <%= radio_button_tag "avatar_action", "new" %>
             <label class='standard-label' for='avatar_action_new'>
diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb
index 8b438e903..62e06fc58 100644
--- a/test/controllers/users_controller_test.rb
+++ b/test/controllers/users_controller_test.rb
@@ -1317,7 +1317,7 @@ class UsersControllerTest < ActionController::TestCase
     assert_equal "", user.description
     assert_nil user.home_lat
     assert_nil user.home_lon
-    assert_equal false, user.image.file?
+    assert_equal false, user.avatar.attached?
     assert_equal false, user.email_valid
     assert_nil user.new_email
     assert_nil user.auth_provider
diff --git a/test/helpers/user_helper_test.rb b/test/helpers/user_helper_test.rb
index 0fcbc3d90..397a90a63 100644
--- a/test/helpers/user_helper_test.rb
+++ b/test/helpers/user_helper_test.rb
@@ -55,11 +55,12 @@ class UserHelperTest < ActionView::TestCase
   end
 
   def test_user_image_url
-    user = create(:user, :image_file_name => "test.jpg", :image_fingerprint => "d41d8cd98f00b204e9800998ecf8427e")
+    user = create(:user)
+    user.avatar.attach(:io => File.open("test/gpx/fixtures/a.gif"), :filename => "a.gif")
     gravatar_user = create(:user, :image_use_gravatar => true)
 
     url = user_image_url(user)
-    assert_match %r{^/attachments/users/images/\d{3}/\d{3}/\d{3}/large/d41d8cd98f00b204e9800998ecf8427e.jpg$}, url
+    assert_match %r{^http://test.host/rails/active_storage/representations/[^/]+/[^/]+/a.gif$}, url
 
     url = user_image_url(gravatar_user)
     assert_match %r{^http://www.gravatar.com/avatar/}, url
diff --git a/test/models/user_test.rb b/test/models/user_test.rb
index 96c6632ff..2368960e1 100644
--- a/test/models/user_test.rb
+++ b/test/models/user_test.rb
@@ -263,7 +263,7 @@ class UserTest < ActiveSupport::TestCase
     assert user.description.blank?
     assert_nil user.home_lat
     assert_nil user.home_lon
-    assert_equal false, user.image.file?
+    assert_equal false, user.avatar.attached?
     assert_equal "deleted", user.status
     assert_equal false, user.visible?
     assert_equal false, user.active?