From 426849af881d289800be29b459052c2f9b168501 Mon Sep 17 00:00:00 2001 From: adrunkhuman <16039109+adrunkhuman@users.noreply.github.com> Date: Sun, 28 Jun 2026 16:47:54 +0200 Subject: [PATCH] fix: split oversized Discord result posts --- AGENTS.md | 2 + README.md | 1 + .../test_fixture_panel_results_actions.py | 101 +++++++++- tests/conftest.py | 11 +- tests/test_admin_commands.py | 95 +++++++++- tests/test_calculation_posting.py | 126 +++++++++++++ tests/test_discord_messages.py | 72 ++++++++ tests/user_commands/test_standings_command.py | 38 ++++ typer_bot/commands/admin_commands.py | 1 + .../commands/admin_panel/unified_actions.py | 55 ++++-- typer_bot/commands/user_commands.py | 32 +--- typer_bot/services/calculation_posting.py | 44 +++-- typer_bot/utils/__init__.py | 10 + typer_bot/utils/discord_messages.py | 173 ++++++++++++++++++ 14 files changed, 690 insertions(+), 71 deletions(-) create mode 100644 tests/test_discord_messages.py create mode 100644 typer_bot/utils/discord_messages.py diff --git a/AGENTS.md b/AGENTS.md index 35bb43f..81bdf2e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -115,6 +115,7 @@ guild_config ( - `typer_bot/commands/admin_commands.py`: `/admin` command surface and orchestration for admin workflows, including admin calculation cooldown state. - `typer_bot/services/calculation_posting.py`: Post-calculation side effects: best-effort DB backup, league-channel publishing, and admin interaction responses. - `typer_bot/utils/config.py`: Centralized configuration (data paths via env vars). +- `typer_bot/utils/discord_messages.py`: Shared Discord 2000-character-safe message chunking helpers. - `typer_bot/utils/prediction_parser.py`: Central logic for parsing "2-1" or "2:1" strings. - `typer_bot/utils/scoring.py`: Point calculation using season scoring rules. - `typer_bot/utils/logger.py`: plain stdout logging setup with contextual fields. @@ -128,6 +129,7 @@ guild_config ( - **New Commands:** Add Cog to `commands/` folder, load in `bot.py`. - **Database Changes:** Edit `typer_bot/database/connection.py` `initialize()` and the focused repositories in `typer_bot/database/`. Keep fresh schema creation and startup validation explicit; handle production data ports manually when needed. - **Debugging:** Check `utils/logger.py` for config. Set `LOG_LEVEL=DEBUG` in env. +- **Discord Message Lengths:** Use `utils/discord_messages.py` helpers instead of ad-hoc 2000-character splitting. - **Database Restore:** Use `scripts/restore_db.py` from the host or container shell for manual database restoration from backups. The script restores into a temporary SQLite file first, then atomically replaces the live DB only after success. ## 5.5 Testing Guidelines diff --git a/README.md b/README.md index fa0fc7a..7b96f2b 100644 --- a/README.md +++ b/README.md @@ -11,6 +11,7 @@ Server admins invite and configure the bot. They do not self-host it. - Standings use the server's active season only. - Scoring rules are stored per season and lock once scores exist. - SQLite stores league data; backups run after successful score calculation. +- Large result/standings posts may be split across multiple Discord messages; participant mentions may be posted as separate `Participants` chunks. ## Server Admin Setup diff --git a/tests/admin_panel/test_fixture_panel_results_actions.py b/tests/admin_panel/test_fixture_panel_results_actions.py index 54c370e..8c23528 100644 --- a/tests/admin_panel/test_fixture_panel_results_actions.py +++ b/tests/admin_panel/test_fixture_panel_results_actions.py @@ -123,6 +123,49 @@ async def test_unified_panel_calculate_scores_button_posts_results( assert _has_button(view, "Delete Fixture") is False assert mock_interaction_admin.message.edit.await_count == 1 + @pytest.mark.asyncio + async def test_unified_panel_calculate_scores_ignores_missing_panel_message( + self, + admin_cog, + mock_interaction_admin, + sample_games, + monkeypatch, + ): + fixture_id = await admin_cog.db.fixtures.create_fixture( + "111111", 50, sample_games, datetime.now(UTC) + timedelta(days=1) + ) + await admin_cog.db.results.save_results(fixture_id, ["2-1", "1-1", "0-2"]) + await admin_cog.db.predictions.save_prediction( + fixture_id, + "111", + "User One", + ["2-1", "1-1", "0-2"], + False, + ) + mock_interaction_admin.message = MagicMock() + mock_interaction_admin.message.edit = AsyncMock( + side_effect=discord.NotFound(MagicMock(), "missing") + ) + monkeypatch.setattr(unified_actions, "post_calculation_result", AsyncMock()) + + view = UnifiedAdminPanelView( + admin_cog.db, + admin_cog.service, + str(mock_interaction_admin.user.id), + "111111", + admin_commands=admin_cog, + bot=admin_cog.bot, + ) + await view.load_fixture_options() + view.fixture_select._values = [str(fixture_id)] + await view.fixture_select.callback(mock_interaction_admin) + + calculate_button = _get_button(view, "Calculate Scores") + await calculate_button.callback(mock_interaction_admin) + + assert view.selection.fixture_label == "Week 50 [CLOSED]" + assert mock_interaction_admin.message.edit.await_count == 1 + @pytest.mark.asyncio async def test_stale_calculate_scores_button_refreshes_when_fixture_already_scored( self, @@ -236,7 +279,7 @@ async def test_unified_panel_calculate_scores_button_handles_service_error( await calculate_button.callback(mock_interaction_admin) assert ( - mock_interaction_admin.response_sent[-1]["content"] + mock_interaction_admin.followup_sent[-1]["content"] == "No predictions found for this fixture" ) post_calculation_result.assert_not_awaited() @@ -335,6 +378,62 @@ async def test_unified_panel_post_results_button_opens_confirmation( assert isinstance(confirm_view, PostResultsConfirmView) assert confirm_view.channel is league_channel + @pytest.mark.asyncio + async def test_unified_panel_post_results_button_splits_oversized_preview( + self, + admin_cog, + mock_interaction_admin, + ): + league_channel = MagicMock(spec=discord.TextChannel) + league_channel.id = 123456 + admin_cog.bot.get_channel.return_value = league_channel + admin_cog.db.scores.get_last_fixture_scores = AsyncMock( + return_value={ + "week_number": 13, + "games": ["A - B"], + "results": ["2-1"], + "scores": [ + { + "user_id": str(index), + "user_name": f"VeryLongUserName{index:03d}", + "points": index, + "exact_scores": index % 4, + "correct_results": index % 7, + } + for index in range(1, 90) + ], + } + ) + admin_cog.db.scores.get_standings = AsyncMock( + return_value=[ + { + "user_id": str(index), + "user_name": f"VeryLongUserName{index:03d}", + "total_points": index * 2, + "total_exact": index % 5, + "total_correct": index % 9, + } + for index in range(1, 90) + ] + ) + + view = UnifiedAdminPanelView( + admin_cog.db, + admin_cog.service, + str(mock_interaction_admin.user.id), + "111111", + admin_commands=admin_cog, + bot=admin_cog.bot, + ) + post_button = _get_button(view, "Re-post Results") + await post_button.callback(mock_interaction_admin) + + sent_messages = mock_interaction_admin.response_sent + mock_interaction_admin.followup_sent + assert len(sent_messages) > 1 + assert all(message["ephemeral"] is True for message in sent_messages) + assert all(len(message["content"]) <= 2000 for message in sent_messages) + assert isinstance(mock_interaction_admin.followup_sent[-1]["view"], PostResultsConfirmView) + @pytest.mark.asyncio async def test_unified_panel_post_results_only_previews_current_guild_scores( self, diff --git a/tests/conftest.py b/tests/conftest.py index 0915544..8cc91d6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -302,32 +302,41 @@ def __init__( self.response_sent = [] self.followup_sent = [] self.id = 123456789 + self._response_done = False self.response = MagicMock() - self.response.is_done.return_value = False + self.response.is_done.side_effect = lambda: self._response_done self.response.send_message = self._response_send_message self.response.edit_message = self._response_edit_message self.response.send_modal = self._response_send_modal + self.response.defer = self._response_defer self.followup = MagicMock() self.followup.send = self._followup_send async def _response_send_message(self, content: str = None, **kwargs): + self._response_done = True msg = {"content": content} msg.update(kwargs) self.response_sent.append(msg) + async def _response_defer(self, **kwargs): + self._response_done = True + self.response_sent.append({"deferred": True, **kwargs}) + async def _followup_send(self, content: str = None, **kwargs): msg = {"content": content} msg.update(kwargs) self.followup_sent.append(msg) async def _response_edit_message(self, content: str = None, **kwargs): + self._response_done = True msg = {"content": content} msg.update(kwargs) self.response_sent.append(msg) async def _response_send_modal(self, modal, **kwargs): + self._response_done = True self.modal_sent = {"modal": modal, **kwargs} async def response_send_message(self, content: str = None, **kwargs): diff --git a/tests/test_admin_commands.py b/tests/test_admin_commands.py index f39f326..d8c420d 100644 --- a/tests/test_admin_commands.py +++ b/tests/test_admin_commands.py @@ -16,7 +16,12 @@ from typer_bot.commands.admin_panel import PostResultsConfirmView, UnifiedAdminPanelView from typer_bot.commands.admin_panel.unified import SetupBotButton from typer_bot.database import Database -from typer_bot.utils import get_admin_permission_error, has_setup_permission, now +from typer_bot.utils import ( + DISCORD_MESSAGE_LIMIT, + get_admin_permission_error, + has_setup_permission, + now, +) from typer_bot.utils.permissions import is_admin @@ -296,7 +301,6 @@ def admin_cog(self, mock_bot, database): async def test_post_results_view_posts_without_mentions( self, mock_text_channel, mock_interaction_admin ): - """NO branch posts standings without pinging participants.""" fixture_data = { "week_number": 1, "games": ["A - B"], @@ -336,7 +340,6 @@ async def test_post_results_view_posts_without_mentions( async def test_post_results_view_posts_with_mentions( self, mock_text_channel, mock_interaction_admin ): - """YES branch appends participant mentions to the public post.""" fixture_data = { "week_number": 1, "games": ["A - B"], @@ -484,10 +487,88 @@ async def test_post_results_view_mentions_branch_aborts_when_acknowledgement_fai assert mock_text_channel.messages_sent == [] + @pytest.mark.asyncio + async def test_post_results_view_splits_oversized_scoreboard_and_mentions( + self, mock_text_channel, mock_interaction_admin + ): + fixture_data = { + "week_number": 12, + "games": ["A - B"], + "results": ["2-1"], + "scores": [ + { + "user_id": str(index), + "user_name": f"User{index:03d}", + "points": index, + "exact_scores": index % 4, + "correct_results": index % 7, + } + for index in range(1, 260) + ], + } + standings = [ + { + "user_id": str(index), + "user_name": f"User{index:03d}", + "total_points": index * 2, + "total_exact": index % 5, + "total_correct": index % 9, + } + for index in range(1, 80) + ] + view = PostResultsConfirmView(fixture_data, standings, mock_text_channel) + yes_button = next(child for child in view.children if child.label == "Mention Users") -class TestCalculationPostFormat: - """Test that the calculation announcement includes entered match results.""" + await yes_button.callback(mock_interaction_admin) + + sent_contents = [message["content"] for message in mock_text_channel.messages_sent] + assert len(sent_contents) > 1 + assert all(len(content) <= DISCORD_MESSAGE_LIMIT for content in sent_contents) + assert any(content.startswith("**Participants:**") for content in sent_contents) + assert "<@259>" in sent_contents[-1] + @pytest.mark.asyncio + async def test_post_results_view_splits_oversized_scoreboard_without_mentions( + self, mock_text_channel, mock_interaction_admin + ): + fixture_data = { + "week_number": 12, + "games": ["A - B"], + "results": ["2-1"], + "scores": [ + { + "user_id": str(index), + "user_name": f"User{index:03d}", + "points": index, + "exact_scores": index % 4, + "correct_results": index % 7, + } + for index in range(1, 260) + ], + } + standings = [ + { + "user_id": str(index), + "user_name": f"User{index:03d}", + "total_points": index * 2, + "total_exact": index % 5, + "total_correct": index % 9, + } + for index in range(1, 80) + ] + view = PostResultsConfirmView(fixture_data, standings, mock_text_channel) + no_button = next(child for child in view.children if child.label == "No Mentions") + + await no_button.callback(mock_interaction_admin) + + sent_contents = [message["content"] for message in mock_text_channel.messages_sent] + assert len(sent_contents) > 1 + assert all(len(content) <= DISCORD_MESSAGE_LIMIT for content in sent_contents) + assert "```\n```" not in sent_contents + assert any("User259" in content for content in sent_contents) + + +class TestCalculationPostFormat: def test_format_fixture_results_included_in_post(self, sample_games): from typer_bot.utils import format_fixture_results @@ -502,15 +583,12 @@ def test_format_fixture_results_included_in_post(self, sample_games): class TestCooldownLogic: - """Test suite for rate limiting cooldown.""" - @pytest.fixture def admin_cog(self, mock_bot, database): mock_bot.db = database return AdminCommands(mock_bot) def test_cooldown_enforced(self, admin_cog): - """Rate limiting prevents leaderboard recalculation spam.""" import time user_id = "user123" @@ -527,7 +605,6 @@ def test_cooldown_enforced(self, admin_cog): assert remaining > 0 def test_cooldown_expires(self, admin_cog): - """Cooldown expires after 30 seconds.""" import time user_id = "user123" diff --git a/tests/test_calculation_posting.py b/tests/test_calculation_posting.py index 5b0ad0f..53847ce 100644 --- a/tests/test_calculation_posting.py +++ b/tests/test_calculation_posting.py @@ -26,6 +26,38 @@ def _score_result(sample_games: list[str]) -> FixtureScoreResult: ) +def _large_score_result() -> FixtureScoreResult: + games = [f"Team {index:02d} Home - Team {index:02d} Away" for index in range(1, 25)] + standings = [ + { + "user_id": str(index), + "user_name": f"VeryLongUserName{index:03d}", + "total_points": index * 3, + "total_exact": index, + "total_correct": index + 2, + } + for index in range(1, 55) + ] + scores = [ + { + "user_id": str(index), + "user_name": f"VeryLongUserName{index:03d}", + "points": index, + "exact_scores": index % 4, + "correct_results": index % 7, + } + for index in range(1, 55) + ] + return FixtureScoreResult( + fixture={"games": games, "week_number": 11}, + results=["2-1"] * len(games), + predictions=[], + scores=scores, + standings=standings, + last_fixture={"week_number": 11, "scores": scores}, + ) + + def _bot_with_executor() -> MagicMock: bot = MagicMock(spec=discord.Client) bot.loop = MagicMock() @@ -61,6 +93,74 @@ async def test_post_calculation_result_posts_to_configured_league_channel( assert "posted to the league channel" in mock_interaction_admin.response_sent[-1]["content"] +@pytest.mark.asyncio +async def test_post_calculation_result_uses_followup_after_defer( + database, + mock_interaction_admin, + sample_games, + monkeypatch, +): + bot = _bot_with_executor() + channel = MagicMock(spec=discord.TextChannel) + channel.send = AsyncMock() + bot.get_channel.return_value = channel + monkeypatch.setattr(calculation_posting, "create_backup", MagicMock(return_value="backup.sql")) + monkeypatch.setattr(calculation_posting, "cleanup_old_backups", MagicMock(return_value=0)) + await mock_interaction_admin.response.defer(ephemeral=True, thinking=True) + + await calculation_posting.post_calculation_result( + bot, database, mock_interaction_admin, _score_result(sample_games) + ) + + assert "posted to the league channel" in mock_interaction_admin.followup_sent[-1]["content"] + + +@pytest.mark.asyncio +async def test_post_calculation_result_uses_followup_for_deferred_unavailable_channel( + database, + mock_interaction_admin, + sample_games, + monkeypatch, +): + bot = _bot_with_executor() + bot.get_channel.return_value = None + bot.fetch_channel = AsyncMock(side_effect=discord.InvalidData("unknown channel type")) + monkeypatch.setattr(calculation_posting, "create_backup", MagicMock(return_value="backup.sql")) + monkeypatch.setattr(calculation_posting, "cleanup_old_backups", MagicMock(return_value=0)) + await mock_interaction_admin.response.defer(ephemeral=True, thinking=True) + + await calculation_posting.post_calculation_result( + bot, database, mock_interaction_admin, _score_result(sample_games) + ) + + assert ( + "configured league channel is unavailable" + in mock_interaction_admin.followup_sent[-1]["content"].lower() + ) + + +@pytest.mark.asyncio +async def test_post_calculation_result_uses_followup_for_deferred_send_failure( + database, + mock_interaction_admin, + sample_games, + monkeypatch, +): + bot = _bot_with_executor() + channel = MagicMock(spec=discord.TextChannel) + channel.send = AsyncMock(side_effect=RuntimeError("discord unavailable")) + bot.get_channel.return_value = channel + monkeypatch.setattr(calculation_posting, "create_backup", MagicMock(return_value="backup.sql")) + monkeypatch.setattr(calculation_posting, "cleanup_old_backups", MagicMock(return_value=0)) + await mock_interaction_admin.response.defer(ephemeral=True, thinking=True) + + await calculation_posting.post_calculation_result( + bot, database, mock_interaction_admin, _score_result(sample_games) + ) + + assert "failed to post" in mock_interaction_admin.followup_sent[-1]["content"] + + @pytest.mark.asyncio async def test_post_calculation_result_posts_when_backup_fails( database, @@ -128,3 +228,29 @@ async def test_post_calculation_result_reports_unavailable_league_channel( "configured league channel is unavailable" in mock_interaction_admin.response_sent[-1]["content"].lower() ) + + +@pytest.mark.asyncio +async def test_post_calculation_result_splits_oversized_public_post( + database, + mock_interaction_admin, + monkeypatch, +): + bot = _bot_with_executor() + channel = MagicMock(spec=discord.TextChannel) + channel.send = AsyncMock() + bot.get_channel.return_value = channel + monkeypatch.setattr(calculation_posting, "create_backup", MagicMock(return_value="backup.sql")) + monkeypatch.setattr(calculation_posting, "cleanup_old_backups", MagicMock(return_value=0)) + + await calculation_posting.post_calculation_result( + bot, database, mock_interaction_admin, _large_score_result() + ) + + assert channel.send.await_count > 1 + sent_contents = [call.args[0] for call in channel.send.await_args_list] + assert all(len(content) <= 2000 for content in sent_contents) + assert "Week 11 Results" in sent_contents[0] + assert any("Overall Standings" in content for content in sent_contents) + assert any("Team 24 Home - Team 24 Away" in content for content in sent_contents) + assert any("VeryLongUserName054" in content for content in sent_contents) diff --git a/tests/test_discord_messages.py b/tests/test_discord_messages.py new file mode 100644 index 0000000..47121f9 --- /dev/null +++ b/tests/test_discord_messages.py @@ -0,0 +1,72 @@ +from typer_bot.utils import ( + DISCORD_MESSAGE_LIMIT, + build_discord_message_chunks, + build_scoreboard_with_mentions_chunks, + chunk_discord_message, +) + + +def test_chunk_discord_message_preserves_code_block_limit_for_long_line(): + payload = "x" * 5000 + chunks = chunk_discord_message(f"```\n{payload}\n```") + + assert len(chunks) > 1 + assert all(len(chunk) <= DISCORD_MESSAGE_LIMIT for chunk in chunks) + assert all(chunk.count("```") % 2 == 0 for chunk in chunks) + assert sum(chunk.count("x") for chunk in chunks) == len(payload) + + +def test_chunk_discord_message_accounts_for_inserted_code_fences(): + payload = "x" * 1993 + chunks = chunk_discord_message(f"```\n{payload}\n```") + + assert len(chunks) > 1 + assert all(len(chunk) <= DISCORD_MESSAGE_LIMIT for chunk in chunks) + assert "```\n```" not in chunks + assert sum(chunk.count("x") for chunk in chunks) == len(payload) + + +def test_chunk_discord_message_reserves_space_before_opening_code_block(): + chunks = chunk_discord_message(f"{'A' * 1996}\n```\nx\n```") + + assert all(len(chunk) <= DISCORD_MESSAGE_LIMIT for chunk in chunks) + assert "```\n```" not in chunks + + +def test_chunk_discord_message_does_not_flush_title_with_empty_code_block(): + payload = "x" * 1993 + chunks = chunk_discord_message(f"Title\n```\n{payload}\n```") + + assert all(len(chunk) <= DISCORD_MESSAGE_LIMIT for chunk in chunks) + assert "Title\n```\n```" not in chunks + assert sum(chunk.count("x") for chunk in chunks) == len(payload) + + +def test_chunk_discord_message_does_not_flush_blank_only_code_block(): + payload = "x" * 1993 + chunks = chunk_discord_message(f"```\n\n{payload}\n```") + + assert all(len(chunk) <= DISCORD_MESSAGE_LIMIT for chunk in chunks) + assert "```\n\n```" not in chunks + assert sum(chunk.count("x") for chunk in chunks) == len(payload) + + +def test_build_discord_message_chunks_separates_oversized_sections(): + first = "A" * 1500 + second = "B" * 1500 + + chunks = build_discord_message_chunks([first, second]) + + assert chunks == [first, second] + + +def test_build_scoreboard_with_mentions_chunks_splits_long_mentions_without_breaking_tokens(): + scoreboard = "Short scoreboard" + mentions = [f"<@{index}>" for index in range(1, 500)] + + chunks = build_scoreboard_with_mentions_chunks(scoreboard, mentions) + + assert len(chunks) > 1 + assert all(len(chunk) <= DISCORD_MESSAGE_LIMIT for chunk in chunks) + assert any(chunk.startswith("**Participants:**") for chunk in chunks) + assert f"<@{len(mentions)}>" in chunks[-1] diff --git a/tests/user_commands/test_standings_command.py b/tests/user_commands/test_standings_command.py index bb07ceb..00a6029 100644 --- a/tests/user_commands/test_standings_command.py +++ b/tests/user_commands/test_standings_command.py @@ -3,6 +3,8 @@ import pytest +from typer_bot.utils import DISCORD_MESSAGE_LIMIT + class TestStandingsCommand: @pytest.mark.asyncio @@ -84,3 +86,39 @@ async def test_standings_only_shows_current_guild_scores( content = mock_interaction.response_sent[0]["content"] assert "Current Guild" in content assert "Other Guild" not in content + + @pytest.mark.asyncio + async def test_standings_splits_oversized_response(self, user_commands, mock_interaction): + standings = [ + { + "user_id": str(index), + "user_name": f"VeryLongUserName{index:03d}", + "total_points": index * 3, + "total_exact": index % 7, + "total_correct": index % 11, + } + for index in range(1, 80) + ] + last_fixture = { + "week_number": 9, + "scores": [ + { + "user_id": str(index), + "user_name": f"VeryLongUserName{index:03d}", + "points": index, + "exact_scores": index % 4, + "correct_results": index % 6, + } + for index in range(1, 80) + ], + } + user_commands.db.scores.get_standings = AsyncMock(return_value=standings) + user_commands.db.scores.get_last_fixture_scores = AsyncMock(return_value=last_fixture) + + await user_commands.standings.callback(user_commands, mock_interaction) + + sent_messages = mock_interaction.response_sent + mock_interaction.followup_sent + assert len(sent_messages) > 1 + assert all(len(message["content"]) <= DISCORD_MESSAGE_LIMIT for message in sent_messages) + assert all(message["ephemeral"] is True for message in sent_messages) + assert any("VeryLongUserName079" in message["content"] for message in sent_messages) diff --git a/typer_bot/commands/admin_commands.py b/typer_bot/commands/admin_commands.py index 84541ad..af6d30b 100644 --- a/typer_bot/commands/admin_commands.py +++ b/typer_bot/commands/admin_commands.py @@ -22,6 +22,7 @@ ) CALCULATE_COOLDOWN = 30.0 +# Bounds the process-local map; the visible calculate cooldown remains 30s. COOLDOWN_ENTRY_EXPIRY = timedelta(hours=1) diff --git a/typer_bot/commands/admin_panel/unified_actions.py b/typer_bot/commands/admin_panel/unified_actions.py index 1c2b2ef..6cd4178 100644 --- a/typer_bot/commands/admin_panel/unified_actions.py +++ b/typer_bot/commands/admin_panel/unified_actions.py @@ -7,7 +7,15 @@ import discord from typer_bot.services import post_calculation_result -from typer_bot.utils import format_standings, get_admin_permission_error, has_setup_permission, now +from typer_bot.utils import ( + DISCORD_MESSAGE_LIMIT, + build_discord_message_chunks, + build_scoreboard_with_mentions_chunks, + format_standings, + get_admin_permission_error, + has_setup_permission, + now, +) from .modals import CreateFixtureModal, EnterResultsModal @@ -392,12 +400,13 @@ async def callback(self, interaction: discord.Interaction): ) return + await interaction.response.defer(ephemeral=True, thinking=True) try: score_result = await self.parent_view.service.calculate_fixture_scores( fixture_id, self.parent_view.guild_id ) except ValueError as exc: - await interaction.response.send_message(str(exc), ephemeral=True) + await interaction.followup.send(str(exc), ephemeral=True) return admin_commands.record_calculate_cooldown( @@ -413,7 +422,10 @@ async def _edit_parent_message(self, interaction: discord.Interaction) -> None: message = getattr(interaction, "message", None) edit_message = getattr(message, "edit", None) if callable(edit_message): - await edit_message(content=self.parent_view.render_content(), view=self.parent_view) + try: + await edit_message(content=self.parent_view.render_content(), view=self.parent_view) + except (discord.HTTPException, discord.NotFound): + return async def _refresh_parent_panel(self, fixture_id: int) -> None: fixture = await self.parent_view.db.fixtures.get_fixture_by_id( @@ -446,31 +458,39 @@ def __init__( @discord.ui.button(label="No Mentions", style=discord.ButtonStyle.primary) async def no_mentions(self, interaction: discord.Interaction, _button: discord.ui.Button): - message = format_standings(self.standings, self.fixture_data) + message_chunks = build_discord_message_chunks( + [format_standings(self.standings, self.fixture_data)] + ) try: + # Public posts only start after the admin interaction is acknowledged. await interaction.response.edit_message( content="Results posted without mentions!", view=None ) except Exception: return try: - await self.channel.send(message) + for message_chunk in message_chunks: + await self.channel.send(message_chunk) except Exception as exc: await interaction.followup.send(f"Failed to post results: {exc}") @discord.ui.button(label="Mention Users", style=discord.ButtonStyle.green) async def with_mentions(self, interaction: discord.Interaction, _button: discord.ui.Button): - message = format_standings(self.standings, self.fixture_data) mentions = [f"<@{score['user_id']}>" for score in self.fixture_data["scores"]] - message += f"\n\n**Participants:**\n{' '.join(mentions)}" + message_chunks = build_scoreboard_with_mentions_chunks( + format_standings(self.standings, self.fixture_data), + mentions, + ) try: + # Public posts only start after the admin interaction is acknowledged. await interaction.response.edit_message( content="Results posted with mentions!", view=None ) except Exception: return try: - await self.channel.send(message) + for message_chunk in message_chunks: + await self.channel.send(message_chunk) except Exception as exc: await interaction.followup.send(f"Failed to post results: {exc}") @@ -517,8 +537,17 @@ async def callback(self, interaction: discord.Interaction): preview = format_standings(standings, fixture_data) view = PostResultsConfirmView(fixture_data, standings, channel) - await interaction.response.send_message( - f"{preview}\n\nMention users in this post?", - view=view, - ephemeral=True, - ) + prompt = "Mention users in this post?" + if len(f"{preview}\n\n{prompt}") <= DISCORD_MESSAGE_LIMIT: + await interaction.response.send_message( + f"{preview}\n\n{prompt}", + view=view, + ephemeral=True, + ) + return + + preview_chunks = build_discord_message_chunks([preview]) + await interaction.response.send_message(preview_chunks[0], ephemeral=True) + for preview_chunk in preview_chunks[1:]: + await interaction.followup.send(preview_chunk, ephemeral=True) + await interaction.followup.send(prompt, view=view, ephemeral=True) diff --git a/typer_bot/commands/user_commands.py b/typer_bot/commands/user_commands.py index 9f89f0f..68e6b0b 100644 --- a/typer_bot/commands/user_commands.py +++ b/typer_bot/commands/user_commands.py @@ -12,6 +12,7 @@ from typer_bot.database import Database, SaveResult from typer_bot.utils import ( build_prediction_submission, + chunk_discord_message, format_for_discord, format_predictions_preview, format_standings, @@ -570,34 +571,7 @@ def __init__(self, bot: commands.Bot): @staticmethod def _chunk_message(content: str, limit: int = 2000) -> list[str]: """Split long responses into Discord-safe chunks.""" - if len(content) <= limit: - return [content] - - chunks: list[str] = [] - current = "" - for line in content.split("\n"): - candidate = f"{current}\n{line}" if current else line - if len(candidate) <= limit: - current = candidate - continue - - if current: - chunks.append(current) - - if len(line) <= limit: - current = line - continue - - start = 0 - while start < len(line): - end = min(start + limit, len(line)) - chunks.append(line[start:end]) - start = end - current = "" - - if current: - chunks.append(current) - return chunks + return chunk_discord_message(content, limit=limit) async def _send_chunked_ephemeral(self, interaction: discord.Interaction, content: str): """Send an ephemeral response split across followups if needed.""" @@ -773,7 +747,7 @@ async def standings(self, interaction: discord.Interaction): message = format_standings(standings, last_fixture) - await interaction.response.send_message(message, ephemeral=True) + await self._send_chunked_ephemeral(interaction, message) @app_commands.command( name="mypredictions", description="View your predictions for open fixtures" diff --git a/typer_bot/services/calculation_posting.py b/typer_bot/services/calculation_posting.py index b4554d7..3daefbc 100644 --- a/typer_bot/services/calculation_posting.py +++ b/typer_bot/services/calculation_posting.py @@ -9,7 +9,7 @@ from typer_bot.database import Database from typer_bot.services.admin_service import FixtureScoreResult -from typer_bot.utils import format_fixture_results, format_standings +from typer_bot.utils import build_discord_message_chunks, format_fixture_results, format_standings from typer_bot.utils.config import BACKUP_DIR from typer_bot.utils.db_backup import cleanup_old_backups, create_backup @@ -24,14 +24,22 @@ async def post_calculation_result( ) -> None: """Run best-effort DB backup, then publish fixture results and standings. - The interaction response must still be unused. Backup failures are logged but - do not fail score calculation; posting failures are reported ephemerally to - the admin. + The interaction may already be deferred. Admin feedback is sent through the + initial response or an ephemeral followup based on interaction state. Backup + failures are logged but do not fail score calculation; posting failures are + reported ephemerally to the admin. """ await _create_backup(bot, db.db_path) await _post_calculation_to_channel(bot, db, interaction, score_result) +async def _send_interaction_feedback(interaction: discord.Interaction, content: str) -> None: + if interaction.response.is_done(): + await interaction.followup.send(content, ephemeral=True) + return + await interaction.response.send_message(content, ephemeral=True) + + async def _create_backup(bot: commands.Bot | discord.Client, db_path: str) -> None: try: await bot.loop.run_in_executor(None, lambda: create_backup(db_path, BACKUP_DIR)) @@ -47,8 +55,9 @@ async def _post_calculation_to_channel( score_result: FixtureScoreResult, ) -> None: if interaction.guild_id is None: - await interaction.response.send_message( - "Scores calculated but could not resolve this server.", ephemeral=True + await _send_interaction_feedback( + interaction, + "Scores calculated but could not resolve this server.", ) return @@ -70,9 +79,9 @@ async def _post_calculation_to_channel( channel = None if not isinstance(channel, discord.TextChannel): - await interaction.response.send_message( + await _send_interaction_feedback( + interaction, "Scores calculated but the configured league channel is unavailable.", - ephemeral=True, ) return @@ -81,20 +90,19 @@ async def _post_calculation_to_channel( score_result.results, score_result.fixture["week_number"], ) - message = ( - results_section - + "\n\n" - + format_standings(score_result.standings, score_result.last_fixture) - ) + standings_section = format_standings(score_result.standings, score_result.last_fixture) + message_chunks = build_discord_message_chunks([results_section, standings_section]) try: - await channel.send(message) - await interaction.response.send_message( + for message_chunk in message_chunks: + await channel.send(message_chunk) + await _send_interaction_feedback( + interaction, f"Week {score_result.fixture['week_number']} results calculated and posted to the league channel!", - ephemeral=True, ) except Exception as exc: logger.error(f"Failed to post results to channel: {exc}") - await interaction.response.send_message( - "Scores calculated but failed to post to channel.", ephemeral=True + await _send_interaction_feedback( + interaction, + "Scores calculated but failed to post to channel.", ) diff --git a/typer_bot/utils/__init__.py b/typer_bot/utils/__init__.py index 401c893..071ddd0 100644 --- a/typer_bot/utils/__init__.py +++ b/typer_bot/utils/__init__.py @@ -1,5 +1,11 @@ """Utility functions and helpers.""" +from .discord_messages import ( + DISCORD_MESSAGE_LIMIT, + build_discord_message_chunks, + build_scoreboard_with_mentions_chunks, + chunk_discord_message, +) from .permissions import ( SETUP_REQUIRED_MESSAGE, get_admin_permission_error, @@ -53,4 +59,8 @@ "parse_iso", "APP_TZ", "SETUP_REQUIRED_MESSAGE", + "DISCORD_MESSAGE_LIMIT", + "build_discord_message_chunks", + "build_scoreboard_with_mentions_chunks", + "chunk_discord_message", ] diff --git a/typer_bot/utils/discord_messages.py b/typer_bot/utils/discord_messages.py new file mode 100644 index 0000000..0def67a --- /dev/null +++ b/typer_bot/utils/discord_messages.py @@ -0,0 +1,173 @@ +"""Discord message length helpers.""" + +from __future__ import annotations + +from collections.abc import Sequence + +DISCORD_MESSAGE_LIMIT = 2000 +CODE_FENCE = "```" + + +def _append_code_line_chunks(chunks: list[str], line: str, limit: int) -> None: + payload_limit = limit - len(f"{CODE_FENCE}\n\n{CODE_FENCE}") + for index in range(0, len(line), payload_limit): + chunks.append(f"{CODE_FENCE}\n{line[index : index + payload_limit]}\n{CODE_FENCE}") + + +def _last_code_fence_index(lines: list[str]) -> int | None: + for index in range(len(lines) - 1, -1, -1): + if lines[index].strip().startswith(CODE_FENCE): + return index + return None + + +def _flush_current_chunk( + chunks: list[str], current_lines: list[str], *, in_code_block: bool +) -> list[str]: + if not current_lines: + return [] + if not in_code_block: + chunks.append("\n".join(current_lines)) + return [] + + last_fence_index = _last_code_fence_index(current_lines) + code_payload_lines = ( + current_lines[last_fence_index + 1 :] if last_fence_index is not None else [] + ) + if last_fence_index == len(current_lines) - 1 or not any( + line.strip() for line in code_payload_lines + ): + prefix = current_lines[:last_fence_index] + if prefix: + chunks.append("\n".join(prefix)) + return [CODE_FENCE] + + chunks.append("\n".join(current_lines) + f"\n{CODE_FENCE}") + return [CODE_FENCE] + + +def chunk_discord_message(content: str, limit: int = DISCORD_MESSAGE_LIMIT) -> list[str]: + """Split content into Discord-safe chunks. + + Returns chunks no longer than ``limit``. Code blocks are closed and reopened + across chunks; long non-code lines are split only when needed. + """ + if len(content) <= limit: + return [content] + + chunks: list[str] = [] + current_lines: list[str] = [] + in_code_block = False + + for line in content.splitlines(): + current = "\n".join(current_lines) + candidate = f"{current}\n{line}" if current else line + closing_fence_length = len(f"\n{CODE_FENCE}") if in_code_block else 0 + opens_code_block = line.strip().startswith(CODE_FENCE) and not in_code_block + + if current_lines and opens_code_block and len(candidate) + len(f"\n{CODE_FENCE}") > limit: + current_lines = _flush_current_chunk(chunks, current_lines, in_code_block=in_code_block) + + if ( + current_lines + and current_lines != [CODE_FENCE] + and len(candidate) + closing_fence_length > limit + ): + current_lines = _flush_current_chunk(chunks, current_lines, in_code_block=in_code_block) + + line_limit = ( + limit - len(f"{CODE_FENCE}\n\n{CODE_FENCE}") + if in_code_block and not line.strip().startswith(CODE_FENCE) + else limit + ) + if len(line) > line_limit: + if current_lines: + current_lines = _flush_current_chunk( + chunks, current_lines, in_code_block=in_code_block + ) + if in_code_block: + _append_code_line_chunks(chunks, line, limit) + current_lines = [CODE_FENCE] + else: + chunks.extend(line[index : index + limit] for index in range(0, len(line), limit)) + continue + + if current_lines == [CODE_FENCE] and line.strip().startswith(CODE_FENCE): + current_lines = [] + in_code_block = False + continue + + current_lines.append(line) + if line.strip().startswith(CODE_FENCE): + in_code_block = not in_code_block + + if current_lines: + chunks.append("\n".join(current_lines)) + + return chunks + + +def build_discord_message_chunks( + sections: Sequence[str], + *, + separator: str = "\n\n", + limit: int = DISCORD_MESSAGE_LIMIT, +) -> list[str]: + """Combine sections when the joined message fits, otherwise split by section.""" + non_empty_sections = [section for section in sections if section] + combined = separator.join(non_empty_sections) + if len(combined) <= limit: + return [combined] + + chunks: list[str] = [] + for section in non_empty_sections: + chunks.extend(chunk_discord_message(section, limit=limit)) + return chunks + + +def _chunk_token_section( + header: str, + tokens: Sequence[str], + *, + separator: str = " ", + limit: int = DISCORD_MESSAGE_LIMIT, +) -> list[str]: + """Split a header plus token list, keeping tokens intact when possible.""" + if not tokens: + return [header] + + chunks: list[str] = [] + current = header + for token in tokens: + joiner = "\n" if current == header else separator + candidate = f"{current}{joiner}{token}" + if len(candidate) <= limit: + current = candidate + continue + + if current != header: + chunks.append(current) + current = f"{header}\n{token}" if header else token + + if len(current) > limit: + chunks.extend(chunk_discord_message(current, limit=limit)) + current = header + + if current != header or not chunks: + chunks.append(current) + + return chunks + + +def build_scoreboard_with_mentions_chunks( + scoreboard: str, + mentions: Sequence[str], + *, + limit: int = DISCORD_MESSAGE_LIMIT, +) -> list[str]: + """Build standings chunks, separating participant mentions when needed.""" + mention_chunks = _chunk_token_section("**Participants:**", mentions, limit=limit) + if len(mention_chunks) == 1: + return build_discord_message_chunks([scoreboard, mention_chunks[0]], limit=limit) + + return build_discord_message_chunks([scoreboard], limit=limit) + mention_chunks