mattermost-plugin-remind icon indicating copy to clipboard operation
mattermost-plugin-remind copied to clipboard

Cannot set reminders for `@channel`, `@all`, and `@here`

Open Roy-Orbison opened this issue 4 years ago • 11 comments

Summary

Plugin doesn't seem to correctly resolve special username-like keywords @channel, @all, and @here when attempting to post reminders.

Steps to reproduce

Steps to reproduce the behavior:

  1. Go to a public channel
  2. Post a message like /remind @channel test in 1 minute
  3. Wait until message should have been posted

Expected behavior

Reminder should be posted as if one had used ~channel-name, explicitly.

Observed behavior (that appears unintentional)

Reminder does not appear and shows as "Past and incomplete" in /remind list output.

Server configuration

Operating system: Debian
Mattermost version: 5.36.1
Remind version: 0.4.4
Updated from an older Remind bot version or fresh install: Updated

Mattermost server logs
{"level":"error","ts":1626123456.78901,"caller":"mlog/sugar.go:25","msg":"failed to find target user @channel","plugin_id":"com.github.scottleedavis.mattermost-plugin-remind"}
{"level":"error","ts":1626123456.78902,"caller":"mlog/sugar.go:25","msg":"failed to find target user @here","plugin_id":"com.github.scottleedavis.mattermost-plugin-remind"}
{"level":"error","ts":1626123456.78903,"caller":"mlog/sugar.go:25","msg":"failed to find target user @all","plugin_id":"com.github.scottleedavis.mattermost-plugin-remind"}

Additional context

Related to #188 but I think distinct from that because, according to others, even ~channel-name notation fails for private channels. Also mentioned here.

Viewing the raw database records for such reminders, most ID strings looked internal to the plugin (searching for them in other tables yielded no results). The external IDs/refs I found were .TeamId, .Username, and .Target. I could find no absolute reference (name or ID) to the channel in which the command was issued, .Target was just @channel.

Using the channel name internally seems like a bad choice anyway since it can be changed between setting a reminder and it being posted. If both specials and explicit channel names were resolved to an ID when a command gets parsed, and that ID were added to the reminder data, wouldn't that be sufficient to post to the (correct) channel? I assume that differentiating the specials in a reminder post's content, in order to handle who gets notified, is up to Mattermost core, not for this plugin to worry about.

https://github.com/scottleedavis/mattermost-plugin-remind/blob/39d6b97ce3f70a6ea274218a974e90ce9e58ede7/server/reminder.go#L93

Roy-Orbison avatar Jul 16 '21 04:07 Roy-Orbison

This issue occurs because when creating the Post, the Mattermost server sees that the Post has an empty Post.Type, and the bot user doesn't have permissions to post channel mentions. Thus CreatePost marks the post as mentionHighlightDisabled, and no channel mentions are triggered.

There are several possible workarounds:

  1. Pass a non-default Post.Type to CreatePost – for instance model.POST_CUSTOM_TYPE_PREFIX + "reminder". This lift the need for user permissions to use channel mentions.

    It looks like this: Capture d’écran 2022-04-06 à 11 58 35

  2. Put the reminder body in a Slack Attachement – similar to what is done for user reminders.

    It looks like this: Capture d’écran 2022-04-06 à 11 55 31

I'd personnaly be in favor of the option 2, because is consistent with the way user reminders are presented, and seems cleaner than option 1.

kemenaran avatar Apr 06 '22 10:04 kemenaran

@kemenaran I think you're referring to the problem described by #188, where the bot can find the channel but fails to post to it. I don't think that's actually fixed, but people are working around it by adding the bot to the channel.

This issue is about the bot not even understanding the channel to begin with, before any posts are created, so permissions are irrelevant. When creating a reminder, its data gets saved into the PluginKeyValueStore table as a JSON object, not as a post. That object has some IDs added to it, but not those of the channel in which the /remind command was executed. When the pending reminders are read in order to create the reminder posts, the targets like @here aren't understood because the plugin did not remember which channel the alias refers to. This could be fixed by resolving and storing the all targets as IDs at the time the reminder is created.

Roy-Orbison avatar Apr 07 '22 00:04 Roy-Orbison

Hmm, I wasn't referring to the issue in #188, but really to the issue description:

