diff --git a/app/controllers/api/v1/leaderboard_controller.rb b/app/controllers/api/v1/leaderboard_controller.rb index 51e9affb8..fac7e5883 100644 --- a/app/controllers/api/v1/leaderboard_controller.rb +++ b/app/controllers/api/v1/leaderboard_controller.rb @@ -14,15 +14,10 @@ def render_period(period) end def format_leaderboard(leaderboard) - entries = leaderboard.entries - .joins(:user) - .where(users: { leaderboard_shadowbanned: false }) - .preload(:user) - .order(total_seconds: :desc) - .map.with_index do |entry, idx| - { rank: idx + 1, - user: { id: entry.user.id, username: entry.user.display_name, avatar_url: entry.user.avatar_url }, - total_seconds: entry.total_seconds } + entries = LeaderboardEntries.fetch_public(leaderboard: leaderboard)[:entries].map do |entry| + { rank: entry[:rank], + user: { id: entry.dig(:user, :id), username: entry.dig(:user, :display_name), avatar_url: entry.dig(:user, :avatar_url) }, + total_seconds: entry[:total_seconds] } end { diff --git a/app/controllers/leaderboards_controller.rb b/app/controllers/leaderboards_controller.rb index 27f98224f..27b5ab7fb 100644 --- a/app/controllers/leaderboards_controller.rb +++ b/app/controllers/leaderboards_controller.rb @@ -64,44 +64,13 @@ def leaderboard_metadata(leaderboard) end def entries_payload(leaderboard, scope, country) - return { entries: [], total: 0 } unless leaderboard&.persisted? - country_code = (scope == :country && country[:available]) ? country[:code] : nil - payload = LeaderboardPageCache.fetch( + LeaderboardEntries.fetch( leaderboard: leaderboard, scope: scope, - country_code: country_code + country_code: country_code, + viewer: current_user, + include_active_projects: true ) - - active_projects = Cache::ActiveProjectsJob.perform_now - - visible_entries = payload[:entries].reject do |e| - e.dig(:user, :shadowbanned) && e[:user_id] != current_user&.id - end - - entries = visible_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], - 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] - }, - active_project: proj ? { name: proj.project_name, repo_url: proj.repo_url } : nil - } - end - - { - entries: entries, - total: visible_entries.size - } end end diff --git a/app/jobs/leaderboard_update_job.rb b/app/jobs/leaderboard_update_job.rb index cc2c2463e..7f33fc33c 100644 --- a/app/jobs/leaderboard_update_job.rb +++ b/app/jobs/leaderboard_update_job.rb @@ -62,6 +62,7 @@ def build_leaderboard(date, period, force_update = false) LeaderboardCache.write(LeaderboardCache.global_key(period, date), board) LeaderboardPageCache.warm(leaderboard: board) + LeaderboardEntries.warm_public(leaderboard: board) Rails.logger.debug "Persisted leaderboard for #{period} with #{board.entries.count} entries" board end diff --git a/app/services/leaderboard_entries.rb b/app/services/leaderboard_entries.rb new file mode 100644 index 000000000..efea064ce --- /dev/null +++ b/app/services/leaderboard_entries.rb @@ -0,0 +1,116 @@ +class LeaderboardEntries + CACHE_EXPIRATION = 10.minutes + + def self.fetch(...) = new(...).fetch + def self.fetch_public(leaderboard:) = new(leaderboard:).fetch_public + def self.warm_public(leaderboard:) = fetch_public(leaderboard:) + + def initialize(leaderboard:, viewer: nil, scope: :global, country_code: nil, include_active_projects: false) + @leaderboard = leaderboard + @viewer = viewer + @scope = scope.to_sym + @country_code = country_code + @include_active_projects = include_active_projects + end + + def fetch + return { entries: [], total: 0 } unless @leaderboard&.persisted? + + active_projects = @include_active_projects ? Cache::ActiveProjectsJob.perform_now : nil + entries = visible_cached_entries.map.with_index(1) do |entry, rank| + entry_payload(entry, rank:, active_projects:) + end + + { entries:, total: entries.size } + end + + def fetch_public + return { entries: [], total: 0 } unless @leaderboard&.persisted? + + entries = Rails.cache.fetch(public_cache_key, expires_in: CACHE_EXPIRATION) do + public_entries_from_database + end + + { entries:, total: entries.size } + end + + private + + def public_cache_key + "leaderboard_entries/public/v1/#{LeaderboardPageCache.version}/#{@leaderboard.cache_key_with_version}" + end + + def public_entries_from_database + @leaderboard.entries + .joins(:user) + .where(users: { leaderboard_shadowbanned: false }) + .preload(:user) + .order(total_seconds: :desc, user_id: :asc) + .map.with_index(1) do |row, rank| + public_entry_payload(row, rank:) + end + end + + def public_entry_payload(entry, rank:) + { + rank:, + user_id: entry.user_id, + total_seconds: entry.total_seconds, + streak_count: entry.streak_count, + is_current_user: false, + user: { + id: entry.user.id, + display_name: entry.user.display_name, + avatar_url: entry.user.avatar_url + }, + active_project: nil + } + end + + def visible_cached_entries + cached_entries.reject { |entry| entry_hidden_from_viewer?(entry) } + end + + def cached_entries + LeaderboardPageCache.fetch( + leaderboard: @leaderboard, + scope: @scope, + country_code: @country_code + )[:entries] + end + + def entry_hidden_from_viewer?(entry) + entry.dig(:user, :shadowbanned) && entry[:user_id] != @viewer&.id + end + + def entry_payload(entry, rank:, active_projects:) + user = entry[:user] + { + rank:, + user_id: entry[:user_id], + total_seconds: entry[:total_seconds], + streak_count: entry[:streak_count], + is_current_user: entry[:user_id] == @viewer&.id, + user: user_payload(user), + active_project: active_project_payload(active_projects&.dig(entry[:user_id])) + } + end + + def user_payload(user) + { + id: user[:id], + 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] + } + end + + def active_project_payload(active_project) + return nil unless active_project + + { name: active_project.project_name, repo_url: active_project.repo_url } + end +end diff --git a/app/services/leaderboard_page_cache.rb b/app/services/leaderboard_page_cache.rb index 0d80183c1..4b53d47d6 100644 --- a/app/services/leaderboard_page_cache.rb +++ b/app/services/leaderboard_page_cache.rb @@ -14,6 +14,8 @@ def clear! Rails.cache.write(version_key, SecureRandom.uuid) end + def version = cache_version + private def version_key = "leaderboard_page/v2/version" @@ -34,7 +36,7 @@ def build_payload(leaderboard:, scope:, country_code:) end def entries_scope(leaderboard:, scope:, country_code:) - q = leaderboard.entries.order(total_seconds: :desc) + q = leaderboard.entries.order(total_seconds: :desc, user_id: :asc) q = q.joins(:user).where(users: { country_code: }) if scope.to_sym == :country && country_code.present? q.preload(user: :email_addresses) end diff --git a/test/services/leaderboard_entries_test.rb b/test/services/leaderboard_entries_test.rb new file mode 100644 index 000000000..f9b83b5df --- /dev/null +++ b/test/services/leaderboard_entries_test.rb @@ -0,0 +1,128 @@ +require "test_helper" + +class LeaderboardEntriesTest < ActiveSupport::TestCase + setup do + Rails.cache.clear + end + + teardown do + Rails.cache.clear + end + + test "fetch hides leaderboard shadowbanned entries from public viewers and reranks visible entries" do + hidden_user = create_user(username: "entries_hidden", leaderboard_shadowbanned: true) + visible_user = create_user(username: "entries_visible") + board = create_board + board.entries.create!(user: hidden_user, total_seconds: 500, streak_count: 1) + board.entries.create!(user: visible_user, total_seconds: 300, streak_count: 2) + + payload = LeaderboardEntries.fetch(leaderboard: board) + + assert_equal 1, payload[:total] + assert_equal [ visible_user.id ], payload[:entries].map { |entry| entry[:user_id] } + assert_equal [ 1 ], payload[:entries].map { |entry| entry[:rank] } + end + + test "fetch shows a leaderboard shadowbanned entry to its own user" do + hidden_user = create_user(username: "entries_self", leaderboard_shadowbanned: true) + board = create_board + board.entries.create!(user: hidden_user, total_seconds: 500, streak_count: 1) + + payload = LeaderboardEntries.fetch(leaderboard: board, viewer: hidden_user) + + assert_equal 1, payload[:total] + assert_equal hidden_user.id, payload[:entries].first[:user_id] + assert_equal true, payload[:entries].first[:is_current_user] + end + + test "fetch applies country scope" do + us_user = create_user(username: "entries_us", country_code: "US") + ca_user = create_user(username: "entries_ca", country_code: "CA") + board = create_board + board.entries.create!(user: us_user, total_seconds: 300, streak_count: 1) + board.entries.create!(user: ca_user, total_seconds: 200, streak_count: 1) + + payload = LeaderboardEntries.fetch(leaderboard: board, scope: :country, country_code: "US") + + assert_equal 1, payload[:total] + assert_equal [ us_user.id ], payload[:entries].map { |entry| entry[:user_id] } + end + + test "fetch can include active project enrichment" do + user = create_user(username: "entries_active") + board = create_board + board.entries.create!(user: user, total_seconds: 300, streak_count: 1) + user.project_repo_mappings.create!(project_name: "active-project") + Heartbeat.create!( + user: user, + project: "active-project", + category: "coding", + time: Time.current.to_f, + source_type: :direct_entry + ) + + payload = LeaderboardEntries.fetch(leaderboard: board, include_active_projects: true) + + assert_equal({ name: "active-project", repo_url: nil }, payload[:entries].first[:active_project]) + end + + test "fetch_public uses lean public-visible entries" do + hidden_user = create_user(username: "public_hidden", leaderboard_shadowbanned: true) + visible_user = create_user(username: "public_visible", trust_level: :green) + board = create_board + board.entries.create!(user: hidden_user, total_seconds: 500, streak_count: 1) + board.entries.create!(user: visible_user, total_seconds: 300, streak_count: 2) + + payload = LeaderboardEntries.fetch_public(leaderboard: board) + + assert_equal 1, payload[:total] + assert_equal visible_user.id, payload[:entries].first[:user_id] + assert_equal 1, payload[:entries].first[:rank] + assert_equal visible_user.display_name, payload[:entries].first.dig(:user, :display_name) + end + + test "fetch_public cache follows leaderboard page cache version" do + original_cache = Rails.cache + Rails.cache = ActiveSupport::Cache.lookup_store(:memory_store) + Rails.cache.clear + + user = create_user(username: "public_cache_visible") + board = create_board + board.entries.create!(user: user, total_seconds: 300, streak_count: 1) + + assert_equal 1, LeaderboardEntries.fetch_public(leaderboard: board)[:total] + + user.set_leaderboard_shadowban( + banned: true, + changed_by_user: User.create!(timezone: "UTC", admin_level: :superadmin), + reason: "test shadowban" + ) + + assert_equal 0, LeaderboardEntries.fetch_public(leaderboard: board)[:total] + ensure + Rails.cache.clear + Rails.cache = original_cache + end + + private + + def create_user(username:, country_code: nil, leaderboard_shadowbanned: false, trust_level: :blue) + User.create!( + username: username, + country_code: country_code, + timezone: "UTC", + trust_level: trust_level, + leaderboard_shadowbanned: leaderboard_shadowbanned, + leaderboard_shadowban_reason: leaderboard_shadowbanned ? "test shadowban" : nil + ) + end + + def create_board + Leaderboard.create!( + start_date: Date.current, + period_type: :daily, + timezone_utc_offset: nil, + finished_generating_at: Time.current + ) + end +end