From 3d9503c2150467f8d8c005f1c49de029141e5098 Mon Sep 17 00:00:00 2001 From: adrunkhuman <16039109+adrunkhuman@users.noreply.github.com> Date: Sun, 10 May 2026 22:43:37 +0200 Subject: [PATCH] fix: close v3 umbrella gaps --- tests/test_database.py | 80 +++++++++++++++++++ tests/test_prediction_submission.py | 43 ++++++++++ typer_bot/commands/user_commands.py | 37 ++++++--- typer_bot/database/connection.py | 27 +++++++ typer_bot/database/scores.py | 23 ++++-- .../handlers/thread_prediction_handler.py | 35 +++++--- typer_bot/utils/__init__.py | 3 + typer_bot/utils/prediction_submission.py | 35 ++++++++ 8 files changed, 250 insertions(+), 33 deletions(-) create mode 100644 tests/test_prediction_submission.py create mode 100644 typer_bot/utils/prediction_submission.py diff --git a/tests/test_database.py b/tests/test_database.py index 1c69339..c52ab95 100644 --- a/tests/test_database.py +++ b/tests/test_database.py @@ -1145,6 +1145,50 @@ async def test_standings_are_active_season_scoped(self, temp_db_path): assert standings[0]["user_name"] == "Active Shared User" assert standings[0]["total_points"] == 3 + @pytest.mark.asyncio + async def test_get_standings_for_season_returns_archived_season_scores(self, temp_db_path): + db = Database(temp_db_path) + await db.initialize() + old_fixture_id = await db.create_fixture( + "111111", 1, ["Old Team A - Old Team B"], datetime.now(UTC) + ) + await db.save_scores( + old_fixture_id, + [ + { + "user_id": "old-user", + "user_name": "Old User", + "points": 30, + "exact_scores": 10, + "correct_results": 0, + } + ], + ) + old_season = await db.get_active_season("111111") + await _start_new_active_season(temp_db_path, "111111") + active_fixture_id = await db.create_fixture( + "111111", 1, ["New Team A - New Team B"], datetime.now(UTC) + ) + await db.save_scores( + active_fixture_id, + [ + { + "user_id": "active-user", + "user_name": "Active User", + "points": 3, + "exact_scores": 1, + "correct_results": 0, + } + ], + ) + + standings = await db.get_standings_for_season("111111", old_season["id"]) + wrong_guild_standings = await db.get_standings_for_season("222222", old_season["id"]) + + assert [row["user_id"] for row in standings] == ["old-user"] + assert standings[0]["total_points"] == 30 + assert wrong_guild_standings == [] + @pytest.mark.asyncio async def test_last_fixture_scores_are_active_season_scoped(self, temp_db_path): db = Database(temp_db_path) @@ -1813,6 +1857,42 @@ async def test_initialize_rejects_null_fixture_guild_ownership(self, temp_db_pat with pytest.raises(RuntimeError, match="fixtures.guild_id has empty rows"): await db.initialize() + @pytest.mark.asyncio + @pytest.mark.parametrize( + "sql", + [ + "UPDATE fixtures SET season_id = NULL WHERE id = ?", + "UPDATE fixtures SET season_id = 999999 WHERE id = ?", + ], + ) + async def test_initialize_rejects_fixture_without_valid_season(self, temp_db_path, sql): + db = Database(temp_db_path) + await db.initialize() + fixture_id = await db.create_fixture("111111", 1, ["A - B"], datetime.now(UTC)) + async with aiosqlite.connect(temp_db_path) as conn: + await conn.execute(sql, (fixture_id,)) + await conn.commit() + + with pytest.raises(RuntimeError, match="same-guild season_id"): + await db.initialize() + + @pytest.mark.asyncio + async def test_initialize_rejects_fixture_with_other_guild_season(self, temp_db_path): + db = Database(temp_db_path) + await db.initialize() + fixture_id = await db.create_fixture("111111", 1, ["A - B"], datetime.now(UTC)) + other_fixture_id = await db.create_fixture("222222", 1, ["C - D"], datetime.now(UTC)) + other_fixture = await db.get_fixture_by_id(other_fixture_id, "222222") + async with aiosqlite.connect(temp_db_path) as conn: + await conn.execute( + "UPDATE fixtures SET season_id = ? WHERE id = ?", + (other_fixture["season_id"], fixture_id), + ) + await conn.commit() + + with pytest.raises(RuntimeError, match="same-guild season_id"): + await db.initialize() + @pytest.fixture async def prediction_db(temp_db_path): diff --git a/tests/test_prediction_submission.py b/tests/test_prediction_submission.py new file mode 100644 index 0000000..1fb8f0f --- /dev/null +++ b/tests/test_prediction_submission.py @@ -0,0 +1,43 @@ +"""Tests for shared prediction submission decisions.""" + +from datetime import UTC, datetime, timedelta + +from typer_bot.utils import build_prediction_submission + + +def test_on_time_full_submission_is_ready_for_scoring(): + deadline = datetime.now(UTC) + timedelta(hours=1) + fixture = {"deadline": deadline, "games": ["A - B", "C - D"]} + + submission = build_prediction_submission( + fixture=fixture, + predicted_game_indexes=[0, 1], + submitted_at=deadline - timedelta(minutes=1), + public_message_id="123", + public_message_kind="thread_message", + ) + + assert submission.is_late is False + assert submission.is_partial is False + assert submission.pending_partial_approval is False + assert submission.public_message_id is None + assert submission.public_message_kind is None + + +def test_late_partial_submission_keeps_public_review_metadata(): + deadline = datetime.now(UTC) - timedelta(hours=1) + fixture = {"deadline": deadline, "games": ["A - B", "C - D"]} + + submission = build_prediction_submission( + fixture=fixture, + predicted_game_indexes=[0], + submitted_at=deadline + timedelta(minutes=1), + public_message_id="123", + public_message_kind="thread_message", + ) + + assert submission.is_late is True + assert submission.is_partial is True + assert submission.pending_partial_approval is True + assert submission.public_message_id == "123" + assert submission.public_message_kind == "thread_message" diff --git a/typer_bot/commands/user_commands.py b/typer_bot/commands/user_commands.py index dc4b429..b48741d 100644 --- a/typer_bot/commands/user_commands.py +++ b/typer_bot/commands/user_commands.py @@ -11,6 +11,7 @@ from typer_bot.database import Database, SaveResult from typer_bot.utils import ( + build_prediction_submission, format_for_discord, format_predictions_preview, format_standings, @@ -425,15 +426,18 @@ async def on_submit(self, interaction: discord.Interaction): ) return - is_late = now() > fixture["deadline"] + submitted_at = now() existing_prediction = await self.db.get_prediction( fixture["id"], str(interaction.user.id), fixture["guild_id"] ) - is_partial = len(predicted_game_indexes) < len(fixture["games"]) - pending_partial_approval = is_late and is_partial + submission = build_prediction_submission( + fixture=fixture, + predicted_game_indexes=predicted_game_indexes, + submitted_at=submitted_at, + ) admin_role_mention = ( await get_configured_admin_role_mention(str(interaction.guild_id), self.db) - if pending_partial_approval + if submission.pending_partial_approval else None ) public_message = None @@ -445,8 +449,8 @@ async def on_submit(self, interaction: discord.Interaction): predictions, predicted_game_indexes, is_update=existing_prediction is not None, - is_late=is_late, - pending_partial_approval=pending_partial_approval, + is_late=submission.is_late, + pending_partial_approval=submission.pending_partial_approval, admin_role_mention=admin_role_mention, ) ) @@ -457,17 +461,24 @@ async def on_submit(self, interaction: discord.Interaction): ) return + submission = build_prediction_submission( + fixture=fixture, + predicted_game_indexes=predicted_game_indexes, + submitted_at=submitted_at, + public_message_id=str(public_message.id), + public_message_kind="bot_post", + ) try: result = await self.db.save_prediction_guarded( fixture["id"], str(interaction.user.id), self.user_name, predictions, - is_late, + submission.is_late, predicted_game_indexes=predicted_game_indexes, - pending_partial_approval=pending_partial_approval, - public_message_id=str(public_message.id) if pending_partial_approval else None, - public_message_kind="bot_post" if pending_partial_approval else None, + pending_partial_approval=submission.pending_partial_approval, + public_message_id=submission.public_message_id, + public_message_kind=submission.public_message_kind, ) except Exception: logger.exception( @@ -508,14 +519,14 @@ async def on_submit(self, interaction: discord.Interaction): deadline_str = format_for_discord(fixture["deadline"], "F") relative_str = format_for_discord(fixture["deadline"], "R") content += f"\n\n**Posted publicly in the fixture thread.**\n**Deadline:** {deadline_str} ({relative_str})" - if pending_partial_approval: + if submission.pending_partial_approval: content += ( "\n\n⏳ **Late prediction awaiting admin review:** your predicted games will only count " "if an admin approves this late submission with missing games." ) - elif is_late: + elif submission.is_late: content += "\n\n⚠️ **Late prediction!** The active season's late penalty applies unless an admin waives it." - elif is_partial: + elif submission.is_partial: content += ( "\n\nℹ️ **Partial prediction saved:** any missing games will count as no prediction. " "If the deadline has not passed yet, use `/predict` again to fill the rest." diff --git a/typer_bot/database/connection.py b/typer_bot/database/connection.py index 2badfe5..c0e54ed 100644 --- a/typer_bot/database/connection.py +++ b/typer_bot/database/connection.py @@ -129,6 +129,28 @@ async def _validate_fixture_guild_ownership(db: aiosqlite.Connection) -> None: ) +async def _validate_fixture_season_ownership(db: aiosqlite.Connection) -> None: + async with db.execute( + """ + SELECT f.id + FROM fixtures f + LEFT JOIN seasons s ON s.id = f.season_id AND s.guild_id = f.guild_id + WHERE f.season_id IS NULL OR s.id IS NULL + ORDER BY f.id + LIMIT 5 + """ + ) as cursor: + rows = await cursor.fetchall() + if not rows: + return + + fixture_ids = ", ".join(str(row[0]) for row in rows) + raise RuntimeError( + "fixtures has rows without a valid same-guild season_id: " + f"{fixture_ids}. Assign every fixture to a season owned by the same guild before starting the bot." + ) + + async def _validate_current_schema(db: aiosqlite.Connection) -> None: for table_name, required_columns in REQUIRED_COLUMNS.items(): columns = await _table_columns(db, table_name) @@ -218,6 +240,7 @@ async def initialize(self) -> None: if has_existing_schema: await _validate_current_schema(db) await _validate_fixture_guild_ownership(db) + await _validate_fixture_season_ownership(db) await _validate_unique_results(db) await db.execute(""" @@ -310,6 +333,7 @@ async def initialize(self) -> None: if not has_existing_schema: await _validate_current_schema(db) await _validate_fixture_guild_ownership(db) + await _validate_fixture_season_ownership(db) await _validate_unique_results(db) await db.execute( @@ -549,5 +573,8 @@ async def recalculate_fixture_scores(self, fixture_id): async def get_standings(self, guild_id): return await self._scores.get_standings(guild_id) + async def get_standings_for_season(self, guild_id, season_id): + return await self._scores.get_standings_for_season(guild_id, season_id) + async def get_last_fixture_scores(self, guild_id): return await self._scores.get_last_fixture_scores(guild_id) diff --git a/typer_bot/database/scores.py b/typer_bot/database/scores.py index 8249cf0..f90fff8 100644 --- a/typer_bot/database/scores.py +++ b/typer_bot/database/scores.py @@ -261,15 +261,24 @@ async def recalculate_fixture_scores(self, fixture_id: int) -> None: async def get_standings(self, guild_id: str) -> list[dict]: """Get standings for one guild.""" + return await self._get_standings(guild_id) + + async def get_standings_for_season(self, guild_id: str, season_id: int) -> list[dict]: + """Get standings for one season owned by one guild.""" + return await self._get_standings(guild_id, season_id=season_id) + + async def _get_standings(self, guild_id: str, season_id: int | None = None) -> list[dict]: + season_filter = "season.id = ?" if season_id is not None else "season.status = 'active'" + params = (guild_id, season_id) if season_id is not None else (guild_id,) async with aiosqlite.connect(self.db_path) as db: db.row_factory = aiosqlite.Row async with db.execute( - """WITH guild_scores AS ( + f"""WITH guild_scores AS ( SELECT s.* FROM scores s JOIN fixtures f ON f.id = s.fixture_id JOIN seasons season ON season.id = f.season_id AND season.guild_id = f.guild_id - WHERE f.guild_id = ? AND season.status = 'active' + WHERE f.guild_id = ? AND {season_filter} ), latest_names AS ( SELECT user_id, user_name @@ -292,11 +301,11 @@ async def get_standings(self, guild_id: str) -> list[dict]: SUM(gs.exact_scores) as total_exact, SUM(gs.correct_results) as total_correct, COUNT(DISTINCT gs.fixture_id) as weeks_played - FROM guild_scores gs - JOIN latest_names ln ON ln.user_id = gs.user_id - GROUP BY gs.user_id, ln.user_name - ORDER BY total_points DESC, total_exact DESC, total_correct DESC, ln.user_name ASC""", - (guild_id,), + FROM guild_scores gs + JOIN latest_names ln ON ln.user_id = gs.user_id + GROUP BY gs.user_id, ln.user_name + ORDER BY total_points DESC, total_exact DESC, total_correct DESC, ln.user_name ASC""", + params, ) as cursor: rows = await cursor.fetchall() return [ diff --git a/typer_bot/handlers/thread_prediction_handler.py b/typer_bot/handlers/thread_prediction_handler.py index d8b2f53..af67560 100644 --- a/typer_bot/handlers/thread_prediction_handler.py +++ b/typer_bot/handlers/thread_prediction_handler.py @@ -8,7 +8,12 @@ import discord from typer_bot.database import Database, SaveResult -from typer_bot.utils import get_configured_admin_role_mention, now, parse_prediction_lines +from typer_bot.utils import ( + build_prediction_submission, + get_configured_admin_role_mention, + now, + parse_prediction_lines, +) from typer_bot.utils.logger import LogContextManager, log_event logger = logging.getLogger(__name__) @@ -153,9 +158,13 @@ async def on_message(self, message: discord.Message): logger.debug(f"Ignoring message with no valid scores from {message.author.id}") return False - is_late = current_time > fixture["deadline"] - is_partial = len(predicted_game_indexes) < len(fixture["games"]) - pending_partial_approval = is_late and is_partial + submission = build_prediction_submission( + fixture=fixture, + predicted_game_indexes=predicted_game_indexes, + submitted_at=current_time, + public_message_id=str(message.id), + public_message_kind="thread_message", + ) try: result = await self.db.try_save_prediction( @@ -163,11 +172,11 @@ async def on_message(self, message: discord.Message): user_id, message.author.display_name, predictions, - is_late, + submission.is_late, predicted_game_indexes=predicted_game_indexes, - pending_partial_approval=pending_partial_approval, - public_message_id=str(message.id) if pending_partial_approval else None, - public_message_kind="thread_message" if pending_partial_approval else None, + pending_partial_approval=submission.pending_partial_approval, + public_message_id=submission.public_message_id, + public_message_kind=submission.public_message_kind, ) if result == SaveResult.FIXTURE_CLOSED: @@ -205,7 +214,7 @@ async def on_message(self, message: discord.Message): return True try: - await message.add_reaction("⏳" if pending_partial_approval else "✅") + await message.add_reaction("⏳" if submission.pending_partial_approval else "✅") except discord.Forbidden: logger.warning( f"Could not add reaction to thread prediction from {message.author.id}. " @@ -228,12 +237,12 @@ async def on_message(self, message: discord.Message): week_number=fixture["week_number"], source="thread", predictions_count=len(predictions), - is_late=is_late, + is_late=submission.is_late, ) - if is_late: + if submission.is_late: with suppress(discord.Forbidden): - if pending_partial_approval: + if submission.pending_partial_approval: admin_role_mention = ( await get_configured_admin_role_mention(str(message.guild.id), self.db) if message.guild @@ -252,7 +261,7 @@ async def on_message(self, message: discord.Message): "⚠️ **Late prediction!** Your prediction was saved but you will receive " "the active season's late penalty unless an admin waives it." ) - elif is_partial: + elif submission.is_partial: with suppress(discord.Forbidden): await message.author.send( "ℹ️ **Partial prediction saved.** Any missing games will count as no prediction. If the deadline has not passed yet, use `/predict` again to fill the rest." diff --git a/typer_bot/utils/__init__.py b/typer_bot/utils/__init__.py index 09f6500..914e0af 100644 --- a/typer_bot/utils/__init__.py +++ b/typer_bot/utils/__init__.py @@ -19,6 +19,7 @@ parse_prediction_lines, parse_predictions, ) +from .prediction_submission import PredictionSubmission, build_prediction_submission from .scoring import ( DEFAULT_SCORING_RULES, align_predictions_to_fixture, @@ -43,6 +44,8 @@ "format_fixture_results", "format_predictions_preview", "format_standings", + "PredictionSubmission", + "build_prediction_submission", "align_predictions_to_fixture", "DEFAULT_SCORING_RULES", "calculate_points", diff --git a/typer_bot/utils/prediction_submission.py b/typer_bot/utils/prediction_submission.py new file mode 100644 index 0000000..5b7c6d7 --- /dev/null +++ b/typer_bot/utils/prediction_submission.py @@ -0,0 +1,35 @@ +"""Shared prediction submission decisions.""" + +from __future__ import annotations + +from dataclasses import dataclass +from datetime import datetime + + +@dataclass(frozen=True, slots=True) +class PredictionSubmission: + is_late: bool + is_partial: bool + pending_partial_approval: bool + public_message_id: str | None + public_message_kind: str | None + + +def build_prediction_submission( + *, + fixture: dict, + predicted_game_indexes: list[int], + submitted_at: datetime, + public_message_id: str | None = None, + public_message_kind: str | None = None, +) -> PredictionSubmission: + is_late = submitted_at > fixture["deadline"] + is_partial = len(predicted_game_indexes) < len(fixture["games"]) + pending_partial_approval = is_late and is_partial + return PredictionSubmission( + is_late=is_late, + is_partial=is_partial, + pending_partial_approval=pending_partial_approval, + public_message_id=public_message_id if pending_partial_approval else None, + public_message_kind=public_message_kind if pending_partial_approval else None, + )