fix: correct bugs found during test coverage expansion

Author: Bulletdev

app/controllers/api/v1/base_controller.rb

@@ -32,6 +32,7 @@ module V1
     #     end
     #   end
     class BaseController < ApplicationController
+      include ActionController::MimeResponds
       include Authenticatable
       include Pundit::Authorization
       include TrialChecker
@@ -94,7 +95,8 @@ def render_validation_errors(exception)
       end
 
       def render_not_found(exception = nil)
-        resource_name = exception&.model&.humanize || 'Resource'
+        resource_name = exception.respond_to?(:model) ? exception.model&.humanize : nil
+        resource_name ||= 'Resource'
         render_error(
           message: "#{resource_name} not found",
           code: 'NOT_FOUND',

app/controllers/api/v1/profile_controller.rb

@@ -21,47 +21,83 @@ def update
             user: UserSerializer.render(@current_user)
           }, status: :ok
         else
-          render json: { errors: @current_user.errors.full_messages }, status: :unprocessable_entity
+          render_error(
+            message: 'Validation failed',
+            code: 'VALIDATION_ERROR',
+            status: :unprocessable_entity,
+            details: @current_user.errors.as_json
+          )
         end
       end
 
       # PATCH /api/v1/profile/password
       # Changes user password
       def update_password
-        unless @current_user.authenticate(password_params[:current_password])
-          return render json: { error: 'Current password is incorrect' }, status: :unauthorized
+        unless @current_user.authenticate(params[:current_password])
+          return render_error(
+            message: 'Current password is incorrect',
+            code: 'INVALID_PASSWORD',
+            status: :unprocessable_entity
+          )
         end
 
-        if @current_user.update(password: password_params[:new_password])
+        new_password = params[:password]
+        new_password_confirmation = params[:password_confirmation]
+
+        if new_password != new_password_confirmation
+          return render_error(
+            message: 'Password confirmation does not match',
+            code: 'VALIDATION_ERROR',
+            status: :unprocessable_entity
+          )
+        end
+
+        if @current_user.update(password: new_password)
           log_password_change
           render json: { message: 'Password updated successfully' }, status: :ok
         else
-          render json: { errors: @current_user.errors.full_messages }, status: :unprocessable_entity
+          render_error(
+            message: 'Validation failed',
+            code: 'VALIDATION_ERROR',
+            status: :unprocessable_entity,
+            details: @current_user.errors.as_json
+          )
         end
       end
 
       # PATCH /api/v1/profile/notifications
       # Updates notification preferences
       def update_notifications
-        if @current_user.update(notification_params)
-          render json: {
-            message: 'Notification preferences updated successfully',
-            notifications_enabled: @current_user.notifications_enabled,
-            notification_preferences: @current_user.notification_preferences
-          }, status: :ok
+        prefs = params[:notification_preferences]
+
+        if prefs.nil?
+          return render_error(
+            message: 'Validation failed',
+            code: 'VALIDATION_ERROR',
+            status: :unprocessable_entity
+          )
+        end
+
+        notification_prefs = @current_user.notification_preferences.merge(prefs.to_unsafe_h.stringify_keys)
+
+        if @current_user.update(notification_preferences: notification_prefs)
+          render_success({
+                           notification_preferences: @current_user.notification_preferences
+                         }, message: 'Notification preferences updated successfully')
         else
-          render json: { errors: @current_user.errors.full_messages }, status: :unprocessable_entity
+          render_error(
+            message: 'Validation failed',
+            code: 'VALIDATION_ERROR',
+            status: :unprocessable_entity,
+            details: @current_user.errors.as_json
+          )
         end
       end
 
       private
 
       def profile_params
-        params.require(:user).permit(:full_name, :avatar_url, :timezone, :language)
-      end
-
-      def password_params
-        params.require(:user).permit(:current_password, :new_password)
+        params.require(:user).permit(:full_name, :email, :avatar_url, :timezone, :language)
       end
 
       def notification_params

app/modules/analytics/controllers/objectives_controller.rb

@@ -141,8 +141,7 @@ def build_objective_score(matches, total)
       end
 
       def build_objective_trend(matches)
-        matches.includes(:match)
-               .order('game_start ASC')
+        matches.order('game_start ASC')
                .last(30)
                .map do |m|
           next unless m.game_start

app/modules/analytics/controllers/teamfights_controller.rb

