From 96ccc6444ebbe5a8bd233201801593f5fac8d6d0 Mon Sep 17 00:00:00 2001 From: adrunkhuman <16039109+adrunkhuman@users.noreply.github.com> Date: Sun, 22 Mar 2026 02:22:55 +0100 Subject: [PATCH 1/4] fix: avoid cooldown on failed result calculation --- temp-pr-1.md | 7 +++++++ tests/test_admin_commands.py | 22 ++++++++++++++++++++++ typer_bot/commands/admin_commands.py | 4 ++-- 3 files changed, 31 insertions(+), 2 deletions(-) create mode 100644 temp-pr-1.md diff --git a/temp-pr-1.md b/temp-pr-1.md new file mode 100644 index 0000000..419e4a9 --- /dev/null +++ b/temp-pr-1.md @@ -0,0 +1,7 @@ +## Summary +- Move the `/admin results calculate` cooldown write to the successful scoring path so validation failures do not throttle admins. +- Add a regression test covering the failed-calculation retry case. + +## Testing +- `uv run pytest tests/test_admin_commands.py -k "results_calculate"` +- `uv run ruff check typer_bot/commands/admin_commands.py tests/test_admin_commands.py` diff --git a/tests/test_admin_commands.py b/tests/test_admin_commands.py index 08018e9..105a13a 100644 --- a/tests/test_admin_commands.py +++ b/tests/test_admin_commands.py @@ -300,6 +300,28 @@ async def test_results_calculate_records_cooldown_and_calls_followup_steps( mock_interaction_admin, score_result ) + @pytest.mark.asyncio + async def test_results_calculate_does_not_record_cooldown_when_scoring_fails( + self, admin_cog, mock_interaction_admin, monkeypatch + ): + """Validation failures should not throttle a retry after the admin fixes results state.""" + fixture = {"id": 7, "week_number": 7, "games": ["A - B"], "deadline": datetime.now(UTC)} + + monkeypatch.setattr(admin_cog.db, "get_open_fixtures", AsyncMock(return_value=[fixture])) + admin_cog.service.calculate_fixture_scores = AsyncMock( + side_effect=ValueError("No results entered") + ) + admin_cog._create_backup = AsyncMock() + admin_cog._post_calculation_to_channel = AsyncMock() + + await admin_cog.results_calculate.callback(admin_cog, mock_interaction_admin, None) + + user_id = str(mock_interaction_admin.user.id) + assert admin_cog.workflow_state.get_calculate_cooldown(user_id) is None + assert mock_interaction_admin.response_sent[-1]["content"] == "No results entered" + admin_cog._create_backup.assert_not_called() + admin_cog._post_calculation_to_channel.assert_not_called() + class TestResultsPostFlow: @pytest.fixture diff --git a/typer_bot/commands/admin_commands.py b/typer_bot/commands/admin_commands.py index 5823167..c49a218 100644 --- a/typer_bot/commands/admin_commands.py +++ b/typer_bot/commands/admin_commands.py @@ -373,14 +373,14 @@ async def results_calculate(self, interaction: discord.Interaction, week: int | if not fixture: return - self.workflow_state.record_calculate_cooldown(user_id, current_time=current_time) - try: score_result = await self.service.calculate_fixture_scores(fixture["id"]) except ValueError as exc: await interaction.response.send_message(str(exc), ephemeral=True) return + self.workflow_state.record_calculate_cooldown(user_id, current_time=current_time) + await self._create_backup() await self._post_calculation_to_channel(interaction, score_result) From ac0f625f76369506083f794f91c132d7fbc491ed Mon Sep 17 00:00:00 2001 From: adrunkhuman <16039109+adrunkhuman@users.noreply.github.com> Date: Sun, 22 Mar 2026 02:23:13 +0100 Subject: [PATCH 2/4] chore: remove temporary PR notes --- temp-pr-1.md | 7 ------- 1 file changed, 7 deletions(-) delete mode 100644 temp-pr-1.md diff --git a/temp-pr-1.md b/temp-pr-1.md deleted file mode 100644 index 419e4a9..0000000 --- a/temp-pr-1.md +++ /dev/null @@ -1,7 +0,0 @@ -## Summary -- Move the `/admin results calculate` cooldown write to the successful scoring path so validation failures do not throttle admins. -- Add a regression test covering the failed-calculation retry case. - -## Testing -- `uv run pytest tests/test_admin_commands.py -k "results_calculate"` -- `uv run ruff check typer_bot/commands/admin_commands.py tests/test_admin_commands.py` From 3b994f1ceca303c43efd1450e33d4ea1d4cc2a4c Mon Sep 17 00:00:00 2001 From: adrunkhuman <16039109+adrunkhuman@users.noreply.github.com> Date: Sun, 22 Mar 2026 02:24:30 +0100 Subject: [PATCH 3/4] fix: narrow fixture cleanup channel typing --- typer_bot/commands/admin_panel/fixtures.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/typer_bot/commands/admin_panel/fixtures.py b/typer_bot/commands/admin_panel/fixtures.py index 7420356..47c7817 100644 --- a/typer_bot/commands/admin_panel/fixtures.py +++ b/typer_bot/commands/admin_panel/fixtures.py @@ -4,7 +4,7 @@ import logging from collections.abc import Callable, Coroutine -from typing import Any +from typing import Any, Protocol, cast import discord @@ -17,6 +17,10 @@ logger = logging.getLogger(__name__) +class _MessageLookupChannel(Protocol): + async def fetch_message(self, message_id: int, /) -> discord.Message: ... + + async def _cleanup_discord_announcement( bot: discord.Client, channel_id: str, @@ -26,9 +30,10 @@ async def _cleanup_discord_announcement( """Best-effort Discord cleanup — logs on failure.""" try: channel = bot.get_channel(int(channel_id)) - if channel is None: + if channel is None or not hasattr(channel, "fetch_message"): return - message = await channel.fetch_message(int(message_id)) + message_channel = cast(_MessageLookupChannel, channel) + message = await message_channel.fetch_message(int(message_id)) if message.thread is not None: await message.thread.delete() await message.delete() From 3ff31aab5ab114cf294d33ef57654ebef182cc52 Mon Sep 17 00:00:00 2001 From: adrunkhuman <16039109+adrunkhuman@users.noreply.github.com> Date: Sun, 22 Mar 2026 02:26:30 +0100 Subject: [PATCH 4/4] test: cover trust-critical command surfaces --- tests/test_admin_commands.py | 24 ++++++ tests/test_user_commands.py | 149 +++++++++++++++++++++++++++++++++++ 2 files changed, 173 insertions(+) diff --git a/tests/test_admin_commands.py b/tests/test_admin_commands.py index 105a13a..886e91b 100644 --- a/tests/test_admin_commands.py +++ b/tests/test_admin_commands.py @@ -12,6 +12,7 @@ ) from typer_bot.commands.admin_panel import OpenFixtureWarningView from typer_bot.services.admin_service import FixtureScoreResult +from typer_bot.utils import now from typer_bot.utils.permissions import is_admin @@ -322,6 +323,29 @@ async def test_results_calculate_does_not_record_cooldown_when_scoring_fails( admin_cog._create_backup.assert_not_called() admin_cog._post_calculation_to_channel.assert_not_called() + @pytest.mark.asyncio + async def test_results_calculate_rejects_active_cooldown( + self, admin_cog, mock_interaction_admin + ): + """Cooldown should stop duplicate clicks before any scoring work starts.""" + user_id = str(mock_interaction_admin.user.id) + admin_cog.workflow_state.record_calculate_cooldown(user_id, current_time=now().timestamp()) + admin_cog.service.calculate_fixture_scores = AsyncMock() + + await admin_cog.results_calculate.callback(admin_cog, mock_interaction_admin, None) + + assert "Please wait" in mock_interaction_admin.response_sent[-1]["content"] + admin_cog.service.calculate_fixture_scores.assert_not_called() + + @pytest.mark.asyncio + async def test_results_calculate_reports_missing_open_fixture( + self, admin_cog, mock_interaction_admin + ): + """Missing fixture selection should fail through the slash-command response path.""" + await admin_cog.results_calculate.callback(admin_cog, mock_interaction_admin, None) + + assert mock_interaction_admin.response_sent[-1]["content"] == "No open fixtures found!" + class TestResultsPostFlow: @pytest.fixture diff --git a/tests/test_user_commands.py b/tests/test_user_commands.py index a04aaad..2742333 100644 --- a/tests/test_user_commands.py +++ b/tests/test_user_commands.py @@ -1,11 +1,13 @@ """Tests for user command wiring.""" +from datetime import UTC, datetime, timedelta from unittest.mock import AsyncMock, MagicMock import discord import pytest from typer_bot.commands.user_commands import UserCommands +from typer_bot.utils import format_standings @pytest.fixture @@ -48,3 +50,150 @@ async def test_handles_dm_permission_error(self, user_commands, mock_interaction assert len(mock_interaction.followup_sent) == 1 assert "can't send you DMs" in mock_interaction.followup_sent[0]["content"] + + +class TestFixturesCommand: + @pytest.mark.asyncio + async def test_no_open_fixture_shows_error(self, user_commands, mock_interaction): + await user_commands.fixtures.callback(user_commands, mock_interaction) + + assert mock_interaction.response_sent[0]["content"] == "❌ No active fixture found!" + + @pytest.mark.asyncio + async def test_single_open_fixture_lists_games_and_deadline( + self, user_commands, mock_interaction, database, sample_games + ): + await database.create_fixture(1, sample_games, datetime.now(UTC) + timedelta(days=1)) + + await user_commands.fixtures.callback(user_commands, mock_interaction) + + content = mock_interaction.response_sent[0]["content"] + assert "Week 1 Fixtures" in content + assert sample_games[0] in content + assert "Deadline:" in content + assert mock_interaction.response_sent[0]["ephemeral"] is True + + @pytest.mark.asyncio + async def test_multiple_open_fixtures_list_each_week( + self, user_commands, mock_interaction, database, sample_games + ): + deadline = datetime.now(UTC) + timedelta(days=1) + await database.create_fixture(1, sample_games, deadline) + await database.create_fixture(2, sample_games, deadline) + + await user_commands.fixtures.callback(user_commands, mock_interaction) + + content = mock_interaction.response_sent[0]["content"] + assert "Open Fixtures" in content + assert "Week 1" in content + assert "Week 2" in content + + +class TestStandingsCommand: + @pytest.mark.asyncio + async def test_standings_sends_empty_state(self, user_commands, mock_interaction): + await user_commands.standings.callback(user_commands, mock_interaction) + + assert mock_interaction.response_sent[0]["content"] == format_standings([], None) + assert mock_interaction.response_sent[0]["ephemeral"] is True + + @pytest.mark.asyncio + async def test_standings_sends_formatted_leaderboard(self, user_commands, mock_interaction): + standings = [ + { + "user_id": "123", + "user_name": "User1", + "total_points": 9, + "total_exact": 3, + "total_correct": 3, + } + ] + last_fixture = { + "week_number": 4, + "games": ["A - B"], + "results": ["2-1"], + "scores": [ + { + "user_id": "123", + "user_name": "User1", + "points": 3, + "exact_scores": 1, + "correct_results": 1, + } + ], + } + user_commands.db.get_standings = AsyncMock(return_value=standings) + user_commands.db.get_last_fixture_scores = AsyncMock(return_value=last_fixture) + + await user_commands.standings.callback(user_commands, mock_interaction) + + assert mock_interaction.response_sent[0]["content"] == format_standings( + standings, last_fixture + ) + + +class TestMyPredictionsCommand: + @pytest.mark.asyncio + async def test_no_open_fixture_shows_error(self, user_commands, mock_interaction): + await user_commands.my_predictions.callback(user_commands, mock_interaction) + + assert mock_interaction.response_sent[0]["content"] == "❌ No active fixture found!" + + @pytest.mark.asyncio + async def test_single_fixture_without_prediction_shows_prompt( + self, user_commands, mock_interaction, database, sample_games + ): + await database.create_fixture(1, sample_games, datetime.now(UTC) + timedelta(days=1)) + + await user_commands.my_predictions.callback(user_commands, mock_interaction) + + content = mock_interaction.response_sent[0]["content"] + assert "haven't submitted predictions" in content + assert "Use `/predict`" in content + + @pytest.mark.asyncio + async def test_single_fixture_prediction_shows_saved_scores( + self, user_commands, mock_interaction, database, sample_games + ): + fixture_id = await database.create_fixture( + 1, sample_games, datetime.now(UTC) + timedelta(days=1) + ) + await database.save_prediction( + fixture_id, + str(mock_interaction.user.id), + mock_interaction.user.name, + ["2-1", "1-1", "0-2"], + False, + ) + + await user_commands.my_predictions.callback(user_commands, mock_interaction) + + content = mock_interaction.response_sent[0]["content"] + assert "Your Predictions:" in content + assert f"1. {sample_games[0]} **2-1**" in content + assert "Status:" in content + assert "Submitted:" in content + + @pytest.mark.asyncio + async def test_multiple_open_fixtures_show_mixed_prediction_state( + self, user_commands, mock_interaction, database, sample_games + ): + deadline = datetime.now(UTC) + timedelta(days=1) + fixture_week_1 = await database.create_fixture(1, sample_games, deadline) + await database.create_fixture(2, sample_games, deadline) + await database.save_prediction( + fixture_week_1, + str(mock_interaction.user.id), + mock_interaction.user.name, + ["2-1", "1-1", "0-2"], + False, + ) + + await user_commands.my_predictions.callback(user_commands, mock_interaction) + + content = mock_interaction.response_sent[0]["content"] + assert "Your Predictions (Open Fixtures):" in content + assert "Week 1" in content + assert "Week 2" in content + assert f"1. {sample_games[0]} **2-1**" in content + assert "No prediction submitted yet." in content