]> git.openstreetmap.org Git - rails.git/commitdiff
Move user images to Active Storage with paperclip as a fallback
authorTom Hughes <tom@compton.nu>
Tue, 2 Jul 2019 21:31:31 +0000 (22:31 +0100)
committerTom Hughes <tom@compton.nu>
Tue, 9 Jul 2019 18:17:30 +0000 (19:17 +0100)
14 files changed:
Gemfile
Gemfile.lock
app/assets/images/avatar_large.png [moved from app/assets/images/users/images/large.png with 100% similarity]
app/assets/images/avatar_small.png [moved from app/assets/images/users/images/small.png with 100% similarity]
app/assets/images/avatars.svg [moved from app/assets/images/users/images/user-icons.svg with 100% similarity]
app/assets/javascripts/user.js
app/controllers/users_controller.rb
app/helpers/user_helper.rb
app/mailers/notifier.rb
app/models/user.rb
app/views/api/users/_user.builder
app/views/users/account.html.erb
test/controllers/users_controller_test.rb
test/helpers/user_helper_test.rb

diff --git a/Gemfile b/Gemfile
index f41ac015bf4adb14c28f3502724c7f63dcf782a1..06530b7315340fbba3a1df76b6b9babbd7b9a2d4 100644 (file)
--- a/Gemfile
+++ b/Gemfile
@@ -126,6 +126,9 @@ gem "mimemagic"
 # Used for browser detection
 gem "browser"
 
+# Used to resize user images
+gem "mini_magick"
+
 # Gems useful for development
 group :development do
   gem "annotate"
index 65c6f2a89ae57088dbe256c1af638459df5228a7..a29ad998992af8900ff6693fba3285afc387055d 100644 (file)
@@ -241,6 +241,7 @@ GEM
       mime-types-data (~> 3.2015)
     mime-types-data (3.2019.0331)
     mimemagic (0.3.3)
+    mini_magick (4.9.3)
     mini_mime (1.0.1)
     mini_portile2 (2.4.0)
     minitest (5.11.3)
@@ -482,6 +483,7 @@ DEPENDENCIES
   listen
   logstasher
   mimemagic
+  mini_magick
   minitest (~> 5.1)
   oauth-plugin (>= 0.5.1)
   omniauth
index 69cc259f0c9e0195ecf5157ebb8d20d3e7703cac..7cbdb1e110b24c843dc8504c505a1363b15e0acb 100644 (file)
@@ -85,8 +85,8 @@ $(document).ready(function () {
 
   $("select#user_auth_provider").on("change", updateAuthUID);
 
-  $("input#user_image").on("change", function () {
-    $("#image_action_new").prop("checked", true);
+  $("input#user_avatar").on("change", function () {
+    $("#avatar_action_new").prop("checked", true);
   });
 
   function enableAuth() {
index 17031848d7c4503f822400dcbb4a3bf8b5587596..8c234006e4477172d457eeb583928b8872256e79 100644 (file)
@@ -660,15 +660,15 @@ class UsersController < ApplicationController
 
     user.languages = params[:user][:languages].split(",")
 
-    case params[:image_action]
+    case params[:avatar_action]
     when "new" then
-      user.image = params[:user][:image]
+      user.avatar.attach(params[:user][:avatar])
       user.image_use_gravatar = false
     when "delete" then
-      user.image = nil
+      user.avatar.purge
       user.image_use_gravatar = false
     when "gravatar" then
-      user.image = nil
+      user.avatar.purge
       user.image_use_gravatar = true
     end
 
index 1fc8e3ed2f1e0b6f82f6e325d0425c278a3d1e87..c22fef1ea2fa71e8d2f0a922a89e13316d5c5564 100644 (file)
@@ -7,8 +7,12 @@ module UserHelper
 
     if user.image_use_gravatar
       user_gravatar_tag(user, options)
-    else
+    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
   end
 
@@ -18,8 +22,12 @@ module UserHelper
 
     if user.image_use_gravatar
       user_gravatar_tag(user, options)
-    else
+    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
   end
 
@@ -29,16 +37,24 @@ module UserHelper
 
     if user.image_use_gravatar
       user_gravatar_tag(user, options)
-    else
+    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
   end
 
   def user_image_url(user, options = {})
     if user.image_use_gravatar
       user_gravatar_url(user, options)
-    else
+    elsif user.avatar.attached?
+      url_for(user.avatar.variant(:resize => "100x100>"))
+    elsif user.image.file?
       image_url(user.image.url(:large))
+    else
+      image_url("avatar_large.png")
     end
   end
 
@@ -65,7 +81,7 @@ module UserHelper
   def user_gravatar_url(user, options = {})
     size = options[:size] || 100
     hash = Digest::MD5.hexdigest(user.email.downcase)
-    default_image_url = image_url("users/images/large.png")
+    default_image_url = image_url("avatar_large.png")
     "#{request.protocol}www.gravatar.com/avatar/#{hash}.jpg?s=#{size}&d=#{u(default_image_url)}"
   end
 
index e705efb006756f09a6f5ff21ea8b7665689725cc..bd2c83b56e245f34f5058003f1b382fe2f2a4517 100644 (file)
@@ -1,4 +1,6 @@
 class Notifier < ActionMailer::Base
+  include ActionView::Helpers::AssetUrlHelper
+
   default :from => Settings.email_from,
           :return_path => Settings.email_return_path,
           :auto_submitted => "auto-generated"
@@ -177,7 +179,16 @@ class Notifier < ActionMailer::Base
   end
 
   def attach_user_avatar(user)
-    attachments.inline["avatar.png"] = File.read(user_avatar_file_path(user))
+    attachments.inline["avatar.png"] = user_avatar_file(user)
+  end
+
+  def user_avatar_file(user)
+    avatar = user&.avatar
+    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)
@@ -185,7 +196,7 @@ class Notifier < ActionMailer::Base
     if image&.file?
       return image.path(:small)
     else
-      return Rails.root.join("app", "assets", "images", "users", "images", "small.png")
+      return Rails.root.join("app", "assets", "images", "avatar_small.png")
     end
   end
 
index 1d008959905e85a50428c0433c04dbe16799d8a8..13248e4fb4c4136273b3b2028fe272bbdfea6a12 100644 (file)
@@ -85,6 +85,8 @@ class User < ActiveRecord::Base
   scope :active, -> { where(:status => %w[active confirmed]) }
   scope :identifiable, -> { where(:data_public => true) }
 
+  has_one_attached :avatar
+
   has_attached_file :image,
                     :default_url => "/assets/:class/:attachment/:style.png",
                     :styles => { :large => "100x100>", :small => "50x50>" }
@@ -267,6 +269,8 @@ class User < ActiveRecord::Base
   ##
   # delete a user - leave the account but purge most personal data
   def delete
+    avatar.purge
+
     self.display_name = "user_#{id}"
     self.description = ""
     self.home_lat = nil
@@ -277,6 +281,7 @@ class User < ActiveRecord::Base
     self.auth_provider = nil
     self.auth_uid = nil
     self.status = "deleted"
+
     save
   end
 
index 7bf8e18a5a3019537bc1f2ab24940be03aa7db41..638e8583fb5b0de6ff454847dda8270b83a92a50 100644 (file)
@@ -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.image.file? || user.image_use_gravatar
+  xml.tag! "img", :href => user_image_url(user) if user.avatar.attached? || user.image.file? || user.image_use_gravatar
   xml.tag! "roles" do
     user.roles.each do |role|
       xml.tag! role.role
index b0839ad563573efb82b541a885c8f52ce4419196..7e8d533b066eb45046a92d5ea3e00357652e7c12 100644 (file)
       <label class="standard-label"><%= t ".image" %></label>
         <%= user_image current_user %>
         <ul class='form-list accountImage-options'>
-        <% if current_user.image.file? %>
+        <% if current_user.avatar.attached? || current_user.image.file? %>
         <li>
-          <%= radio_button_tag "image_action", "keep", !current_user.image_use_gravatar %>
-          <label class='standard-label' for='image_action_keep'><%= t ".keep image" %></label>
+          <%= 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.image.file? || current_user.image_use_gravatar? %>
+        <% if current_user.avatar.attached? || current_user.image.file? || current_user.image_use_gravatar? %>
         <li>
-          <%= radio_button_tag "image_action", "delete" %>
-          <label class='standard-label' for='image_action_delete'><%= t ".delete image" %></label>
+          <%= radio_button_tag "avatar_action", "delete" %>
+          <label class='standard-label' for='avatar_action_delete'><%= t ".delete image" %></label>
         </li>
         <% end %>
-        <% if current_user.image.file? %>
+        <% if current_user.avatar.attached? || current_user.image.file? %>
           <li>
-            <%= radio_button_tag "image_action", "new" %>
-            <label class='standard-label' for='image_action_new'>
+            <%= radio_button_tag "avatar_action", "new" %>
+            <label class='standard-label' for='avatar_action_new'>
                 <%= t ".replace image" %>
                 <span class="form-help deemphasize"><%= t ".image size hint" %></span>
             </label>
-            <%= f.file_field :image %>
+            <%= f.file_field :avatar %>
           </li>
         <% else %>
         <li>
-          <%= radio_button_tag "image_action", "new" %>
-          <label class='standard-label' for='image_action_new'>
+          <%= radio_button_tag "avatar_action", "new" %>
+          <label class='standard-label' for='avatar_action_new'>
             <%= t ".new image" %>
             <span class="form-help deemphasize"><%= t ".image size hint" %></span>
           </label>
-          <%= f.file_field :image %>
+          <%= f.file_field :avatar %>
         </li>
         <% end %>
         <li>
-          <%= radio_button_tag "image_action", "gravatar", current_user.image_use_gravatar %>
-          <label class='standard-label' for='image_action_gravatar'>
+          <%= radio_button_tag "avatar_action", "gravatar", current_user.image_use_gravatar %>
+          <label class='standard-label' for='avatar_action_gravatar'>
             <%= t ".gravatar.gravatar" %>
             <span class='form-help deemphasize'> (<a href="<%= t ".gravatar.link" %>" target="_new"><%= t ".gravatar.link text" %></a>)</span>
           </label>
index 984ea8d46cce6bfc3fa49a3c1ec752fe2f87c337..8b438e903283e849d23f7971c62fdecedae10e22 100644 (file)
@@ -876,28 +876,28 @@ class UsersControllerTest < ActionController::TestCase
 
     # Changing to an uploaded image should work
     image = Rack::Test::UploadedFile.new("test/gpx/fixtures/a.gif", "image/gif")
-    post :account, :params => { :display_name => user.display_name, :image_action => "new", :user => user.attributes.merge(:image => image) }, :session => { :user => user }
+    post :account, :params => { :display_name => user.display_name, :avatar_action => "new", :user => user.attributes.merge(:avatar => image) }, :session => { :user => user }
     assert_response :success
     assert_template :account
     assert_select "div#errorExplanation", false
     assert_select ".notice", /^User information updated successfully/
-    assert_select "form#accountForm > fieldset > div.form-row.accountImage input[name=image_action][checked][value=?]", "keep"
+    assert_select "form#accountForm > fieldset > div.form-row.accountImage input[name=avatar_action][checked][value=?]", "keep"
 
     # Changing to a gravatar image should work
-    post :account, :params => { :display_name => user.display_name, :image_action => "gravatar", :user => user.attributes }, :session => { :user => user }
+    post :account, :params => { :display_name => user.display_name, :avatar_action => "gravatar", :user => user.attributes }, :session => { :user => user }
     assert_response :success
     assert_template :account
     assert_select "div#errorExplanation", false
     assert_select ".notice", /^User information updated successfully/
-    assert_select "form#accountForm > fieldset > div.form-row.accountImage input[name=image_action][checked][value=?]", "gravatar"
+    assert_select "form#accountForm > fieldset > div.form-row.accountImage input[name=avatar_action][checked][value=?]", "gravatar"
 
     # Removing the image should work
-    post :account, :params => { :display_name => user.display_name, :image_action => "delete", :user => user.attributes }, :session => { :user => user }
+    post :account, :params => { :display_name => user.display_name, :avatar_action => "delete", :user => user.attributes }, :session => { :user => user }
     assert_response :success
     assert_template :account
     assert_select "div#errorExplanation", false
     assert_select ".notice", /^User information updated successfully/
-    assert_select "form#accountForm > fieldset > div.form-row.accountImage input[name=image_action][checked]", false
+    assert_select "form#accountForm > fieldset > div.form-row.accountImage input[name=avatar_action][checked]", false
 
     # Adding external authentication should redirect to the auth provider
     post :account, :params => { :display_name => user.display_name, :user => user.attributes.merge(:auth_provider => "openid", :auth_uid => "gmail.com") }, :session => { :user => user }
index c57299a96d4f3e4db339d710cfc18e078380dabf..0fcbc3d90cfb7060d4f7ef77ec25f00890712af7 100644 (file)
@@ -8,10 +8,10 @@ class UserHelperTest < ActionView::TestCase
     gravatar_user = create(:user, :image_use_gravatar => true)
 
     image = user_image(user)
-    assert_match %r{^<img class="user_image" .* src="/assets/users/images/large-.*" />$}, image
+    assert_match %r{^<img class="user_image" .* src="/images/avatar_large.png" />$}, image
 
     image = user_image(user, :class => "foo")
-    assert_match %r{^<img class="foo" .* src="/assets/users/images/large-.*" />$}, image
+    assert_match %r{^<img class="foo" .* src="/images/avatar_large.png" />$}, image
 
     image = user_image(gravatar_user)
     assert_match %r{^<img class="user_image" .* src="http://www.gravatar.com/avatar/.*" />$}, image
@@ -25,10 +25,10 @@ class UserHelperTest < ActionView::TestCase
     gravatar_user = create(:user, :image_use_gravatar => true)
 
     image = user_thumbnail(user)
-    assert_match %r{^<img class="user_thumbnail" .* src="/assets/users/images/small-.*" />$}, image
+    assert_match %r{^<img class="user_thumbnail" .* src="/images/avatar_small.png" />$}, image
 
     image = user_thumbnail(user, :class => "foo")
-    assert_match %r{^<img class="foo" .* src="/assets/users/images/small-.*" />$}, image
+    assert_match %r{^<img class="foo" .* src="/images/avatar_small.png" />$}, image
 
     image = user_thumbnail(gravatar_user)
     assert_match %r{^<img class="user_thumbnail" .* src="http://www.gravatar.com/avatar/.*" />$}, image
@@ -42,10 +42,10 @@ class UserHelperTest < ActionView::TestCase
     gravatar_user = create(:user, :image_use_gravatar => true)
 
     image = user_thumbnail_tiny(user)
-    assert_match %r{^<img class="user_thumbnail_tiny" .* src="/assets/users/images/small-.*" />$}, image
+    assert_match %r{^<img class="user_thumbnail_tiny" .* src="/images/avatar_small.png" />$}, image
 
     image = user_thumbnail_tiny(user, :class => "foo")
-    assert_match %r{^<img class="foo" .* src="/assets/users/images/small-.*" />$}, image
+    assert_match %r{^<img class="foo" .* src="/images/avatar_small.png" />$}, image
 
     image = user_thumbnail_tiny(gravatar_user)
     assert_match %r{^<img class="user_thumbnail_tiny" .* src="http://www.gravatar.com/avatar/.*" />$}, image