Skip to content

Commit

Permalink
Merge pull request rapidpro#489 from nyaruka/notifications
Browse files Browse the repository at this point in the history
📟 Notifications
  • Loading branch information
rowanseymour authored Sep 8, 2021
2 parents 560e149 + 76bdedd commit 0b67cac
Show file tree
Hide file tree
Showing 10 changed files with 307 additions and 10 deletions.
10 changes: 10 additions & 0 deletions core/handlers/ticket_opened_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,16 @@ func TestTicketOpened(t *testing.T) {
Args: []interface{}{testdata.Admin.ID},
Count: 1,
},
{ // admin will have a ticket assigned notification for the ticket directly assigned to them
SQL: "select count(*) from notifications_notification where user_id = $1 and notification_type = 'tickets:activity'",
Args: []interface{}{testdata.Admin.ID},
Count: 1,
},
{ // all assignable users will have a ticket opened notification for the unassigned ticket
SQL: "select count(*) from notifications_notification where notification_type = 'tickets:opened'",
Args: nil,
Count: 3,
},
},
},
}
Expand Down
11 changes: 10 additions & 1 deletion core/hooks/insert_tickets.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,11 @@ func (h *insertTicketsHook) Apply(ctx context.Context, tx *sqlx.Tx, rp *redis.Po

// generate opened events for each ticket
openEvents := make([]*models.TicketEvent, len(tickets))
eventsByTicket := make(map[*models.Ticket]*models.TicketEvent, len(tickets))
for i, ticket := range tickets {
openEvents[i] = models.NewTicketOpenedEvent(ticket, models.NilUserID, ticket.AssigneeID())
evt := models.NewTicketOpenedEvent(ticket, models.NilUserID, ticket.AssigneeID())
openEvents[i] = evt
eventsByTicket[ticket] = evt
}

// and insert those too
Expand All @@ -44,5 +47,11 @@ func (h *insertTicketsHook) Apply(ctx context.Context, tx *sqlx.Tx, rp *redis.Po
return errors.Wrapf(err, "error inserting ticket opened events")
}

// and insert logs/notifications for those
err = models.NotificationsFromTicketEvents(ctx, tx, oa, eventsByTicket)
if err != nil {
return errors.Wrapf(err, "error inserting notifications")
}

return nil
}
9 changes: 8 additions & 1 deletion core/models/imports.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package models

