From 28b769b9d6ab30543322fde616088e1e20d88f0e Mon Sep 17 00:00:00 2001 From: Nikolay Tatarinov Date: Tue, 24 Feb 2026 11:49:07 +0300 Subject: [PATCH] refactor: update group duty pin functionality and documentation - Changed the behavior of the group duty pin feature to send a new message, unpin the old one, and pin the new one instead of editing the existing message. This ensures the pinned message is always fresh. - Updated the `DUTY_PIN_NOTIFY` configuration description in the documentation to reflect the new message handling approach. - Revised the architecture documentation to clarify the updated group duty pin process. - Enhanced tests to verify the new behavior of the group duty pin functionality, ensuring proper message handling and scheduling. --- .coverage | Bin 53248 -> 53248 bytes CHANGELOG.md | 4 ++ docs/architecture.md | 2 +- docs/configuration.md | 2 +- duty_teller/handlers/group_duty_pin.py | 45 +++++++-------- tests/test_handlers_group_duty_pin.py | 76 +++++++++++++++---------- 6 files changed, 72 insertions(+), 57 deletions(-) diff --git a/.coverage b/.coverage index 5d4fa23f168ae7acc0cf477dd4e9c354c357a981..d5f46e177cf100634b9612a1bdbfa8b7fd5211ab 100644 GIT binary patch delta 35 tcmV+;0Nnq8paX!Q1F&L_6o2=p<$Cpd Literal["updated", "no_message", "failed"]: - """Refresh pinned duty message for a chat: edit text, optionally re-pin, schedule next. + """Refresh pinned duty message: send new message, unpin old, pin new, save new message_id. Returns: - "updated" if the message was updated successfully; + "updated" if the message was sent, pinned and saved successfully; "no_message" if there is no pin record for this chat; - "failed" if edit or permissions failed. + "failed" if send_message or permissions failed. """ loop = asyncio.get_running_loop() message_id = await loop.run_in_executor(None, _sync_get_message_id, chat_id) @@ -110,28 +110,27 @@ async def _refresh_pin_for_chat( None, lambda: _get_duty_message_text_sync(config.DEFAULT_LANGUAGE) ) try: - await context.bot.edit_message_text( - chat_id=chat_id, - message_id=message_id, - text=text, - ) + msg = await context.bot.send_message(chat_id=chat_id, text=text) except (BadRequest, Forbidden) as e: - logger.warning("Failed to edit pinned message chat_id=%s: %s", chat_id, e) + logger.warning( + "Failed to send duty message for pin refresh chat_id=%s: %s", chat_id, e + ) next_end = await loop.run_in_executor(None, _get_next_shift_end_sync) await _schedule_next_update(context.application, chat_id, next_end) return "failed" - if config.DUTY_PIN_NOTIFY: - try: - await context.bot.unpin_chat_message(chat_id=chat_id) - await context.bot.pin_chat_message( - chat_id=chat_id, - message_id=message_id, - disable_notification=False, - ) - except (BadRequest, Forbidden) as e: - logger.warning( - "Re-pin after update failed chat_id=%s: %s", chat_id, e - ) + try: + await context.bot.unpin_chat_message(chat_id=chat_id) + await context.bot.pin_chat_message( + chat_id=chat_id, + message_id=msg.message_id, + disable_notification=not config.DUTY_PIN_NOTIFY, + ) + except (BadRequest, Forbidden) as e: + logger.warning("Unpin or pin after refresh failed chat_id=%s: %s", chat_id, e) + next_end = await loop.run_in_executor(None, _get_next_shift_end_sync) + 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) next_end = await loop.run_in_executor(None, _get_next_shift_end_sync) await _schedule_next_update(context.application, chat_id, next_end) return "updated" @@ -253,9 +252,7 @@ async def pin_duty_cmd(update: Update, context: ContextTypes.DEFAULT_TYPE) -> No await update.message.reply_text(t(lang, "pin_duty.failed")) -async def refresh_pin_cmd( - update: Update, context: ContextTypes.DEFAULT_TYPE -) -> None: +async def refresh_pin_cmd(update: Update, context: ContextTypes.DEFAULT_TYPE) -> None: """Handle /refresh_pin: immediately refresh pinned duty message in the group.""" if not update.message or not update.effective_chat or not update.effective_user: return diff --git a/tests/test_handlers_group_duty_pin.py b/tests/test_handlers_group_duty_pin.py index ccd1041..743e669 100644 --- a/tests/test_handlers_group_duty_pin.py +++ b/tests/test_handlers_group_duty_pin.py @@ -126,13 +126,15 @@ async def test_schedule_next_update_when_utc_none_runs_once_with_retry_delay(): @pytest.mark.asyncio -async def test_update_group_pin_edits_message_and_schedules_next(): - """update_group_pin: with message_id and text, edits message, re-pins with notification, schedules next.""" +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.""" + new_msg = MagicMock() + new_msg.message_id = 999 context = MagicMock() context.job = MagicMock() context.job.data = {"chat_id": 123} context.bot = MagicMock() - context.bot.edit_message_text = AsyncMock() + context.bot.send_message = AsyncMock(return_value=new_msg) context.bot.unpin_chat_message = AsyncMock() context.bot.pin_chat_message = AsyncMock() context.application = MagicMock() @@ -147,40 +149,38 @@ async def test_update_group_pin_edits_message_and_schedules_next(): ): with patch.object(mod, "_get_next_shift_end_sync", return_value=None): with patch.object(mod, "_schedule_next_update", AsyncMock()): - await mod.update_group_pin(context) - context.bot.edit_message_text.assert_called_once_with( - chat_id=123, message_id=1, text="Current duty" - ) + with patch.object(mod, "_sync_save_pin") as mock_save: + await mod.update_group_pin(context) + context.bot.send_message.assert_called_once_with(chat_id=123, text="Current duty") context.bot.unpin_chat_message.assert_called_once_with(chat_id=123) context.bot.pin_chat_message.assert_called_once_with( - chat_id=123, message_id=1, disable_notification=False + chat_id=123, message_id=999, disable_notification=False ) + mock_save.assert_called_once_with(123, 999) @pytest.mark.asyncio async def test_update_group_pin_no_message_id_skips(): - """update_group_pin: when no pin record (message_id None), does not edit.""" + """update_group_pin: when no pin record (message_id None), does not send_message.""" context = MagicMock() context.job = MagicMock() context.job.data = {"chat_id": 456} context.bot = MagicMock() - context.bot.edit_message_text = AsyncMock() + context.bot.send_message = AsyncMock() with patch.object(mod, "_sync_get_message_id", return_value=None): await mod.update_group_pin(context) - context.bot.edit_message_text.assert_not_called() + context.bot.send_message.assert_not_called() @pytest.mark.asyncio -async def test_update_group_pin_edit_raises_bad_request_still_schedules_next(): - """update_group_pin: edit_message_text BadRequest -> no unpin/pin, _schedule_next_update still called.""" +async def test_update_group_pin_send_raises_no_unpin_pin_schedule_still_called(): + """update_group_pin: send_message raises BadRequest -> no unpin/pin/save, _schedule_next_update still called.""" context = MagicMock() context.job = MagicMock() context.job.data = {"chat_id": 111} context.bot = MagicMock() - context.bot.edit_message_text = AsyncMock( - side_effect=BadRequest("Message not modified") - ) + context.bot.send_message = AsyncMock(side_effect=BadRequest("Chat not found")) context.bot.unpin_chat_message = AsyncMock() context.bot.pin_chat_message = AsyncMock() context.application = MagicMock() @@ -199,13 +199,17 @@ async def test_update_group_pin_edit_raises_bad_request_still_schedules_next(): @pytest.mark.asyncio async def test_update_group_pin_repin_raises_still_schedules_next(): - """update_group_pin: edit succeeds, unpin or pin raises -> log, _schedule_next_update still called.""" + """update_group_pin: send_message ok, unpin or pin raises -> no _sync_save_pin, schedule still called, log.""" + new_msg = MagicMock() + new_msg.message_id = 888 context = MagicMock() context.job = MagicMock() context.job.data = {"chat_id": 222} context.bot = MagicMock() - context.bot.edit_message_text = AsyncMock() - context.bot.unpin_chat_message = AsyncMock(side_effect=Forbidden("Not enough rights")) + context.bot.send_message = AsyncMock(return_value=new_msg) + context.bot.unpin_chat_message = AsyncMock( + side_effect=Forbidden("Not enough rights") + ) context.bot.pin_chat_message = AsyncMock() context.application = MagicMock() @@ -216,22 +220,26 @@ async def test_update_group_pin_repin_raises_still_schedules_next(): with patch.object( mod, "_schedule_next_update", AsyncMock() ) as mock_schedule: - with patch.object(mod, "logger") as mock_logger: - await mod.update_group_pin(context) - context.bot.edit_message_text.assert_called_once() + with patch.object(mod, "_sync_save_pin") as mock_save: + with patch.object(mod, "logger") as mock_logger: + await mod.update_group_pin(context) + context.bot.send_message.assert_called_once_with(chat_id=222, text="Text") + mock_save.assert_not_called() mock_logger.warning.assert_called_once() - assert "Re-pin" in mock_logger.warning.call_args[0][0] + assert "Unpin or pin" in mock_logger.warning.call_args[0][0] mock_schedule.assert_called_once_with(context.application, 222, None) @pytest.mark.asyncio -async def test_update_group_pin_duty_pin_notify_false_skips_repin(): - """update_group_pin: DUTY_PIN_NOTIFY False -> edit only, no unpin/pin, schedule next.""" +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.""" + new_msg = MagicMock() + new_msg.message_id = 777 context = MagicMock() context.job = MagicMock() context.job.data = {"chat_id": 333} context.bot = MagicMock() - context.bot.edit_message_text = AsyncMock() + context.bot.send_message = AsyncMock(return_value=new_msg) context.bot.unpin_chat_message = AsyncMock() context.bot.pin_chat_message = AsyncMock() context.application = MagicMock() @@ -240,11 +248,17 @@ async def test_update_group_pin_duty_pin_notify_false_skips_repin(): with patch.object(mod, "_sync_get_message_id", return_value=4): with patch.object(mod, "_get_duty_message_text_sync", return_value="Text"): with patch.object(mod, "_get_next_shift_end_sync", return_value=None): - with patch.object(mod, "_schedule_next_update", AsyncMock()) as mock_schedule: - await mod.update_group_pin(context) - context.bot.edit_message_text.assert_called_once() - context.bot.unpin_chat_message.assert_not_called() - context.bot.pin_chat_message.assert_not_called() + with patch.object( + mod, "_schedule_next_update", AsyncMock() + ) as mock_schedule: + with patch.object(mod, "_sync_save_pin") as mock_save: + await mod.update_group_pin(context) + context.bot.send_message.assert_called_once_with(chat_id=333, text="Text") + context.bot.unpin_chat_message.assert_called_once_with(chat_id=333) + context.bot.pin_chat_message.assert_called_once_with( + chat_id=333, message_id=777, disable_notification=True + ) + mock_save.assert_called_once_with(333, 777) mock_schedule.assert_called_once_with(context.application, 333, None)