More validations for advancement conditions (thewca#7376)

* Replace Round.find_for

It's my bad but using it easily leads to a N+1 query on
competition_events. Instead implementing something on `Competition`
makes more sense as we can easily tweak the `includes` before actually
requesting the round.

* Improve validation for advancement conditions

WcaOnRails/app/controllers/api/v0/competitions_controller.rb

@@ -33,7 +33,7 @@ def results
   end
 
   def event_results
-    competition = competition_from_params
+    competition = competition_from_params(associations: [:rounds])
     event = Event.c_find!(params[:event_id])
     results_by_round = competition.results
                                   .where(eventId: event.id)
@@ -43,7 +43,7 @@ def event_results
       # I think all competitions now have round data, but let's be cautious
       # and assume they may not.
       # round data.
-      round = Round.find_for(competition.id, event.id, round_type.id)
+      round = competition.find_round_for(event.id, round_type.id)
       {
         id: round&.id,
         roundTypeId: round_type.id,
@@ -135,9 +135,9 @@ def update_wcif
     }
   end
 
-  private def competition_from_params
+  private def competition_from_params(associations: {})
     id = params[:competition_id] || params[:id]
-    competition = Competition.find_by_id(id)
+    competition = Competition.includes(associations).find_by_id(id)
 
     # If this competition exists, but is not publicly visible, then only show it
     # to the user if they are able to manage the competition.

WcaOnRails/app/models/competition.rb

@@ -1936,4 +1936,11 @@ def series_sibling_competitions
     series_competitions
       .where.not(id: self.id)
   end
+
+  def find_round_for(event_id, round_type_id, format_id = nil)
+    rounds.find do |r|
+      r.event.id == event_id && r.round_type_id == round_type_id &&
+        (format_id.nil? || format_id == r.format_id)
+    end
+  end
 end

WcaOnRails/app/models/concerns/resultable.rb

@@ -41,7 +41,17 @@ def format
     end
 
     def round
-      Round.find_for(competitionId, eventId, roundTypeId, formatId)
+      # This method is actually relatively expensive, it's definitely fine to
+      # use it if you're dealing with a single result, but if you're manipulating
+      # a bunch of them please don't use it as you likely have another mean
+      # to get a 'round' for your set of results.
+      # Using a 'find' here is intentional to pass the `includes(:rounds)` to
+      # avoid the n+1 query on competition_events if we were directly using
+      # competition.find_round_for.
+      Competition
+        .includes(:rounds)
+        .find_by_id(competition_id)
+        &.find_round_for(event_id, round_type_id, format_id)
     end
 
     validate :belongs_to_a_round

WcaOnRails/app/models/round.rb

@@ -238,18 +238,4 @@ def self.name_from_attributes_id(event_id, round_type_id)
   def self.name_from_attributes(event, round_type)
     I18n.t("round.name", event_name: event.name, round_name: round_type.name)
   end
-
-  # Find a matching round the given (competition_id, event_id, round_type_id)
-  # tuple.
-  # Optionally take a format if we want a strict match.
-  def self.find_for(competition_id, event_id, round_type_id, format_id = nil)
-    Competition.find_by(id: competition_id)
-               &.competition_events
-               &.find { |ce| ce.event_id == event_id }
-               &.rounds
-               &.find do |r|
-                 r.round_type_id == round_type_id &&
-                   (format_id.nil? || format_id == r.format_id)
-               end
-  end
 end

WcaOnRails/lib/advancement_conditions/attempt_result_condition.rb

@@ -18,5 +18,13 @@ def to_s(round, short: false)
         I18n.t("advancement_condition#{".short" if short}.attempt_result.points", round_format: round_form, points: SolveTime.multibld_attempt_to_points(attempt_result))
       end
     end
+
+    def max_advancing(results)
+      return 0 if results.empty?
+      field = results.first.format.sort_by == "single" ? :best : :average
+      results.select do |r|
+        r.to_solve_time(field).complete? && r.send(field) < attempt_result
+      end.size
+    end
   end
 end

WcaOnRails/lib/advancement_conditions/percent_condition.rb

@@ -11,5 +11,9 @@ def self.wcif_type
     def to_s(round, short: false)
       I18n.t("advancement_condition#{".short" if short}.percent", percent: percent)
     end
+
+    def max_advancing(results)
+      results.size * percent / 100
+    end
   end
 end

WcaOnRails/lib/advancement_conditions/ranking_condition.rb

@@ -11,5 +11,9 @@ def self.wcif_type
     def to_s(round, short: false)
       I18n.t("advancement_condition#{".short" if short}.ranking", ranking: ranking)
     end
+
+    def max_advancing(results)
+      ranking
+    end
   end
 end

WcaOnRails/lib/results_validators/advancement_conditions_validator.rb

@@ -2,12 +2,22 @@
 
 module ResultsValidators
   class AdvancementConditionsValidator < GenericValidator
-    REGULATION_9M_ERROR = "Event %{event_id} has more than four rounds, which is not permitted as per Regulation 9m."
+    # PV: There used to be an error for regulation 9M, but now all results
+    # must belong to a round, and rails validations make sure the total
+    # number of rounds is <= 4, and that each round number is unique.
     REGULATION_9M1_ERROR = "Round %{round_id} has 99 or fewer competitors but has more than two subsequent rounds, which is not permitted as per Regulation 9m1."
     REGULATION_9M2_ERROR = "Round %{round_id} has 15 or fewer competitors but has more than one subsequent round, which is not permitted as per Regulation 9m2."
     REGULATION_9M3_ERROR = "Round %{round_id} has 7 or fewer competitors but has at least one subsequent round, which is not permitted as per Regulation 9m3."
     REGULATION_9P1_ERROR = "Round %{round_id}: Fewer than 25%% of competitors were eliminated, which is not permitted as per Regulation 9p1."
     OLD_REGULATION_9P_ERROR = "Round %{round_id}: There must be at least one competitor eliminated, which is required as per Regulation 9p (competitions before April 2010)."
+    ROUND_9P1_ERROR = "Round %{round_id}: according to the advancement condition (%{condition}), fewer than 25%% of competitors would be eliminated," \
+                      "which is not permitted as per Regulation 9p1. Please update the round information in the manage events page."
+    TOO_MANY_QUALIFIED_WARNING = "Round %{round_id}: more competitors qualified than what the advancement condition planned (%{actual} instead of %{expected}, " \
+                                 "the condition was: %{condition}). Please update the round information in the manage events page."
+    NOT_ENOUGH_QUALIFIED_WARNING = "Round %{round_id}: according to the events data, at most %{expected} could have proceed, but only %{actual} competed in the round. " \
+                                   "Please leave a comment about that (or fix the events data if you applied a different advancement condition)."
+    COMPETED_NOT_QUALIFIED_ERROR = "Round %{round_id}: %{ids} competed but did not meet the attempt result advancement condition (%{condition}). " \
+                                   "Please make sure the advancement condition reflects what was used during the competition, and remove the results if needed."
 
     # These are the old "(combined) qualification" and "b-final" rounds.
     # They are not taken into account in advancement conditions.
@@ -31,6 +41,7 @@ def validate(competition_ids: [], model: Result, results: nil)
       end
 
       results_by_competition_id.each do |competition_id, results_for_comp|
+        competition = Competition.includes(:rounds).find(competition_id)
         comp_start_date = competitions_start_dates[competition_id]
         results_by_event_id = results_for_comp.group_by(&:eventId)
         results_by_event_id.each do |event_id, results_for_event|
@@ -42,14 +53,7 @@ def validate(competition_ids: [], model: Result, results: nil)
             IGNORE_ROUND_TYPES.include?(round_type_id)
           end
           remaining_number_of_rounds = round_types_in_results.size
-          if remaining_number_of_rounds > 4
-            # https://www.worldcubeassociation.org/regulations/#9m: Events must have at most four rounds.
-            # Should not happen as we already have a validation to create rounds, but who knows...
-            @errors << ValidationError.new(:rounds, competition_id,
-                                           REGULATION_9M_ERROR,
-                                           event_id: event_id)
-          end
-          number_of_people_in_previous_round = nil
+          previous_round_type_id = nil
           (ordered_round_type_ids & round_types_in_results).each do |round_type_id|
             remaining_number_of_rounds -= 1
             number_of_people_in_round = results_by_round_type_id[round_type_id].size
@@ -75,7 +79,30 @@ def validate(competition_ids: [], model: Result, results: nil)
 
             # Check for the number of qualified competitors (only if we are not
             # in a first round).
-            if number_of_people_in_previous_round
+            if previous_round_type_id
+              # Get the actual Round from the website: they are populated for all
+              # competitions and we can check both what actually happens and what
+              # was set to happen.
+              previous_round = competition.find_round_for(event_id, previous_round_type_id)
+              previous_results = results_by_round_type_id[previous_round_type_id]
+              number_of_people_in_previous_round = previous_results.size
+              condition = previous_round.advancement_condition
+
+              # Check that no one proceeded if they shouldn't have
+              if condition.instance_of? AdvancementConditions::AttemptResultCondition
+                current_persons = results_by_round_type_id[round_type_id].map(&:wca_id)
+                people_over_condition = previous_results.filter do |r|
+                  current_persons.include?(r.wca_id) && r.send(r.format.sort_by) > condition.attempt_result
+                end.map(&:wca_id)
+                if people_over_condition.any?
+                  @errors << ValidationError.new(:rounds, competition_id,
+                                                 COMPETED_NOT_QUALIFIED_ERROR,
+                                                 round_id: round_id,
+                                                 ids: people_over_condition.join(","),
+                                                 condition: condition.to_s(previous_round))
+                end
+              end
+
               # Article 9p, since July 20, 2006 until April 13, 2010
               if Date.new(2006, 7, 20) <= comp_start_date &&
                  comp_start_date <= Date.new(2010, 4, 13)
@@ -85,16 +112,43 @@ def validate(competition_ids: [], model: Result, results: nil)
                                                  round_id: round_id)
                 end
               else
+                max_advancing = 3 * number_of_people_in_previous_round / 4
                 # Article 9p1, since April 14, 2010
                 # https://www.worldcubeassociation.org/regulations/#9p1: At least 25% of competitors must be eliminated between consecutive rounds of the same event.
-                if number_of_people_in_round > 3 * number_of_people_in_previous_round / 4
+                if number_of_people_in_round > max_advancing
                   @errors << ValidationError.new(:rounds, competition_id,
                                                  REGULATION_9P1_ERROR,
                                                  round_id: round_id)
                 end
+                if condition
+                  theoritical_number_of_people = condition.max_advancing(previous_results)
+                  if number_of_people_in_round > theoritical_number_of_people
+                    @warnings << ValidationWarning.new(:rounds, competition_id,
+                                                       TOO_MANY_QUALIFIED_WARNING,
+                                                       round_id: round_id,
+                                                       actual: number_of_people_in_round,
+                                                       expected: theoritical_number_of_people,
+                                                       condition: condition.to_s(previous_round))
+                  end
+                  if theoritical_number_of_people > max_advancing
+                    @errors << ValidationError.new(:rounds, competition_id,
+                                                   ROUND_9P1_ERROR,
+                                                   round_id: round_id,
+                                                   condition: condition.to_s(previous_round))
+                  end
+                  # This comes from https://github.com/thewca/worldcubeassociation.org/issues/5587
+                  if theoritical_number_of_people - number_of_people_in_round >= 3 &&
+                     (number_of_people_in_round / theoritical_number_of_people) <= 0.8
+                    @warnings << ValidationWarning.new(:rounds, competition_id,
+                                                       NOT_ENOUGH_QUALIFIED_WARNING,
+                                                       round_id: round_id,
+                                                       expected: theoritical_number_of_people,
+                                                       actual: number_of_people_in_round)
+                  end
+                end
               end
             end
-            number_of_people_in_previous_round = number_of_people_in_round
+            previous_round_type_id = round_type_id
           end
         end
       end