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

Add initial prototype of Slack integration #172

Open
wants to merge 1 commit into
base: stable
Choose a base branch
from

Conversation

Kobzol
Copy link
Collaborator

@Kobzol Kobzol commented Jan 11, 2020

This PR adds Slack integration:

  • After a task solution changes its status or after it receives a new comment, a Slack app will send a notification (a direct message) to the affected user on https://pyladiescz.slack.com.
  • The corresponding Slack user is looked up in the Slack by their e-mail, the users are cached locally in memory (not in MongoDB).
  • If no user is found or Slack tokens are not set, the integration will not do anything.
  • Coaches are looked up too, if they are found, they will be @mentioned in the Slack message so that you can send them a DM easily on Slack right from the notification.

Notes:

  • I had to get a session slug from a task solution to present a link in the Slack message to the user. I added a method to Course to find a session from a task id. It's not ideal, but as long as task IDs are unique across sessions in a single course, it should work.
  • Right now the Slack message is sent within the backend request that changes the task solution, therefore until the Slack message is sent, the request will not complete. I don't suppose that Slack servers would be often slow or offline and there is a 10 second timeout. But if you consider this a problem, I can instead spawn the Slack notification handling as a new async task so that it will not block the HTTP request.
  • The integration uses a very simple event system to lay a foundation for possible other future integrations.

TODO:

  • Specify Slack tokens and website URL in deployment configuration (the Slack is already prepared, so I can send the tokens to you privately)

@Kobzol
Copy link
Collaborator Author

Kobzol commented Jan 11, 2020

Screenshot of how it currently looks in Slack:
image

@frenzymadness
Copy link
Collaborator

Awesome idea! It'll add another motivation to join and use Slack!
One question. Some courses use international PyLadies Slack instead of PyLadiesCZ Slack. Does this integration need admin privileges? I mean, how complicated is it to enable this on Slack?

@Kobzol
Copy link
Collaborator Author

Kobzol commented Jan 12, 2020

I was able to activate it on PyladiesCZ from my account and I'm not an admin if I'm not mistaken. The only permissions I need is to lookup an user by e-mail and send a message to an user, and you can already do that if you are a member of the Slack, so I don't think that we require admin to add it. However, in this case we would need to set the Slack URL per course.

@Kobzol
Copy link
Collaborator Author

Kobzol commented Jan 16, 2020

@frenzymadness I installed the app on the international PyLadies Slack, it seems to work fine. Should I add support for multiple Slack workspaces? (I'm a bit sad that the courses are fragmented in two Slacks though 😞)

@Kobzol
Copy link
Collaborator Author

Kobzol commented Jan 20, 2020

It would be nice to notify coaches that some participant has submitted a solution. However, to avoid spam, this should probably be opt-in.

What about a subscribe button in a course/lesson? It could be used by coaches, and when active, these events would be then sent to their Slack account.

@frenzymadness
Copy link
Collaborator

Could we please finish this. Even without support for coaches, it is a good enhancement. We need it currently only for beginners course which uses PyLadies CZ slack.

@Kobzol
Copy link
Collaborator Author

Kobzol commented Feb 8, 2020

Sure. There are some pending questions, I think that @messa didn't have time to check this out yet. It should work as it is if you want to test it ASAP. I won't be able to code until the next weekend though, so if there are bugs I'll fix them next week.

@fivaldi
Copy link
Collaborator

fivaldi commented Mar 6, 2020

👍 On this Slack integration feature. We'd really welcome this in Olomouc for improving response between us and participants.
Is there anything needed for this PR to be reviewed? Feel free to ping @Kobzol or me @fivaldi, if so.

@frenzymadness
Copy link
Collaborator

ping!

@Kobzol
Copy link
Collaborator Author

Kobzol commented Jul 22, 2020

@messa could you please look at this before the next PyLadies course? Thanks :)

@frenzymadness
Copy link
Collaborator

I'd very like this to be merged. @messa what is blocking us?

@Kobzol
Copy link
Collaborator Author

Kobzol commented Sep 17, 2021

Rebased on top of master.

@fivaldi
Copy link
Collaborator

fivaldi commented Oct 14, 2021

@messa I'd like to highlight this PR... PyLadies Olomouc would really love this enhancement!

@Kobzol Kobzol changed the base branch from master to stable October 21, 2021 07:52
@Kobzol
Copy link
Collaborator Author

Kobzol commented Oct 21, 2021

Changed the PR base to stable.

@Kobzol Kobzol requested a review from messa October 21, 2021 07:53
@Kobzol Kobzol requested a review from brabemi November 2, 2021 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants