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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New slack webhooks not accepting channels modifications #56

Open
TBG-FR opened this issue Jul 30, 2021 · 7 comments
Open

New slack webhooks not accepting channels modifications #56

TBG-FR opened this issue Jul 30, 2021 · 7 comments

Comments

@TBG-FR
Copy link

TBG-FR commented Jul 30, 2021

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
Copy link
Owner

infojunkie commented Jul 30, 2021

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
Copy link
Author

TBG-FR commented Jul 30, 2021

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
Copy link
Owner

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
Copy link
Author

TBG-FR commented Aug 2, 2021

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.

@TBG-FR TBG-FR changed the title Remove channels mapping Remove channels mapping // New slack webhooks not accepting channels modifications Aug 2, 2021
@TBG-FR TBG-FR changed the title Remove channels mapping // New slack webhooks not accepting channels modifications New slack webhooks not accepting channels modifications Aug 2, 2021
@infojunkie
Copy link
Owner

infojunkie commented Aug 2, 2021

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:

  • If there is a way to detect whether a given webhook is legacy or not, then the plugin code can warn the user that the channel mapping is ineffective in case of non-legacy webhooks, upon settings submission.

  • Add a general warning on the plugin settings page explaining what you said above.

  • Wait until legacy webhooks are completely removed from Slack to remove this mapping.

@littlehongkong
Copy link

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

@infojunkie
Copy link
Owner

infojunkie commented Feb 14, 2023

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.

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