diff --git a/duty_teller/handlers/group_duty_pin.py b/duty_teller/handlers/group_duty_pin.py index 032dfcc..f4c2f47 100644 --- a/duty_teller/handlers/group_duty_pin.py +++ b/duty_teller/handlers/group_duty_pin.py @@ -103,7 +103,7 @@ async def _schedule_next_update( async def _refresh_pin_for_chat( context: ContextTypes.DEFAULT_TYPE, chat_id: int ) -> Literal["updated", "no_message", "failed"]: - """Refresh pinned duty message: send new message, unpin old, pin new, save new message_id. + """Refresh pinned duty message: send new message, unpin old, pin new, save new message_id, delete old. Uses single DB session for message_id, text, next_shift_end (consolidated). @@ -120,6 +120,7 @@ async def _refresh_pin_for_chat( if message_id is None: logger.info("No pin record for chat_id=%s, skipping update", chat_id) return "no_message" + old_message_id = message_id try: msg = await context.bot.send_message(chat_id=chat_id, text=text) except (BadRequest, Forbidden) as e: @@ -140,6 +141,16 @@ async def _refresh_pin_for_chat( await _schedule_next_update(context.application, chat_id, next_end) return "failed" await loop.run_in_executor(None, _sync_save_pin, chat_id, msg.message_id) + if old_message_id is not None: + try: + await context.bot.delete_message(chat_id=chat_id, message_id=old_message_id) + except (BadRequest, Forbidden) as e: + logger.warning( + "Could not delete old pinned message %s in chat_id=%s: %s", + old_message_id, + chat_id, + e, + ) await _schedule_next_update(context.application, chat_id, next_end) return "updated" diff --git a/tests/test_handlers_group_duty_pin.py b/tests/test_handlers_group_duty_pin.py index c4e007f..cd0abd8 100644 --- a/tests/test_handlers_group_duty_pin.py +++ b/tests/test_handlers_group_duty_pin.py @@ -127,7 +127,7 @@ async def test_schedule_next_update_when_utc_none_runs_once_with_retry_delay(): @pytest.mark.asyncio async def test_update_group_pin_sends_new_unpins_pins_saves_schedules_next(): - """update_group_pin: with message_id and text, sends new message, unpins, pins new, saves id, schedules next.""" + """update_group_pin: with message_id and text, sends new message, unpins, pins new, saves id, deletes old, schedules next.""" new_msg = MagicMock() new_msg.message_id = 999 context = MagicMock() @@ -137,6 +137,7 @@ async def test_update_group_pin_sends_new_unpins_pins_saves_schedules_next(): context.bot.send_message = AsyncMock(return_value=new_msg) context.bot.unpin_chat_message = AsyncMock() context.bot.pin_chat_message = AsyncMock() + context.bot.delete_message = AsyncMock() context.application = MagicMock() context.application.job_queue = MagicMock() context.application.job_queue.get_jobs_by_name = MagicMock(return_value=[]) @@ -155,6 +156,39 @@ async def test_update_group_pin_sends_new_unpins_pins_saves_schedules_next(): chat_id=123, message_id=999, disable_notification=False ) mock_save.assert_called_once_with(123, 999) + context.bot.delete_message.assert_called_once_with(chat_id=123, message_id=1) + + +@pytest.mark.asyncio +async def test_update_group_pin_delete_message_raises_bad_request_still_schedules(): + """update_group_pin: delete_message raises BadRequest -> save and schedule still done, log warning.""" + new_msg = MagicMock() + new_msg.message_id = 999 + context = MagicMock() + context.job = MagicMock() + context.job.data = {"chat_id": 123} + context.bot = MagicMock() + context.bot.send_message = AsyncMock(return_value=new_msg) + context.bot.unpin_chat_message = AsyncMock() + context.bot.pin_chat_message = AsyncMock() + context.bot.delete_message = AsyncMock(side_effect=BadRequest("Message to delete not found")) + context.application = MagicMock() + context.application.job_queue = MagicMock() + context.application.job_queue.get_jobs_by_name = MagicMock(return_value=[]) + context.application.job_queue.run_once = MagicMock() + + with patch.object(config, "DUTY_PIN_NOTIFY", True): + with patch.object( + mod, "_sync_get_pin_refresh_data", return_value=(1, "Current duty", None) + ): + with patch.object(mod, "_schedule_next_update", AsyncMock()) as mock_schedule: + with patch.object(mod, "_sync_save_pin") as mock_save: + with patch.object(mod, "logger") as mock_logger: + await mod.update_group_pin(context) + mock_save.assert_called_once_with(123, 999) + mock_schedule.assert_called_once_with(context.application, 123, None) + mock_logger.warning.assert_called_once() + assert "Could not delete old pinned message" in mock_logger.warning.call_args[0][0] @pytest.mark.asyncio @@ -230,7 +264,7 @@ async def test_update_group_pin_repin_raises_still_schedules_next(): @pytest.mark.asyncio async def test_update_group_pin_duty_pin_notify_false_pins_silent(): - """update_group_pin: DUTY_PIN_NOTIFY False -> send new, unpin, pin new with disable_notification=True, save, schedule.""" + """update_group_pin: DUTY_PIN_NOTIFY False -> send new, unpin, pin new with disable_notification=True, save, delete old, schedule.""" new_msg = MagicMock() new_msg.message_id = 777 context = MagicMock() @@ -240,6 +274,7 @@ async def test_update_group_pin_duty_pin_notify_false_pins_silent(): context.bot.send_message = AsyncMock(return_value=new_msg) context.bot.unpin_chat_message = AsyncMock() context.bot.pin_chat_message = AsyncMock() + context.bot.delete_message = AsyncMock() context.application = MagicMock() with patch.object(config, "DUTY_PIN_NOTIFY", False): @@ -257,6 +292,7 @@ async def test_update_group_pin_duty_pin_notify_false_pins_silent(): chat_id=333, message_id=777, disable_notification=True ) mock_save.assert_called_once_with(333, 777) + context.bot.delete_message.assert_called_once_with(chat_id=333, message_id=4) mock_schedule.assert_called_once_with(context.application, 333, None)