Slack slash commands using Slack app integration on GitLab.com

added gitlab-ce~~874211 ~~19492 gitlab-ce~~1042912 gitlab-ce~~584040 labels

changed milestone to gitlab-ce%29

removed gitlab-ce~584040 label

changed milestone to gitlab-ce%37

@zj you know more about this than I do

I think, as this is just for GitLab.com, we could just auto-enable the Slack slash commands API and then have the app use that? We'd also need to set up a bot to allow configuration, like the video.

@smcgivern That is harder than it seems.

For .com this flow from the videos will work. But keep in mind we are not using OAuth right now, which is what we would need. If we're running on .com we should know the OAuth app secrets etc to setup a OAuth dance, but once we direct the user to Slack we'll eventually get a payload back, encrypted with a shared secret, which we in turn can use to enable the service. After that we can and should actually use the OAuth flow for user authentication too, instead of the ChatName model we use now. Technically it is possible to have both, but it will get weird gitlab-ce~~2024184 wise and probably some amounts of gitlab-ce~~168008 will be gained/endured (Whatever the right term is)

Thanks @zj! Does that mean we should set this up as a (third) project-level Slack service, or maybe something else entirely?

A couple more things to consider:

This doesn't actually need the ability to configure services in the project at all if we use OAuth - we just use the token of the user themselves.
We'd eventually want some way of letting people set aliases, because this would just have a static list of commands as entry points. (Probably /gitlab, but we could also do /gitlab-issue if we thought that was better - I don't think it is, though.) So a sample command would probably look like /gitlab gitlab-org/gitlab-ce issue create foo, and it would be nice if people could get that down to /gitlab ce or something if they chose to.

added gitlab-ce~901060 label

@smcgivern We don't need a new service, checkout how we do that with the Mattermost service. We should be able to do the same here.

@zj which part? Just to be clear, the reason I'm asking is because if our (GitLab's) Slack team has this new app set up, we shouldn't have to configure a project service at all - we should all just be able to use the /gitlab command on any project our individual user has access to.

added gitlab-ce~992791 label

@sarrahvesselov : This is the other Slack issue for 9.4 that is actually higher priority than https://gitlab.com/gitlab-org/gitlab-ce/issues/32164. But it'll likely be driven by engineering, and it's hard to do UX for it ahead of time because we can't predict the flow just yet. Because the engineering team needs to do investigation / POC on the integration and how the screens would flow between GitLab.com and Slack. So it may look a little UX-rough in this first release. You can click on the links in the description to see how it works with other apps. Essentially, it's just a bunch of UX screens. Hopefully the flow that's discovered will design itself.

I don't think there's anything actionable for UX just yet, beyond maybe reviewing the links in the description.

which part? Just to be clear, the reason I'm asking is because if our (GitLab's) Slack team has this new app set up, we shouldn't have to configure a project service at all - we should all just be able to use the /gitlab command on any project our individual user has access to.

That will be a bit harder, as there is no way to detect the group/project name just yet, but should be doable. I wonder how this is going to work with subgroups, though. But you should be alright.

If you, or any other developer, needs help to understand, please ping me on Slack for faster response times.

changed the description

assigned to @vsizov

@vsizov : This one I think needs a little bit of technical investigation. In the description there's examples of how other apps are doing it. So you can take that as a baseline set of requirements. Please ping me and the Slack people (in the Slack channel) if there's any questions/concerns. Once you have a better idea of what's possible, we can chat about any particular UX details, etc.

@jschatz1 : I don't think there's any FE work for this particular issue. Maybe some very basic settings pages. But let's let @vsizov let us know what's required.

