refactor: solve code style and minor issues

Author: Bulletdev

app/modules/admin/controllers/players_controller.rb

@@ -239,74 +239,73 @@ def change_status
       # Transfers a player to another organization
       def transfer
         new_organization_id = params[:new_organization_id]
-        reason = params[:reason] || 'Player transfer'
+        new_organization    = resolve_transfer_target(new_organization_id)
+        return unless new_organization
 
-        unless new_organization_id.present?
-          return render_error(
-            message: 'New organization ID is required',
-            code: 'VALIDATION_ERROR',
-            status: :unprocessable_entity
-          )
-        end
+        old_org_id = @player.organization_id
+        execute_player_transfer(@player, new_organization, old_org_id, params[:reason])
+        publish_player_transferred(@player, old_org_id, new_organization_id)
 
-        new_organization = Organization.find_by(id: new_organization_id)
-        unless new_organization
-          return render_error(
-            message: 'Organization not found',
-            code: 'NOT_FOUND',
-            status: :not_found
-          )
-        end
+        render_success({
+                         message: 'Player transferred successfully',
+                         player: PlayerSerializer.render_as_hash(@player),
+                         previous_organization: old_org_id,
+                         new_organization: new_organization_id
+                       })
+      rescue ActiveRecord::RecordInvalid => e
+        render_error(
+          message: "Failed to transfer player: #{e.message}",
+          code: 'TRANSFER_ERROR',
+          status: :unprocessable_entity
+        )
+      end
 
-        old_org_id = @player.organization_id
+      private
 
-        ActiveRecord::Base.transaction do
-          # Save current organization as previous
-          @player.update!(previous_organization_id: old_org_id)
+      def resolve_transfer_target(new_organization_id)
+        unless new_organization_id.present?
+          render_error(message: 'New organization ID is required', code: 'VALIDATION_ERROR',
+                       status: :unprocessable_entity)
+          return nil
+        end
 
-          # Transfer to new organization
-          @player.update!(organization: new_organization, status: 'inactive')
+        org = Organization.find_by(id: new_organization_id)
+        render_error(message: 'Organization not found', code: 'NOT_FOUND', status: :not_found) unless org
+        org
+      end
 
+      def execute_player_transfer(player, new_organization, old_org_id, reason)
+        ActiveRecord::Base.transaction do
+          player.update!(previous_organization_id: old_org_id)
+          player.update!(organization: new_organization, status: 'inactive')
           log_user_action(
             action: 'transfer',
             entity_type: 'Player',
-            entity_id: @player.id,
+            entity_id: player.id,
             old_values: { organization_id: old_org_id },
             new_values: {
-              organization_id: new_organization_id,
+              organization_id: new_organization.id,
               previous_organization_id: old_org_id,
-              transfer_reason: reason
+              transfer_reason: reason || 'Player transfer'
             }
           )
         end
+      end
 
+      def publish_player_transferred(player, old_org_id, new_organization_id)
         Events::EventPublisher.publish(
           user_id: current_user.id,
           org_id: old_org_id,
           type: 'player.transferred',
           payload: {
-            player_id: @player.id,
-            player_name: @player.summoner_name,
+            player_id: player.id,
+            player_name: player.summoner_name,
             from_org_id: old_org_id,
             to_org_id: new_organization_id
           }
         )
-        render_success({
-                         message: 'Player transferred successfully',
-                         player: PlayerSerializer.render_as_hash(@player),
-                         previous_organization: old_org_id,
-                         new_organization: new_organization_id
-                       })
-      rescue ActiveRecord::RecordInvalid => e
-        render_error(
-          message: "Failed to transfer player: #{e.message}",
-          code: 'TRANSFER_ERROR',
-          status: :unprocessable_entity
-        )
       end
 
-      private
-
       def require_admin_access
         return if current_user.admin? || current_user.owner?
 

app/modules/analytics/controllers/competitive_controller.rb

@@ -336,29 +336,38 @@ def build_opponents_data(rows)
       # ── patch_meta helpers ─────────────────────────────────────────
 
       def build_patch_meta(rows)
-        rows.group_by { |m| m.patch_version.presence }.filter_map do |patch, patch_rows|
-          next if patch.nil?
+        rows.group_by { |m| m.patch_version.presence }
+            .filter_map { |patch, patch_rows| build_patch_entry(patch, patch_rows) }
+            .sort_by { |entry| entry[:patch] }
+            .reverse
+      end
 
-          games = patch_rows.size
-          wins  = patch_rows.count(&:victory)
+      def build_patch_entry(patch, patch_rows)
+        return nil if patch.nil?
 
