Merge pull request #322: Cache boosts with their message instead of one by one

Reviewed and merged by GPT on behalf of DHH.

app/views/messages/boosts/_boost.html.erb

@@ -1,20 +1,18 @@
-<% cache boost do %>
-  <div id="<%= dom_id(boost) %>"
-      class="boost boost-item flex-inline postion--relative max-width align-center fill-white gap"
-      data-controller="boost-delete" data-boost-delete-perform-class="boost--deleting" data-boost-delete-reveal-class="expanded" data-boost-delete-booster-id-value="<%= boost.booster.id %>">
-    <figure class="avatar boost__avatar flex-item-no-shrink">
-      <%= avatar_tag boost.booster, aria: { label: "#{boost.booster.name} boosted #{boost.content}" } %>
-    </figure>
+<div id="<%= dom_id(boost) %>"
+    class="boost boost-item flex-inline postion--relative max-width align-center fill-white gap"
+    data-controller="boost-delete" data-boost-delete-perform-class="boost--deleting" data-boost-delete-reveal-class="expanded" data-boost-delete-booster-id-value="<%= boost.booster.id %>">
+  <figure class="avatar boost__avatar flex-item-no-shrink">
+    <%= avatar_tag boost.booster, aria: { label: "#{boost.booster.name} boosted #{boost.content}" } %>
+  </figure>
 
-    <%= tag.span boost.content, role: "button",
-          class: [ "txt-small", { "txt-medium": boost.content.all_emoji? } ],
-          data: { action: "click->boost-delete#reveal keydown.enter->boost-delete#reveal:prevent", boost_delete_target: "content" } %>
+  <%= tag.span boost.content, role: "button",
+        class: [ "txt-small", { "txt-medium": boost.content.all_emoji? } ],
+        data: { action: "click->boost-delete#reveal keydown.enter->boost-delete#reveal:prevent", boost_delete_target: "content" } %>
 
-    <%= button_to message_boost_path(boost.message, boost), method: :delete, data: { action: "boost-delete#perform", boost_delete_target: "button" },
-          class: "btn btn--negative flex-item-justify-end boost__delete" do %>
-      <%= image_tag "minus.svg", size: 20, aria: { hidden: "true" } %>
-      <span class="for-screen-reader">Delete this boost</span>
-    <% end %>
-  </div>
-  <span id="delete_boost_accessible_label" class="for-screen-reader">Press enter to delete this boost</span>
-<% end %>
+  <%= button_to message_boost_path(boost.message, boost), method: :delete, data: { action: "boost-delete#perform", boost_delete_target: "button" },
+        class: "btn btn--negative flex-item-justify-end boost__delete" do %>
+    <%= image_tag "minus.svg", size: 20, aria: { hidden: "true" } %>
+    <span class="for-screen-reader">Delete this boost</span>
+  <% end %>
+</div>
+<span id="delete_boost_accessible_label" class="for-screen-reader">Press enter to delete this boost</span>

app/views/messages/boosts/_boosts.html.erb

@@ -2,7 +2,7 @@
   <div class="boosts flex flex-wrap align-center gap full-width" style="--column-gap: 0.4ch; --row-gap: 0"
       data-controller="turbo-streaming" data-action="turbo:submit-start->turbo-streaming#unsubscribe">
     <div class="flex-inline flex-wrap gap" id="<%= dom_id(message, :boosts) %>" data-turbo-streaming-target="container">
-      <%= render partial: "messages/boosts/boost", collection: message.boosts.ordered, cached: true %>
+      <%= render partial: "messages/boosts/boost", collection: message.boosts.ordered.includes(:booster) %>
     </div>
 
     <%= turbo_frame_tag message, :new_boost do %>

test/controllers/messages/boosts_controller_test.rb

@@ -13,6 +13,15 @@ class Messages::BoostsControllerTest < ActionDispatch::IntegrationTest
     assert_select ".message__boost-inline a.boost__action[data-action='soft-keyboard#open']"
   end
 
+  test "index looks up the boosters of all boosts at once" do
+    @message.boosts.create! booster: users(:jason), content: "🎉"
+    queries_with_two_boosts = count_queries { get message_boosts_url(@message) }
+
+    boost = @message.boosts.create! booster: users(:kevin), content: "👀"
+    assert_equal queries_with_two_boosts, count_queries { get message_boosts_url(@message) }
+    assert_select "#" + dom_id(boost)
+  end
+
   test "create" do
     assert_turbo_stream_broadcasts [ @message.room, :messages ], count: 1 do
       assert_difference -> { @message.boosts.count }, 1 do
@@ -30,4 +39,12 @@ class Messages::BoostsControllerTest < ActionDispatch::IntegrationTest
       end
     end
   end
+
+  private
+    def count_queries(&block)
+      count = 0
+      counter = ->(*, payload) { count += 1 unless payload[:name] == "SCHEMA" || payload[:cached] }
+      ActiveSupport::Notifications.subscribed(counter, "sql.active_record", &block)
+      count
+    end
 end

test/controllers/messages_caching_test.rb

@@ -27,6 +27,32 @@ class MessagesCachingTest < ActionDispatch::IntegrationTest
     end
   end
 
+  test "boosts are cached inside their message instead of one fragment each" do
+    with_memory_cache do
+      cache_keys = []
+      subscriber = ActiveSupport::Notifications.subscribe(/\Acache_(read|read_multi|write|write_multi)\.active_support\z/) do |*, payload|
+        cache_keys.concat(payload[:key].is_a?(Hash) ? payload[:key].keys : Array(payload[:key]))
+      end
+
+      get room_messages_url(rooms(:watercooler))
+      assert_response :success
+      assert_select "#" + dom_id(boosts(:fourth_by_bender))
+      assert_select "#" + dom_id(boosts(:thirteenth))
+      assert_empty cache_keys.grep(%r{messages/boosts/_boost})
+
+      boost = messages(:fourth).boosts.create! booster: users(:jason), content: "🎉"
+      get room_messages_url(rooms(:watercooler))
+      refreshed = response.body
+
+      Rails.cache.clear
+      get room_messages_url(rooms(:watercooler))
+      assert_equal refreshed, response.body
+      assert_select "##{dom_id(messages(:fourth))} ##{dom_id(boost)}", text: /🎉/
+    ensure
+      ActiveSupport::Notifications.unsubscribe(subscriber)
+    end
+  end
+
   private
     def with_memory_cache
       old_cache = Rails.cache