@@ -20,9 +20,10 @@ def show
         player = organization_scoped(Player).find(params[:player_id])
 
         stats = PlayerMatchStat.joins(:match)
-                               .includes(:match)
-                               .where(player: player, match: { organization: current_organization })
+                               .where(player: player)
+                               .where('matches.organization_id = ?', current_organization.id)
                                .order('matches.game_start DESC')
+                               .preload(:match)
                                .limit(20)
 
         teamfight_data = {

app/modules/competitive/services/draft_comparator_service.rb

@@ -34,7 +34,7 @@ def self.compare_draft(our_picks:, opponent_picks:, organization:, our_bans: [],
       our_picks: our_picks,
       opponent_picks: opponent_picks,
       our_bans: our_bans,
-      opponent_bans: opponent_bans,
+      _opponent_bans: opponent_bans,
       patch: patch,
       organization: organization
     )
@@ -60,7 +60,7 @@ def compare_draft(our_picks:, opponent_picks:, our_bans:, _opponent_bans:, patch
 
     # Generate insights
     insights = analyzer.generate_insights(
-      our_picks: our_picks,
+      _our_picks: our_picks,
       opponent_picks: opponent_picks,
       our_bans: our_bans,
       similar_matches: similar_matches,

app/modules/matches/controllers/matches_controller.rb

@@ -120,7 +120,7 @@ def stats
           end,
           comparison: {
             total_gold: stats.sum(:gold_earned),
-            total_damage: stats.sum(:total_damage_dealt),
+            total_damage: stats.sum(:damage_dealt_total),
             total_vision_score: stats.sum(:vision_score),
             avg_kda: calculate_avg_kda(stats)
           }
@@ -138,63 +138,22 @@ def import
         unless player.riot_puuid.present?
           return render_error(
             message: 'Player does not have a Riot PUUID. Please sync player from Riot first.',
-            code: 'VALIDATION_ERROR',
-            status: :unprocessable_entity
+            code: 'MISSING_PUUID',
+            status: :bad_request
           )
         end
 
-        begin
-          riot_service = RiotApiService.new
-          region = player.region || 'BR'
-
-          match_ids = riot_service.get_match_history(
-            puuid: player.riot_puuid,
-            region: region,
-            count: count
-          )
-
-          imported_count = 0
-          match_ids.each do |match_id|
-            next if Match.exists?(riot_match_id: match_id)
+        job_id = ImportPlayerMatchesJob.perform_later(
+          player.id,
+          current_organization.id,
+          count
+        ).job_id
 
-            SyncMatchJob.perform_later(match_id, current_organization.id, region)
-            imported_count += 1
-          end
-
-          render_success({
-                           message: "Queued #{imported_count} matches for import",
-                           total_matches_found: match_ids.count,
-                           already_imported: match_ids.count - imported_count,
-                           player: PlayerSerializer.render_as_hash(player)
-                         })
-        rescue RedisClient::CannotConnectError, Redis::CannotConnectError => e
-          Rails.logger.error "Redis connection failed during match import: #{e.message}"
-
-          render_error(
-            message: 'Background job service is temporarily unavailable. Please try again later.',
-            code: 'BACKGROUND_SERVICE_UNAVAILABLE',
-            status: :service_unavailable,
-            details: {
-              hint: 'The import service is currently down. Contact your administrator if this persists.',
-              player_id: player.id
-            }
-          )
-        rescue RiotApiService::RiotApiError => e
-          render_error(
-            message: "Failed to fetch matches from Riot API: #{e.message}",
-            code: 'RIOT_API_ERROR',
-            status: :bad_gateway
-          )
-        rescue StandardError => e
-          Rails.logger.error "Unexpected error during match import: #{e.class} - #{e.message}"
-          Rails.logger.error e.backtrace.first(5).join("\n")
-
-          render_error(
-            message: "Failed to import matches: #{e.message}",
-            code: 'IMPORT_ERROR',
-            status: :internal_server_error
-          )
-        end
+        render_success({
+                         job_id: job_id,
+                         player_id: player.id.to_s,
+                         count: count
+                       }, message: 'Match import queued successfully')
       end
 
       private
@@ -265,7 +224,7 @@ def calculate_matches_summary(matches)
           victories: matches.victories.count,
           defeats: matches.defeats.count,
           win_rate: calculate_win_rate(matches),
-          by_type: matches.group(:match_type).count,
+          by_type: matches.unscope(:order).group(:match_type).count,
           avg_duration: matches.average(:game_duration)&.round(0)
         }
       end
@@ -276,8 +235,8 @@ def calculate_team_stats(stats)
           total_deaths: stats.sum(:deaths),
           total_assists: stats.sum(:assists),
           total_gold: stats.sum(:gold_earned),
-          total_damage: stats.sum(:total_damage_dealt),
-          total_cs: stats.sum(:minions_killed),
+          total_damage: stats.sum(:damage_dealt_total),
+          total_cs: stats.sum(:cs),
           total_vision_score: stats.sum(:vision_score)
         }
       end

app/modules/matches/jobs/import_player_matches_job.rb

@@ -0,0 +1,40 @@
+# frozen_string_literal: true
+
+module Matches
+  # Background job that fetches match history from Riot API for a specific player
+  # and queues individual SyncMatchJob jobs for each match to import.
+  class ImportPlayerMatchesJob < ApplicationJob
+    queue_as :default
+
+    def perform(player_id, organization_id, count = 20)
+      Current.organization_id = organization_id
+
+      organization = Organization.find(organization_id)
+      player = organization.players.find(player_id)
+
+      return unless player.riot_puuid.present?
+
+      riot_service = RiotApiService.new
+      region = player.region || 'BR'
+
+      match_ids = riot_service.get_match_history(
+        puuid: player.riot_puuid,
+        region: region,
+        count: count
+      )
+
+      match_ids.each do |match_id|
+        next if Match.exists?(riot_match_id: match_id)
+
+        SyncMatchJob.perform_later(match_id, organization_id, region)
+      end
+    rescue RiotApiService::RiotApiError => e
+      Rails.logger.error("ImportPlayerMatchesJob: Riot API error for player #{player_id}: #{e.message}")
+    rescue StandardError => e
+      Rails.logger.error("ImportPlayerMatchesJob: Unexpected error for player #{player_id}: #{e.class} - #{e.message}")
+      raise
+    ensure
+      Current.organization_id = nil
+    end
+  end
+end

app/modules/notifications/controllers/notifications_controller.rb

@@ -65,15 +65,15 @@ def destroy
 
         render_deleted(message: 'Notification deleted successfully')
       rescue ActiveRecord::RecordNotFound
-        render_not_found('Notification not found')
+        render_not_found
       end
 
       private
 
       def set_notification
         @notification = current_user.notifications.find(params[:id])
       rescue ActiveRecord::RecordNotFound
-        render_not_found('Notification not found')
+        render_not_found
       end
     end
   end

app/modules/scouting/controllers/players_controller.rb

@@ -77,7 +77,8 @@ def create
       # Updates global target data OR watchlist data
       def update
         ActiveRecord::Base.transaction do
-          @target.update!(target_params) if target_params.any?
+          tp = target_params.to_h
+          @target.update!(tp) if tp.any?
           update_watchlist_if_params_present
           render_updated(serialized_target_response)
         end
@@ -134,13 +135,15 @@ def create_watchlist_for(target)
       end
 
       def update_watchlist_if_params_present
-        return unless watchlist_params.any?
+        wp = watchlist_params.to_h
+        wp = scouting_target_watchlist_params.to_h if wp.empty?
+        return if wp.empty?
 
         watchlist = @target.scouting_watchlists.find_or_create_by!(organization: current_organization) do |w|
           w.added_by = current_user
         end
         old_values = watchlist.attributes.dup
-        watchlist.update!(watchlist_params)
+        watchlist.update!(wp)
         log_user_action(action: 'update', entity_type: 'ScoutingWatchlist',
                         entity_id: watchlist.id, old_values: old_values, new_values: watchlist.attributes)
       end
@@ -306,6 +309,12 @@ def watchlist_params
         )
       end
 
+      def scouting_target_watchlist_params
+        params.fetch(:scouting_target, {}).permit(
+          :priority, :status, :notes, :assigned_to_id
+        )
+      end
+
       def target_params
         # :role is the LoL in-game position (top/jungle/mid/adc/support), not an authorization role.
         params.fetch(:target, {}).permit( # nosemgrep: ruby.lang.security.model-attr-accessible.model-attr-accessible

app/modules/scouting/controllers/regions_controller.rb

@@ -33,7 +33,7 @@ def index
           { code: 'TR', name: 'Turkey', platform: 'TR1' }
         ]
 
-        render_success(regions)
+        render_success({ regions: regions })
       end
     end
   end