From 741eff7bdc70ff31ab0e045630a4fe62b9efee59 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Mon, 15 Dec 2025 13:07:13 +0000 Subject: [PATCH] Revert "Merge pull request #1865 from basecamp/public-avatar-caching" This reverts commit c628f14c016e146a6c81f90f26907a5c0be18f40, reversing changes made to 4bafc7323642bac684e5422809e6d207d8639534. --- app/assets/stylesheets/avatars.css | 5 --- app/controllers/my/avatars_controller.rb | 16 --------- app/controllers/users/avatars_controller.rb | 13 +++++--- app/helpers/avatars_helper.rb | 7 +--- app/views/my/avatars/show.svg.erb | 1 - app/views/users/_initials.svg.erb | 22 ------------- app/views/users/avatars/show.svg.erb | 23 ++++++++++++- config/routes.rb | 1 - .../controllers/my/avatars_controller_test.rb | 33 ------------------- .../users/avatars_controller_test.rb | 11 +++++-- 10 files changed, 41 insertions(+), 91 deletions(-) delete mode 100644 app/controllers/my/avatars_controller.rb delete mode 100644 app/views/my/avatars/show.svg.erb delete mode 100644 app/views/users/_initials.svg.erb delete mode 100644 test/controllers/my/avatars_controller_test.rb diff --git a/app/assets/stylesheets/avatars.css b/app/assets/stylesheets/avatars.css index 9c55ef4e9..e3f15d4ab 100644 --- a/app/assets/stylesheets/avatars.css +++ b/app/assets/stylesheets/avatars.css @@ -22,11 +22,6 @@ max-inline-size: 100%; object-fit: cover; } - - span[data-creator-id] { - display: grid; - place-items: center; - } } .avatar__form { diff --git a/app/controllers/my/avatars_controller.rb b/app/controllers/my/avatars_controller.rb deleted file mode 100644 index 70f714ae2..000000000 --- a/app/controllers/my/avatars_controller.rb +++ /dev/null @@ -1,16 +0,0 @@ -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 diff --git a/app/controllers/users/avatars_controller.rb b/app/controllers/users/avatars_controller.rb index 69ed38f0c..420dbe5b6 100644 --- a/app/controllers/users/avatars_controller.rb +++ b/app/controllers/users/avatars_controller.rb @@ -5,20 +5,17 @@ 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") - else + elsif stale? @user, cache_control: cache_control render_initials end end def destroy @user.avatar.destroy - @user.touch redirect_to @user end @@ -31,6 +28,14 @@ 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 diff --git a/app/helpers/avatars_helper.rb b/app/helpers/avatars_helper.rb index 503430c9a..7c064a650 100644 --- a/app/helpers/avatars_helper.rb +++ b/app/helpers/avatars_helper.rb @@ -42,11 +42,6 @@ module AvatarsHelper end def avatar_image_tag(user, **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 + image_tag user_avatar_url(user, script_name: user.account.slug), aria: { hidden: "true" }, size: 48, title: user.name, **options end end diff --git a/app/views/my/avatars/show.svg.erb b/app/views/my/avatars/show.svg.erb deleted file mode 100644 index adf20ff54..000000000 --- a/app/views/my/avatars/show.svg.erb +++ /dev/null @@ -1 +0,0 @@ -<%= render "users/initials", user: Current.user %> diff --git a/app/views/users/_initials.svg.erb b/app/views/users/_initials.svg.erb deleted file mode 100644 index 7d79adfb2..000000000 --- a/app/views/users/_initials.svg.erb +++ /dev/null @@ -1,22 +0,0 @@ - diff --git a/app/views/users/avatars/show.svg.erb b/app/views/users/avatars/show.svg.erb index b6e34aeb7..06b2dec0d 100644 --- a/app/views/users/avatars/show.svg.erb +++ b/app/views/users/avatars/show.svg.erb @@ -1 +1,22 @@ -<%= render "users/initials", user: @user %> + diff --git a/config/routes.rb b/config/routes.rb index ce7d4fe90..aa2815433 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -164,7 +164,6 @@ Rails.application.routes.draw do resource :landing namespace :my do - resource :avatar, only: :show resource :identity, only: :show resources :access_tokens resources :pins diff --git a/test/controllers/my/avatars_controller_test.rb b/test/controllers/my/avatars_controller_test.rb deleted file mode 100644 index c2081d121..000000000 --- a/test/controllers/my/avatars_controller_test.rb +++ /dev/null @@ -1,33 +0,0 @@ -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 diff --git a/test/controllers/users/avatars_controller_test.rb b/test/controllers/users/avatars_controller_test.rb index 783d5aeba..3ded884b8 100644 --- a/test/controllers/users/avatars_controller_test.rb +++ b/test/controllers/users/avatars_controller_test.rb @@ -12,10 +12,17 @@ class Users::AvatarsControllerTest < ActionDispatch::IntegrationTest assert_redirected_to ActionController::Base.helpers.image_path("system_user.png") end - test "show initials with public 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[:public] + 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_equal 30.minutes.to_s, @response.cache_control[:max_age] end