The new Slack app feature for GitLab.com is not like the "Slack slash commands" feature we have now as it should work on a global level. Essentially, this is just another OAuth provider for us as GitHub, BitBucket, Google, etc. We only need to treat it in another way: we don't need a sign up/sign in and we don't show it in the user profile. But it still needs to be added to gitlab.yml file (app secret and app id). When a user clicks "Add to Slack", one, in fact, connects the slack team with the GitLab user profile (note! NOT with a GitLab project). But we can specify what exactly project was meant to be connected to slack team by setting a special flag in the project or something.
Trello connects its internal teams with slack teams which makes more sense. They don’t connect any particular board to slack. However they allow to adjust it latter, see The Trello App for Slack - Trello Help We can consider using group for this as well
The relation between GitLab project and Slack team is hard one :) I mean there are questions here: 1) Visibility. How do we check permissions? I mean if user has requested some issue to show and someone in the slack team is not supposed to see it. This case should be handled correctly. The rest of checks seems like already implemented in our existing Slack service.
The URL(s) for command(s) should be hardcoded in Slack settings. So this will be a single entry point for all the slack commands for the whole GitLab.com (something like gitlab.com/slack/trigger)
We could also use a Slack's webhooks to overlap our "Slack notification service". Not sure if this makes sense though.
We can check if we're on GitLab.com then we don't show existing services "Slack slash commands" and I think "Slack notification service" (has to be decided)?
This feature should be implemented on EE side I believe. Should we move the issue?

@sarrahvesselov @tauriedavis I do not believe the gitlab-ce~~2024184 is done here yet right? So I cannot begin on the frontend work until gitlab-ce~~2024186 is added.

@jschatz1 Correct, no action from either FE or UX at this stage per @victorwu.

@sarrahvesselov : This is the other Slack issue for 9.4 that is actually higher priority than https://gitlab.com/gitlab-org/gitlab-ce/issues/32164. But it'll likely be driven by engineering, and it's hard to do UX for it ahead of time because we can't predict the flow just yet...I don't think there's anything actionable for UX just yet, beyond maybe reviewing the links in the description.

@jschatz1 : I don't think there's any FE work for this particular issue. Maybe some very basic settings pages.

@vsizov : I moved this to EE since it's for GitLab.com specifically, which is running EE. Thanks for the reminder.

Chatted with @vsizov. The main goal here is to get something in the slack app directory. That might be group-level integration or project-level integration. @vsizov will do more investigation to see what's feasible and best path forward.

assigned to @vsizov and @jschatz1

@vsizov let me know what you want me to work on.

@smcgivern There are two possible flows I can think of, I'm currently implementing the first one. This list should answer your comment:

Slack team has this new app set up, we shouldn't have to configure a project service at all - we should all just be able to use the /gitlab command on any project our individual user has access to.

Flow 1 (GitLab project <-> Slack team)

This flow is compatible with the functionality we have now so most of the code can be reused

Button on project settings page follows to Slack and sets redirect URL that will take us to /:project_full_path/slack/enable where we create slack_integration record (team_id, project_id, project_alias (by default, project full path))
On project settings page in Slack section we see a form with an alias field that can be edited by users.
We create a global trigger URL for Slack where we lookup a project by team_id and alias.

Problems

If Slack is not a service (third) we can't use ChatName entity. If Slack is a service we would have to store team_id and alias as serialized values which makes it impossible to look up a project (effectively)

Flow 2 (GitLab user <-> Slack user)

The configuration happens in user profile. Please note that application still can be installed on team level only. So every Slack team member can fire commands like /gitlab gitlab-org/gitlab-ce issue new "Awesome issue" and other. At the same time all the command responses should be “ephemeral” (response is visible for command author only). Project aliases can be configured by every user separately in their own “User Settings”.

Problems

We can't use existing ChatName enity to map Slack and GitLab users

Generally, this flow seems pretty confusing to me. I would stick with the first one.

UPDATE: I was talking about "GitLab group <-> Slack user" flow recently but it seems very similar to the first one in a nutshell

Also, one note about fixing the timeout bug. Slack has a feature "delayed response" it allows to send the response later but it has bad UX for "in_channnel" messages as the initial command will not be visible in the channel. Quote from Slack docs:

The only user-facing difference between immediate responses and delayed responses is that "in channel" delayed responses will not include the initial command sent by the user. To echo the command back to the channel, you'll still need to provide a response to Slack's original visit to your invocation URL.

See https://api.slack.com/slash-commands#delayed_responses_and_multiple_responses

