From e97aa0d6801ce52f727450ca28a77cdfdfa18822 Mon Sep 17 00:00:00 2001
From: Jacob Cody Wimer <jacob.wimer@gmail.com>
Date: Fri, 13 Feb 2026 18:02:04 -0500
Subject: [PATCH] Fixed N+1 on up_matches and added html cache.

---
 app/models/mat.rb                             | 24 +++++-
 .../tournaments/_up_matches_mat_row.html.erb  | 34 ++++++++
 app/views/tournaments/up_matches.html.erb     | 34 +-------
 test/controllers/up_matches_cache_test.rb     | 84 +++++++++++++++++++
 test/models/mat_test.rb                       | 15 ++++
 5 files changed, 155 insertions(+), 36 deletions(-)
 create mode 100644 app/views/tournaments/_up_matches_mat_row.html.erb
 create mode 100644 test/controllers/up_matches_cache_test.rb

diff --git a/app/models/mat.rb b/app/models/mat.rb
index 2733b28..9a60b16 100644
--- a/app/models/mat.rb
+++ b/app/models/mat.rb
@@ -8,6 +8,8 @@ class Mat < ApplicationRecord
 
 	QUEUE_SLOTS = %w[queue1 queue2 queue3 queue4].freeze
 
+	after_save :clear_queue_matches_cache
+
 	def assign_next_match
 		slot = first_empty_queue_slot
 		return true unless slot
@@ -86,8 +88,20 @@ class Mat < ApplicationRecord
 		QUEUE_SLOTS.map { |slot| public_send(slot) }
 	end
 
+	# used to prevent N+1 query on each mat
 	def queue_matches
-		queue_match_ids.map { |match_id| match_id ? Match.find_by(id: match_id) : nil }
+		slot_ids = queue_match_ids
+		if @queue_matches.nil? || @queue_match_slot_ids != slot_ids
+			ids = slot_ids.compact
+			@queue_matches = if ids.empty?
+				[nil, nil, nil, nil]
+			else
+				matches_by_id = Match.where(id: ids).index_by(&:id)
+				slot_ids.map { |match_id| match_id ? matches_by_id[match_id] : nil }
+			end
+			@queue_match_slot_ids = slot_ids
+		end
+		@queue_matches
 	end
 
 	def queue1_match
@@ -175,9 +189,13 @@ class Mat < ApplicationRecord
 
 	private
 
+	def clear_queue_matches_cache
+		@queue_matches = nil
+		@queue_match_slot_ids = nil
+	end
+
 	def queue_match_at(position)
-		match_id = public_send("queue#{position}")
-		match_id ? Match.find_by(id: match_id) : nil
+		queue_matches[position - 1]
 	end
 
 	def first_empty_queue_slot
diff --git a/app/views/tournaments/_up_matches_mat_row.html.erb b/app/views/tournaments/_up_matches_mat_row.html.erb
new file mode 100644
index 0000000..f4095ed
--- /dev/null
+++ b/app/views/tournaments/_up_matches_mat_row.html.erb
@@ -0,0 +1,34 @@
+<% queue1_match, queue2_match, queue3_match, queue4_match = mat.queue_matches %>
+<% cache ["up_matches_mat_row", mat, mat.queue1, mat.queue2, mat.queue3, mat.queue4] do %>
+  <tr>
+    <td><%= mat.name %></td>
+    <td>
+      <% if queue1_match %><strong><%= queue1_match.bout_number %></strong> (<%= queue1_match.bracket_position %>)<br>
+        <%= queue1_match.weight_max %> lbs
+        <br><%= queue1_match.w1_bracket_name %> vs. <br>
+        <%= queue1_match.w2_bracket_name %>
+      <% end %>
+    </td>
+    <td>
+      <% if queue2_match %><strong><%= queue2_match.bout_number %></strong> (<%= queue2_match.bracket_position %>)<br>
+        <%= queue2_match.weight_max %> lbs
+        <br><%= queue2_match.w1_bracket_name %> vs. <br>
+        <%= queue2_match.w2_bracket_name %>
+      <% end %>
+    </td>
+    <td>
+      <% if queue3_match %><strong><%= queue3_match.bout_number %></strong> (<%= queue3_match.bracket_position %>)<br>
+        <%= queue3_match.weight_max %> lbs
+        <br><%= queue3_match.w1_bracket_name %> vs. <br>
+        <%= queue3_match.w2_bracket_name %>
+      <% end %>
+    </td>
+    <td>
+      <% if queue4_match %><strong><%= queue4_match.bout_number %></strong> (<%= queue4_match.bracket_position %>)<br>
+        <%= queue4_match.weight_max %> lbs
+        <br><%= queue4_match.w1_bracket_name %> vs. <br>
+        <%= queue4_match.w2_bracket_name %>
+      <% end %>
+    </td>
+  </tr>
+<% end %>
diff --git a/app/views/tournaments/up_matches.html.erb b/app/views/tournaments/up_matches.html.erb
index eebbf4b..d2c6960 100644
--- a/app/views/tournaments/up_matches.html.erb
+++ b/app/views/tournaments/up_matches.html.erb
@@ -1,4 +1,3 @@
-<% cache ["#{@tournament.id}_up_matches", @tournament] do %>
   <script>
     // $(document).ready(function() {
     //   $('#matchList').dataTable();
@@ -42,37 +41,7 @@
 
     <tbody>
       <% @mats.each.map do |m| %>
