message.reply doesn't inherit parent_id

[blampe]

Describe the bug
reply doesn't expose any way to configure the pid or parent_id of the message being replied to: https://github.com/attzonko/mmpy_bot/blob/master/mmpy_bot/dispatcher.py#L267

By itself this isn't unreasonable, as it would be a clumsy developer experience to always have to plumb that around. However, the method doesn't automatically infer pid/parent_id either, and following the call chain from reply to send to channel_msg shows pid is never correctly populated:

https://github.com/attzonko/mmpy_bot/blob/master/mmpy_bot/mattermost.py#L223

Expected behavior
I would expect message.reply and message.reply_webhook to preserve the chat thread by automatically populating the correct parent_id in the POST request.

Additional context
This is accurate as of 1.3.3.

[attzonko]

I think I understand what you mean. I think the original intent of the message.reply hook was simply to reply to whoever triggered the bot by @ mentioning them in the message being sent back.

What you would like to be able to do is for the bot to actually reply back and preserve the message thread. I am not comfortable simply changing the default behavior of the existing reply functions, because we would be potentially changing how existing plugins work unexpectedly.

I think we can either add new functions with a different name which somehow still makes sense, or perhaps change the current functions to have the option to preserve the thread. I am leaning toward that option. Thoughts?

[blampe]

Preserving the current behavior is totally reasonable! An alternative entry point (reply_thread?) or option (threaded=True?) would be great 👍

…

On Wed, Jan 16, 2019 at 22:31 Alex Tzonkov ***@***.***> wrote: I think I understand what you mean. I think the original intent of the message.reply hook was simply to reply to whoever triggered the bot by @ mentioning them in the message being sent back. What you would like to be able to do is for the bot to actually reply back and preserve the message thread. I am not comfortable simply changing the default behavior of the existing reply functions, because we would be potentially changing how existing plugins work unexpectedly. I think we can either add new functions with a different name which somehow still makes sense, or perhaps change the current functions to have the option to preserve the thread. I am leaning toward that option. Thoughts? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#90 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAzzyyLISanUPR505EKYDfsvkAOQZ2ZBks5vEBjJgaJpZM4aCEl3> .

[ralgozino]

I think I'm late to the discussion, but isn't the message.comment() exactly this? I for one, am using it for this purpose.

[attzonko]

@ralgozino The message.comment() allows you to provide a reaction to a message via an emoji. The new message.reply_thread() actually provides a written text response which keeps the messages as part of the same thread. Hope that helps clarify.

[ralgozino]

@attzonko doesn't message.react() do the emoji thing?

Using message.comment() I get this:

This is the plugin code:

# *-* coding:utf-8 *-*

import re
import logging
import ipinfo

from mmpy_bot.bot import listen_to
from mmpy_bot.bot import respond_to

@listen_to(r'!whois (.+)')
@respond_to(r'!whois (.+)')
def whois(message, host):
    msg = 'WHOIS |  **%s**\n' % host
    msg += '---------|--------\n'
    handler = ipinfo.getHandler()
    details = handler.getDetails(host)
    all_details = details.all
    for d in sorted(all_details):
        msg += '%s | %s\n' % (d.title(), all_details[d])
    message.comment(msg)

[attzonko]

@ralgozino That is weird, I'll have to dig into and understand why it works that way. It could be just how the Mattermost API works. The code for message.comment() just calls the message.react()

    def react(self, emoji_name):
        self._client.react_msg(
            self._body['data']['post']['id'], emoji_name)

    def comment(self, message):
        self.react(message)

And the _client.react_msg() uses the /reactions API endpoint which from the documentation says:

Endpoints for creating, getting and removing emoji reactions.

[attzonko]

It almost seems the implementation is taking advantage of the API working for both.

I think we should change it so message.comment() uses the message.reply_thread() so we can be explicit with the intent. I don't like the sloppiness of passing in message to the emoji_name parameter 😃

[attzonko]

Re-opening the issue so I can remember to change the message.comment() to use the message.reply_thread() unless anyone have a major objection 😸

[ralgozino]

No objections here! :) Il giorno Gio 31 Gen 2019, 22:17 Alex Tzonkov <notifications@github.com> ha scritto:

…

Re-opening the issue so I can remember to change the message.comment() to use the message.reply_thread() unless anyone have a major objection 😸 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#90 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGEWSg2jlHCDhaC2XRaHrFAVl8MSepyNks5vI5WlgaJpZM4aCEl3> .