feat: enhance group duty pin functionality to delete old messages

- Updated the `_refresh_pin_for_chat` function to delete the old pinned message after sending a new one, ensuring a cleaner chat experience.
- Modified related unit tests to verify the new deletion behavior, including handling exceptions when the old message cannot be deleted.
- Improved documentation in test cases to reflect the updated functionality and error handling.

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"
 

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)