-        <tr>
-          <td><%= m.name %></td>
-          <td>
-            <% if m.queue1_match %><strong><%=m.queue1_match.bout_number%></strong> (<%= m.queue1_match.bracket_position %>)<br>
-              <%= m.queue1_match.weight_max %> lbs
-              <br><%= m.queue1_match.w1_bracket_name %> vs. <br>
-              <%= m.queue1_match.w2_bracket_name %>
-            <% end %>
-          </td>
-          <td>
-            <% if m.queue2_match %><strong><%=m.queue2_match.bout_number%></strong> (<%= m.queue2_match.bracket_position %>)<br>
-              <%= m.queue2_match.weight_max %> lbs
-              <br><%= m.queue2_match.w1_bracket_name %> vs. <br>
-              <%= m.queue2_match.w2_bracket_name %>
-            <% end %>
-          </td>
-          <td>
-            <% if m.queue3_match %><strong><%=m.queue3_match.bout_number%></strong> (<%= m.queue3_match.bracket_position %>)<br>
-              <%= m.queue3_match.weight_max %> lbs
-              <br><%= m.queue3_match.w1_bracket_name %> vs. <br>
-              <%= m.queue3_match.w2_bracket_name %>
-            <% end %>
-          </td>
-          <td>
-            <% if m.queue4_match %><strong><%=m.queue4_match.bout_number%></strong> (<%= m.queue4_match.bracket_position %>)<br>
-              <%= m.queue4_match.weight_max %> lbs
-              <br><%= m.queue4_match.w1_bracket_name %> vs. <br>
-              <%= m.queue4_match.w2_bracket_name %>
-            <% end %>
-          </td>
-        </tr>
+        <%= render "up_matches_mat_row", mat: m %>
       <% end %>
     </tbody>
   </table>
@@ -104,4 +73,3 @@
   </table>
 
   <br>
-<% end %>
diff --git a/test/controllers/up_matches_cache_test.rb b/test/controllers/up_matches_cache_test.rb
new file mode 100644
index 0000000..81901ad
--- /dev/null
+++ b/test/controllers/up_matches_cache_test.rb
@@ -0,0 +1,84 @@
+require "test_helper"
+
+class UpMatchesCacheTest < ActionController::TestCase
+  tests TournamentsController
+
+  setup do
+    create_double_elim_tournament_1_6_with_multiple_weights_and_multiple_mats(16, 4, 2)
+    @tournament.update!(user_id: users(:one).id)
+    @tournament.reset_and_fill_bout_board
+
+    sign_in users(:one)
+
+    @original_perform_caching = ActionController::Base.perform_caching
+    ActionController::Base.perform_caching = true
+    Rails.cache.clear
+  end
+
+  teardown do
+    Rails.cache.clear
+    ActionController::Base.perform_caching = @original_perform_caching
+  end
+
+  test "up_matches row fragments hit cache and invalidate when a mat queue changes" do
+    first_events = cache_events_for_up_matches do
+      get :up_matches, params: { id: @tournament.id }
+      assert_response :success
+    end
+
+    assert_operator cache_writes(first_events), :>, 0, "Expected initial render to write row fragments"
+
+    second_events = cache_events_for_up_matches do
+      get :up_matches, params: { id: @tournament.id }
+      assert_response :success
+    end
+
+    assert_equal 0, cache_writes(second_events), "Expected second render to reuse cached row fragments"
+    assert_operator cache_hits(second_events), :>, 0, "Expected second render to have cache hits"
+
+    mat = @tournament.mats.first
+    mat.reload
+    movable_match = mat.queue2_match || mat.queue1_match
+    assert movable_match, "Expected at least one queued match to move"
+    mat.assign_match_to_queue!(movable_match, 4)
+
+    third_events = cache_events_for_up_matches do
+      get :up_matches, params: { id: @tournament.id }
+      assert_response :success
+    end
+
+    assert_operator cache_writes(third_events), :>, 0, "Expected queue change to invalidate and rewrite at least one row fragment"
+  end
+
+  private
+
+  def cache_events_for_up_matches
+    events = []
+    subscriber = lambda do |name, _start, _finish, _id, payload|
+      key = payload[:key].to_s
+      next unless key.include?("up_matches_mat_row")
+
+      events << { name: name, hit: payload[:hit] }
+    end
+
+    ActiveSupport::Notifications.subscribed(
+      subscriber,
+      /cache_(read|write|fetch_hit|generate)\.active_support/
+    ) do
+      yield
+    end
+
+    events
+  end
+
+  def cache_writes(events)
+    events.count { |event| event[:name] == "cache_write.active_support" }
+  end
+
+  def cache_hits(events)
+    events.count do |event|
+      event[:name] == "cache_fetch_hit.active_support" ||
+        (event[:name] == "cache_read.active_support" && event[:hit])
+    end
+  end
+end
diff --git a/test/models/mat_test.rb b/test/models/mat_test.rb
index 4026932..743e6e1 100644
--- a/test/models/mat_test.rb
+++ b/test/models/mat_test.rb
@@ -10,5 +10,20 @@ class MatTest < ActiveSupport::TestCase
     assert_not mat.valid?
     assert_equal [:tournament, :name], mat.errors.attribute_names
   end
+
+  test "queue_matches refreshes after queue slots change and record reloads" do
+    create_double_elim_tournament_1_6_with_multiple_weights_and_multiple_mats(16, 4, 1)
+    @tournament.reset_and_fill_bout_board
+
+    mat = @tournament.mats.first
+    initial_queue_ids = mat.queue_matches.map { |match| match&.id }
+    assert initial_queue_ids.compact.any?, "Expected initial queue to contain matches"
+
+    Mat.where(id: mat.id).update_all(queue1: nil, queue2: nil, queue3: nil, queue4: nil)
+    mat.reload
+
+    refreshed_queue_ids = mat.queue_matches.map { |match| match&.id }
+    assert_equal [nil, nil, nil, nil], refreshed_queue_ids, "Expected queue_matches to refresh after reload and slot changes"
+  end
   
 end