fix: refactor methods to reduce cyclomatic complexity

  All changes maintain backward compatibility while improving code quality, maintainability, and readability. The refactoring follows SOLID principles by extracting methods with single responsibilities

Author: Bulletdev

app/controllers/api/v1/analytics/champions_controller.rb

@@ -20,42 +20,56 @@ module Analytics
       class ChampionsController < Api::V1::BaseController
         def show
           player = organization_scoped(Player).find(params[:player_id])
+          stats = fetch_champion_stats(player)
+          champion_stats = build_champion_stats(stats)
 
-          stats = PlayerMatchStat.where(player: player)
-                                 .group(:champion)
-                                 .select(
-                                   'champion',
-                                   'COUNT(*) as games_played',
-                                   'SUM(CASE WHEN matches.victory THEN 1 ELSE 0 END) as wins',
-                                   'AVG((kills + assists)::float / NULLIF(deaths, 0)) as avg_kda',
-                                   'AVG(cs_per_min) as avg_cs_per_min',
-                                   'AVG(damage_dealt_total) as avg_damage_dealt',
-                                   'AVG(damage_taken) as avg_damage_taken',
-                                   'AVG(gold_per_min) as avg_gold_per_min',
-                                   'AVG(vision_score) as avg_vision_score'
-                                 )
-                                 .joins(:match)
-                                 .order('games_played DESC')
+          render_success(build_champion_data(player, champion_stats))
+        end
+
+        private
 
+        def fetch_champion_stats(player)
+          PlayerMatchStat.where(player: player)
+                         .group(:champion)
+                         .select(
+                           'champion',
+                           'COUNT(*) as games_played',
+                           'SUM(CASE WHEN matches.victory THEN 1 ELSE 0 END) as wins',
+                           'AVG((kills + assists)::float / NULLIF(deaths, 0)) as avg_kda',
+                           'AVG(cs_per_min) as avg_cs_per_min',
+                           'AVG(damage_dealt_total) as avg_damage_dealt',
+                           'AVG(damage_taken) as avg_damage_taken',
+                           'AVG(gold_per_min) as avg_gold_per_min',
+                           'AVG(vision_score) as avg_vision_score'
+                         )
+                         .joins(:match)
+                         .order('games_played DESC')
+        end
+
+        def build_champion_stats(stats)
           riot_service = RiotCdnService.new
-          champion_stats = stats.map do |stat|
-            win_rate = stat.games_played.zero? ? 0 : (stat.wins.to_f / stat.games_played)
-            {
-              champion: stat.champion,
-              games_played: stat.games_played,
-              win_rate: win_rate,
-              avg_kda: stat.avg_kda&.round(2) || 0,
-              avg_cs_per_min: stat.avg_cs_per_min&.round(1) || 0.0,
-              avg_damage_dealt: stat.avg_damage_dealt&.round(0) || 0,
-              avg_damage_taken: stat.avg_damage_taken&.round(0) || 0,
-              avg_gold_per_min: stat.avg_gold_per_min&.round(0) || 0,
-              avg_vision_score: stat.avg_vision_score&.round(1) || 0.0,
-              mastery_grade: calculate_mastery_grade(win_rate, stat.avg_kda),
-              icon_url: riot_service.champion_icon_url(stat.champion)
-            }
-          end
+          stats.map { |stat| build_champion_stat_hash(stat, riot_service) }
+        end
+
+        def build_champion_stat_hash(stat, riot_service)
+          win_rate = stat.games_played.zero? ? 0 : (stat.wins.to_f / stat.games_played)
+          {
+            champion: stat.champion,
+            games_played: stat.games_played,
+            win_rate: win_rate,
+            avg_kda: stat.avg_kda&.round(2) || 0,
+            avg_cs_per_min: stat.avg_cs_per_min&.round(1) || 0.0,
+            avg_damage_dealt: stat.avg_damage_dealt&.round(0) || 0,
+            avg_damage_taken: stat.avg_damage_taken&.round(0) || 0,
+            avg_gold_per_min: stat.avg_gold_per_min&.round(0) || 0,
+            avg_vision_score: stat.avg_vision_score&.round(1) || 0.0,
+            mastery_grade: calculate_mastery_grade(win_rate, stat.avg_kda),
+            icon_url: riot_service.champion_icon_url(stat.champion)
+          }
+        end
 