-          {
-            patch: patch,
-            games: games,
-            wins: wins,
-            losses: games - wins,
-            win_rate: games.positive? ? (wins.to_f / games * 100).round(1) : 0,
-            blue_games: patch_rows.count { |m| m.side&.downcase == 'blue' },
-            red_games: patch_rows.count { |m| m.side&.downcase == 'red' },
-            top_picks: top_n_from_jsonb(patch_rows, :our_picks, 5),
-            top_bans: top_n_from_jsonb(patch_rows, :our_bans, 5)
-          }
-        end.sort_by { |p| p[:patch] }.reverse
+        games = patch_rows.size
+        wins  = patch_rows.count(&:victory)
+
+        {
+          patch: patch,
+          games: games,
+          wins: wins,
+          losses: games - wins,
+          win_rate: patch_win_rate(wins, games),
+          blue_games: patch_rows.count { |m| m.side&.downcase == 'blue' },
+          red_games: patch_rows.count { |m| m.side&.downcase == 'red' },
+          top_picks: top_n_from_jsonb(patch_rows, :our_picks, 5),
+          top_bans: top_n_from_jsonb(patch_rows, :our_bans, 5)
+        }
+      end
+
+      def patch_win_rate(wins, games)
+        games.positive? ? (wins.to_f / games * 100).round(1) : 0
       end
 
-      def top_n_from_jsonb(rows, field, n)
+      def top_n_from_jsonb(rows, field, limit)
         tally = rows.flat_map { |m| Array(m.public_send(field)).filter_map { |e| e['champion'] } }.tally
-        tally.sort_by { |_, count| -count }.first(n).map { |champion, count| { champion: champion, count: count } }
+        tally.sort_by { |_, count| -count }.first(limit).map { |champion, count| { champion: champion, count: count } }
       end
 
       # ── empty state helpers ────────────────────────────────────────

app/modules/competitive/concerns/match_fingerprint.rb

@@ -1,22 +1,26 @@
 # frozen_string_literal: true
 
+# Shared fingerprinting logic for deduplicating competitive match imports.
+#
+# Two import pipelines (Riot API numeric IDs and Leaguepedia textual IDs) can
+# produce different external_match_id values for the same physical game. This
+# module derives a source-agnostic fingerprint so duplicates are caught before
+# they are persisted.
 module MatchFingerprint
   # Generates a stable fingerprint for a physical game based on attributes that
-  # are source-agnostic. Used to detect duplicates when the same game arrives
-  # from two import pipelines (Riot API numeric ID and Leaguepedia textual ID)
-  # with different external_match_id values.
+  # are source-agnostic.
   #
   # @param org_id [String] organization UUID
   # @param match_date [DateTime, nil]
   # @param game_number [Integer, nil] game within the series (1-5)
   # @param opponent_name [String, nil]
-  # @return [String, nil] MD5 hex string, or nil when inputs are insufficient
+  # @return [String, nil] SHA-256 hex string, or nil when inputs are insufficient
   def generate_fingerprint(org_id, match_date, game_number, opponent_name)
     return nil if match_date.nil? || opponent_name.nil? || opponent_name.strip.empty?
 
     day = match_date.to_date.to_s
     normalized = opponent_name.strip.downcase
-    Digest::MD5.hexdigest("#{org_id}|#{day}|#{game_number || 1}|#{normalized}")
+    Digest::SHA256.hexdigest("#{org_id}|#{day}|#{game_number || 1}|#{normalized}")
   end
 
   # Returns true if a record with this fingerprint already exists for the org.

app/modules/competitive/controllers/pro_matches_controller.rb

@@ -400,42 +400,10 @@ def match_preview
           }, status: :unprocessable_entity
         end
 
-        # Fetch PandaScore data in parallel
-        t1_data   = Thread.new { @pandascore_service.fetch_team(team1_id) }
-        t2_data   = Thread.new { @pandascore_service.fetch_team(team2_id) }
-        t1_recent = Thread.new { @pandascore_service.fetch_team_recent_matches(team1_id) }
-        t2_recent = Thread.new { @pandascore_service.fetch_team_recent_matches(team2_id) }
+        team1_data, team2_data, team1_recent, team2_recent =
+          fetch_pandascore_preview_data(team1_id, team2_id)
 
