Skip to content

Commit b852a68

Browse files
PoolitzerBibo-Joshi
Poolitzer
authored and
Bibo-Joshi
committed
Refactor Warnings in ConversationHandler (python-telegram-bot#2755, python-telegram-bot#2784)
1 parent 44f1ce3 commit b852a68

3 files changed

Lines changed: 219 additions & 117 deletions

File tree

‎.github/pull_request_template.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ Hey! You're PRing? Cool! Please have a look at the below checklist. It's here to
2929
- [ ] Link new and existing constants in docstrings instead of hard coded number and strings
3030
- [ ] Add new message types to `Message.effective_attachment`
3131
- [ ] Added new handlers for new update types
32+
- [ ] Add the handlers to the warning loop in the `ConversationHandler`
3233
- [ ] Added new filters for new message (sub)types
3334
- [ ] Added or updated documentation for the changed class(es) and/or method(s)
3435
- [ ] 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`

‎telegram/ext/_conversationhandler.py‎

Lines changed: 79 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@
4545
DispatcherHandlerStop,
4646
Handler,
4747
InlineQueryHandler,
48+
StringCommandHandler,
49+
StringRegexHandler,
50+
TypeHandler,
4851
)
4952
from telegram._utils.warnings import warn
5053
from telegram.ext._utils.promise import Promise
@@ -239,6 +242,14 @@ def __init__(
239242
map_to_parent: Dict[object, object] = None,
240243
run_async: bool = False,
241244
):
245+
# these imports need to be here because of circular import error otherwise
246+
from telegram.ext import ( # pylint: disable=import-outside-toplevel
247+
ShippingQueryHandler,
248+
PreCheckoutQueryHandler,
249+
PollHandler,
250+
PollAnswerHandler,
251+
)
252+
242253
self.run_async = run_async
243254

244255
self._entry_points = entry_points
@@ -283,49 +294,77 @@ def __init__(
283294
for state_handlers in states.values():
284295
all_handlers.extend(state_handlers)
285296

286-
if self.per_message:
287-
for handler in all_handlers:
288-
if not isinstance(handler, CallbackQueryHandler):
289-
warn(
290-
"If 'per_message=True', all entry points, state handlers, and fallbacks"
291-
" must be 'CallbackQueryHandler', since no other handlers "
292-
"have a message context.",
293-
stacklevel=2,
294-
)
295-
break
296-
else:
297-
for handler in all_handlers:
298-
if isinstance(handler, CallbackQueryHandler):
299-
warn(
300-
"If 'per_message=False', 'CallbackQueryHandler' will not be "
301-
"tracked for every message.",
302-
stacklevel=2,
303-
)
304-
break
297+
# this loop is going to warn the user about handlers which can work unexpected
298+
# in conversations
305299

306-
if self.per_chat:
307-
for handler in all_handlers:
308-
if isinstance(handler, (InlineQueryHandler, ChosenInlineResultHandler)):
309-
warn(
310-
"If 'per_chat=True', 'InlineQueryHandler' can not be used, "
311-
"since inline queries have no chat context.",
312-
stacklevel=2,
313-
)
314-
break
300+
# this link will be added to all warnings tied to per_* setting
301+
per_faq_link = (
302+
" Read this FAQ entry to learn more about the per_* settings: https://git.io/JtcyU."
303+
)
315304

316-
if self.conversation_timeout:
317-
for handler in all_handlers:
318-
if isinstance(handler, self.__class__):
319-
warn(
320-
"Using `conversation_timeout` with nested conversations is currently not "
321-
"supported. You can still try to use it, but it will likely behave "
322-
"differently from what you expect.",
323-
stacklevel=2,
324-
)
325-
break
305+
for handler in all_handlers:
306+
if isinstance(handler, (StringCommandHandler, StringRegexHandler)):
307+
warn(
308+
"The `ConversationHandler` only handles updates of type `telegram.Update`. "
309+
f"{handler.__class__.__name__} handles updates of type `str`.",
310+
stacklevel=2,
311+
)
312+
elif isinstance(handler, TypeHandler) and not issubclass(handler.type, Update):
313+
warn(
314+
"The `ConversationHandler` only handles updates of type `telegram.Update`."
315+
f" The TypeHandler is set to handle {handler.type.__name__}.",
316+
stacklevel=2,
317+
)
318+
elif isinstance(handler, PollHandler):
319+
warn(
320+
"PollHandler will never trigger in a conversation since it has no information "
321+
"about the chat or the user who voted in it. Do you mean the "
322+
"`PollAnswerHandler`?",
323+
stacklevel=2,
324+
)
325+
326+
elif self.per_chat and (
327+
isinstance(
328+
handler,
329+
(
330+
ShippingQueryHandler,
331+
InlineQueryHandler,
332+
ChosenInlineResultHandler,
333+
PreCheckoutQueryHandler,
334+
PollAnswerHandler,
335+
),
336+
)
337+
):
338+
warn(
339+
f"Updates handled by {handler.__class__.__name__} only have information about "
340+
f"the user, so this handler won't ever be triggered if `per_chat=True`."
341+
f"{per_faq_link}",
342+
stacklevel=2,
343+
)
344+
345+
elif self.per_message and not isinstance(handler, CallbackQueryHandler):
346+
warn(
347+
"If 'per_message=True', all entry points, state handlers, and fallbacks"
348+
" must be 'CallbackQueryHandler', since no other handlers "
349+
f"have a message context.{per_faq_link}",
350+
stacklevel=2,
351+
)
352+
elif not self.per_message and isinstance(handler, CallbackQueryHandler):
353+
warn(
354+
"If 'per_message=False', 'CallbackQueryHandler' will not be "
355+
f"tracked for every message.{per_faq_link}",
356+
stacklevel=2,
357+
)
358+
359+
if self.conversation_timeout and isinstance(handler, self.__class__):
360+
warn(
361+
"Using `conversation_timeout` with nested conversations is currently not "
362+
"supported. You can still try to use it, but it will likely behave "
363+
"differently from what you expect.",
364+
stacklevel=2,
365+
)
326366

327-
if self.run_async:
328-
for handler in all_handlers:
367+
if self.run_async:
329368
handler.run_async = True
330369

331370
@property

0 commit comments

Comments
 (0)