From eec96ff3843c4a7593cd36ad5e963ede98e30ed5 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Tue, 25 Nov 2025 14:40:49 +0000 Subject: [PATCH 1/8] Serve own avatar from its own endpoint This allows us to have different cache controls depending on whether you're viewing your own avatar, or someone else's. Your own avatar will always be fresh, while other folks' avatars can be pulled from the CDN. --- 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 ++----- 9 files changed, 86 insertions(+), 41 deletions(-) create mode 100644 app/controllers/my/avatars_controller.rb create mode 100644 app/views/my/avatars/show.svg.erb create mode 100644 app/views/users/_initials.svg.erb create mode 100644 test/controllers/my/avatars_controller_test.rb diff --git a/app/controllers/my/avatars_controller.rb b/app/controllers/my/avatars_controller.rb new file mode 100644 index 000000000..70f714ae2 --- /dev/null +++ b/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 diff --git a/app/controllers/users/avatars_controller.rb b/app/controllers/users/avatars_controller.rb index d85e5f869..989387981 100644 --- a/app/controllers/users/avatars_controller.rb +++ b/app/controllers/users/avatars_controller.rb @@ -7,17 +7,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 @@ -30,14 +33,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 diff --git a/app/helpers/avatars_helper.rb b/app/helpers/avatars_helper.rb index 7c064a650..a30f20962 100644 --- a/app/helpers/avatars_helper.rb +++ b/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(user_avatar_url(user, script_name: user.account.slug), aria: { hidden: "true" }, size: 48, title: user.name, data: { only_visible_to_others: true }, **options), + 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) + ] + end end end diff --git a/app/views/my/avatars/show.svg.erb b/app/views/my/avatars/show.svg.erb new file mode 100644 index 000000000..adf20ff54 --- /dev/null +++ b/app/views/my/avatars/show.svg.erb @@ -0,0 +1 @@ +<%= render "users/initials", user: Current.user %> diff --git a/app/views/users/_initials.svg.erb b/app/views/users/_initials.svg.erb new file mode 100644 index 000000000..7d79adfb2 --- /dev/null +++ b/app/views/users/_initials.svg.erb @@ -0,0 +1,22 @@ + diff --git a/app/views/users/avatars/show.svg.erb b/app/views/users/avatars/show.svg.erb index 06b2dec0d..b6e34aeb7 100644 --- a/app/views/users/avatars/show.svg.erb +++ b/app/views/users/avatars/show.svg.erb @@ -1,22 +1 @@ - +<%= render "users/initials", user: @user %> diff --git a/config/routes.rb b/config/routes.rb index aa2815433..ce7d4fe90 100644 --- a/config/routes.rb +++ b/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 diff --git a/test/controllers/my/avatars_controller_test.rb b/test/controllers/my/avatars_controller_test.rb new file mode 100644 index 000000000..c2081d121 --- /dev/null +++ b/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 diff --git a/test/controllers/users/avatars_controller_test.rb b/test/controllers/users/avatars_controller_test.rb index 3ded884b8..783d5aeba 100644 --- a/test/controllers/users/avatars_controller_test.rb +++ b/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 From 88ab0beb24eb1372229615e8687688b3a5d21349 Mon Sep 17 00:00:00 2001 From: Jason Zimdars Date: Fri, 12 Dec 2025 22:40:53 -0600 Subject: [PATCH 2/8] Make layout bulletproof --- app/assets/stylesheets/avatars.css | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/assets/stylesheets/avatars.css b/app/assets/stylesheets/avatars.css index e3f15d4ab..9c55ef4e9 100644 --- a/app/assets/stylesheets/avatars.css +++ b/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 { From 87c65577476ca97529d6e2013eed913cacc3cb06 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Mon, 15 Dec 2025 09:47:47 +0000 Subject: [PATCH 3/8] Limit length of full name during signup If we don't validate for length, then signups that overflow the database columns will unnecessarily create and cancel a tenant. Adding a validation means we can avoid this. --- app/models/signup.rb | 1 + app/views/signups/completions/new.html.erb | 2 +- test/models/signup_test.rb | 13 +++++++++++++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/app/models/signup.rb b/app/models/signup.rb index 110c253ef..041f461ac 100644 --- a/app/models/signup.rb +++ b/app/models/signup.rb @@ -8,6 +8,7 @@ class Signup validates :email_address, format: { with: URI::MailTo::EMAIL_REGEXP }, on: :identity_creation validates :full_name, :identity, presence: true, on: :completion + validates :full_name, length: { maximum: 240 } def initialize(...) super diff --git a/app/views/signups/completions/new.html.erb b/app/views/signups/completions/new.html.erb index 5be760843..6d4c4c6cf 100644 --- a/app/views/signups/completions/new.html.erb +++ b/app/views/signups/completions/new.html.erb @@ -4,7 +4,7 @@

<%= @page_title %>

<%= form_with model: @signup, url: signup_completion_path, scope: "signup", class: "flex flex-column gap", data: { controller: "form" } do |form| %> - <%= form.text_field :full_name, class: "input txt-large", autocomplete: "name", placeholder: "Enter your full name…", autofocus: true, required: true %> + <%= form.text_field :full_name, class: "input txt-large", autocomplete: "name", placeholder: "Enter your full name…", autofocus: true, required: true, maxlength: 240 %>

You're one step away. Just enter your name to get your own Fizzy account.

