Skip to content
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

[FEATURE] Interruption behaviour of Conversations #1640

Closed
hyzyla opened this issue Nov 24, 2019 · 11 comments
Closed

[FEATURE] Interruption behaviour of Conversations #1640

hyzyla opened this issue Nov 24, 2019 · 11 comments

Comments

@hyzyla
Copy link

hyzyla commented Nov 24, 2019

Issue I am facing

Hi, I have faced a problem where I have a few conversations and a few commands:

    dp.add_handler(
        ConversationHandler(
            entry_points=[CommandHandler('command1', command1)],
            states={
                STATE1: [MessageHandler(Filters.text, do_something1)],
            },
            fallbacks=[AnyHandler(do_something1_fallback)],
            allow_reentry=True,
        )
    )
    dp.add_handler(CommandHandler('command2', command2))
    dp.add_handler(
        ConversationHandler(
            entry_points=[CommandHandler('command3, command3)],
            states={
                STATE2: [MessageHandler(Filters.video, do_something3)],
            },
            fallbacks=[AnyHandler(do_something3_fallback)],
            allow_reentry=True,
        )
   )
   dp.add_handler(AnyHandler(global_fallback))

Now when my bot handles conversation for command3, I can easy to interrupt the conversation by entering command2 or command3. But interruption doesn't reset state of conversation for command3.
For example:

U: /command3
B: Send video
U: (sends a text)
B: Send video, please
U: /command1
B: Send text
U: (sends a text)
B: Thanks, I saved it
U: BLABLA
B: Send video, please.

The last sentence is not obvious, why I need a video (do_something3_fallback) when I finished the conversation in the middle of full conversational? More generaly I expect to see the result of global_fallback.

My idea to add a parameter isolated to conversational that doesn't allow interruption by any command and when such conversational started than dispatcher can only view handlers of started conversation.

I know that I can implement it by nesting Conversational Handlers, but then my code looks very ugly and messy.

@Bibo-Joshi
Copy link
Member

Please habe a look at the group argument of do.add_handler. TLDR: only one handler per group is excecuted. Putting your global_fallback in it's own group should suffice for it to be triggered.

@hyzyla
Copy link
Author

hyzyla commented Nov 24, 2019

@Bibo-Joshi Sorry, I read documentation and source code for add_dispatcher and handle_update and still don't understand how it solves my problem when I need to handle or prevent conversation interruptions? Could you please, help me with that?

@Bibo-Joshi
Copy link
Member

Well, it doesn't prevent a conversation from being interrupted but can be used to solve this:

But interruption doesn't reset state of conversation for command3.

See the discussion in #1447
However, I do find the idea of non-interruptable ConversationHandlers interesting.

@Bibo-Joshi Bibo-Joshi self-assigned this Nov 24, 2019
@Bibo-Joshi Bibo-Joshi added this to the 12.5 milestone Nov 24, 2019
@hyzyla
Copy link
Author

hyzyla commented Nov 24, 2019

I added groups to handlers and it's solved my problem (interrupt conversation after some command), but such a method seems a bit hacky to achieve my goal.

    dp.add_handler(
        ConversationHandler(
            entry_points=[CommandHandler('command1', command1)],
            states={
                STATE1: [MessageHandler(Filters.text, do_something1)],
            },
            fallbacks=[MessageHandler(Filters.command, cancel_conversation), AnyHandler(do_something1_fallback)],
            allow_reentry=True,
        ),
        group=1,
    )
    dp.add_handler(CommandHandler('command2', command2), group=100)
    dp.add_handler(
        ConversationHandler(
            entry_points=[CommandHandler('command3, command3)],
            states={
                STATE2: [MessageHandler(Filters.video, do_something3)],
            },
            fallbacks=[MessageHandler(Filters.command, cancel_conversation), AnyHandler(do_something3_fallback)],
            allow_reentry=True,
        ),
        group=2,
   )
   dp.add_handler(AnyHandler(global_fallback), group=100)

I do not have idea hot improve that, maybe some flag to the conversational handler that interrupts the conversation when update handled by some handler outside of conversation and some parameters interrupt handler. What do you think about it?

@Bibo-Joshi
Copy link
Member

Well your solution fallbacks=[MessageHandler(Filters.command, cancel_conversation), ...] seems pretty reasonable to me.
Still, I've added this issue to the v12.5 milestone. At this point I think a PR should add two kwargs to ConversationHander:

  • cancel_on_interruption (or some similar naming): If True, the CHs state will be set to END, if coversation is interruptet. One could also think about allowing to pass a callback that's executed on cancellation or even a dict (mapping states to callbacks) so that the callback depends on the state in wich the conversation is cancelled.
  • allow_interruption (or some similar naming): If False the conversation will not be interrupted until it has ended

Would you like to give at a go yourself?

@Bibo-Joshi Bibo-Joshi changed the title [QUESTION] Isolated conversation [FEATURE] Interruption behaviour of Conversations Nov 24, 2019
@Bibo-Joshi
Copy link
Member

Bibo-Joshi commented Nov 24, 2019

PS: Preventing your CH from interruption can be done by adding fallback in the "isolated" CH that catches e.g. entrypoints of other conversations. However, this will only work for handlers in the same group.
Also, the DispatcherHandlerStop may be useful for you.

Also, if you'd like to PR, please tell us about your proposed changes beforehand, so we can keep in touch b/c CH is quite complex …

@hyzyla
Copy link
Author

hyzyla commented Dec 7, 2019

Sorry, I don't have time to work on this feature

@TempAccountForResponseToQuestion
Copy link

TempAccountForResponseToQuestion commented Dec 19, 2019

A very dirty hack that ends all conversational handlers. Modify under your task.

all_hand = dispatcher.handlers
for dict_group in all_hand:
    for handler in all_hand[dict_group]:
        if isinstance(handler, ConversationHandler):
            handler.update_state(ConversationHandler.END, handler._get_key(update))

c2hpdCBjb2Rl

2021-07-01, Comment by @Bibo-Joshi : The update_state method was never documented and should be considered internal. By now (v13.6) it's private. I highly recommend not to use it, as its behavior may change without notice.

@Bibo-Joshi
Copy link
Member

Leaving this here for completeness sake: Nested CHs need to be considered, if this is tackled.

@Bibo-Joshi Bibo-Joshi removed their assignment Jan 9, 2021
@Bibo-Joshi
Copy link
Member

Looking back at this, I'm not sure adding this as a feature is a good idea for mainly two reasons:

a) It would mean tightly coupling the Dispatcher.process_update logic with ConversationHandler, which I don't think is a good idea and is bound to be a pain to maintain down the road
b) this sort of thing has a high need for customization, especially when multiple different conversations are involved. Als users might want to do something special (i.e. send a warning) when a converstion is interrupted or not allowed to be interrupted.

Finally there are viable workarounds like storing flags like in_conversation_a=True in chat/user_data which can be used for custom behavior.

@Bibo-Joshi
Copy link
Member

After internal discussion, we don't see this feature being implemented, at least not in the forseable future. If someone finds this comment and really wants this feature: Feel free to comment or open a new issue that references this one.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants