From 5343c92b6ec40816a11cc510c2917179c8b04ff2 Mon Sep 17 00:00:00 2001 From: Mahad Kalam Date: Fri, 8 May 2026 17:16:06 +0100 Subject: [PATCH 1/3] Clean up leaderboard code --- .../api/v1/leaderboard_controller.rb | 4 +- app/controllers/leaderboards_controller.rb | 19 +-- app/jobs/cleanup_old_leaderboards_job.rb | 8 +- app/jobs/leaderboard_update_job.rb | 81 +--------- app/models/leaderboard.rb | 52 +++++- app/models/leaderboard/builder.rb | 123 ++++++++++++++ app/services/leaderboard_cache.rb | 25 --- app/services/leaderboard_date_range.rb | 22 --- app/services/leaderboard_service.rb | 25 --- .../jobs/cleanup_old_leaderboards_job_test.rb | 43 +++++ test/models/leaderboard/builder_test.rb | 143 +++++++++++++++++ test/models/leaderboard_test.rb | 151 ++++++++++++++++++ 12 files changed, 524 insertions(+), 172 deletions(-) create mode 100644 app/models/leaderboard/builder.rb delete mode 100644 app/services/leaderboard_cache.rb delete mode 100644 app/services/leaderboard_date_range.rb delete mode 100644 app/services/leaderboard_service.rb create mode 100644 test/jobs/cleanup_old_leaderboards_job_test.rb create mode 100644 test/models/leaderboard/builder_test.rb create mode 100644 test/models/leaderboard_test.rb diff --git a/app/controllers/api/v1/leaderboard_controller.rb b/app/controllers/api/v1/leaderboard_controller.rb index f67fc6cbf..043aa6a3b 100644 --- a/app/controllers/api/v1/leaderboard_controller.rb +++ b/app/controllers/api/v1/leaderboard_controller.rb @@ -1,6 +1,6 @@ class Api::V1::LeaderboardController < ApplicationController def daily - leaderboard = LeaderboardService.get(period: :daily, date: Date.current) + leaderboard = Leaderboard.fetch(period: :daily, date: Date.current) if leaderboard.nil? render json: { error: "Leaderboard is being generated" }, status: :service_unavailable @@ -10,7 +10,7 @@ def daily end def weekly - leaderboard = LeaderboardService.get(period: :last_7_days, date: Date.current) + leaderboard = Leaderboard.fetch(period: :last_7_days, date: Date.current) if leaderboard.nil? render json: { error: "Leaderboard is being generated" }, status: :service_unavailable diff --git a/app/controllers/leaderboards_controller.rb b/app/controllers/leaderboards_controller.rb index 6358d19cc..95ef23144 100644 --- a/app/controllers/leaderboards_controller.rb +++ b/app/controllers/leaderboards_controller.rb @@ -6,7 +6,7 @@ def index country = load_country_context leaderboard_scope = validated_leaderboard_scope(country) - leaderboard = LeaderboardService.get(period: period_type, date: Date.current) + leaderboard = Leaderboard.fetch(period: period_type, date: Date.current) render inertia: "Leaderboards/Index", props: { period_type: period_type.to_s, @@ -76,23 +76,12 @@ def entries_payload(leaderboard, scope, country) active_projects = Cache::ActiveProjectsJob.perform_now entries = payload[:entries].map do |e| - user = e[:user] proj = active_projects&.dig(e[:user_id]) - { - user_id: e[:user_id], - total_seconds: e[:total_seconds], - streak_count: e[:streak_count], + e.merge( is_current_user: e[:user_id] == current_user&.id, - user: { - display_name: user[:display_name], - avatar_url: user[:avatar_url], - profile_path: user[:profile_path], - verified: user[:verified], - country_code: user[:country_code], - red: user[:red] - }, + user: e[:user].except(:id), active_project: proj ? { name: proj.project_name, repo_url: proj.repo_url } : nil - } + ) end { diff --git a/app/jobs/cleanup_old_leaderboards_job.rb b/app/jobs/cleanup_old_leaderboards_job.rb index 247f217e7..713f464cf 100644 --- a/app/jobs/cleanup_old_leaderboards_job.rb +++ b/app/jobs/cleanup_old_leaderboards_job.rb @@ -1,10 +1,14 @@ class CleanupOldLeaderboardsJob < ApplicationJob queue_as :literally_whenever # fucking wild that this exists + # `RETAIN_DAYS = 2` keeps today + 2 prior days of boards (3 dates total) + # before reaping older ones. Boards with `start_date < (today - 2)` go. + RETAIN_DAYS = 2 + def perform - cutoff = 2.days.ago.beginning_of_day + cutoff = RETAIN_DAYS.days.ago.to_date - old_leaderboards = Leaderboard.where("created_at < ?", cutoff) + old_leaderboards = Leaderboard.where(start_date: ...cutoff) count = old_leaderboards.count return if count.zero? diff --git a/app/jobs/leaderboard_update_job.rb b/app/jobs/leaderboard_update_job.rb index 8aaf64351..562ff62c7 100644 --- a/app/jobs/leaderboard_update_job.rb +++ b/app/jobs/leaderboard_update_job.rb @@ -11,85 +11,6 @@ class LeaderboardUpdateJob < ApplicationJob ) def perform(period = :daily, date = Date.current, force_update: false) - date = LeaderboardDateRange.normalize_date(date, period) - build_leaderboard(date, period, force_update) - end - - private - - def build_leaderboard(date, period, force_update = false) - generation_started_at = Process.clock_gettime(Process::CLOCK_MONOTONIC) - board = ::Leaderboard.find_or_create_by!( - start_date: date, - period_type: period, - timezone_utc_offset: nil - ) - - return board if board.finished_generating_at.present? && !force_update - - Rails.logger.info "Building leaderboard for #{period} on #{date}" - - range = LeaderboardDateRange.calculate(date, period) - timestamp = Time.current - eligible_users = User.where.not(github_uid: nil) - .where.not(trust_level: User.trust_levels[:red]) - - ActiveRecord::Base.transaction do - heartbeat_query = Heartbeat.where(user_id: eligible_users.select(:id), time: range) - .leaderboard_eligible - - data = heartbeat_query.group(:user_id).duration_seconds - .filter { |_, seconds| seconds > 60 } - - # Two-phase streak computation: query 8 days of data first (covers - # most users whose streaks are < 7 days), then extend to 31 days - # only for users whose streak maxed out the short window. - streaks = Heartbeat.daily_streaks_for_users(data.keys, start_date: 8.days.ago, exclude_browser_time: true) - - needs_full_history = streaks.select { |_, streak| streak >= 6 }.keys - if needs_full_history.any? - needs_full_history.each { |id| Rails.cache.delete("user_streak_without_browser_v3_#{id}") } - full_streaks = Heartbeat.daily_streaks_for_users(needs_full_history, start_date: 31.days.ago, exclude_browser_time: true) - streaks.merge!(full_streaks) - end - - entries = data.map do |user_id, seconds| - { - leaderboard_id: board.id, - user_id: user_id, - total_seconds: seconds, - streak_count: streaks[user_id] || 0, - created_at: timestamp, - updated_at: timestamp - } - end - - LeaderboardEntry.upsert_all(entries, unique_by: %i[leaderboard_id user_id]) if entries.any? - - if data.keys.any? - board.entries.where.not(user_id: data.keys).delete_all - else - board.entries.delete_all - end - - finished_at = Time.current - generation_duration_seconds = [ - (Process.clock_gettime(Process::CLOCK_MONOTONIC) - generation_started_at).ceil, - 1 - ].max - - board.update!( - finished_generating_at: finished_at, - generation_duration_seconds: generation_duration_seconds - ) - end - - cache_key = LeaderboardCache.global_key(period, date) - LeaderboardCache.write(cache_key, board) - LeaderboardPageCache.warm(leaderboard: board) - - Rails.logger.debug "Persisted leaderboard for #{period} with #{board.entries.count} entries" - - board + Leaderboard.regenerate(period: period, date: date, force: force_update) end end diff --git a/app/models/leaderboard.rb b/app/models/leaderboard.rb index 9fcc33a37..2b9057cf1 100644 --- a/app/models/leaderboard.rb +++ b/app/models/leaderboard.rb @@ -1,5 +1,5 @@ class Leaderboard < ApplicationRecord - GLOBAL_TIMEZONE = "UTC" + CACHE_EXPIRATION = 10.minutes has_many :entries, class_name: "LeaderboardEntry", @@ -12,6 +12,42 @@ class Leaderboard < ApplicationRecord last_7_days: 2 } + scope :ready, -> { + where.not(finished_generating_at: nil).where(deleted_at: nil, timezone_utc_offset: nil) + } + + def self.fetch(period: :daily, date: Date.current) + period = period.to_sym + date = normalize_date(date, period) + key = cache_key(period, date) + + if (cached = Rails.cache.read(key)) + return cached + end + + board = ready.find_by(start_date: date, period_type: period) + if board + write_cache(board, period: period, date: date) + return board + end + + LeaderboardUpdateJob.perform_later(period, date) + nil + end + + def self.regenerate(period:, date:, force: false) + Builder.new(period: period, date: date).call(force: force) + end + + def self.normalize_date(date, _period) + date = Date.current if date.blank? + date.is_a?(Date) ? date : Date.parse(date.to_s) + end + + def self.write_cache(board, period:, date:) + Rails.cache.write(cache_key(period, date), board, expires_in: CACHE_EXPIRATION) + end + def finished_generating? finished_generating_at.present? end @@ -20,6 +56,15 @@ def period_end_date start_date end + def range + case period_type.to_sym + when :last_7_days + ((start_date - 6.days).beginning_of_day...start_date.end_of_day) + else + 24.hours.ago...Time.current + end + end + def date_range_text if last_7_days? "#{(start_date - 6.days).strftime('%b %d')} - #{start_date.strftime('%b %d, %Y')}" @@ -27,4 +72,9 @@ def date_range_text "Last 24 hours" end end + + def self.cache_key(period, date) + "leaderboard_#{period}_#{date}" + end + private_class_method :cache_key end diff --git a/app/models/leaderboard/builder.rb b/app/models/leaderboard/builder.rb new file mode 100644 index 000000000..714431923 --- /dev/null +++ b/app/models/leaderboard/builder.rb @@ -0,0 +1,123 @@ +class Leaderboard + # Internal: builds (or rebuilds) the entries for a Leaderboard row. + # Use Leaderboard.regenerate(period:, date:) as the public entry point. + # + # Responsibilities, all in one place so the build invariants live together: + # - Find/create the persisted Leaderboard row for (period, date). + # - Aggregate eligible heartbeat durations over the board's range. + # - Compute streaks (with a two-phase short/long window optimization). + # - Upsert LeaderboardEntries and prune stale ones. + # - Mark the board finished and warm both the lookup + page caches. + class Builder + MIN_TOTAL_SECONDS = 60 + SHORT_STREAK_WINDOW = 8.days + FULL_STREAK_WINDOW = 31.days + SHORT_STREAK_MAX = 6 + + def initialize(period:, date:) + @period = period.to_sym + @date = Leaderboard.normalize_date(date, @period) + end + + def call(force: false) + board = find_or_create_board + return board if board.finished_generating? && !force + + Rails.logger.info "Building leaderboard for #{@period} on #{@date}" + generation_started = Process.clock_gettime(Process::CLOCK_MONOTONIC) + + ActiveRecord::Base.transaction do + upsert_entries(board) + finalize(board, generation_started) + end + + Leaderboard.write_cache(board, period: @period, date: @date) + LeaderboardPageCache.warm(leaderboard: board) + + Rails.logger.debug "Persisted leaderboard for #{@period} with #{board.entries.count} entries" + board + end + + private + + def find_or_create_board + Leaderboard.find_or_create_by!( + start_date: @date, + period_type: @period, + timezone_utc_offset: nil + ) + end + + def upsert_entries(board) + data = heartbeat_durations(board.range) + streaks = streaks_for(data.keys) + timestamp = Time.current + + entries = data.map do |user_id, seconds| + { + leaderboard_id: board.id, + user_id: user_id, + total_seconds: seconds, + streak_count: streaks[user_id] || 0, + created_at: timestamp, + updated_at: timestamp + } + end + + LeaderboardEntry.upsert_all(entries, unique_by: %i[leaderboard_id user_id]) if entries.any? + + if data.keys.any? + board.entries.where.not(user_id: data.keys).delete_all + else + board.entries.delete_all + end + end + + def heartbeat_durations(range) + eligible_users = User.where.not(github_uid: nil) + .where.not(trust_level: User.trust_levels[:red]) + + Heartbeat.where(user_id: eligible_users.select(:id), time: range) + .leaderboard_eligible + .group(:user_id) + .duration_seconds + .filter { |_, seconds| seconds > MIN_TOTAL_SECONDS } + end + + # Two-phase streak computation: query a short window first (covers most + # users whose streaks are < 7 days), then extend to the full window only + # for users whose streak maxed out the short window. + def streaks_for(user_ids) + return {} if user_ids.empty? + + streaks = Heartbeat.daily_streaks_for_users( + user_ids, + start_date: SHORT_STREAK_WINDOW.ago, + exclude_browser_time: true + ) + + maxed = streaks.select { |_, s| s >= SHORT_STREAK_MAX }.keys + return streaks if maxed.empty? + + maxed.each { |id| Rails.cache.delete("user_streak_without_browser_v3_#{id}") } + streaks.merge( + Heartbeat.daily_streaks_for_users( + maxed, + start_date: FULL_STREAK_WINDOW.ago, + exclude_browser_time: true + ) + ) + end + + def finalize(board, started_at) + duration = [ + (Process.clock_gettime(Process::CLOCK_MONOTONIC) - started_at).ceil, + 1 + ].max + board.update!( + finished_generating_at: Time.current, + generation_duration_seconds: duration + ) + end + end +end diff --git a/app/services/leaderboard_cache.rb b/app/services/leaderboard_cache.rb deleted file mode 100644 index 740d27d83..000000000 --- a/app/services/leaderboard_cache.rb +++ /dev/null @@ -1,25 +0,0 @@ -module LeaderboardCache - CACHE_EXPIRATION = 10.minutes - - module_function - - def global_key(period, date) - "leaderboard_#{period}_#{date}" - end - - def timezone_key(offset, date, period) - "tz_leaderboard_#{offset}_#{date}_#{period}" - end - - def write(key, data) - Rails.cache.write(key, data, expires_in: CACHE_EXPIRATION) - end - - def read(key) - Rails.cache.read(key) - end - - def fetch(key, &block) - Rails.cache.fetch(key, expires_in: CACHE_EXPIRATION, &block) - end -end diff --git a/app/services/leaderboard_date_range.rb b/app/services/leaderboard_date_range.rb deleted file mode 100644 index d803b43aa..000000000 --- a/app/services/leaderboard_date_range.rb +++ /dev/null @@ -1,22 +0,0 @@ -module LeaderboardDateRange - module_function - - def calculate(date, period) - case period - when :weekly - (date.beginning_of_day...(date + 7.days).beginning_of_day) - when :last_7_days - ((date - 6.days).beginning_of_day...date.end_of_day) - else - # Daily leaderboards are intentionally rolling; date only keys the persisted board. - (24.hours.ago...Time.current) - end - end - - def normalize_date(date, period) - date = Date.current if date.blank? - date = date.is_a?(Date) ? date : Date.parse(date.to_s) - date = date.beginning_of_week if period == :weekly - date - end -end diff --git a/app/services/leaderboard_service.rb b/app/services/leaderboard_service.rb deleted file mode 100644 index d7f0d3288..000000000 --- a/app/services/leaderboard_service.rb +++ /dev/null @@ -1,25 +0,0 @@ -class LeaderboardService - def self.get(period: :daily, date: Date.current) - new.get(period: period, date: date) - end - - def get(period: :daily, date: Date.current) - date = Date.current if date.blank? - date = LeaderboardDateRange.normalize_date(date, period) - - key = LeaderboardCache.global_key(period, date) - board = LeaderboardCache.read(key) - return board if board.present? - - board = ::Leaderboard.where.not(finished_generating_at: nil) - .find_by(start_date: date, period_type: period, timezone_utc_offset: nil, deleted_at: nil) - - if board.present? - LeaderboardCache.write(key, board) - return board - end - - ::LeaderboardUpdateJob.perform_later(period, date) - nil - end -end diff --git a/test/jobs/cleanup_old_leaderboards_job_test.rb b/test/jobs/cleanup_old_leaderboards_job_test.rb new file mode 100644 index 000000000..d7e02bb23 --- /dev/null +++ b/test/jobs/cleanup_old_leaderboards_job_test.rb @@ -0,0 +1,43 @@ +require "test_helper" + +class CleanupOldLeaderboardsJobTest < ActiveJob::TestCase + test "deletes boards whose start_date is older than the retention window" do + old = Leaderboard.create!( + start_date: 5.days.ago.to_date, + period_type: :daily, + finished_generating_at: 5.days.ago + ) + + CleanupOldLeaderboardsJob.perform_now + + refute Leaderboard.exists?(old.id) + end + + test "preserves today's board even if its created_at is older than retention" do + # Boards are upserted by (start_date, period_type) — today's daily board's + # row may have been created days ago, but the period itself is still active. + # Reaping by created_at would delete a live board out from under the cron + # job; this test pins the corrected behavior. + today_board = Leaderboard.create!( + start_date: Date.current, + period_type: :daily, + finished_generating_at: Time.current + ) + today_board.update_columns(created_at: 30.days.ago) + + CleanupOldLeaderboardsJob.perform_now + + assert Leaderboard.exists?(today_board.id) + end + + test "is a no-op when nothing is past retention" do + Leaderboard.create!( + start_date: Date.current, + period_type: :daily, + finished_generating_at: Time.current + ) + + assert_nothing_raised { CleanupOldLeaderboardsJob.perform_now } + assert_equal 1, Leaderboard.count + end +end diff --git a/test/models/leaderboard/builder_test.rb b/test/models/leaderboard/builder_test.rb new file mode 100644 index 000000000..3a85e6ff4 --- /dev/null +++ b/test/models/leaderboard/builder_test.rb @@ -0,0 +1,143 @@ +require "test_helper" + +class Leaderboard::BuilderTest < ActiveSupport::TestCase + setup do + Rails.cache.clear + end + + teardown do + Rails.cache.clear + end + + test "force: true rebuilds a finished board (overwrites entries and bumps timestamp)" do + user = create_user(username: "force_user", github_uid: "GH_FORCE") + board = Leaderboard.create!( + start_date: Date.current, + period_type: :daily, + finished_generating_at: 1.hour.ago + ) + # Pre-existing stale entry from a previous build. + LeaderboardEntry.create!(leaderboard: board, user: user, total_seconds: 999) + original_finished_at = board.finished_generating_at + + create_heartbeat_pair(user: user, started_at: 2.hours.ago) + + Leaderboard::Builder.new(period: :daily, date: Date.current).call(force: true) + + board.reload + assert_operator board.finished_generating_at, :>, original_finished_at + # Stale 999 row got overwritten with the real ~120s aggregation. + assert_equal 120, board.entries.find_by!(user_id: user.id).total_seconds + end + + test "filters users below MIN_TOTAL_SECONDS (60s)" do + short_user = create_user(username: "short_user", github_uid: "GH_SHORT") + long_user = create_user(username: "long_user", github_uid: "GH_LONG") + + # 30s pair — below the 60s floor. + create_heartbeat_pair(user: short_user, started_at: 1.hour.ago, gap_seconds: 30) + # 120s pair — above the floor. + create_heartbeat_pair(user: long_user, started_at: 1.hour.ago, gap_seconds: 120) + + Leaderboard::Builder.new(period: :daily, date: Date.current).call(force: true) + + board = Leaderboard.find_by!(start_date: Date.current, period_type: :daily) + assert_equal [ long_user.id ], board.entries.pluck(:user_id) + end + + test "empty data path prunes all stale entries" do + user = create_user(username: "stale_user", github_uid: "GH_STALE") + board = Leaderboard.create!(start_date: Date.current, period_type: :daily) + LeaderboardEntry.create!(leaderboard: board, user: user, total_seconds: 500) + # No heartbeats produced — but a stale entry exists. The builder should + # delete it on rebuild. + + Leaderboard::Builder.new(period: :daily, date: Date.current).call + + assert_equal 0, board.reload.entries.count + end + + test "delegates two-phase streak optimization to Heartbeat.daily_streaks_for_users" do + # The two-phase optimization queries Heartbeat.daily_streaks_for_users + # twice when any user's streak hits the SHORT_STREAK_MAX (6) ceiling. + # We pin the contract by stubbing the underlying call and asserting + # both calls occur with the expected windows. + user = create_user(username: "streaky", github_uid: "GH_STREAKY") + create_heartbeat_pair(user: user, started_at: 2.hours.ago) + + short_window_seen = false + full_window_seen = false + + Heartbeat.singleton_class.alias_method :__orig_daily_streaks, :daily_streaks_for_users + Heartbeat.define_singleton_method(:daily_streaks_for_users) do |ids, start_date:, **rest| + window_days = ((Time.current - start_date) / 1.day).round + if window_days <= 8 + short_window_seen = true + # Force every queried user to look maxed-out so the second phase fires. + ids.to_h { |id| [ id, Leaderboard::Builder::SHORT_STREAK_MAX ] } + else + full_window_seen = true + ids.to_h { |id| [ id, 12 ] } + end + end + + begin + Leaderboard::Builder.new(period: :daily, date: Date.current).call(force: true) + ensure + Heartbeat.singleton_class.alias_method :daily_streaks_for_users, :__orig_daily_streaks + Heartbeat.singleton_class.remove_method :__orig_daily_streaks + end + + assert short_window_seen, "expected short-window streak query to run" + assert full_window_seen, "expected full-window streak query to run for maxed users" + + board = Leaderboard.find_by!(start_date: Date.current, period_type: :daily) + assert_equal 12, board.entries.find_by!(user_id: user.id).streak_count + end + + test "skips full-window streak query when no user maxes the short window" do + user = create_user(username: "low_streak", github_uid: "GH_LOWS") + create_heartbeat_pair(user: user, started_at: 2.hours.ago) + + full_window_called = false + + Heartbeat.singleton_class.alias_method :__orig_daily_streaks, :daily_streaks_for_users + Heartbeat.define_singleton_method(:daily_streaks_for_users) do |ids, start_date:, **rest| + window_days = ((Time.current - start_date) / 1.day).round + if window_days <= 8 + ids.to_h { |id| [ id, 1 ] } + else + full_window_called = true + ids.to_h { |id| [ id, 99 ] } + end + end + + begin + Leaderboard::Builder.new(period: :daily, date: Date.current).call(force: true) + ensure + Heartbeat.singleton_class.alias_method :daily_streaks_for_users, :__orig_daily_streaks + Heartbeat.singleton_class.remove_method :__orig_daily_streaks + end + + refute full_window_called, "full-window query should be skipped when no user maxes short window" + board = Leaderboard.find_by!(start_date: Date.current, period_type: :daily) + assert_equal 1, board.entries.find_by!(user_id: user.id).streak_count + end + + private + + def create_user(username:, github_uid:) + User.create!(username: username, github_uid: github_uid, timezone: "UTC") + end + + def create_heartbeat_pair(user:, started_at:, gap_seconds: 120) + user.heartbeats.create!( + entity: "src/file.rb", type: "file", category: "coding", editor: "vscode", + time: started_at.to_f, project: "builder-test", source_type: :test_entry + ) + user.heartbeats.create!( + entity: "src/file.rb", type: "file", category: "coding", editor: "vscode", + time: (started_at + gap_seconds).to_f, project: "builder-test", source_type: :test_entry + ) + end +end diff --git a/test/models/leaderboard_test.rb b/test/models/leaderboard_test.rb new file mode 100644 index 000000000..f79407e93 --- /dev/null +++ b/test/models/leaderboard_test.rb @@ -0,0 +1,151 @@ +require "test_helper" + +class LeaderboardTest < ActiveSupport::TestCase + include ActiveJob::TestHelper + + setup do + Rails.cache.clear + # `perform_later` enqueues onto the GoodJob backend, which we don't + # want firing in unit tests; capture into ActiveJob's TestAdapter. + ActiveJob::Base.queue_adapter = :test + end + + teardown do + Rails.cache.clear + end + + # --- normalize_date ------------------------------------------------------- + + test "normalize_date returns today when blank" do + travel_to Time.zone.local(2024, 3, 20, 12) do + assert_equal Date.new(2024, 3, 20), Leaderboard.normalize_date(nil, :daily) + assert_equal Date.new(2024, 3, 20), Leaderboard.normalize_date("", :daily) + end + end + + test "normalize_date parses string dates" do + assert_equal Date.new(2024, 3, 20), Leaderboard.normalize_date("2024-03-20", :daily) + assert_equal Date.new(2024, 3, 20), Leaderboard.normalize_date("2024-03-20", :last_7_days) + end + + test "normalize_date passes through Date objects" do + d = Date.new(2024, 3, 20) + assert_equal d, Leaderboard.normalize_date(d, :daily) + end + + # --- range --------------------------------------------------------------- + + test "range for daily is rolling 24h ending now" do + travel_to Time.zone.local(2024, 3, 20, 15, 30) do + board = Leaderboard.new(start_date: Date.current, period_type: :daily) + r = board.range + assert_in_delta 24.hours.ago.to_f, r.first.to_f, 1 + assert_in_delta Time.current.to_f, r.last.to_f, 1 + end + end + + test "range for last_7_days is anchored on start_date" do + board = Leaderboard.new(start_date: Date.new(2024, 3, 20), period_type: :last_7_days) + r = board.range + assert_equal Date.new(2024, 3, 14).beginning_of_day, r.first + assert_equal Date.new(2024, 3, 20).end_of_day.to_i, r.last.to_i + end + + # --- fetch --------------------------------------------------------------- + + test "fetch returns nil and enqueues regeneration when no finished board exists" do + assert_enqueued_with(job: LeaderboardUpdateJob, args: [ :daily, Date.current ]) do + assert_nil Leaderboard.fetch(period: :daily, date: Date.current) + end + end + + test "fetch returns the persisted finished board" do + board = Leaderboard.create!( + start_date: Date.current, + period_type: :daily, + finished_generating_at: Time.current + ) + + fetched = Leaderboard.fetch(period: :daily, date: Date.current) + assert_equal board.id, fetched.id + end + + test "fetch ignores in-progress (unfinished) boards and enqueues regeneration" do + Leaderboard.create!(start_date: Date.current, period_type: :daily, finished_generating_at: nil) + + assert_enqueued_with(job: LeaderboardUpdateJob) do + assert_nil Leaderboard.fetch(period: :daily, date: Date.current) + end + end + + test "fetch ignores soft-deleted boards" do + Leaderboard.create!( + start_date: Date.current, + period_type: :daily, + finished_generating_at: Time.current, + deleted_at: Time.current + ) + + assert_enqueued_with(job: LeaderboardUpdateJob) do + assert_nil Leaderboard.fetch(period: :daily, date: Date.current) + end + end + + test "fetch caches subsequent lookups, avoiding repeated DB hits" do + board = Leaderboard.create!( + start_date: Date.current, + period_type: :daily, + finished_generating_at: Time.current + ) + + with_memory_cache do + Leaderboard.fetch(period: :daily, date: Date.current) + Leaderboard.where(id: board.id).delete_all + cached = Leaderboard.fetch(period: :daily, date: Date.current) + assert_equal board.id, cached.id + end + end + + test "fetch accepts string period and date" do + Leaderboard.create!( + start_date: Date.new(2024, 3, 20), + period_type: :last_7_days, + finished_generating_at: Time.current + ) + + fetched = Leaderboard.fetch(period: "last_7_days", date: "2024-03-20") + refute_nil fetched + assert_equal "last_7_days", fetched.period_type + end + + test "regenerate creates a board when none exists" do + travel_to Time.zone.local(2024, 3, 20, 12) do + board = Leaderboard.regenerate(period: :daily, date: Date.current, force: true) + assert board.persisted? + assert board.finished_generating? + assert_equal "daily", board.period_type + end + end + + test "regenerate is a no-op for an already-finished board unless forced" do + board = Leaderboard.create!( + start_date: Date.current, + period_type: :daily, + finished_generating_at: 1.hour.ago + ) + finished_at = board.finished_generating_at + + Leaderboard.regenerate(period: :daily, date: Date.current) + assert_equal finished_at.to_i, board.reload.finished_generating_at.to_i + end + + private + + def with_memory_cache + original = Rails.cache + Rails.cache = ActiveSupport::Cache::MemoryStore.new + yield + ensure + Rails.cache = original + end +end From 5078c46225698a10d9b24b1e76c7385f22886901 Mon Sep 17 00:00:00 2001 From: Mahad Kalam Date: Fri, 8 May 2026 17:29:20 +0100 Subject: [PATCH 2/3] Fix tests --- test/models/leaderboard_test.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/models/leaderboard_test.rb b/test/models/leaderboard_test.rb index f79407e93..6e0022c27 100644 --- a/test/models/leaderboard_test.rb +++ b/test/models/leaderboard_test.rb @@ -7,11 +7,16 @@ class LeaderboardTest < ActiveSupport::TestCase Rails.cache.clear # `perform_later` enqueues onto the GoodJob backend, which we don't # want firing in unit tests; capture into ActiveJob's TestAdapter. + # Save the original adapter so we restore it in teardown — without + # this, the :test adapter leaks into other test files that depend + # on the real GoodJob backend (e.g. HeartbeatExportJobTest). + @original_queue_adapter = ActiveJob::Base.queue_adapter ActiveJob::Base.queue_adapter = :test end teardown do Rails.cache.clear + ActiveJob::Base.queue_adapter = @original_queue_adapter end # --- normalize_date ------------------------------------------------------- From 613304926aa8c0edcf0b778d03a5b168108ecd78 Mon Sep 17 00:00:00 2001 From: Mahad Kalam Date: Fri, 8 May 2026 17:35:20 +0100 Subject: [PATCH 3/3] warg --- app/models/leaderboard.rb | 4 ++-- app/models/leaderboard/builder.rb | 2 +- test/models/leaderboard_test.rb | 9 ++++----- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/app/models/leaderboard.rb b/app/models/leaderboard.rb index 2b9057cf1..fcec9b6e6 100644 --- a/app/models/leaderboard.rb +++ b/app/models/leaderboard.rb @@ -18,7 +18,7 @@ class Leaderboard < ApplicationRecord def self.fetch(period: :daily, date: Date.current) period = period.to_sym - date = normalize_date(date, period) + date = normalize_date(date) key = cache_key(period, date) if (cached = Rails.cache.read(key)) @@ -39,7 +39,7 @@ def self.regenerate(period:, date:, force: false) Builder.new(period: period, date: date).call(force: force) end - def self.normalize_date(date, _period) + def self.normalize_date(date) date = Date.current if date.blank? date.is_a?(Date) ? date : Date.parse(date.to_s) end diff --git a/app/models/leaderboard/builder.rb b/app/models/leaderboard/builder.rb index 714431923..4c829d14c 100644 --- a/app/models/leaderboard/builder.rb +++ b/app/models/leaderboard/builder.rb @@ -16,7 +16,7 @@ class Builder def initialize(period:, date:) @period = period.to_sym - @date = Leaderboard.normalize_date(date, @period) + @date = Leaderboard.normalize_date(date) end def call(force: false) diff --git a/test/models/leaderboard_test.rb b/test/models/leaderboard_test.rb index 6e0022c27..5024cc3f9 100644 --- a/test/models/leaderboard_test.rb +++ b/test/models/leaderboard_test.rb @@ -23,19 +23,18 @@ class LeaderboardTest < ActiveSupport::TestCase test "normalize_date returns today when blank" do travel_to Time.zone.local(2024, 3, 20, 12) do - assert_equal Date.new(2024, 3, 20), Leaderboard.normalize_date(nil, :daily) - assert_equal Date.new(2024, 3, 20), Leaderboard.normalize_date("", :daily) + assert_equal Date.new(2024, 3, 20), Leaderboard.normalize_date(nil) + assert_equal Date.new(2024, 3, 20), Leaderboard.normalize_date("") end end test "normalize_date parses string dates" do - assert_equal Date.new(2024, 3, 20), Leaderboard.normalize_date("2024-03-20", :daily) - assert_equal Date.new(2024, 3, 20), Leaderboard.normalize_date("2024-03-20", :last_7_days) + assert_equal Date.new(2024, 3, 20), Leaderboard.normalize_date("2024-03-20") end test "normalize_date passes through Date objects" do d = Date.new(2024, 3, 20) - assert_equal d, Leaderboard.normalize_date(d, :daily) + assert_equal d, Leaderboard.normalize_date(d) end # --- range ---------------------------------------------------------------