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

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

joshuaboniface
Copy link

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.

This commit 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.
@unode
Copy link
Collaborator

unode commented Aug 15, 2024

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
Copy link
Author

joshuaboniface commented Aug 16, 2024

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]))
  1. 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.

Copy link

codeclimate bot commented Dec 12, 2024

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

Here's the issue category breakdown:

Category Count
Complexity 1

View more on Code Climate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants