attzonko / mmpy_bot

A python-based chatbot for Mattermost (http://www.mattermost.org).
MIT License
261 stars 103 forks source link

Permit per-plugin/per-listener reply-to-self #544

Open joshuaboniface opened 1 month ago

joshuaboniface commented 1 month ago

This PR implements a per-plugin reply-to-self functionality, allowing the bot to, for selected listen_to statements, reply to its own previous messages. For example, this would allow the bot to send a reply like "Hey @sender do the thing" to a trigger, and then handle its own "@sender" listener for that message.

First, we turn the EventHandler ignore_own_messages into a global mmpy_bot setting, which defaults to True to preserve the existing behaviour; the bot user must turn this off to use this new feature.

Second, we add a handler within the MessageFunction that duplicates the functionality in EventHandler; this ensures that the existing behaviour continues to be preserved, even if the global setting is False.

Finally, we add another kwarg to the listen_to decorator to permit passing an ignore_own_message=False option, which would then allow that particular listener to ignore the previous conditions and reply to itself.

Information about this functionality, including a warning about loops, has been added to the documentation on Plugins at the bottom of the page, to ensure users become aware of this new feature and what needs to be done to activate it.

codeclimate[bot] commented 1 month ago

Code Climate has analyzed commit cb934b53 and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1

View more on Code Climate.

unode commented 1 month ago

Hi Joshua, thanks a lot for the contribution.

Trying to understand the use-case and considering the risks, can you elaborate on how you envision that this will be used?

My main concern is that with the above code, the more plugins you enable that use this feature, the more likely you are to potentially run into self triggers and loops, for which we currently do not have stop or control mechanisms.

Understanding also that you would be using this functionality between different plugins, I would also like to contrast the proposed approach with simply calling the corresponding code/function directly.

For instance, I currently have cases that seem identical to yours but instead of doing the extra round-trip between Mattermost and the bot, directly call the function that triggers the behavior. Meaning @bot Do A and B instead of @bot Do A that creates the reply @bot Do B. The code reads:

@listen_to(...)
def a_and_b(...):
    a()
    b()

This approach also has the advantage that most of the listener logic is refactored out into a callable function which @listen_to decorated functions are not. You can then include, for example, report messages in between.

@listen_to(...)
def a_and_b(...):
    send_message_to_channel("Hey I'm about to do A")
    a()
    send_message_to_channel("A complete, doing B next")
    b()
    send_message_to_channel("A and B done")

All this works especially well with non blocking code.

joshuaboniface commented 1 month ago

Hey @unode no problem, happy to explain a bit. I'll preface by saying I fully understand if this is a bit to specific/esoteric for general availability or maintainability, but I figured after implementing it that it was simple enough to at least put out there!

The idea of multiple function calls is of course preferable, but it won't work with what I'm doing. Consider the following two plugins:

  1. A custom group "ping", implemented as a reply to something like @triage. For instance:

    @listen_to('@triage', ...)
    def triage_custom_ping(message):
    users = get_users_on_triage_right_now()
    self.driver.reply_to(message, ' '.join([f'@{user}' for user in users]))
  2. A user-specified reminder system. In this conception, I have a plugin where the user can provide arbitrary message, and the bot replies with it at a later time:

    
    @listen_to('remind me later to (.*)', ...)
    def reminder(message, reminder_string):
    schedule_reminder(reminder_string, message.channel_id)

def schedule_reminder(reminder_string, channel_id): apscheduler.add_job(send_a_reminder, args=[reminder_string, channel_id])

def send_a_reminder(reminder_string, channel_id, ...): driver = ... driver.send_message(reminder_string, channel_id, ...)



I won't detail the code any more here since it's a bit complex, but it uses `apscheduler` and such to trigger a future run of a function, passing the `reminder_message`, then uses a custom `Driver` instance to send a message.

So, what I wanted to happen was to have a user be able to enter something like `@triage remember to check your tickets`, and then when the bot later sends that message, have the bot then respond to its own message with the `@triage` custom ping. But since this is arbitrary user-entered content that is then later posted by the bot, the custom group ping wouldn't work in it without this sort of functionality. Or, put another way, it might be possible to hack this in for a few specific cases, but I wanted it to be more general since I have several custom team pings and many reasons to ping them in these reminders, so that would be a bit explosive in complexity if hardcoded in. Providing a way for the bot to see and reply to its own messages in a limited way (i.e. just for that ping plugin) seemed like a far easier solution, and from there it was trivial to just expand it to a part of the `listen_to` decorator.

Hopefully that clarifies why the direct "just call both functions" method wouldn't work for what I'm trying to do, and what sort of usecases might be possible with this functionality.