From bcb43305b6f8baa7aaaa454aa9d5ca28fecff057 Mon Sep 17 00:00:00 2001
From: Mike Dalessio <mike@37signals.com>
Date: Fri, 21 Nov 2025 13:47:34 -0500
Subject: [PATCH] Improve avatar image handling

- redirect avatar image requests to the rails_blob_url, instead of
  streaming them through the web app
- use a thumbnail variant for avatar images
- only put avatar initials behind the stale? check (not the image
  redirect, which would result in browsers rendering broken images when
  an avatar is changed, until max-age expires)
---
 app/controllers/users/avatars_controller.rb   | 14 ++++--------
 app/models/user.rb                            |  4 +++-
 .../users/avatars_controller_test.rb          | 22 +++++++++++++++++--
 3 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/app/controllers/users/avatars_controller.rb b/app/controllers/users/avatars_controller.rb
index 76b2825ed..9101ee401 100644
--- a/app/controllers/users/avatars_controller.rb
+++ b/app/controllers/users/avatars_controller.rb
@@ -7,8 +7,10 @@ class Users::AvatarsController < ApplicationController
   before_action :ensure_permission_to_administer_user, only: :destroy
 
   def show
-    if stale? @user, cache_control: cache_control
-      render_avatar_or_initials
+    if @user.avatar.attached?
+      redirect_to rails_blob_url(@user.avatar.variant(:thumb), disposition: "inline")
+    elsif stale? @user, cache_control: cache_control
+      render_initials
     end
   end
 
@@ -34,14 +36,6 @@ class Users::AvatarsController < ApplicationController
       end
     end
 
-    def render_avatar_or_initials
-      if @user.avatar.attached?
-        send_blob_stream @user.avatar
-      else
-        render_initials
-      end
-    end
-
     def render_initials
       render formats: :svg
     end
diff --git a/app/models/user.rb b/app/models/user.rb
index 4458712a5..94380b8d9 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -3,7 +3,9 @@ class User < ApplicationRecord
     Mentionable, Named, Notifiable, Role, Searcher, Watcher
   include Timelined # Depends on Accessor
 
-  has_one_attached :avatar
+  has_one_attached :avatar do |attachable|
+    attachable.variant :thumb, resize_to_limit: [ 256, 256 ]
+  end
 
   belongs_to :account
   belongs_to :identity, optional: true
diff --git a/test/controllers/users/avatars_controller_test.rb b/test/controllers/users/avatars_controller_test.rb
index f23044ef3..afdf89e1a 100644
--- a/test/controllers/users/avatars_controller_test.rb
+++ b/test/controllers/users/avatars_controller_test.rb
@@ -5,19 +5,37 @@ class Users::AvatarsControllerTest < ActionDispatch::IntegrationTest
     sign_in_as :david
   end
 
-  test "show self without caching" do
+  test "show own initials without caching" do
     get user_avatar_path(users(:david))
     assert_match "image/svg+xml", @response.content_type
     assert @response.cache_control[:private]
     assert_equal "0", @response.cache_control[:max_age]
   end
 
-  test "show other with caching" do
+  test "show other initials with caching" do
     get user_avatar_path(users(:kevin))
     assert_match "image/svg+xml", @response.content_type
     assert_equal 30.minutes.to_s, @response.cache_control[:max_age]
   end
 
+  test "show own image redirects to the blob url" do
+    users(:david).avatar.attach(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg")
+    assert users(:david).avatar.attached?
+
+    get user_avatar_path(users(:david))
+
+    assert_redirected_to rails_blob_url(users(:david).avatar.variant(:thumb), disposition: "inline")
+  end
+
+  test "show other image redirects to the blob url" do
+    users(:kevin).avatar.attach(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg")
+    assert users(:kevin).avatar.attached?
+
+    get user_avatar_path(users(:kevin))
+
+    assert_redirected_to rails_blob_url(users(:kevin).avatar.variant(:thumb), disposition: "inline")
+  end
+
   test "delete self" do
     delete user_avatar_path(users(:david))
     assert_redirected_to users(:david)