Plugin doesn't seem to correctly resolve special username-like keywords @channel, @all, and @here when attempting to post reminders.

I agree that there's an issue on how channels are stored (especially for private channels), but I was trying to move forward the discussion on a generic-case issue, that is probably causing a lot of these troubles. (Nanely "Why when creating the post the server explicitly disables mentions?")

kemenaran avatar Apr 08 '22 11:04 kemenaran

Is this going to be taken in a release? Would be very useful!

rochaporto avatar Jun 08 '22 21:06 rochaporto

At the point it tries to execute the request, it appears to exclude the channel ID form the parser's input data even though it's available in that context as args.ChannelId. It's just @scottleedavis (or anyone who's decent at Go) having the time and will to patch in a fix.

Roy-Orbison avatar Jun 09 '22 00:06 Roy-Orbison

Guys, will this be fixed in the next releases? I really need this fix. Maybe there is some kind of night build?

shamanis avatar Sep 07 '22 13:09 shamanis

I fixed the issue on the CodeursEnLiberte fork, but never submitted a PR. I just opened #220 with a fix for this issue.

kemenaran avatar Sep 07 '22 20:09 kemenaran

@kemenaran Can you explain how setting a custom message type will solve this issue? The problem is that the plugin does not know where to post the message, not that it's unable to be posted.

Maybe your PR helps resolve #188?

Roy-Orbison avatar Sep 08 '22 01:09 Roy-Orbison

It solves the problem because global mentions (such as @here, etc.) are normally triggered only if the user posting the message is allowed to trigger such global mentions. Which bot users such as Remindbot never are.

The officially sanctioned workaround is to use a custom message type. In that case, the message is always allowed to trigger a global mention. And thus writing a reminder containing @here or @channel properly works.

kemenaran avatar Sep 08 '22 05:09 kemenaran

@kemenaran I've explained the details of this issue numerous times, and every time you've responded, you've described the problem of being not allowed to use special mentions in the reminder content, which is not the issue I'm describing. This issue occurs even if there are no special mentions in the reminder content. You keep ignoring the fact that when you're passing a custom PostType to CreatePost you already have the channel ID!

Please refrain from commenting any further on this issue until you've actually tried the test case:

  1. Post this to any channel you like:
    /remind @all foo in 2 minutes
    
  2. The database will then contain something like the following in the PluginKeyValueStore table:
    PluginId PKey PValue ExpireAt
    com.github.scottleedavis.mattermost-plugin-remind 2022-09-08 07:01:32 +0000 UTC [{"Hostname":"test","Id":"oooooooooooooooooooooooooo","Username":"roy","ReminderId":"rrrrrrrrrrrrrrrrrrrrrrrrrr","Repeat":"","Occurrence":"2022-09-08T07:01:32Z","Snoozed":"0002-02-02T00:00:00Z"}] 0
    com.github.scottleedavis.mattermost-plugin-remind roy [{"TeamId":"tttttttttttttttttttttttttt","Id":"rrrrrrrrrrrrrrrrrrrrrrrrrr","Username":"roy","Target":"@all","Message":"foo","When":"in 2 minutes","Occurrences":[{"Hostname":"test","Id":"oooooooooooooooooooooooooo","Username":"roy","ReminderId":"rrrrrrrrrrrrrrrrrrrrrrrrrr","Repeat":"","Occurrence":"2022-09-08T07:01:32Z","Snoozed":"0002-02-02T00:00:00Z"}],"Completed":"0002-02-02T00:00:00Z"}] 0
  3. Inspect the data. Note that the reminder contains only the word foo, the Target is not associated with any specific channel, it is not a channel's real name, it's not a channel ID, thus the bot does not know which channel to post to, and fails to remind the user.
  4. Check your logs, you will see an entry like so:
    {"timestamp":"2022-09-08 07:01:31.123 +00:00","level":"error","msg":"failed to find target user @all","caller":"app/plugin_api.go:940","plugin_id":"com.github.scottleedavis.mattermost-plugin-remind"}
    
    This error is caused here, well away from your patch.

Roy-Orbison avatar Sep 08 '22 07:09 Roy-Orbison

awaiting response from mm team regarding https://github.com/scottleedavis/mattermost-plugin-remind/issues/195#issuecomment-1416785427 before movement on this issue

scottleedavis avatar Feb 04 '23 15:02 scottleedavis