Thanks for the detailed comment @vsizov!

Button on project settings page follows to Slack and sets redirect URL that will take us to /:project_full_path/slack/enable where we create slack_integration record (team_id, project_id, project_alias (by default, project full path))

So this means that it must be set up initially by a project master, but afterwards anyone in the Slack team can use that GitLab project?

On project settings page in Slack section we see a form with an alias field that can be edited by users.

We can start without aliases at first, if that helps.

If Slack is not a service (third) we can't use ChatName entity. If Slack is a service we would have to store team_id and alias as serialized values which makes it impossible to look up a project (effectively)

Can we make it a service, but add a table specifically for this service's configuration, with a 1:1 relation to the main services table? Project services settings being serialised is a problem that we need to solve at some point anyway, probably by splitting service configuration out into separate tables, so we may as well start now.

Generally, this flow seems pretty confusing to me. I would stick with the first one.

Agreed

Can we make it a service, but add a table specifically for this service's configuration, with a 1:1 relation to the main services table?

Haha, this is what I started implementing yesterday. That proves the fact that this is a good idea :)

So this means that it must be set up initially by a project master, but afterwards anyone in the Slack team can use that GitLab project?

My first POC was with Slack not being a service then I revised this approach, so now it's a regular service.

Thanks @vsizov. One other thing - can you associate the same GitLab project to multiple Slack teams, or is that also out of scope for now?

How do we check permissions? I mean if user has requested some issue to show and someone in the slack team is not supposed to see it. This case should be handled correctly.

@vsizov it would be great if you could take a look at this discussion in https://gitlab.com/gitlab-org/gitlab-ce/issues/25248 and contribute with your own suggestions.

Thanks @vsizov. One other thing - can you associate the same GitLab project to multiple Slack teams, or is that also out of scope for now?

It's possible, but I think we can do that later if the feature will be requested.

@vsizov it would be great if you could take a look at this discussion in https://gitlab.com/gitlab-org/gitlab-ce/issues/25248 and contribute with your own suggestions.

@pedroms Thanks for heads up. It's very interesting issue and I totally agree with your suggestion. Fortunately, I will reuse existing code for chat commands so fixing this security bug will be out of the scope of the current issue.

@vsizov ok, thanks for the update.

Good news: I already have my POC working

Bad news: During my local testing, I caught "Request timeout" several times. So the bug https://gitlab.com/gitlab-org/gitlab-ce/issues/31417 have to be addressed somehow... see also my comment https://gitlab.com/gitlab-org/gitlab-ee/issues/2629#note_33088381

To test the feature I created an instance on GCE and made a port forwarding with SSH to my local machine as I don't have a public IP from my ISP.

Video https://drive.google.com/a/gitlab.com/file/d/0B5tAA38RRmPUdlNMaF9YQW9CZEk/view?usp=sharing

It will look the same if we start from Slack, you just need to install Gitlab app to your team. On gitlab.com, we also need to add an App data to ~~yaml configuration file~~ application settings.

@vsizov thanks for doing that, and creating the video! cc @victorwu

For the timeouts, do you think you'd be able to spend some time looking at fixing those? Which are the endpoint that you see timing out most often?

@smcgivern The last ten times I checked it, this worked without any timeouts (port forwarding + mobile internet). As I understand from the gitlab-ce#31417 issue the problem is a time we spent on issue creation itself. As we introduced a lot of performance improvements + I don't like the idea with ephemeral responses, let's deploy feature with 9.4 release and then test it on gitlab.com directly. It should not be hard with a flipper.

TODO

Hide everything when not gitlab.com
Handling Slack errors when user has not allowed the access with OAuth2
Active flag should be respected
TODOs in the code
/gitlab help does not work
Think of UX. Maybe we can improve something...
Specs
Make a separate HTTP query to get username
Ability to disable/enable the feature
Security: Check team_id and alias substitution
Create a documentation page
Removing a team from the integration
Save button is not needed anymore

@vsizov thanks, sounds good to me!

@vsizov : This is amazing! If we can get what you showed in the video into prod I think that would be a great first release. I few points:

