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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mattermost attachment support #801

Open
RichyHBM opened this issue Jan 18, 2023 · 4 comments
Open

Mattermost attachment support #801

RichyHBM opened this issue Jan 18, 2023 · 4 comments

Comments

@RichyHBM
Copy link

馃挕 The Idea
The current Mattermost implementation doesn't support attachments, but Mattermost does. This should extend the Mattermost implementation to add support for attachments

馃敤 Breaking Feature
Nope

@phea
Copy link
Contributor

phea commented Mar 8, 2023

I plan on tackling this feature request, this will require adding a 'Bot' mode similar to the Slack plugin as file uploads are only available through Mattermost's API and not webhooks. However, the challenge is to differentiate between the two modes, as both webhook and bot tokens in Mattermost are a 27-digit alphanumeric string -- unlike with Slack that uses different token formats.

The current schema does include a botname:

  • mmost://{botname}@{hostname}/{token}

Screenshot from 2023-03-08 09-03-49

Looking through the code, botname is an alias for user and with Mattermost v5 setting this field does not change the notification message (see screenshot). However for the sake of backwards compatibility I am reluctant to use botname as a way to determine the mode.

One possible solution would be to have a isBot parameter, set it to yes to activate Bot mode. I believe this would be a more reliable way to differentiate between the two modes, and it would not break any existing implementations.

@caronc thoughts or other ideas?

@caronc
Copy link
Owner

caronc commented Mar 9, 2023

Probably a new mode (or modes) would be better vs a mode paired with a bool. We have a few plugins that detect the mode to use based on the key/token provided. But a mode that can also be set to enforce the detection (to override). I'm not at home right now, but i can dig up some examples.

If the tokens are different as you say, then this works in our favor because it makes the auto detection easier (and accurate).

Thoughts?

P.s. thanks for taking this on. Attachments are a bit harry, but there are lots of examples you should be able to borrow from the cover all angles most services use. Might be some copying and pasting i think to make your life easier

@phea
Copy link
Contributor

phea commented Mar 10, 2023

Sorry I guess I wasn't clear, but Mattermost tokens for webhooks and bots use the same format, so it's not possible to auto detect based on token. One big difference between the two modes is how channels are passed. Webhooks use slugs like off-topic or general, whereas API requests require channel ids that are 26-digit alphanumeric strings like h6e9o6yhzinr9845f4faac7hme.

I thought the ids had some special encoding similar to uuids that we can use to validate channel ids, but looking through their source code they just assume any 26-digit alphanumeric string is a valid id: https://github.com/mattermost/mattermost-server/blob/v6.7.2/model/utils.go#L608

We can do the same, but this seems unreliable as channel names have a max length of 64(src)

I can create the initial PR where any notifications with attachments use API calls over hooks, and then we can revisit whether to use a bool flag or some other mechanism.

References:
[1] Webhooks
[2] REST API

@caronc
Copy link
Owner

caronc commented Mar 10, 2023

Nah, if that is the case, definitely don't auto-detect. But i think I'd still rather have 4 distinctive modes over a separate switch that toggles a bot flag paired with the 2 existing modes.

Otherwise, whatever you come up with will be amazing no doubt! 馃殌馃憤馃檪

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

No branches or pull requests

3 participants