Skip to content

Added checks for regulation 9m #215

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 47 additions & 27 deletions lib/wca_live/scoretaking.ex
Original file line number Diff line number Diff line change
Expand Up @@ -224,41 +224,61 @@ defmodule WcaLive.Scoretaking do
if Round.open?(round) do
{:error, "cannot open this round as it is already open"}
else
Multi.new()
|> Multi.run(:finish_previous, fn _repo, _changes ->
if round.number > 1 do
previous = get_previous_round(round) |> Repo.preload(:results)
finish_round(previous)
else
{:ok, nil}
message = validate_previous_rounds(round)
if message do
Comment on lines +227 to +228
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid nesting you can do this:

cond do
  Round.open?(round) ->
    {:error, ...}

  message = validate_previous_rounds(round) ->
    {:error, error}

  true ->
    Multi.new()
    |> ...
end

{:error, message}
else
Multi.new()
|> Multi.run(:finish_previous, fn _repo, _changes ->
if round.number > 1 do
previous = get_previous_round(round) |> Repo.preload(:results)
finish_round(previous)
else
{:ok, nil}
end
end)
|> Multi.run(:create_results, fn _repo, _changes ->
create_empty_results(round)
end)
|> Repo.transaction()
|> case do
{:ok, %{create_results: round}} -> {:ok, round}
{:error, _, reason, _} -> {:error, reason}
end
end)
|> Multi.run(:create_results, fn _repo, _changes ->
create_empty_results(round)
end)
|> Repo.transaction()
|> case do
{:ok, %{create_results: round}} -> {:ok, round}
{:error, _, reason, _} -> {:error, reason}
end
end
end

# Check that enough people have competed in the previous round in order to open the next round
# See https://www.worldcubeassociation.org/regulations/#9m
defp validate_previous_rounds(round) do
min = [8, 16, 100]
round_results =
Enum.to_list(1..round.number - 1 // 1)
|> Enum.map(
fn round_offset ->
Round |> Round.where_sibling(round, -round_offset) |> Repo.one() |> Repo.preload(:results)
Copy link
Member

@jonatanklosko jonatanklosko Mar 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of querying each round separately, let's add list_previous_rounds. It should return rounds ordered by round number, and here we can reverse for the check.

end
)
|> Enum.map(fn round -> round.results |> Enum.reject(&Result.empty?/1) |> length() end)

Enum.find_index(Enum.zip(round_results, min), fn {x, y} -> x < y end)
|> case do
0 -> "Rounds with 7 or fewer competitors must not have subsequent rounds"
1 -> "Rounds with 15 or fewer competitors must have at most one subsequent round"
2 -> "Rounds with 99 or fewer competitors must have at most two subsequent rounds"
Comment on lines +267 to +269
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually we keep {:error, _} messages lowercased. We transform to uppercase in other places as needed.

Suggested change
0 -> "Rounds with 7 or fewer competitors must not have subsequent rounds"
1 -> "Rounds with 15 or fewer competitors must have at most one subsequent round"
2 -> "Rounds with 99 or fewer competitors must have at most two subsequent rounds"
0 -> "rounds with 7 or fewer competitors must not have subsequent rounds"
1 -> "rounds with 15 or fewer competitors must have at most one subsequent round"
2 -> "rounds with 99 or fewer competitors must have at most two subsequent rounds"

_ -> :nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only other value we expected is nil (not found), so let's be explicit:

Suggested change
_ -> :nil
nil -> nil

(also, nil is an atom, but you don't need : because it's special, same with true and false :))

end
end

# Finishes `round` by removing empty results, so that the next round may be opened.
defp finish_round(round) do
actual_results = Enum.reject(round.results, &Result.empty?/1)

if length(actual_results) < 8 do
# See: https://www.worldcubeassociation.org/regulations/#9m3
{:error, "rounds with less than 8 competitors cannot have a subsequent round"}
else
# Note: we remove empty results, so we need to recompute advancing results,
# because an advancement condition may depend on the total number of results (i.e. "percent" type).
# Note: we remove empty results, so we need to recompute advancing results,
# because an advancement condition may depend on the total number of results (i.e. "percent" type).

actual_results
|> Round.put_results_in_round(round)
|> update_round_and_advancing()
end
Enum.reject(round.results, &Result.empty?/1)
|> Round.put_results_in_round(round)
|> update_round_and_advancing()
end

defp create_empty_results(round) do
Expand Down