Mattermost-server: Resolve mentions in slash command payload

[lieut-data]

In a discussion on community, we we noted that Slack supports an Escape channels, users, and links sent to your app setting for slash commands that, among other things, resolves @username to the corresponding user id, encoded as <@user-id> and sometimes as <@user-id|username>.

Design and implement a means to extend the payload of Mattermost slash commands to do something similar. There’s no need to maintain explicit Slack compatibility here. Considerations include:

• Where to include in the payload? Copying Slack would require a configuration setting to avoid breaking existing slash commands, whereas adding to the existing payload /should/ be backwards compatible.
• What to include? User mentions should probably specify the offset in the string where it was found, the matching username and user id.
• Extend mentions to include channels (only public).
• How does this extend to slash commands registered by plugins?

A full resolution of this ticket should include changes to the server covering both third-party (“custom commands”) and plugin slash commands. Take a look at https://github.com/mattermost/mattermost-server/blob/master/app/command.go to get started. To illustrate the functionality, update https://github.com/mattermost/mattermost-plugin-demo to receive the expanded payload and perhaps dump it back out in the form of a post for verification. Be sure to document in the PR exactly how third-party (“custom commands”) should expect to receive the mentions in the payload.

---

If you're interested please comment here and come join our "Contributors" community channel on our daily build server, where you can discuss questions with community members and the Mattermost core team. For technical advice or questions, please join our "Developers" community channel.

New contributors please see our Developer's Guide.

JIRA: https://mattermost.atlassian.net/browse/MM-21987

[lieut-data]

@agarciamontoro, I've created this GitHub issue to mirror https://mattermost.atlassian.net/browse/MM-21987. If you can comment in reply, GitHub will let me assign this ticket your way. Thanks again!

[agarciamontoro]

Thank you for creating the issue!

[agarciamontoro]

Design Proposal

EDIT: as discussed in ~Toolkit, we will get rid of the offset and just consider the map between mentions and identifiers.

EDIT 2: I ended up adding separate fields for user and channel mentions, as the main use case of the single field with the special character identifying the type of the mention does not hold after removing the offset information of the payload.

Summary

The basic idea here is to extend the payload sent to the command providers —either via URL values for custom commands or via the command arguments structure for plugin commands— with an additional structure that maps the mentions found in the message to their user IDs (or to the channel IDs, in case they are channel mentions), also specifying the offset in the string where those mentions were found.

Implementation Details

The structure that holds the data should contain, for each mention found (either to a user or to a specific channel):

1. the mention found in the message, starting with a @ for users and with a ~ for channels,
2. the identifier that corresponds to that mention,
3. the offset in the string where the mention occurs.

Custom Commands

For custom commands, we need to serialize the data, and the current implementation uses URL-encoded values, so I am proposing to add three additional values that will contain arrays of the same length, where their i-th element corresponds to the i-th mention found in the message:

• mentions: an array of strings containing all the found mentions, even if the mentions are repeated.
• mentions_ids: an array of strings containing the corresponding IDs to the found mentions.
• mentions_offset: an array of numbers containing the offset in the string where the mention was found. I need to do a bit of research here about what the offset should refer to: runes (probably too Go-specific), characters, bytes...

Plugin Commands

For plugin commands, we can modify the CommandArgs structure in model/command_args.go, adding a new Mentions field that maps found mentions in the message to their identifiers, as in:

type Mention struct {
 Offset int
 ID string
}

type CommandArgs struct {
 // ... other fields ...
 Mentions  map[string]Mention   `json:"-"`
}

For Developers

The only work needed from developers is for them to parse the first character of the mention, so they know if the mention is to a channel or to a user.

We could solve this by duplicating the structure, splitting it in something like user_mentions and channel_mentions. I do not think this is really useful, and having the character @ or ~ into the mention string, there is no doubt about what the offset refers to: it refers to the position of this special character, not the position of the first character in the name itself.

Discussion on Backward Compatibility

With this solution, slash commands built before this change (either custom or plugin-based) will still work as usual:

• The custom slash commands will receive an additional payload in the request, but they will just ignore it when parsing it and, as we are not modifying the message itself, everything stays the same from their point of view.
• The plugin-based slash commands will still receive the same CommandArgs structure, and as they are not accessing the new field, it will be transparent for them. Marshaling and unmarshaling the structure into json will not change because we are omitting the field for that step.

Testing

I'm planning on modifying the corresponding unit tests to check the new feature, as well as adding an example of the usage to mattermost-plugin-demo.

Possible Issues

This solution adds a bunch of information to the payload, and the size of the data sent will be a bit larger. A solution to this possible problem is to add a setting to enable/disable the parsing and the information returned to the developer, but I don't think it will be a bottleneck at all, so I would keep it simple for the moment.

I don't have data to back this guess, so feel free to comment here if you think it will cause a performance problem. I can come up with some numbers when I implement the solution.

Alternative Solution

Copying what Slack does, we could just replace the mentions in the message itself with the user IDs (or with channel IDs), using a special syntax that could be easily parsed by developer. Slack uses <@userid> for user IDs and <#channel-id>, so if a command is

/command mention @someone in #mychannel

the text received by the developer would be converted into

/command mention <@12345ABCD> in <#09876ZYXW>

This is a sensible solution, but it has a couple of caveats:

1. The user sending the command may want to write <@12345>, and that would be converted to something like <<@10293>>, if 12345 happens to be a username, even if the user does not want to. To solve this we would need to discuss how to escape characters, and I think a generic solution to that problem may rapidly fall into the over-engineering side of things.
2. The developer will need to parse this special syntax. This work has been already done by the server, so we are duplicating efforts.

The solution proposed above solve the first issue by not modifying the command written at all, and tackle the second one by just telling the developers where the mentions are and to which identifiers they are mapped.

The obvious advantage of this solution is that it does not add additional data to the payload, so the size will not increase. (See Possible Issues for my thoughts on this).

---

I will start with the implementation right away, but any comments, ideas or suggestions are more than welcome! For example, I still need to

• check how Mattermost scopes the mentions to users
• check whether it makes sense to resolve any mention found or limit that to the users in the channel (I think I will go for this first), as well as how to resolve only public channels.

[lieut-data]

@agarciamontoro, I like the first approach best. I agree with your intuition that the additional overhead in the payload should be negligible and not a performance concern.

I think the mentions and mention_ids should be sufficient. Let's make a note to supply sample code illustrating how to work with this data once the feature is available -- actually, maybe we can even define a type MentionsMap map[string]Mention and implement ToURLValues / FromURLValues or some such that streamlines this for non-plugin, Go authors. Other languages could look to this for guidance on how to consume the parameters.

With respect to resolving mentions, I think we'll want to resolve them globally within the team vs. just in the channel. A big use case for this feature will be to support integrations that might want to manage permissions, e.g. /nc allow_user @alejandro to interact with them, but the command issued by the system administrator might not be in a channel with that user.

Let me know if I can help any further!

[agarciamontoro]

I think the mentions and mention_ids should be sufficient.

I agree.

Let's make a note to supply sample code illustrating how to work with this data once the feature is available -- actually, maybe we can even define a type MentionsMap map[string]Mention and implement ToURLValues / FromURLValues or some such that streamlines this for non-plugin, Go authors. Other languages could look to this for guidance on how to consume the parameters.

Makes sense, but without considering the offset, the new Mention structure is not needed anymore, so the map would be just a regular map[string]string. I do like the idea of implementing ToURLValues / FromURLValues for the whole payload, as it would make clear the data included in it.

With respect to resolving mentions, I think we'll want to resolve them globally within the team vs. just in the channel. A big use case for this feature will be to support integrations that might want to manage permissions, e.g. /nc allow_user @alejandro to interact with them, but the command issued by the system administrator might not be in a channel with that user.

Sounds reasonable, I'll do it this way, thanks!

[lieut-data]

Thanks again, @agarciamontoro :)