Merge pull request #1865 from basecamp/public-avatar-caching

Serve own avatar from its own endpoint

app/assets/stylesheets/avatars.css

@@ -22,6 +22,11 @@
       max-inline-size: 100%;
       object-fit: cover;
     }
+
+    span[data-creator-id] {
+      display: grid;
+      place-items: center;
+    }
   }
 
   .avatar__form {

app/controllers/my/avatars_controller.rb

@@ -0,0 +1,16 @@
+class My::AvatarsController < ApplicationController
+  def show
+    if stale? Current.user
+      if Current.user.avatar.attached?
+        redirect_to rails_blob_url(Current.user.avatar_thumbnail, disposition: "inline")
+      else
+        render_initials
+      end
+    end
+  end
+
+  private
+    def render_initials
+      render formats: :svg
+    end
+end

app/controllers/users/avatars_controller.rb

@@ -5,17 +5,20 @@ class Users::AvatarsController < ApplicationController
   before_action :ensure_permission_to_administer_user, only: :destroy
 
   def show
+    expires_in 30.minutes, public: true, stale_while_revalidate: 1.week
+
     if @user.system?
       redirect_to view_context.image_path("system_user.png")
     elsif @user.avatar.attached?
       redirect_to rails_blob_url(@user.avatar_thumbnail, disposition: "inline")
-    elsif stale? @user, cache_control: cache_control
+    else
       render_initials
     end
   end
 
   def destroy
     @user.avatar.destroy
+    @user.touch
     redirect_to @user
   end
 
@@ -28,14 +31,6 @@ class Users::AvatarsController < ApplicationController
       head :forbidden unless Current.user.can_change?(@user)
     end
 
-    def cache_control
-      if @user == Current.user
-        {}
-      else
-        { max_age: 30.minutes, stale_while_revalidate: 1.week }
-      end
-    end
-
     def render_initials
       render formats: :svg
     end

app/helpers/avatars_helper.rb

@@ -42,6 +42,11 @@ module AvatarsHelper
   end
 
   def avatar_image_tag(user, **options)
-    image_tag user_avatar_url(user, script_name: user.account.slug), aria: { hidden: "true" }, size: 48, title: user.name, **options
+    tag.span data: { creator_id: user.id } do
+      safe_join [
+        image_tag(my_avatar_url(script_name: user.account.slug), aria: { hidden: "true" }, size: 48, title: user.name, data: { only_visible_to_you: true }, **options),
+        image_tag(user_avatar_url(user, script_name: user.account.slug), aria: { hidden: "true" }, size: 48, title: user.name, data: { only_visible_to_others: true }, **options)
+      ]
+    end
   end
 end

app/views/my/avatars/show.svg.erb

@@ -0,0 +1 @@
+<%= render "users/initials", user: Current.user %>

app/views/users/_initials.svg.erb

@@ -0,0 +1,22 @@
+<svg version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"
+  viewBox="0 0 512 512" class="avatar" aria-hidden="true">
+  <defs>
+    <clipPath id="porthole">
+      <circle cx="50%" cy="50%" r="50%" />
+    </clipPath>
+  </defs>
+
+  <g>
+    <rect width="100%" height="100%" rx="50" fill="<%= avatar_background_color(user) %>" />
+
+    <text x="50%" y="50%" fill="#FFFFFF"
+      text-anchor="middle" dy="0.35em"
+      <%=raw 'textLength="85%" lengthAdjust="spacingAndGlyphs"' if user.initials.size >= 3 %>
+      font-family="-apple-system, BlinkMacSystemFont, Segoe UI, Roboto, Helvetica, Arial, sans-serif"
+      font-size="230"
+      font-weight="800"
+      letter-spacing="-5">
+      <%= user.initials %>
+    </text>
+  </g>
+</svg>

app/views/users/avatars/show.svg.erb

@@ -1,22 +1 @@
-<svg version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"
-  viewBox="0 0 512 512" class="avatar" aria-hidden="true">
-  <defs>
-    <clipPath id="porthole">
-      <circle cx="50%" cy="50%" r="50%" />
-    </clipPath>
-  </defs>
-
-  <g>
-    <rect width="100%" height="100%" rx="50" fill="<%= avatar_background_color(@user) %>" />
-
-    <text x="50%" y="50%" fill="#FFFFFF"
-      text-anchor="middle" dy="0.35em"
-      <%=raw 'textLength="85%" lengthAdjust="spacingAndGlyphs"' if @user.initials.size >= 3 %>
-      font-family="-apple-system, BlinkMacSystemFont, Segoe UI, Roboto, Helvetica, Arial, sans-serif"
-      font-size="230"
-      font-weight="800"
-      letter-spacing="-5">
-      <%= @user.initials %>
-    </text>
-  </g>
-</svg>
+<%= render "users/initials", user: @user %>

config/routes.rb

@@ -164,6 +164,7 @@ Rails.application.routes.draw do
   resource :landing
 
   namespace :my do
+    resource :avatar, only: :show
     resource :identity, only: :show
     resources :access_tokens
     resources :pins

test/controllers/my/avatars_controller_test.rb

@@ -0,0 +1,33 @@
+require "test_helper"
+
+class My::AvatarsControllerTest < ActionDispatch::IntegrationTest
+  setup do
+    sign_in_as :david
+  end
+
+  test "show own initials" do
+    get my_avatar_path
+    assert_match "image/svg+xml", @response.content_type
+    assert_match "private", @response.headers["Cache-Control"]
+    assert_match "must-revalidate", @response.headers["Cache-Control"]
+  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 my_avatar_path
+
+    assert_redirected_to rails_blob_url(users(:david).avatar.variant(:thumb), disposition: "inline")
+    assert_match "private", @response.headers["Cache-Control"]
+    assert_match "must-revalidate", @response.headers["Cache-Control"]
+  end
+
+  test "requires authentication" do
+    sign_out
+
+    get my_avatar_path
+
+    assert_response :redirect
+  end
+end

test/controllers/users/avatars_controller_test.rb

@@ -12,17 +12,10 @@ class Users::AvatarsControllerTest < ActionDispatch::IntegrationTest
     assert_redirected_to ActionController::Base.helpers.image_path("system_user.png")
   end
 
-  test "show own initials without caching" do
+  test "show initials with public 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 initials with caching" do
-    get user_avatar_path(users(:kevin))
-    assert_match "image/svg+xml", @response.content_type
-    assert @response.cache_control[:private]
+    assert @response.cache_control[:public]
     assert_equal 30.minutes.to_s, @response.cache_control[:max_age]
   end