diff --git a/test/models/signup_test.rb b/test/models/signup_test.rb index 589bcd49a..947d55d6b 100644 --- a/test/models/signup_test.rb +++ b/test/models/signup_test.rb @@ -72,4 +72,17 @@ class SignupTest < ActiveSupport::TestCase assert_nil signup.user end end + + test "#complete with name that is too long" do + Current.without_account do + signup = Signup.new(full_name: "A" * 241, identity: identities(:kevin)) + signup.expects(:create_tenant).never + + assert_not signup.complete + + assert signup.errors[:full_name].any? + assert_nil signup.account + assert_nil signup.user + end + end end From d4cbc86113c12f9e6964fb202ccfb30ec3bafa57 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Mon, 15 Dec 2025 11:22:00 +0000 Subject: [PATCH 4/8] Swap order of avatar links If the conditional CSS rules failed to match for some reason, we should prefer the general avatar image rather than the "my" avatar one. That way the fallback mode will still look correct. --- app/helpers/avatars_helper.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/helpers/avatars_helper.rb b/app/helpers/avatars_helper.rb index a30f20962..503430c9a 100644 --- a/app/helpers/avatars_helper.rb +++ b/app/helpers/avatars_helper.rb @@ -44,8 +44,8 @@ module AvatarsHelper def avatar_image_tag(user, **options) tag.span data: { creator_id: user.id } do safe_join [ - 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), - 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(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 From 87081aa61798c11501f7ed4c401a34e17116348d Mon Sep 17 00:00:00 2001 From: "Stanko K.R." Date: Mon, 15 Dec 2025 13:15:36 +0100 Subject: [PATCH 5/8] Show only public cards on public boards --- app/controllers/public/base_controller.rb | 2 +- test/controllers/public/cards_controller_test.rb | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/app/controllers/public/base_controller.rb b/app/controllers/public/base_controller.rb index b4c33e95b..018b1ab36 100644 --- a/app/controllers/public/base_controller.rb +++ b/app/controllers/public/base_controller.rb @@ -11,7 +11,7 @@ class Public::BaseController < ApplicationController end def set_card - @card = @board.cards.find_by!(number: params[:id]) if params[:board_id] && params[:id] + @card = @board.cards.published.find_by!(number: params[:id]) if params[:board_id] && params[:id] end def set_public_cache_expiration diff --git a/test/controllers/public/cards_controller_test.rb b/test/controllers/public/cards_controller_test.rb index 652da3e8a..8b3b9e029 100644 --- a/test/controllers/public/cards_controller_test.rb +++ b/test/controllers/public/cards_controller_test.rb @@ -18,4 +18,10 @@ class Public::CardsControllerTest < ActionDispatch::IntegrationTest get public_board_card_path(@board.publication.key, @card) assert_response :not_found end + + test "not found if the card is drafted" do + @card.update!(status: :drafted) + get public_board_card_path(@board.publication.key, @card) + assert_response :not_found + end end From 741eff7bdc70ff31ab0e045630a4fe62b9efee59 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Mon, 15 Dec 2025 13:07:13 +0000 Subject: [PATCH 6/8] 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 From e2292a1739bf154a3d7d7cdddcf3074deff99f57 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Mon, 15 Dec 2025 13:23:57 +0000 Subject: [PATCH 7/8] Update cached fragments --- app/views/cards/comments/_comment.html.erb | 1 + app/views/events/_event.html.erb | 1 + app/views/notifications/_notification.html.erb | 1 + 3 files changed, 3 insertions(+) diff --git a/app/views/cards/comments/_comment.html.erb b/app/views/cards/comments/_comment.html.erb index fee6d6d25..6a825e9ec 100644 --- a/app/views/cards/comments/_comment.html.erb +++ b/app/views/cards/comments/_comment.html.erb @@ -1,4 +1,5 @@ <% cache comment do %> + <%# Helper Dependency Updated: avatar_image_tag 2025-12-15 %> <%= turbo_frame_tag comment, :container do %> <%# Cache bump 2025-12-14: action text attachment rendering changed for lightbox -%>
"> diff --git a/app/views/events/_event.html.erb b/app/views/events/_event.html.erb index 99f7a945e..733bb68df 100644 --- a/app/views/events/_event.html.erb +++ b/app/views/events/_event.html.erb @@ -1,4 +1,5 @@ <% cache event do %> + <%# Helper Dependency Updated: avatar_image_tag 2025-12-15 %> <% if lookup_context.exists?("events/event/eventable/_#{event.action}") %> <%= render "events/event/eventable/#{event.action}", event: event %> <% else %> diff --git a/app/views/notifications/_notification.html.erb b/app/views/notifications/_notification.html.erb index 1596d8529..02f547c4e 100644 --- a/app/views/notifications/_notification.html.erb +++ b/app/views/notifications/_notification.html.erb @@ -1,4 +1,5 @@ <% cache notification do %> + <%# Helper Dependency Updated: avatar_image_tag 2025-12-15 %> <%= notification_tag notification do %> <%= render "notifications/notification/header", notification: notification do %> <%= notification_toggle_read_button(notification, url: card_reading_path(notification.card)) %> From 2ea6f4e070cc1bc382bd69ca451707f322c419a5 Mon Sep 17 00:00:00 2001 From: Andy Smith Date: Mon, 15 Dec 2025 13:14:11 -0600 Subject: [PATCH 8/8] Use decimals for ordered lists --- app/assets/stylesheets/rich-text-content.css | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/app/assets/stylesheets/rich-text-content.css b/app/assets/stylesheets/rich-text-content.css index 5a63f9eac..5a8ba3fd7 100644 --- a/app/assets/stylesheets/rich-text-content.css +++ b/app/assets/stylesheets/rich-text-content.css @@ -29,10 +29,17 @@ } ol, ul { - list-style: disc; padding-inline-start: 3ch; } + ol { + list-style: decimal; + } + + ul { + list-style: disc; + } + li:has(li) { list-style: none;