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 App does not allow selecting private channels for pulses #20867

Closed
mj3052 opened this issue Mar 5, 2022 · 10 comments · Fixed by #23232
Closed

New Slack App does not allow selecting private channels for pulses #20867

mj3052 opened this issue Mar 5, 2022 · 10 comments · Fixed by #23232
Assignees
Labels
.Frontend Notifications/Slack Priority:P2 Average run of the mill bug .Regression Bugs that were previously fixed and/or bugs unintentionally shipped with new features. Type:Bug Product defects
Milestone

Comments

@mj3052
Copy link

mj3052 commented Mar 5, 2022

Is your feature request related to a problem? Please describe.
With the old Slack bot integration it used to be possible to select a private channel for pulses in Slack. This option is now gone with the introduction of the new Slack App.

Describe the solution you'd like
We would like for it to, once again, be possible to select private channels when setting up a Slack pulse.

Describe alternatives you've considered
A workaround was suggested in #2694 here, but it seems a bit too bad, as this was possible with the old integration.

It has also been suggested to just make the text field free form, so private channels can be added directly by name (and then might just not exist). This solution would also work, but the UX of having them available in the dropdown as before seems nicer.

How important is this feature to you?
We do not use pulses for anything but posting to private channels, as it is only select people who needs the info. Right now we did the manual DB edit as in the workaround. It was historically also a very popular addition in #2694, so it seems like a lot of people will miss this option.

Additional context
I found that the decision to remove list of private_channel was made here. It was done to reduce the need of the scope groups:read, which is a bit too bad. I would like to note that this scope still only gives the app read access to private channels it has been added to. I verified this using the conversations.list endpoint. So the scope does not give excessive access to lots of unneeded information.


Just thought I would raise this as its own issue, to show that we would very much like this option to still be available.

⬇️ Please click the 👍 reaction instead of leaving a +1 or update? comment

@flamber flamber added .Regression Bugs that were previously fixed and/or bugs unintentionally shipped with new features. Notifications/Slack Priority:P2 Average run of the mill bug Type:Bug Product defects and removed Type:New Feature .Needs Triage labels Mar 5, 2022
@noahmoss
Copy link
Member

noahmoss commented Mar 7, 2022

My 2¢:

  • We must not give the impression that data sent to a private channel is actually private. This is not and has never been the case. Due to limitations with the Slack API, graphs & tables must be uploaded to a public files channel (e.g. #metabase_files) before the actual subscription message is sent. Allowing private channels to be selected for subscriptions can give the false impression that the data will remain private when sent to Slack.
  • I don't think fetching & displaying the names of all public and private channels to all Metabase users is the right thing to do. Private channels are private for a reason and some could have sensitive names. Having a free-form text field (or perhaps a combination of a text field + a dropdown for public channels) might be a decent solution, if we can solve the issue in the previous bullet point.

But someone like @brunobergher should probably weigh in here from the product side.

@brunobergher
Copy link
Contributor

I agree with @noahmoss on all points.

I have seen a UX pattern elsewhere where users can enter a channel id if the channel they're looking for isn't in the list, which could help alleviate this (eg making the last option in the list Other (enter id) and showing a text input below it. I wouldn't make this a priority through.

@barillax
Copy link

Just wanted to share another perspective. At our organization, private channels do not necessarily equate to "where the secrets live"; instead, it's more akin to "we prefer not to have random people join and ask questions in this channel".

In this world, most of our team-specific channels are private. These are also the destinations for most of our pulses. I suspect we're not that unusual in using Slack this way.

In any case, our other SaaS tools will happily send to private channels, so with the recent update Metabase has become the odd man out. I hope you'll consider an option that doesn't require editing the database directly. As is, I'd consider this a regression in the Slack integration.

My preference would be to retain the groups:read permission and conditionally show a stern warning in the UI when setting up a pulse to a private channel.

@abhishek-superk
Copy link

A workaround that we started using: send emails to channel instead.

I know it's not the best alert, but it makes sending alerts possible. (And also sends raw data, which can be a pro or a con depending on your use-case).

Just open the channel and you'll see the following in the integrations. Click on Send emails to this channel and you'll get an email that you can send your alert/pulse to.
image

@pranaygp
Copy link

My workaround has been manipulating the dropdown using the React Dev Tools and then saving it

  1. First
    Save props.details as a global variable

  2. Run $reactTemp0.channel = "#private-channel" in the console

@noahmoss
Copy link
Member

noahmoss commented May 2, 2022

Categorizing as a frontend bug, since a fix to this should be to change the Slack UI to allow free-form input alongside the channel drop-down. If this is done, the backend should work without issue.

@ranquild ranquild self-assigned this May 3, 2022
@ranquild ranquild removed their assignment Jun 6, 2022
@gusaiani gusaiani self-assigned this Jun 6, 2022
@gusaiani
Copy link
Contributor

gusaiani commented Jun 6, 2022

I have seen a UX pattern elsewhere where users can enter a channel id if the channel they're looking for isn't in the list, which could help alleviate this (eg making the last option in the list Other (enter id) and showing a text input below it. I wouldn't make this a priority through.

@brunobergher @cdeweyx, I'm taking up this issue per our pod's weekly planning.

Bruno, Conor suggested we implement the option of posting to private channels the way you suggest above.

One detail we may want to think about: "making the last option in the list Other (enter id)".

The list of public channels / @s to post may get very large, enough to make the last option nearly undiscoverable.

Alternatives I can quickly think of:

  1. having "post to private channel" be the first option in the dropdown list
  2. having a checkbox/link below the main dropdown saying something like "post to private channel", that when clicked will show the text input (maybe hiding the initial dropdown at the same time)
  3. there could be a private/public toggle component, defaulted to public, switching between the dropdown and a text input

Any thoughts?

@abhishek-superk
Copy link

Works great for dashboards, but don't see any change in setting alerts for a question.
Sending dashboard subscriptions and question alerts should work the same, right?

@flamber
Copy link
Contributor

flamber commented Jun 22, 2022

@abhishek-superk There's now an issue for it #23483.

@abhishek-superk
Copy link

@flamber : Thankyou, much appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
.Frontend Notifications/Slack Priority:P2 Average run of the mill bug .Regression Bugs that were previously fixed and/or bugs unintentionally shipped with new features. Type:Bug Product defects
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants