-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
fix(slack): replace slack GetChannels by GetConversations #1350
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1350 +/- ##
==========================================
+ Coverage 70.01% 70.05% +0.03%
==========================================
Files 74 74
Lines 5556 5563 +7
==========================================
+ Hits 3890 3897 +7
Misses 1307 1307
Partials 359 359
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Hi @lkysow , when do you think this can be released ? |
@nishkrishnan Do you know about when would be able to new release that is cover this fix ? |
we're going to try and get one out next week |
Can somebody share a working configuration? I'm still getting
|
@jmreicha I was used to deploy via helm as following, the link about configmap that is keeping following slack config
|
Ah ok, so it needs to be in a separate |
Splitting the configurations seems to work, erroring now with |
For anybody else that comes across the error, these permissions seem to be enough to fix it - |
fixes #1210
channels.list has been deprecated by slack and will break for existing applications in February 2021 (always breaks for new applications).
This PR replaces this call (used to check that the specified channel exists) by the new conversations.list which should by default work in a similar way.
It was needed to bump
github.com/nlopes/slack
as GetConversations was not available in the current version, but I decided not to go up to the latest as they changed the signature of PostMessage, which was trickier to mock in the tests.