Skip to content

feat: возможность для пользователя передать action как строку#118

Open
someqst wants to merge 6 commits intolove-apples:mainfrom
someqst:feat/send-action-as-string
Open

feat: возможность для пользователя передать action как строку#118
someqst wants to merge 6 commits intolove-apples:mainfrom
someqst:feat/send-action-as-string

Conversation

@someqst
Copy link
Copy Markdown

@someqst someqst commented Apr 18, 2026

Столкнулся с проблемой, что ожидается StrEnum SenderAction при попытке вызова метода send_action.
Думаю, было бы неплохо позволить пользователю присылать action строкой, дабы увеличить удобство использования метода.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@someqst someqst force-pushed the feat/send-action-as-string branch from f1e2ce9 to b728e3e Compare April 18, 2026 21:43
@Olegt0rr Olegt0rr self-requested a review April 22, 2026 22:00
Copy link
Copy Markdown
Collaborator

@Olegt0rr Olegt0rr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Изменение небольшое и понятное, но есть пара моментов.

Тесты — PR #119 идёт в связке с этим, и там полноценные тесты на валидацию. Здесь тестов нет. Хотелось бы хотя бы два: передать валидную строку "typing_on" и проверить что всё ок; передать невалидную — убедиться что летит ValueError с понятным сообщением.

Поведение при SenderAction — если передать уже готовый SenderAction.TYPING_ON, он прогоняется через SenderAction(action) ещё раз. Для StrEnum это работает, но немного избыточно. Можно добавить isinstance(action, SenderAction) и не конвертировать тогда вообще. Не критично, но аккуратнее.

Жду тесты)

@someqst
Copy link
Copy Markdown
Author

someqst commented Apr 23, 2026

@Olegt0rr
Тесты сделал. Логику улучшил через isinstance.
Сначала думал, что это оптимизация ради оптимизации (про SenderAction(action) - поэтому не добавил сначала. Но вы дали уверенности, что это не просто так)

@love-apples
Copy link
Copy Markdown
Owner

methods/send_action.py на строке 32 всё ещё принимает только SenderAction, хотя этот класс публично документирован через docs/methods/send_action.md. Поэтому bot.send_action(action="typing_on") теперь работает, а прямой вызов SendAction(bot, action="typing_on").fetch() всё ещё упадёт на self.action.value.

Лучше перенести нормализацию строки в SendAction.init или общий helper, чтобы оба публичных входа вели себя одинаково.

@Olegt0rr Olegt0rr added the bug Something isn't working label Apr 28, 2026
@Olegt0rr Olegt0rr requested a review from Copilot April 28, 2026 18:30
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

PR добавляет более удобный интерфейс для Bot.send_action(): теперь action можно передавать не только как SenderAction, но и как строку (значение enum), с тестами на валидные/невалидные строки.

Changes:

  • Расширен тип параметра action в Bot.send_action до SenderAction | str и добавлена конвертация строки в SenderAction.
  • Добавлено сообщение об ошибке для невалидной строки action со списком допустимых значений.
  • Добавлены unit-тесты для send_action со строковым action (валидный/невалидный) и вызова без явного action.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
maxapi/bot.py Принимает action как SenderAction или строку и валидирует/конвертирует перед вызовом SendAction.
tests/test_bot.py Добавляет тесты для строкового action и дефолтного поведения.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/test_bot.py Outdated
Comment thread maxapi/bot.py Outdated
Comment thread maxapi/bot.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants