Skip to content

Refactor conv warnings #2755

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Nov 6, 2021
1 change: 1 addition & 0 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ Hey! You're PRing? Cool! Please have a look at the below checklist. It's here to
- [ ] Link new and existing constants in docstrings instead of hard coded number and strings
- [ ] Add new message types to `Message.effective_attachment`
- [ ] Added new handlers for new update types
- [ ] Add the handlers to the warning loop in the `ConversationHandler`
- [ ] Added new filters for new message (sub)types
- [ ] Added or updated documentation for the changed class(es) and/or method(s)
- [ ] Updated the Bot API version number in all places: `README.rst` and `README_RAW.rst` (including the badge), as well as `telegram.constants.BOT_API_VERSION`
Expand Down
119 changes: 79 additions & 40 deletions telegram/ext/_conversationhandler.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@
DispatcherHandlerStop,
Handler,
InlineQueryHandler,
StringCommandHandler,
StringRegexHandler,
TypeHandler,
)
from telegram._utils.warnings import warn
from telegram.ext._utils.promise import Promise
Expand Down Expand Up @@ -239,6 +242,14 @@ def __init__(
map_to_parent: Dict[object, object] = None,
run_async: bool = False,
):
# these imports need to be here because of circular import error otherwise
from telegram.ext import ( # pylint: disable=import-outside-toplevel
ShippingQueryHandler,
PreCheckoutQueryHandler,
PollHandler,
PollAnswerHandler,
)

self.run_async = run_async

self._entry_points = entry_points
Expand Down Expand Up @@ -283,49 +294,77 @@ def __init__(
for state_handlers in states.values():
all_handlers.extend(state_handlers)

if self.per_message:
for handler in all_handlers:
if not isinstance(handler, CallbackQueryHandler):
warn(
"If 'per_message=True', all entry points, state handlers, and fallbacks"
" must be 'CallbackQueryHandler', since no other handlers "
"have a message context.",
stacklevel=2,
)
break
else:
for handler in all_handlers:
if isinstance(handler, CallbackQueryHandler):
warn(
"If 'per_message=False', 'CallbackQueryHandler' will not be "
"tracked for every message.",
stacklevel=2,
)
break
# this loop is going to warn the user about handlers which can work unexpected
# in conversations

if self.per_chat:
for handler in all_handlers:
if isinstance(handler, (InlineQueryHandler, ChosenInlineResultHandler)):
warn(
"If 'per_chat=True', 'InlineQueryHandler' can not be used, "
"since inline queries have no chat context.",
stacklevel=2,
)
break
# this link will be added to all warnings tied to per_* setting
per_faq_link = (
" Read this FAQ entry to learn more about the per_* settings: https://git.io/JtcyU."
)

if self.conversation_timeout:
for handler in all_handlers:
if isinstance(handler, self.__class__):
warn(
"Using `conversation_timeout` with nested conversations is currently not "
"supported. You can still try to use it, but it will likely behave "
"differently from what you expect.",
stacklevel=2,
)
break
for handler in all_handlers:
if isinstance(handler, (StringCommandHandler, StringRegexHandler)):
warn(
"The `ConversationHandler` only handles updates of type `telegram.Update`. "
f"{handler.__class__.__name__} handles updates of type `str`.",
stacklevel=2,
)
elif isinstance(handler, TypeHandler) and not issubclass(handler.type, Update):
warn(
"The `ConversationHandler` only handles updates of type `telegram.Update`."
f" The TypeHandler is set to handle {handler.type.__name__}.",
stacklevel=2,
)
elif isinstance(handler, PollHandler):
warn(
"PollHandler will never trigger in a conversation since it has no information "
"about the chat or the user who voted in it. Do you mean the "
"`PollAnswerHandler`?",
stacklevel=2,
)

elif self.per_chat and (
isinstance(
handler,
(
ShippingQueryHandler,
InlineQueryHandler,
ChosenInlineResultHandler,
PreCheckoutQueryHandler,
PollAnswerHandler,
),
)
):
warn(
f"Updates handled by {handler.__class__.__name__} only have information about "
f"the user, so this handler won't ever be triggered if `per_chat=True`."
f"{per_faq_link}",
stacklevel=2,
)

elif self.per_message and not isinstance(handler, CallbackQueryHandler):
warn(
"If 'per_message=True', all entry points, state handlers, and fallbacks"
" must be 'CallbackQueryHandler', since no other handlers "
f"have a message context.{per_faq_link}",
stacklevel=2,
)
elif not self.per_message and isinstance(handler, CallbackQueryHandler):
warn(
"If 'per_message=False', 'CallbackQueryHandler' will not be "
f"tracked for every message.{per_faq_link}",
stacklevel=2,
)

if self.conversation_timeout and isinstance(handler, self.__class__):
warn(
"Using `conversation_timeout` with nested conversations is currently not "
"supported. You can still try to use it, but it will likely behave "
"differently from what you expect.",
stacklevel=2,
)

if self.run_async:
for handler in all_handlers:
if self.run_async:
handler.run_async = True

@property
Expand Down
Loading