Add notifications for task events

[joshsmith]

Problem

We should generate notifications for users on tasks for the following events:

• task description changed
• PR was opened for the task
• comment was added
• comment was edited

These notifications may need us to first have the notion of an event model. See how GitHub does this.

We may also need to distinguish watching notifications from participating notifications. See again how GitHub handles this, and we should look into, e.g., how Trello or Asana handle these.

[begedin]

Some notes

Marking "Notification/Event" as "read"

We talked about this being problematic on github in the past. As soon as a notification is opened, it's marked as "read" by github.

This is really decided on the client, but since the initial discussion is here, it probably should be mentioned here.

My proposal would be to have an explicit "mark read" button. Really, I can't think of another automatic way to do it other than what github does, that would still be intuitive enough.

Improvements to display of notifications

On the Github notifications page, all I see is an icon (indicating open/closed/merged status) with a title linking to an issue or a pull request.

I find it would be more useful to have a list telling me to a degree what actually happened in that issue/pull request.

My proposal is to list these notifications as, for example,

• %{user} commented on %{task}
• %{user} edited their comment on %{task}
• %{user} changed status of %{task} to %{status}
• etc.

Event and Notification model

I don't really think they should be one and the same.

In addition to that, I believe it would be more flexible to have separate TaskEvent, ProjectEvent and Notification models.

A notification is sort of like an event, but

• it needs to have a "read/unread" status
• is tied to a user
• is likely to have multiple ways to send it's content to this user

A TaskEvent or a ProjectEvent on the other hand is not tied to a specific user.

I believe they aren't necessarily built on top of each other (though I can imagine a Notification might end up being created as a consequence of an event being created). I think it may be more flexible to have them built in parallel, from the same source (Task in this case).

This umbrella/discussion issue could deal with Notification models and they could be implemented separately from events.

Proposed general implementation

As a Task is edited from codecorps, the task service module could have a step which generates notifications. The process could be something like

# on update from codecorps
task |> update |> find_participating_users |> generate_notifications

# on create from code_corps
task |> create |> find_participating_users |> generate_notifications

# on github issue created
issue_payload |> sync_to_tasks |> Enum.each(fn task -> task |> find_participating_users |> generate_notifications end)

# etc.

Later, when we have events to, we could easily plug them in along the lines of

with task <- task |> update do
  task |> find_participating_users |> generate_notifications
  task |> create_update_event
end

Parts of this feature that need implementation

Notification model ~2 hours

• Notification model (we should discuss field names here)
  • represents an :action which is probably an atom or a string
  • the :action is performed by :source, which is a User
  • has a :recipient which is also a User
  • has a :task, this action was performed in
  • maybe has a :comment in some cases.

Not sure about notification always happening on a :task. We may need a :project relationship to, for some notifications which aren't specific to a :task.

There's also the case of events such as "task was moved from list a to list b", etc.

We need to think hard which events we may want to support in the future, but I don't think we will able to build this so well that future expansions of the schema won't be necessary.

A UserFinder module ~3-4 hours

This is a module that takes in a Task and finds users who should get notifications. We need to discuss who these are. Some options:

• project users
• organization users
• task author
• all comment authors
• (not implemented yet) task watchers
• (not implemented yet) users who caused events to be created for the task

Later on, it may need a method that instead receives a Project and does the same, but that depends.

We may also have to implement this in a more explicit way, where we specify which type of participating users to find.

We need to figure out what our initial criteria is and start with that. Whatever it is, once we agree, it should not take long to implement.

Handling individual cases for notification creation

When a task gets created ~0-2 hours

I'm not sure notifications are warranted here, in our current model, unless all project users should get them, in this case.

When a task gets updated ~2 hours

This one is a bit clearer. Task author and comment authors should be notified at the very least. Anything more depends on further discussion.

This includes opening and closing from CodeCorps, as well as status changes rom CodeCorps, since we treat them all the same.

When a Comment is created ~2 hours

Probably affects the same users as a Task update.

When a Comment is edited ~ 2 hours

Probably affects the same users as a Task update

When a TaskUser, TaskSkill, etc. is created/deleted ~2 hours

I would not initially consider this MVP, as we can have something usable before we tackle these cases. However, it is probably something users would expect, so it will have to be done soon after MPV.

When a Task is created via a GitHub webhook ~2 hours

Again, similar to task creation from CodeCorps, I'm not sure we need to create notifications here, unless we notify project users/ organization users. There is the additional case where a task can be created as part of an IssueComment webhook, so at the very least, we may have to find participating users in that case.

When a Task is updated via a GitHub webhook ~2 hours

Similar to task update from CodeCorps. Initially participating users are the Task author and any Comment authors (a Task could be updated as part of a comment being created)

When a Comment is created via a GitHub webhook ~2 hours

Similar to Comment creation from CodeCorps.

When a Comment is updated via a GitHub webhook ~2 hours

Similar to Comment update from CodeCorps.

Implementation notes

Presumably, in most of these cases, except maybe those caused by a sync from GitHub, the user causing the notifications to get created should not be notified.

I think in that case, instead of having the UserFinder module deal with this internally, it may be more flexible to exclude that one user from the module doing the creation.

Something like task |> UserFinder.find_participating_users |> exclude(user) |> generate_nofitications

However, this is just a note and should not be taken as something we need to do. In this specific example, it would be just as easy to exclude from within UserFinder, since the Task being passed in has a user_id.