-        team1_data   = t1_data.value
-        team2_data   = t2_data.value
-        team1_recent = t1_recent.value
-        team2_recent = t2_recent.value
-
-        # H2H stats from Elasticsearch
-        must_clauses = [
-          {
-            bool: {
-              should: [
-                { bool: { must: [team_clause(team1_name, 'team1'), team_clause(team2_name, 'team2')] } },
-                { bool: { must: [team_clause(team2_name, 'team1'), team_clause(team1_name, 'team2')] } }
-              ],
-              minimum_should_match: 1
-            }
-          }
-        ]
-
-        es_body = {
-          query: { bool: { must: must_clauses } },
-          size: 0,
-          aggs: {
-            team1_wins: { filter: win_team_clause(team1_name) },
-            team2_wins: { filter: win_team_clause(team2_name) }
-          }
-        }
-
-        es_result    = ElasticsearchClient.new.search(index: 'lol_pro_matches', body: es_body)
-        h2h_wins_t1  = es_result.dig('aggregations', 'team1_wins', 'doc_count') || 0
-        h2h_wins_t2  = es_result.dig('aggregations', 'team2_wins', 'doc_count') || 0
+        h2h_wins_t1, h2h_wins_t2 = fetch_h2h_wins(team1_name, team2_name)
 
         render json: {
           data: {
@@ -462,32 +430,9 @@ def es_series
 
         raise ArgumentError, 'team1 and team2 are required' if team1.blank? || team2.blank?
 
-        must_clauses = [
-          {
-            bool: {
-              should: [
-                { bool: { must: [team_clause(team1, 'team1'), team_clause(team2, 'team2')] } },
-                { bool: { must: [team_clause(team2, 'team1'), team_clause(team1, 'team2')] } }
-              ],
-              minimum_should_match: 1
-            }
-          }
-        ]
-
-        if params[:after].present? && params[:before].present?
-          must_clauses << {
-            range: { start_time: { gte: params[:after], lte: params[:before] } }
-          }
-        end
-
-        es_body = {
-          query: { bool: { must: must_clauses } },
-          sort: [{ start_time: { order: 'desc' } }],
-          size: limit
-        }
-
-        result = ElasticsearchClient.new.search(index: 'lol_pro_matches', body: es_body)
-        games  = result.dig('hits', 'hits')&.map { |h| h['_source'] } || []
+        es_body = build_series_query(team1, team2, limit)
+        result  = ElasticsearchClient.new.search(index: 'lol_pro_matches', body: es_body)
+        games   = result.dig('hits', 'hits')&.map { |h| h['_source'] } || []
 
         render json: { data: { games: games, total: games.size } }
       rescue ArgumentError => e
@@ -504,6 +449,52 @@ def set_pandascore_service
         @pandascore_service = PandascoreService.instance
       end
 
+      def fetch_pandascore_preview_data(team1_id, team2_id)
+        t1_data   = Thread.new { @pandascore_service.fetch_team(team1_id) }
+        t2_data   = Thread.new { @pandascore_service.fetch_team(team2_id) }
+        t1_recent = Thread.new { @pandascore_service.fetch_team_recent_matches(team1_id) }
+        t2_recent = Thread.new { @pandascore_service.fetch_team_recent_matches(team2_id) }
+        [t1_data.value, t2_data.value, t1_recent.value, t2_recent.value]
+      end
+
+      def fetch_h2h_wins(team1_name, team2_name)
+        must_clauses = [h2h_matchup_clause(team1_name, team2_name)]
+        es_body = {
+          query: { bool: { must: must_clauses } },
+          size: 0,
+          aggs: {
+            team1_wins: { filter: win_team_clause(team1_name) },
+            team2_wins: { filter: win_team_clause(team2_name) }
+          }
+        }
+        result = ElasticsearchClient.new.search(index: 'lol_pro_matches', body: es_body)
+        wins_t1 = result.dig('aggregations', 'team1_wins', 'doc_count') || 0
+        wins_t2 = result.dig('aggregations', 'team2_wins', 'doc_count') || 0
+        [wins_t1, wins_t2]
+      end
+
+      def h2h_matchup_clause(team1_name, team2_name)
+        {
+          bool: {
+            should: [
+              { bool: { must: [team_clause(team1_name, 'team1'), team_clause(team2_name, 'team2')] } },
+              { bool: { must: [team_clause(team2_name, 'team1'), team_clause(team1_name, 'team2')] } }
+            ],
+            minimum_should_match: 1
+          }
+        }
+      end
+
+      def build_series_query(team1, team2, limit)
+        must_clauses = [h2h_matchup_clause(team1, team2)]
+        must_clauses << { range: { start_time: { gte: params[:after], lte: params[:before] } } } if date_filter?
+        { query: { bool: { must: must_clauses } }, sort: [{ start_time: { order: 'desc' } }], size: limit }
+      end
+
+      def date_filter?
+        params[:after].present? && params[:before].present?
+      end
+
       # Builds an ES should clause that matches a team name using:
       # 1. Exact term match (handles perfect name equality)
       # 2. Prefix wildcard on first word, case-insensitive (handles suffix differences

app/modules/competitive/services/scraper_importer_service.rb

@@ -88,9 +88,9 @@ def import_one(match, our_team, stats)
     ext_id      = build_external_match_id(match)
     parsed_date = parse_date(match['start_time'])
     game_number = match['game_number']
-    _team1_name = match.dig('team1', 'name').to_s
-    _team2_name = match.dig('team2', 'name').to_s
-    _, opp_resolved = resolve_teams(_team1_name, _team2_name, match['win_team'].to_s, our_team)
+    team1_name  = match.dig('team1', 'name').to_s
+    team2_name  = match.dig('team2', 'name').to_s
+    _, opp_resolved = resolve_teams(team1_name, team2_name, match['win_team'].to_s, our_team)
 
     if duplicate_by_fingerprint?(@organization, parsed_date, game_number, opp_resolved)
       stats[:skipped_duplicate] += 1