Why do you have to authorize yourself again with the Chat accounts page? Is that something based on how we built GitLab? Definitely not in scope to clean this up for this issue. But is that something we can remove in the future? (i.e. skip that entire step.)
I think the UX is great actually for this issue. I don't want to increase scope much more. Maybe clean up the settings page a little bit with the text copy. But I do not want to introduce any additional flows or screens. This is great already!
It's not a big deal if /gitlab help doesn't work yet. Let's put that as a low priority, and we can just say we don't support it as a first release if we don't have time for it.
For the active checkbox in the UI. Let's do the simplest thing. What's the easiest? I'm thinking we should at least allow the user to add the integration and also remove (or just pause) the integration. Whatever is easiest.
You've showed the GitLab --> Slack flow. For the Slack --> GitLab flow (similar to https://drive.google.com/file/d/0B5tDlHAM4iZITEhiVUdoU3p6d1k/view), is there a lot more work to make that happen with the additional screens? If we can launch this without this flow, I think that is totally fine. Let us know what's possible.

@pedroms @sarrahvesselov : I don't think we have capacity to make the flows perfectly smooth here. But please take a look and give any feedback that we can use here or in future issues.

@vsizov made an assume video already: https://gitlab.com/gitlab-org/gitlab-ee/issues/2629#note_33409700.

mentioned in merge request !2259 (merged)

changed the description

@victorwu

Why do you have to authorize yourself again with the Chat accounts page?

Sounds good. I've put it to TODOs for further iterations

It's not a big deal if /gitlab help doesn't work yet

It's already fixed.

For the active checkbox in the UI. Let's do the simplest thing. What's the easiest?

I got your point. Will do whatever is easiest. Thanks.

You've showed the GitLab --> Slack flow. For the Slack --> GitLab flow

I have not investigated that yet, but I've put it to TODOs for further iterations which means that I would really want to see it in 9.4 but in a separate MR. It should be a matter of creating a one page where you chose a project to enable integration for (theoretically).

Perfect thanks @vsizov

The video was awesome, thanks @vsizov for putting that together.

I think the UX is great actually for this issue. I don't want to increase scope much more. Maybe clean up the settings page a little bit with the text copy. But I do not want to introduce any additional flows or screens. This is great already!

I agree with @victorwu here on the point above as well as on the re-authorization on the chat accounts page.

I took a screenshot of the settings page. The Active checkbox seems out of place. I looked at other integrations setting's pages and see that this is consistent across all the screens. I will open a separate issue to look at how we are handling this in the UI. It looks haphazard and unpolished in it's current state.

I removed "active" box and now we calculate active field based on existence of added teams

One note, I can add a support for several teams per project pretty easily, just let me know if we need that /cc @victorwu @smcgivern

Thanks @vsizov . Let's do that in a future iteration. Thanks!

@vsizov thanks for recording the video, that really helped understand the flow

Iterating over what @victorwu and @sarrahvesselov wrote:

Showing the authorize message in Slack and making the user go to GitLab to authorize his chat name seems a bit unnecessary. For the user, pressing “Add to Slack” in GitLab and then “Install” in Slack should've done the trick. It would be great if we could remove the whole “authorize ping-pong”.
The “Active” checkbox doesn't make much sense in this situation. If we remove it, how can the user disable/remove the integration? Is that done in the Slack?
To separate help text/instructions from actions and content, consider moving the “Add to Slack” button and the team/command table out of the gray well. Also, rephrase the instruction to read: “To setupset up this service click the following buttonpress Add to Slack”

The “Active” checkbox doesn't make much sense in this situation

It's already removed.

If we remove it, how can the user disable/remove the integration? Is that done in the Slack?

Please check out the TODO list https://gitlab.com/gitlab-org/gitlab-ee/issues/2629#note_33455571, we are going to add ability to remove a team from the integration. Of course, it's also possible from Slack side but the thing is that it should be possible to do from both sides :)

Thanks @vsizov . Let's use what you listed as the scope for this issue. https://gitlab.com/gitlab-org/gitlab-ee/issues/2629#note_33455571.

If we have more capacity to do improvements afterward, let's do that in a separate issue and merge request.

I'm pinging the Slack people in the Slack Slack channel as you see. Let's see if they can give any feedback to you. Thanks for all the awesome work!

changed the description

The service page now looks like this:

I left it in a help section yet. Will play with this tomorrow. Today I'm off (holiday)

@smcgivern Although it's not finished yet, could you take a quick look? https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2259

Public link of the same awesome video from https://gitlab.com/gitlab-org/gitlab-ee/issues/2629#note_33409700

https://drive.google.com/file/d/0B5tDlHAM4iZIX3c1VkZOV0VNTFU/view

OK, I eliminated that authorization step in the current iteration as it's not that hard. The only thing that I highlighted in the video (below) is that "Nickname" field in ChatName section is empty, this is because Slack does not provide that data during token exchange phase. I propose two options:

We put a placaholder "You" when the name of Slack user is not set.
We make one extra HTTP request to Slack to get that data (I prefer this).

The final video https://drive.google.com/file/d/0B5tAA38RRmPUZFI0ZGx1akpMWlE/view?usp=sharing (public for everyone)

UPDATE: The data Slack give us on exchange phase

=> {"ok"=>true,
 "access_token"=>"XXXXXX",
 "scope"=>"identify,commands",
 "user_id"=>"U5RA7AJXX",
 "team_name"=>"sizovf",
 "team_id"=>"T5R6LAXXX"}

only two of them are really documented

which sounds like they can disappear at any given point because of different Slack's technical decisions, this is one more point for making one more HTTP query.

@vsizov I am in favour of making the extra HTTP request Can we do that in the a background worker, or wouldn't that make sense?

Can we do that in the a background worker, or wouldn't that make sense?

We don't store a token so we would need to pass it to sidekiq job as an argument. I propose to start from realtime query then we can improve it.

@pedroms I applied the text changes you proposed after recording the video, sorry.

LOL, you can't request commands and identity.basic scopes at the same time! https://api.slack.com/docs/oauth-scopes#working_with_scopes

I ran into it when tried to get user's data with a separate HTTP query. OK, I'm going to leave this "Nickname" field empty for this MR and then resolve the problem separately somehow... in a separate MR.

UPDATE: I wrote about it to the slack-platform chat

changed the description

I updated TODOs for further iterations (from Valery)

I'm reading Slack's checklist for applications and I wanted to note few things:

The app should have a good landing page "My landing page contains a detailed summary of what my app does and how it integrates with Slack." This is essentially the same page that provides "Slack --> GitLab flow". So, although, it will be in the next iteration (MR) it should be done in the same release as we can't pass review process without this.
Quote: My landing page contains clear instructions on how to install and use my app (e.g. which parts of my app trigger messages in Slack, and which comments a bot responds to).
Prepare your customer support. We should create a customer support (separate channel) for this. This is interesting point because it seems like we just need to have some public channel for this and have our support team checking it as well... It's a bit unclear what they mean by "support channel", Slack channel or channel in general.

The app should have a good landing page "My landing page contains a detailed summary of what my app does and how it integrates with Slack." This is essentially the same page that provides "Slack --> GitLab flow". So, although, it will be in the next iteration (MR) it should be done in the same release as we can't pass review process without this.

@pedroms do you have time to put together some UX for this? If not, maybe @hazelyang has some bandwidth for this.

@victorwu @smcgivern I think that reviewing our app will take some time so we need to be able to enable new Slack app feature (for review process) but leave the old Slack integration working because it won't be available for other teams except us. Currently I made only one option that actually switches old/new implementation for GitLab.com so we will probably need to change that before release...

changed the description

@vsizov do you mean we should be able to change it by GitLab group? @rymai is working on something similar for the performance bar in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12362, but it's not super-simple.

Wdyt about hard-coding some group IDs until the app is approved? I think we should also check this with the Slack team.

@sarrahvesselov @pedroms Just let me know if I can help you.

@smcgivern I think we need to enable it for all, but only our team and Slack review team will be actually able to install it. This does not look super cool though.

