infojunkie / MantisBT-Slack

Slack integration for Mantis bug tracker
GNU General Public License v2.0
48 stars 37 forks source link

New slack webhooks not accepting channels modifications #56

Open TBG-FR opened 3 years ago

TBG-FR commented 3 years ago

Hello there !

I plan to work on your plugin, and I was a bit disappointed at first by the two existing mappings, "channels" and "webhooks". I just tried to setup a Mantis (v2.25.0), adding MantisBT-Slack, and I found out "channels" seem to bu useless now...

Creating a webhook in Slack requires a defined channel, and it seems that you can't override that thing. So if I understand right, only the webhook mapping is useful ? Shouldn't we remove all the channel-related code ?

Thanks in advance for your answers !

infojunkie commented 3 years ago

It is possible to use a single webhook to send messages to different channels, using the channel field in the JSON payload. I expect, but without testing, that the channel specified when creating a webhook is the "default" channel that would be used if the channel field is empty. Ref: https://api.slack.com/messaging/sending#publishing

The reason why the webhooks mapping exists in this plugin is to allow sending notifications to different Slack workspaces based on the Mantis project.

TBG-FR commented 3 years ago

Thanks for the clarifications. However, I tested it, creating a webhook in Slack, and then filling the channel mapping, and no messages where emitted to the channels... Is the channel field in JSON payload implemented ?

Alright, I understand, that's a good point.

infojunkie commented 3 years ago

However, I tested it, creating a webhook in Slack, and then filling the channel mapping, and no messages where emitted to the channels...

I would say make sure your configuration is correct, and you can trace the code: https://github.com/infojunkie/MantisBT-Slack/blob/master/Slack.php#L312-L315

TBG-FR commented 3 years ago

The documentation page states :

That URL is your shiny new Incoming Webhook, one that's specific to a single user, and a single channel. and also You cannot override the default channel (chosen by the user who installed your app), username, or icon when you're using Incoming Webhooks to post messages. Instead, these values will always inherit from the associated Slack app configuration.

That would explain why my Mantis is always posting in the webhook channel, not taking into account the default_channel value or the mappings..

I think the plugin was made for legacy webhooks and hence does not work with the new ones

The majority of your legacy code for sending messages using incoming webhooks should continue to work within a Slack app without much modification; the only thing you can no longer do is customize the destination channel and author identity at runtime.

infojunkie commented 3 years ago

Thanks for the research and explanation!

Since legacy webhooks are still functional, it does not make sense to remove the channel mapping functionality just yet. I suggest the following:

littlehongkong commented 1 year ago

Is there any plan to change the legacy code for this web hook?

infojunkie commented 1 year ago

I am not familiar with the state of webhooks in Slack today, so my previous comment still stands.

Also, before removing the settings Slack Channels and Default Slack Channel, I need to make sure Mattermost and Discord compatibility will not be broken.