import (
"context"
"database/sql/driver"
"encoding/json"
"fmt"
"strings"
Expand All @@ -14,13 +15,19 @@ import (
"github.com/nyaruka/goflow/envs"
"github.com/nyaruka/goflow/flows"
"github.com/nyaruka/goflow/flows/modifiers"
"github.com/nyaruka/null"
"github.com/pkg/errors"

"github.com/jmoiron/sqlx"
)

// ContactImportID is the type for contact import IDs
type ContactImportID int64
type ContactImportID null.Int

func (i ContactImportID) MarshalJSON() ([]byte, error) { return null.Int(i).MarshalJSON() }
func (i *ContactImportID) UnmarshalJSON(b []byte) error { return null.UnmarshalInt(b, (*null.Int)(i)) }
func (i ContactImportID) Value() (driver.Value, error) { return null.Int(i).Value() }
func (i *ContactImportID) Scan(value interface{}) error { return null.ScanInt(value, (*null.Int)(i)) }

// ContactImportBatchID is the type for contact import batch IDs
type ContactImportBatchID int64
Expand Down
117 changes: 117 additions & 0 deletions core/models/notifications.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
package models

import (
"context"
"time"

"github.com/nyaruka/mailroom/utils/dbutil"
"github.com/pkg/errors"
)

// NotificationID is our type for notification ids
type NotificationID int

type NotificationType string

const (
NotificationTypeChannelAlert NotificationType = "channel:alert"
NotificationTypeExportFinished NotificationType = "export:finished"
NotificationTypeImportFinished NotificationType = "import:finished"
NotificationTypeTicketsOpened NotificationType = "tickets:opened"
NotificationTypeTicketsActivity NotificationType = "tickets:activity"
)

type Notification struct {
ID NotificationID `db:"id"`
OrgID OrgID `db:"org_id"`
Type NotificationType `db:"notification_type"`
Scope string `db:"scope"`
UserID UserID `db:"user_id"`
IsSeen bool `db:"is_seen"`
CreatedOn time.Time `db:"created_on"`

ChannelID ChannelID `db:"channel_id"`
ContactImportID ContactImportID `db:"contact_import_id"`
}

var ticketAssignableToles = []UserRole{UserRoleAdministrator, UserRoleEditor, UserRoleAgent}

// NotificationsFromTicketEvents logs the opening of new tickets and notifies all assignable users if tickets is not already assigned
func NotificationsFromTicketEvents(ctx context.Context, db Queryer, oa *OrgAssets, events map[*Ticket]*TicketEvent) error {
notifyTicketsOpened := make(map[UserID]bool)
notifyTicketsActivity := make(map[UserID]bool)

for ticket, evt := range events {
switch evt.EventType() {
case TicketEventTypeOpened:
// if ticket is unassigned notify all possible assignees
if evt.AssigneeID() == NilUserID {
for _, u := range oa.users {
user := u.(*User)

if hasAnyRole(user, ticketAssignableToles) && evt.CreatedByID() != user.ID() {
notifyTicketsOpened[user.ID()] = true
}
}
} else if evt.AssigneeID() != evt.CreatedByID() {
notifyTicketsActivity[evt.AssigneeID()] = true
}
case TicketEventTypeAssigned:
// notify new ticket assignee if they didn't self-assign
if evt.AssigneeID() != NilUserID && evt.AssigneeID() != evt.CreatedByID() {
notifyTicketsActivity[evt.AssigneeID()] = true
}
case TicketEventTypeNoteAdded:
// notify ticket assignee if they didn't add note themselves
if ticket.AssigneeID() != NilUserID && ticket.AssigneeID() != evt.CreatedByID() {
notifyTicketsActivity[ticket.AssigneeID()] = true
}
}
}

notifications := make([]*Notification, 0, len(events))

for userID := range notifyTicketsOpened {
notifications = append(notifications, &Notification{
OrgID: oa.OrgID(),
Type: NotificationTypeTicketsOpened,
Scope: "",
UserID: userID,
})
}

for userID := range notifyTicketsActivity {
notifications = append(notifications, &Notification{
OrgID: oa.OrgID(),
Type: NotificationTypeTicketsActivity,
Scope: "",
UserID: userID,
})
}

return insertNotifications(ctx, db, notifications)
}

const insertNotificationSQL = `
INSERT INTO notifications_notification(org_id, notification_type, scope, user_id, is_seen, created_on, channel_id, contact_import_id)
VALUES(:org_id, :notification_type, :scope, :user_id, FALSE, NOW(), :channel_id, :contact_import_id)
ON CONFLICT DO NOTHING`

func insertNotifications(ctx context.Context, db Queryer, notifications []*Notification) error {
is := make([]interface{}, len(notifications))
for i := range notifications {
is[i] = notifications[i]
}

err := dbutil.BulkQuery(ctx, db, insertNotificationSQL, is)
return errors.Wrap(err, "error inserting notifications")
}

func hasAnyRole(user *User, roles []UserRole) bool {
for _, r := range roles {
if user.Role() == r {
return true
}
}
return false
}
136 changes: 136 additions & 0 deletions core/models/notifications_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
package models_test

import (
"context"
"testing"
"time"

"github.com/jmoiron/sqlx"
"github.com/nyaruka/mailroom/core/models"
"github.com/nyaruka/mailroom/testsuite"
"github.com/nyaruka/mailroom/testsuite/testdata"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestTicketNotifications(t *testing.T) {
ctx, _, db, _ := testsuite.Get()

defer deleteTickets(db)

oa, err := models.GetOrgAssets(ctx, db, testdata.Org1.ID)
require.NoError(t, err)

t0 := time.Now()

// open unassigned tickets by a flow (i.e. no user)
ticket1, openedEvent1 := openTicket(t, ctx, db, nil, nil)
ticket2, openedEvent2 := openTicket(t, ctx, db, nil, nil)
err = models.NotificationsFromTicketEvents(ctx, db, oa, map[*models.Ticket]*models.TicketEvent{ticket1: openedEvent1, ticket2: openedEvent2})
require.NoError(t, err)

// check that all assignable users are notified once
assertNotifications(t, ctx, db, t0, map[*testdata.User][]models.NotificationType{
testdata.Admin: {models.NotificationTypeTicketsOpened},
testdata.Editor: {models.NotificationTypeTicketsOpened},
testdata.Agent: {models.NotificationTypeTicketsOpened},
})

t1 := time.Now()

// another ticket opened won't create new notifications
ticket3, openedEvent3 := openTicket(t, ctx, db, nil, nil)
err = models.NotificationsFromTicketEvents(ctx, db, oa, map[*models.Ticket]*models.TicketEvent{ticket3: openedEvent3})
require.NoError(t, err)

assertNotifications(t, ctx, db, t1, map[*testdata.User][]models.NotificationType{})

// mark all notifications as seen
db.MustExec(`UPDATE notifications_notification SET is_seen = TRUE`)

// open an unassigned ticket by a user
ticket4, openedEvent4 := openTicket(t, ctx, db, testdata.Editor, nil)
err = models.NotificationsFromTicketEvents(ctx, db, oa, map[*models.Ticket]*models.TicketEvent{ticket4: openedEvent4})
require.NoError(t, err)

// check that all assignable users are notified except the user that opened the ticket
assertNotifications(t, ctx, db, t1, map[*testdata.User][]models.NotificationType{
testdata.Admin: {models.NotificationTypeTicketsOpened},
testdata.Agent: {models.NotificationTypeTicketsOpened},
})

t2 := time.Now()
db.MustExec(`UPDATE notifications_notification SET is_seen = TRUE`)

// open an already assigned ticket
ticket5, openedEvent5 := openTicket(t, ctx, db, nil, testdata.Agent)
err = models.NotificationsFromTicketEvents(ctx, db, oa, map[*models.Ticket]*models.TicketEvent{ticket5: openedEvent5})
require.NoError(t, err)

// check that the assigned user gets a ticket activity notification
assertNotifications(t, ctx, db, t2, map[*testdata.User][]models.NotificationType{
testdata.Agent: {models.NotificationTypeTicketsActivity},
})

t3 := time.Now()

// however if a user opens a ticket which is assigned to themselves, no notification
ticket6, openedEvent6 := openTicket(t, ctx, db, testdata.Admin, testdata.Admin)
err = models.NotificationsFromTicketEvents(ctx, db, oa, map[*models.Ticket]*models.TicketEvent{ticket6: openedEvent6})
require.NoError(t, err)

// check that the assigned user gets a ticket activity notification
assertNotifications(t, ctx, db, t3, map[*testdata.User][]models.NotificationType{})

t4 := time.Now()
db.MustExec(`UPDATE notifications_notification SET is_seen = TRUE`)

// now have a user assign existing tickets to another user
_, err = models.TicketsAssign(ctx, db, oa, testdata.Admin.ID, []*models.Ticket{ticket1, ticket2}, testdata.Agent.ID, "")
require.NoError(t, err)

// check that the assigned user gets a ticket activity notification
assertNotifications(t, ctx, db, t4, map[*testdata.User][]models.NotificationType{
testdata.Agent: {models.NotificationTypeTicketsActivity},
})

t5 := time.Now()
db.MustExec(`UPDATE notifications_notification SET is_seen = TRUE`)

// and finally a user assigning a ticket to themselves
_, err = models.TicketsAssign(ctx, db, oa, testdata.Editor.ID, []*models.Ticket{ticket3}, testdata.Editor.ID, "")
require.NoError(t, err)

// no notifications for self-assignment
assertNotifications(t, ctx, db, t5, map[*testdata.User][]models.NotificationType{})
}

func assertNotifications(t *testing.T, ctx context.Context, db *sqlx.DB, after time.Time, expected map[*testdata.User][]models.NotificationType) {
// check last log
var notifications []*models.Notification
err := db.SelectContext(ctx, &notifications, `SELECT id, org_id, notification_type, scope, user_id, is_seen, created_on FROM notifications_notification WHERE created_on > $1 ORDER BY id`, after)
require.NoError(t, err)

expectedByID := map[models.UserID][]models.NotificationType{}
for user, notificationTypes := range expected {
expectedByID[user.ID] = notificationTypes
}

actual := map[models.UserID][]models.NotificationType{}
for _, notification := range notifications {
actual[notification.UserID] = append(actual[notification.UserID], notification.Type)
}

assert.Equal(t, expectedByID, actual)
}

func openTicket(t *testing.T, ctx context.Context, db *sqlx.DB, openedBy *testdata.User, assignee *testdata.User) (*models.Ticket, *models.TicketEvent) {
ticket := testdata.InsertOpenTicket(db, testdata.Org1, testdata.Cathy, testdata.Internal, testdata.SupportTopic, "", "Where my pants", "", assignee)
modelTicket := ticket.Load(db)

openedEvent := models.NewTicketOpenedEvent(modelTicket, openedBy.SafeID(), assignee.SafeID())
err := models.InsertTicketEvents(ctx, db, []*models.TicketEvent{openedEvent})
require.NoError(t, err)

return modelTicket, openedEvent
}
14 changes: 12 additions & 2 deletions core/models/tickets.go
Original file line number Diff line number Diff line change
Expand Up @@ -420,12 +420,17 @@ func TicketsAssign(ctx context.Context, db Queryer, oa *OrgAssets, userID UserID
// mark the tickets as assigned in the db
err := Exec(ctx, "assign tickets", db, ticketsAssignSQL, pq.Array(ids), assigneeID, now)
if err != nil {
return nil, errors.Wrapf(err, "error updating tickets")
return nil, errors.Wrap(err, "error updating tickets")
}

err = InsertTicketEvents(ctx, db, events)
if err != nil {
return nil, errors.Wrapf(err, "error inserting ticket events")
return nil, errors.Wrap(err, "error inserting ticket events")
}

err = NotificationsFromTicketEvents(ctx, db, oa, eventsByTicket)
if err != nil {
return nil, errors.Wrap(err, "error inserting notifications")
}

return eventsByTicket, nil
Expand All @@ -452,6 +457,11 @@ func TicketsAddNote(ctx context.Context, db Queryer, oa *OrgAssets, userID UserI
return nil, errors.Wrapf(err, "error inserting ticket events")
}

err = NotificationsFromTicketEvents(ctx, db, oa, eventsByTicket)
if err != nil {
return nil, errors.Wrap(err, "error inserting notifications")
}

return eventsByTicket, nil
}

Expand Down
7 changes: 6 additions & 1 deletion core/models/tickets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,8 @@ func TestTicketsAssign(t *testing.T) {

// and there are new assigned events
testsuite.AssertQuery(t, db, `SELECT count(*) FROM tickets_ticketevent WHERE event_type = 'A' AND note = 'please handle these'`).Returns(2)

testsuite.AssertQuery(t, db, `SELECT count(*) FROM notifications_notification WHERE user_id = $1 AND notification_type = 'tickets:activity'`, testdata.Agent.ID).Returns(1)
}

func TestTicketsAddNote(t *testing.T) {
Expand All @@ -222,7 +224,7 @@ func TestTicketsAddNote(t *testing.T) {
ticket1 := testdata.InsertClosedTicket(db, testdata.Org1, testdata.Cathy, testdata.Mailgun, testdata.DefaultTopic, "Problem", "Where my shoes", "123", nil)
modelTicket1 := ticket1.Load(db)

ticket2 := testdata.InsertOpenTicket(db, testdata.Org1, testdata.Cathy, testdata.Zendesk, testdata.DefaultTopic, "Old Problem", "Where my pants", "234", nil)
ticket2 := testdata.InsertOpenTicket(db, testdata.Org1, testdata.Cathy, testdata.Zendesk, testdata.DefaultTopic, "Old Problem", "Where my pants", "234", testdata.Agent)
modelTicket2 := ticket2.Load(db)

testdata.InsertOpenTicket(db, testdata.Org1, testdata.Cathy, testdata.Mailgun, testdata.DefaultTopic, "Ignore", "", "", nil)
Expand All @@ -235,6 +237,8 @@ func TestTicketsAddNote(t *testing.T) {

// check there are new note events
testsuite.AssertQuery(t, db, `SELECT count(*) FROM tickets_ticketevent WHERE event_type = 'N' AND note = 'spam'`).Returns(2)

testsuite.AssertQuery(t, db, `SELECT count(*) FROM notifications_notification WHERE user_id = $1 AND notification_type = 'tickets:activity'`, testdata.Agent.ID).Returns(1)
}

func TestTicketsChangeTopic(t *testing.T) {
Expand Down Expand Up @@ -370,6 +374,7 @@ func TestReopenTickets(t *testing.T) {
}

func deleteTickets(db *sqlx.DB) {
db.MustExec(`DELETE FROM notifications_notification`)
db.MustExec(`DELETE FROM request_logs_httplog`)
db.MustExec(`DELETE FROM tickets_ticketevent`)
db.MustExec(`DELETE FROM tickets_ticket`)
Expand Down
Binary file modified mailroom_test.dump
Binary file not shown.
Loading

0 comments on commit 0b67cac

Please sign in to comment.