UPDATE: I asked in the Slack https://slack-platform.slack.com/archives/C4B465268/p1499155345828900

I think we will need to hide Slack application service from the list and allow it to be installed from the directory only (while we're under review). In this case only we and Slack review team can actually test it.

@vsizov that makes sense! So we can add a feature flag to show / hide the Slack app in the integrations list, and then when it's hidden still allow installation from the directory?

@smcgivern Yes.

MR is under review https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2259!

@vsizov please let @hazelyang know if you are still in need of UX design for the landing page.

changed the description

There is the interesting feature in Slack, you can share your application between teams without actually putting it to the Slack directory. Which makes things even simpler for us. It means that we don't have to wait for the review process at all, we can start using it (I mean make it publicly available) right after internal tests. So the plan is following:

We release an RC version
We create GitLab application. It requires some images, description, links and so on. I created an issue https://gitlab.com/gitlab-org/gitlab-ce/issues/34719.
We put the Slack application data to ApplicationSettings of GitLab.com but not enable the Slack application feature.
Go to the project settings (any project) and trying to enable Slack App by using the direct link as it's hidden while it's disabled.
Test and fix at will :)
Enable the feature so every GitLab.com user can enjoy it.
Submit the application to the Slack directory. At this time we should have landing page ready! Issue https://gitlab.com/gitlab-org/gitlab-ce/issues/34720

The next iterations by priority(let me know if you don't agree):

Make it possible to edit project alias(https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2374)
Landing page
Change the text (hide the Slack button) when there is a team added. Or add a support for multiple teams.(https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2381)
Empty Nickname problem (issue - https://gitlab.com/gitlab-org/gitlab-ee/issues/2894)

Also, out of this list goes "Create Slack Application" as it should be done after we release an RC

The project alias wouldn't delay this, as I suppose this is pretty simple to do?

If so, I agree with the priorities.

@JobV All these items will be in separate MRs. The basic implementation is already done and should be merged soon https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2259

@sarrahvesselov @jschatz1

I'm thinking how to make project alias editabe, and I think that the best UX would be to render the "project alias" as an input field and if it's changed we could show a "Save" button. What do you think?

On the other hand, the most inexpensive solution would be to make a classic "Edit" button that follows to a separate page with a form.

I'm thinking how to make project alias editabe, and I think that the best UX would be to render the "project alias" as an input field and if it's changed we could show a "Save" button. What do you think?

I think this would be the optimal solution @vsizov. If simply adding the Edit gets us there faster, I think that is fine too.

WIP: Project alias for Slack application can be edited (working on specs)

Show Slack button when there is no team added

and trying to enable Slack App by using the direct link as it's hidden while it's disabled.

It seems like I broke this at some point. I need to fix that...

This is pretty important fix that should go to RC https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2392

@vsizov do you need my help to fix?

@jschatz1 No, thanks. I made all UI changes myself. I think they are OK.

@vsizov where is the MR itself that does this? This setting should not be in a .well.

@jschatz1 This one https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2374

and also I made some UI changes in https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2259

@jschatz1 I didn't ask the help because it was really unclear what we really needed. Thanks for the review.

mentioned in issue #2894

changed the description

mentioned in issue gitlab-com/infrastructure#2256 (closed)

changed milestone to %Next 2-3 months

changed milestone to %10.0

Closing this as the Slack app is now implemented on GitLab.com.

changed milestone to %9.4

closed

Slack slash commands using Slack app integration on GitLab.com

Resources

Description

Reference

Blockers

Scope for this issue

TODOs for further iterations (from Valery)

Designs

Child items ...

Activity

Flow 1 (GitLab project <-> Slack team)

Problems

Flow 2 (GitLab user <-> Slack user)

Problems

Admin message

Admin message

Slack slash commands using Slack app integration on GitLab.com

Resources

Description

Reference

Blockers

Scope for this issue

TODOs for further iterations (from Valery)

Activity

Flow 1 (GitLab project <-> Slack team)

Problems

Flow 2 (GitLab user <-> Slack user)

Problems