Skip to content

fix: remove sentinel-player dependency in teams-left #54

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.

Sign up for GitHub

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

Merged
mateu merged 2 commits into from
Apr 6, 2026
Merged

fix: remove sentinel-player dependency in teams-left #54

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ab4fcec
fix: remove player-24 sentinel dependency in teams-left metrics
mateu Apr 6, 2026
adeaa51
test: add regression for max_left=0 when no non-admin players have picks
Copilot Apr 6, 2026
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
24 changes: 14 additions & 10 deletions lib/Bracket/Model/DBIC.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -506,10 +506,9 @@ sub count_player_teams_left {
from games_played
),
games_remaining as (
select p.game
from pick p
where p.player = 24
and p.game not in (select game from games_played)
select g.id as game
from game g
where g.id not in (select game from games_played)
),
player_teams_remaining as (
select player, pick
Expand All @@ -529,7 +528,10 @@ sub count_player_teams_left {
foreach my $row (@{$result}) {
$teams_left_per_player->{$row->[0]} = $row->[1];
}
my $max_left = max map { $teams_left_per_player->{$_} } grep { $_ != 1 } keys %{$teams_left_per_player};
my @players = grep { $_ != 1 } keys %{$teams_left_per_player};
my $max_left = @players
? max(map { $teams_left_per_player->{$_} } @players)
: 0;
Comment on lines +531 to +534
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

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

The new empty-non-admin-player branch for $max_left (returning 0 when there are no players other than player 1 in the result map) isn’t covered by tests. Add a regression case where only the admin/perfect player exists (or where no non-admin players have remaining teams) and assert $max_left is 0, to prevent reintroducing the previous max-on-empty behavior.

Copilot uses AI. Check for mistakes.
return $teams_left_per_player, $max_left;
}
);
Expand Down Expand Up @@ -566,10 +568,9 @@ sub count_player_final4_teams_left {
from games_played
),
games_remaining as (
select p.game
from pick p
where p.player = 24
and p.game not in (select game from games_played)
select g.id as game
from game g
where g.id not in (select game from games_played)
),
player_teams_remaining as (
select player, pick
Expand All @@ -594,7 +595,10 @@ sub count_player_final4_teams_left {
foreach my $row (@{$result}) {
$final4_teams_left_per_player->{$row->[0]} = $row->[1];
}
my $max_left = max map { $final4_teams_left_per_player->{$_} } grep { $_ != 1 } keys %{$final4_teams_left_per_player};
my @players = grep { $_ != 1 } keys %{$final4_teams_left_per_player};
my $max_left = @players
? max(map { $final4_teams_left_per_player->{$_} } @players)
: 0;
Comment on lines +598 to +601
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

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

Similar to count_player_teams_left: the new $max_left fallback to 0 when there are no non-admin players in the computed map is not exercised by the current test. Add a regression test for count_player_final4_teams_left where no non-admin players are present (or none have remaining final4 teams) and assert $max_left is 0.

Copilot uses AI. Check for mistakes.
return $final4_teams_left_per_player, $max_left;
}
);
Expand Down
41 changes: 26 additions & 15 deletions t/model_round_based_final4_counts.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,16 @@ my $dbh = $schema->storage->dbh;
$dbh->sqlite_create_function('get_loser', 1, sub { return -1; });
$dbh->sqlite_create_function('get_first_round_loser', 1, sub { return -1; });

# Regression: max_left must be 0 (not an error) when no non-admin players have picks.
{
my ($tl, $ml) = Bracket::Model::DBIC::count_player_teams_left($model);
is($ml, 0, 'count_player_teams_left: max_left is 0 when no non-admin players have picks');
}
{
my ($tl, $ml) = Bracket::Model::DBIC::count_player_final4_teams_left($model);
is($ml, 0, 'count_player_final4_teams_left: max_left is 0 when no non-admin players have picks');
}

my $player = $schema->resultset('Player')->create({
email => 'round-based-final4@example.com',
password => 'secret',
Expand Down Expand Up @@ -64,21 +74,11 @@ $schema->resultset('Team')->create({
round_out => 7,
});

# Create a player with id 24 to satisfy foreign key constraints on Pick.
$schema->resultset('Player')->create({
id => 24,
email => 'player-24@example.com',
password => 'secret',
first_name => 'Player',
last_name => 'TwentyFour',
});

# player 24 drives games_remaining in count_player_final4_teams_left.
$schema->resultset('Pick')->create({
player => 24,
game => $round4_game_id,
pick => $round4_team_id,
});
# Regression: teams-left queries should not rely on a sentinel player id.
ok(
!$schema->resultset('Player')->find(24),
'test fixture intentionally omits player 24 sentinel',
);
$schema->resultset('Pick')->create({
player => $player->id,
game => $round4_game_id,
Expand All @@ -96,4 +96,15 @@ ok(
'count_player_final4_teams_left reports max_left from computed map',
);

my ($all_teams_left, $all_max_left) = Bracket::Model::DBIC::count_player_teams_left($model);
is(
($all_teams_left->{$player->id} || 0) >= 1,
1,
'count_player_teams_left no longer depends on sentinel player rows',
);
ok(
defined $all_max_left && $all_max_left >= 1,
'count_player_teams_left reports max_left without sentinel player data',
);

done_testing();
Loading