-          champion_data = {
+        def build_champion_data(player, champion_stats)
+          {
             player: PlayerSerializer.render_as_hash(player),
             champion_stats: champion_stats,
             top_champions: champion_stats.take(5),

app/controllers/api/v1/images_controller.rb

@@ -14,6 +14,16 @@ module V1
     class ImagesController < BaseController
       skip_before_action :authenticate_request!, only: [:proxy]
 
+      ALLOWED_DOMAINS = [
+        'upload.wikimedia.org',
+        'ddragon.leagueoflegends.com',
+        'raw.communitydragon.org',
+        'static.wikia.nocookie.net',
+        'commons.wikimedia.org'
+      ].freeze
+
+      HTTP_TIMEOUT_OPTIONS = { open_timeout: 5, read_timeout: 10 }.freeze
+
       # GET /api/v1/images/proxy
       # Proxies and caches external images
       #
@@ -33,16 +43,6 @@ def proxy
 
       private
 
-      ALLOWED_DOMAINS = [
-        'upload.wikimedia.org',
-        'ddragon.leagueoflegends.com',
-        'raw.communitydragon.org',
-        'static.wikia.nocookie.net',
-        'commons.wikimedia.org'
-      ].freeze
-
-      HTTP_TIMEOUT_OPTIONS = { open_timeout: 5, read_timeout: 10 }.freeze
-
       # Validates if the URL is from an allowed domain
       def valid_image_url?(url)
         return false if url.blank?

app/controllers/api/v1/team_goals_controller.rb

@@ -6,28 +6,8 @@ class TeamGoalsController < Api::V1::BaseController
       before_action :set_team_goal, only: %i[show update destroy]
 
       def index
-        goals = organization_scoped(TeamGoal).includes(:player, :assigned_to, :created_by)
-
-        goals = goals.by_status(params[:status]) if params[:status].present?
-        goals = goals.by_category(params[:category]) if params[:category].present?
-        goals = goals.for_player(params[:player_id]) if params[:player_id].present?
-
-        goals = goals.team_goals if params[:type] == 'team'
-        goals = goals.player_goals if params[:type] == 'player'
-        goals = goals.active if params[:active] == 'true'
-        goals = goals.overdue if params[:overdue] == 'true'
-        goals = goals.expiring_soon(params[:expiring_days]&.to_i || 7) if params[:expiring_soon] == 'true'
-
-        goals = goals.where(assigned_to_id: params[:assigned_to_id]) if params[:assigned_to_id].present?
-
-        # Whitelist for sort parameters to prevent SQL injection
-        allowed_sort_fields = %w[created_at updated_at title status category start_date end_date progress]
-        allowed_sort_orders = %w[asc desc]
-
-        sort_by = allowed_sort_fields.include?(params[:sort_by]) ? params[:sort_by] : 'created_at'
-        sort_order = allowed_sort_orders.include?(params[:sort_order]&.downcase) ? params[:sort_order].downcase : 'desc'
-        goals = goals.order(sort_by => sort_order)
-
+        goals = apply_goal_filters(organization_scoped(TeamGoal).includes(:player, :assigned_to, :created_by))
+        goals = apply_goal_sorting(goals)
         result = paginate(goals)
 
         render_success({
@@ -128,6 +108,43 @@ def team_goal_params
         )
       end
 
+      def apply_goal_filters(goals)
+        goals = apply_basic_filters(goals)
+        goals = apply_type_filters(goals)
+        goals = apply_status_filters(goals)
+        goals = goals.where(assigned_to_id: params[:assigned_to_id]) if params[:assigned_to_id].present?
+        goals
+      end
+
+      def apply_basic_filters(goals)
+        goals = goals.by_status(params[:status]) if params[:status].present?
+        goals = goals.by_category(params[:category]) if params[:category].present?
+        goals = goals.for_player(params[:player_id]) if params[:player_id].present?
+        goals
+      end
+
+      def apply_type_filters(goals)
+        goals = goals.team_goals if params[:type] == 'team'
+        goals = goals.player_goals if params[:type] == 'player'
+        goals
+      end
+
+      def apply_status_filters(goals)
+        goals = goals.active if params[:active] == 'true'
+        goals = goals.overdue if params[:overdue] == 'true'
+        goals = goals.expiring_soon(params[:expiring_days]&.to_i || 7) if params[:expiring_soon] == 'true'
+        goals
+      end
+
+      def apply_goal_sorting(goals)
+        allowed_sort_fields = %w[created_at updated_at title status category start_date end_date progress]
+        allowed_sort_orders = %w[asc desc]
+
+        sort_by = allowed_sort_fields.include?(params[:sort_by]) ? params[:sort_by] : 'created_at'
+        sort_order = allowed_sort_orders.include?(params[:sort_order]&.downcase) ? params[:sort_order].downcase : 'desc'
+        goals.order(sort_by => sort_order)
+      end
+
       def calculate_goals_summary(goals)
         {
           total: goals.count,

app/models/audit_log.rb

@@ -61,21 +61,21 @@ def entity_display
   def changes_summary
     return 'Created' if action == 'create'
     return 'Deleted' if action == 'delete'
-
     return 'No changes recorded' if old_values.blank? && new_values.blank?
 
-    changes = []
+    changes = build_changes_list
+    changes.empty? ? 'No changes recorded' : changes.join(', ')
+  end
+
+  def build_changes_list
+    return [] unless old_values.present? && new_values.present?
 
-    if old_values.present? && new_values.present?
-      new_values.each do |key, new_val|
-        old_val = old_values[key]
-        next if old_val == new_val
+    new_values.each_with_object([]) do |(key, new_val), changes|
+      old_val = old_values[key]
+      next if old_val == new_val
 
-        changes << "#{key.humanize}: #{format_value(old_val)} → #{format_value(new_val)}"
-      end
+      changes << "#{key.humanize}: #{format_value(old_val)} → #{format_value(new_val)}"
     end
-
-    changes.empty? ? 'No changes recorded' : changes.join(', ')
   end
 
   def ip_location