diff --git a/core/handlers/ticket_opened_test.go b/core/handlers/ticket_opened_test.go index 252e78ee6..51fa7c50d 100644 --- a/core/handlers/ticket_opened_test.go +++ b/core/handlers/ticket_opened_test.go @@ -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, + }, }, }, } diff --git a/core/hooks/insert_tickets.go b/core/hooks/insert_tickets.go index b0c009c64..4860aeaf4 100644 --- a/core/hooks/insert_tickets.go +++ b/core/hooks/insert_tickets.go @@ -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 @@ -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 } diff --git a/core/models/imports.go b/core/models/imports.go index bd91d3a66..0f6aab119 100644 --- a/core/models/imports.go +++ b/core/models/imports.go @@ -2,6 +2,7 @@ package models import ( "context" + "database/sql/driver" "encoding/json" "fmt" "strings" @@ -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 diff --git a/core/models/notifications.go b/core/models/notifications.go new file mode 100644 index 000000000..1e2f27589 --- /dev/null +++ b/core/models/notifications.go @@ -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 +} diff --git a/core/models/notifications_test.go b/core/models/notifications_test.go new file mode 100644 index 000000000..fe333c0f8 --- /dev/null +++ b/core/models/notifications_test.go @@ -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, ¬ifications, `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 +} diff --git a/core/models/tickets.go b/core/models/tickets.go index 4f787d88f..420f04c5e 100644 --- a/core/models/tickets.go +++ b/core/models/tickets.go @@ -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 @@ -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 } diff --git a/core/models/tickets_test.go b/core/models/tickets_test.go index 2a662183b..f3ae0fbb3 100644 --- a/core/models/tickets_test.go +++ b/core/models/tickets_test.go @@ -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) { @@ -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) @@ -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) { @@ -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`) diff --git a/mailroom_test.dump b/mailroom_test.dump index 2ee9b53cf..0b203ddbc 100644 Binary files a/mailroom_test.dump and b/mailroom_test.dump differ diff --git a/testsuite/testdata/constants.go b/testsuite/testdata/constants.go index 1813dee87..6b5bcfc24 100644 --- a/testsuite/testdata/constants.go +++ b/testsuite/testdata/constants.go @@ -18,6 +18,13 @@ type User struct { Email string } +func (u *User) SafeID() models.UserID { + if u != nil { + return u.ID + } + return models.NilUserID +} + type Classifier struct { ID models.ClassifierID UUID assets.ClassifierUUID diff --git a/testsuite/testdata/tickets.go b/testsuite/testdata/tickets.go index f418665c0..76a2c1ae2 100644 --- a/testsuite/testdata/tickets.go +++ b/testsuite/testdata/tickets.go @@ -51,15 +51,11 @@ func insertTicket(db *sqlx.DB, org *Org, contact *Contact, ticketer *Ticketer, s t := dates.Now() closedOn = &t } - assigneeID := models.NilUserID - if assignee != nil { - assigneeID = assignee.ID - } var id models.TicketID must(db.Get(&id, `INSERT INTO tickets_ticket(uuid, org_id, contact_id, ticketer_id, status, topic_id, subject, body, external_id, opened_on, modified_on, closed_on, last_activity_on, assignee_id) - VALUES($1, $2, $3, $4, $5, $6, $7, $8, $9, NOW(), NOW(), $10, NOW(), $11) RETURNING id`, uuid, org.ID, contact.ID, ticketer.ID, status, topic.ID, subject, body, externalID, closedOn, assigneeID, + VALUES($1, $2, $3, $4, $5, $6, $7, $8, $9, NOW(), NOW(), $10, NOW(), $11) RETURNING id`, uuid, org.ID, contact.ID, ticketer.ID, status, topic.ID, subject, body, externalID, closedOn, assignee.SafeID(), )) return &Ticket{id, uuid} }