diff --git a/.coverage b/.coverage index 5d4fa23..d5f46e1 100644 Binary files a/.coverage and b/.coverage differ diff --git a/CHANGELOG.md b/CHANGELOG.md index cc203d9..38a5cd2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Group duty pin**: when the pinned duty message is updated on schedule, the bot re-pins it so group members get a Telegram notification. Configurable via `DUTY_PIN_NOTIFY` (default: enabled); set to `0` or `false` to only edit the message without re-pinning. +### Changed + +- **Group duty pin refresh**: schedule updates now send a new message, unpin the previous one and pin the new one (instead of editing the existing message and re-pinning). Ensures the pinned message is always a fresh post; on pin/unpin errors the stored message_id is not updated so the next run retries. + ## [0.1.0] - 2025-02-20 ### Added diff --git a/docs/architecture.md b/docs/architecture.md index d37edc9..9bbad1b 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -8,7 +8,7 @@ High-level architecture of Duty Teller: components, data flow, and package relat - **FastAPI** — HTTP server: REST API (`/api/duties`, `/api/calendar-events`, `/api/calendar/ical/{token}.ics`) and static miniapp at `/app`. Runs in a separate thread alongside the bot. - **Database** — SQLAlchemy ORM with Alembic migrations. Default backend: SQLite (`data/duty_teller.db`). Stores users, duties (with event types: duty, unavailable, vacation), group duty pins, calendar subscription tokens. - **Duty-schedule import** — Two-step admin flow: handover time (timezone → UTC), then JSON file. Parser produces per-person date lists; import service deletes existing duties in range and inserts new ones. -- **Group duty pin** — In groups, the bot can pin the current duty message; time/timezone for the pinned text come from `DUTY_DISPLAY_TZ`. Pin state is restored on startup from the database. When the duty changes on schedule, the bot edits the pinned message and, if `DUTY_PIN_NOTIFY` is enabled (default), re-pins it so that members get a Telegram notification; the first pin (bot added to group or `/pin_duty`) is always silent. +- **Group duty pin** — In groups, the bot can pin the current duty message; time/timezone for the pinned text come from `DUTY_DISPLAY_TZ`. Pin state is restored on startup from the database. When the duty changes on schedule, the bot sends a new message, unpins the previous one and pins the new one; if `DUTY_PIN_NOTIFY` is enabled (default), pinning the new message triggers a Telegram notification for members. The first pin (bot added to group or `/pin_duty`) is always silent. ## Data flow diff --git a/docs/configuration.md b/docs/configuration.md index 1eaed10..7c75828 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -17,7 +17,7 @@ All configuration is read from the environment (e.g. `.env` via python-dotenv). | **CORS_ORIGINS** | comma-separated list | `*` | Allowed origins for CORS. Leave unset or set to `*` for allow-all. Example: `https://your-domain.com`. | | **EXTERNAL_CALENDAR_ICS_URL** | string (URL) | *(empty)* | URL of a public ICS calendar (e.g. holidays). If set, those days are highlighted on the duty grid; users can tap "i" on a cell to see the event summary. Empty = no external calendar. | | **DUTY_DISPLAY_TZ** | string (timezone name) | `Europe/Moscow` | Timezone for the pinned duty message in groups. Example: `Europe/Moscow`, `UTC`. | -| **DUTY_PIN_NOTIFY** | `0`, `false`, or `no` to disable | `1` (enabled) | When the pinned duty message is updated on schedule, the bot re-pins it so that group members get a Telegram notification (“Bot pinned a message”). Set to `0`, `false`, or `no` to only edit the message without re-pinning (no notification). The first pin (e.g. when the bot is added to the group or on `/pin_duty`) is always silent. | +| **DUTY_PIN_NOTIFY** | `0`, `false`, or `no` to disable | `1` (enabled) | When the pinned duty message is updated on schedule, the bot sends a new message, unpins the old one and pins the new one. If enabled, pinning the new message sends a Telegram notification (“Bot pinned a message”). Set to `0`, `false`, or `no` to pin without notification. The first pin (e.g. when the bot is added to the group or on `/pin_duty`) is always silent. | | **DEFAULT_LANGUAGE** | `en` or `ru` (normalized) | `en` | Default UI language when the user's Telegram language is unknown. Values starting with `ru` are normalized to `ru`, otherwise `en`. | ## Roles and access diff --git a/duty_teller/handlers/group_duty_pin.py b/duty_teller/handlers/group_duty_pin.py index 29cca17..4df910f 100644 --- a/duty_teller/handlers/group_duty_pin.py +++ b/duty_teller/handlers/group_duty_pin.py @@ -94,12 +94,12 @@ 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 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)