From 64b0e787f047d95c87b205be947122d9fe3953d0 Mon Sep 17 00:00:00 2001
From: Marwin Hochfelsner <50826859+hlfan@users.noreply.github.com>
Date: Sat, 17 May 2025 00:43:03 +0200
Subject: [PATCH] Extract heatmap data assembly into helper

---
 app/controllers/users_controller.rb       | 35 +++++------------------
 app/helpers/heatmap_helper.rb             | 25 ++++++++++++++++
 app/views/users/_heatmap.html.erb         | 10 +++----
 app/views/users/show.html.erb             |  5 ++--
 test/controllers/users_controller_test.rb | 14 ++++-----
 5 files changed, 45 insertions(+), 44 deletions(-)
 create mode 100644 app/helpers/heatmap_helper.rb

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 5bd4809be..de6c8b1c2 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -25,12 +25,12 @@ class UsersController < ApplicationController
       @title = @user.display_name
 
       @heatmap_data = Rails.cache.fetch("heatmap_data_of_user_#{@user.id}", :expires_at => Time.zone.now.end_of_day) do
-        one_year_ago = 1.year.ago.beginning_of_day
-        today = Time.zone.now.end_of_day
+        from = 1.year.ago.beginning_of_day
+        to = Time.zone.now.end_of_day
 
         mapped = Changeset
                  .where(:user_id => @user.id)
-                 .where(:created_at => one_year_ago..today)
+                 .where(:created_at => from..to)
                  .where(:num_changes => 1..)
                  .group("date_trunc('day', created_at)")
                  .select("date_trunc('day', created_at) AS date, SUM(num_changes) AS total_changes, MAX(id) AS max_id")
@@ -43,32 +43,11 @@ class UsersController < ApplicationController
                    }
                  end
 
-        indexed = mapped.index_by { |entry| entry[:date] }
-
-        # Pad the start by one week to ensure the heatmap can start on the first day of the week
-        all_days = ((1.year.ago - 1.week).beginning_of_day.to_date..today.to_date).map do |date|
-          indexed[date] || { :date => date, :total_changes => 0 }
-        end
-
-        # Get unique months with repeating months and count into the next year with numbers over 12
-        month_offset = 0
-        months = ((1.year.ago - 2.weeks).beginning_of_day.to_date..(today + 1.week).to_date)
-                 .map(&:month)
-                 .chunk_while { |before, after| before == after }
-                 .map(&:first)
-                 .map do |month|
-                   month_offset += 12 if month == 1
-                   month + month_offset
-                 end
-
-        total = mapped.sum { |entry| entry[:total_changes] }
-        max_per_day = mapped.map { |entry| entry[:total_changes] }.max
-
         {
-          :days => all_days,
-          :months => months,
-          :total => total,
-          :max_per_day => max_per_day
+          :count => mapped.sum { |entry| entry[:total_changes] },
+          :data => mapped.index_by { |entry| entry[:date] },
+          :from => from,
+          :to => to
         }
       end
     else
diff --git a/app/helpers/heatmap_helper.rb b/app/helpers/heatmap_helper.rb
new file mode 100644
index 000000000..8dfdbf5d3
--- /dev/null
+++ b/app/helpers/heatmap_helper.rb
@@ -0,0 +1,25 @@
+module HeatmapHelper
+  def prepare_heatmap(data, from, to)
+    # Pad the start by one week to ensure the heatmap can start on the first day of the week
+    all_days = ((from - 1.week).to_date..to.to_date).map do |date|
+      data[date] || { :date => date, :total_changes => 0 }
+    end
+
+    # Get unique months with repeating months and count into the next year with numbers over 12
+    month_offset = 0
+    months = ((from - 2.weeks).to_date..(to + 1.week).to_date)
+             .map(&:month)
+             .chunk_while { |before, after| before == after }
+             .map(&:first)
+             .map do |month|
+      month_offset += 12 if month == 1
+      month + month_offset
+    end
+
+    {
+      :days => all_days,
+      :months => months,
+      :max_per_day => data.values.pluck(:total_changes).max
+    }
+  end
+end
diff --git a/app/views/users/_heatmap.html.erb b/app/views/users/_heatmap.html.erb
index c86b1c4a4..73cd2c94a 100644
--- a/app/views/users/_heatmap.html.erb
+++ b/app/views/users/_heatmap.html.erb
@@ -1,11 +1,9 @@
-<h2 class="text-body-secondary fs-5 mt-4">
-  <%= t("users.show.contributions", :count => data[:total]) %>
-</h2>
+<% cal_data = prepare_heatmap(data, from, to) %>
 <div class="row">
   <div class="col overflow-auto">
-    <div class="heatmap d-none align-items-center mb-1 text-center" data-max-per-day="<%= data[:max_per_day] %>">
+    <div class="heatmap d-none align-items-center mb-1 text-center" data-max-per-day="<%= cal_data[:max_per_day] %>">
       <!-- Months -->
-      <% data[:months].each do |month| %>
+      <% cal_data[:months].each do |month| %>
         <span class="mb-1 mx-n2" data-month="<%= month %>"><%= t("date.abbr_month_names")[((month - 1) % 12) + 1] %></span>
       <% end %>
       <!-- Days -->
@@ -13,7 +11,7 @@
         <span class="me-1 my-n1" data-weekday="<%= day %>"><%= t("date.abbr_day_names")[day] %></span>
       <% end %>
       <!-- Heatmap -->
-      <% data[:days].each do |day| %>
+      <% cal_data[:days].each do |day| %>
         <% if day[:total_changes] == 0 %>
           <span data-date="<%= day[:date] %>"></span>
         <% else %>
diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb
index 112bdd894..a9c4ee49e 100644
--- a/app/views/users/show.html.erb
+++ b/app/views/users/show.html.erb
@@ -246,8 +246,9 @@
 
 <div class="richtext text-break"><%= @user.description.to_html %></div>
 
-<% if @heatmap_data[:total].positive? %>
-  <%= render :partial => "heatmap", :locals => { :data => @heatmap_data } %>
+<% if @heatmap_data[:count].positive? %>
+  <h2 class="text-body-secondary fs-5 mt-4"><%= t("users.show.contributions", :count => @heatmap_data[:count]) %></h2>
+  <%= render :partial => "heatmap", :locals => @heatmap_data %>
 <% end %>
 
 <% if current_user and @user.id == current_user.id %>
diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb
index 305932f97..c29f3827e 100644
--- a/test/controllers/users_controller_test.rb
+++ b/test/controllers/users_controller_test.rb
@@ -341,12 +341,10 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     # The data should not be empty
     heatmap_data = assigns(:heatmap_data)
     assert_not_nil heatmap_data
-    assert_predicate heatmap_data[:days], :any?
+    assert_predicate heatmap_data[:data], :any?
     # The data should be in the right format
-    assert(heatmap_data[:days].all? { |entry| entry[:date] && entry[:total_changes] }, "Heatmap data should have :date and :total_changes keys")
-    assert_predicate heatmap_data[:months], :any?
-    assert_equal 30, heatmap_data[:total]
-    assert_equal 20, heatmap_data[:max_per_day]
+    assert(heatmap_data[:data].values.all? { |entry| entry[:date] && entry[:total_changes] }, "Heatmap data should have :date and :total_changes keys")
+    assert_equal 30, heatmap_data[:count]
   end
 
   def test_show_heatmap_data_caching
@@ -364,7 +362,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     get user_path(user.display_name)
     first_response_data = assigns(:heatmap_data)
     assert_not_nil first_response_data, "Expected heatmap data to be assigned on the first request"
-    assert_equal 1, first_response_data[:days].count { |day| day[:total_changes].positive? }, "Expected one entry in the heatmap data"
+    assert_equal 1, first_response_data[:data].values.count { |day| day[:total_changes].positive? }, "Expected one entry in the heatmap data"
 
     # Inspect cache after the first request
     cached_data = Rails.cache.read("heatmap_data_of_user_#{user.id}")
@@ -386,7 +384,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     third_response_data = assigns(:heatmap_data)
 
     # Ensure the new entry is now included
-    assert_equal 2, third_response_data[:days].count { |day| day[:total_changes].positive? }, "Expected two entries in the heatmap data after clearing the cache"
+    assert_equal 2, third_response_data[:data].values.count { |day| day[:total_changes].positive? }, "Expected two entries in the heatmap data after clearing the cache"
 
     # Reset caching config to defaults
     Rails.cache.clear
@@ -398,7 +396,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
 
     get user_path(user.display_name)
     assert_response :success
-    assert_empty(assigns(:heatmap_data)[:days].select { |e| e[:total_changes].positive? })
+    assert_empty(assigns(:heatmap_data)[:data].values)
     assert_select ".heatmap", :count => 0
   end
 
-- 
2.39.5