Refactor representations of messages and users

README.md

@@ -52,13 +52,13 @@ Creates a new incoming message from the client:
 }
 ```
 
-### `ack_msg`
+### `ack_chat`
 
-Acknowledges receipt an outgoing message to the client:
+Acknowledges receipt an outgoing chat message to the client:
 
 ```json
 {
-    "type": "ack_msg",
+    "type": "ack_chat",
     "msg_id": 46363452
 }
 ```
@@ -96,7 +96,6 @@ A chat session for a new contact has been successfully started:
 ```json
 {
     "type": "chat_started",
-    "time": "2024-05-01T17:15:30.123456Z",
     "chat_id": "65vbbDAQCdPdEWlEhDGy4utO"
 }
 ```
@@ -108,34 +107,37 @@ A chat session for an existing contact has been successfully resumed:
 ```json
 {
     "type": "chat_resumed",
-    "time": "2024-05-01T17:15:30.123456Z",
     "chat_id": "65vbbDAQCdPdEWlEhDGy4utO",
     "email": "[email protected]"
 }
 ```
 
-### `msg_out`
+### `chat_out`
 
-A new outgoing message has been created and should be displayed in the client:
+A new outgoing chat event has been created and should be displayed. Thus far `msg_out` is the only type sent.
 
 ```json
 {
-    "type": "msg_out",
-    "time": "2024-05-01T17:15:30.123456Z",
-    "msg_id": 34634,
-    "text": "Thanks for contacting us!",
-    "origin": "flow"
+    "type": "chat_out",
+    "msg_out": {
+        "id": 34634,
+        "text": "Thanks for contacting us!",
+        "origin": "flow",
+        "time": "2024-05-01T17:15:30.123456Z"
+    }
 }
 ```
 
 ```json
 {
-    "type": "msg_out",
-    "time": "2024-05-01T17:15:30.123456Z",
-    "msg_id": 34634,
-    "text": "How can we help?",
-    "origin": "chat",
-    "user": {"name": "Bob McTickets", "email": "[email protected]"}
+    "type": "chat_out",
+    "msg_out": {
+        "id": 34634,
+        "text": "Thanks for contacting us!",
+        "origin": "chat",
+        "user": {"id": 234, "name": "Bob McTickets", "email": "[email protected]", "avatar": "https://example.com/bob.jpg"},
+        "time": "2024-05-01T17:15:30.123456Z"
+    }
 }
 ```
 
@@ -146,20 +148,22 @@ The client previously requested history with a `get_history` command:
 ```json
 {
     "type": "history",
-    "time": "2024-05-01T17:15:30.123456Z",
     "history": [
-        {
-            "type": "msg_in",
-            "time": "2024-04-01T13:15:30.123456Z",
-            "msg_id": 34632,
-            "text": "I need help!"
+        { 
+            "msg_in": {
+                "id": 34632,
+                "text": "I need help!",
+                "time": "2024-04-01T13:15:30.123456Z"
+            }
         },
         {
-            "type": "msg_out",
-            "time": "2024-04-01T13:15:30.123456Z",
-            "msg_id": 34634,
-            "text": "Thanks for contacting us!",
-            "origin": "flow"
+            "msg_out": {
+                "id": 34634,
+                "text": "Thanks for contacting us!",
+                "origin": "chat",
+                "user": {"id": 234, "name": "Bob McTickets", "email": "[email protected]", "avatar": "https://example.com/bob.jpg"},
+                "time": "2024-04-01T13:15:30.123456Z"
+            }
         }
     ]
 }

core/models/msg.go

@@ -22,33 +22,34 @@ const (
 	MsgOriginTicket    MsgOrigin = "ticket"
 	MsgOriginChat      MsgOrigin = "chat"
 
-	MsgStatusSent MsgStatus = "sent"
-
 	DirectionIn  MsgDirection = "I"
 	DirectionOut MsgDirection = "O"
 )
 
-type MsgOut struct {
-	ID          MsgID       `json:"id"`
-	ChannelUUID ChannelUUID `json:"channel_uuid"`
-	ChatID      ChatID      `json:"chat_id"`
-	Text        string      `json:"text"`
-	Attachments []string    `json:"attachments,omitempty"`
-	Origin      MsgOrigin   `json:"origin"`
-	UserID      UserID      `json:"user_id,omitempty"`
-	Time        time.Time   `json:"time"`
+type MsgIn struct {
+	ID   MsgID     `json:"id"`
+	Text string    `json:"text"`
+	Time time.Time `json:"time"`
 }
 
-func NewMsgOut(id MsgID, ch *Channel, chatID ChatID, text string, attachments []string, origin MsgOrigin, u *User, t time.Time) *MsgOut {
-	var userID UserID
-	if u != nil {
-		userID = u.ID
-	}
+func NewMsgIn(id MsgID, text string, t time.Time) *MsgIn {
+	return &MsgIn{ID: id, Text: text, Time: t}
+}
 
-	return &MsgOut{ID: id, ChannelUUID: ch.UUID, ChatID: chatID, Text: text, Origin: origin, UserID: userID, Time: t}
+type MsgOut struct {
+	ID          MsgID     `json:"id"`
+	Text        string    `json:"text"`
+	Attachments []string  `json:"attachments,omitempty"`
+	Origin      MsgOrigin `json:"origin"`
+	User        *User     `json:"user,omitempty"`
+	Time        time.Time `json:"time"`
 }
 
-type Msg struct {
+func NewMsgOut(id MsgID, text string, attachments []string, origin MsgOrigin, user *User, t time.Time) *MsgOut {
+	return &MsgOut{ID: id, Text: text, Attachments: attachments, Origin: origin, User: user, Time: t}
+}
+
+type DBMsg struct {
 	ID          MsgID        `json:"id"`
 	Text        string       `json:"text"`
 	Attachments []string     `json:"attachments"`
@@ -60,7 +61,32 @@ type Msg struct {
 	CreatedOn   time.Time    `json:"created_on"`
 }
 
-func (m *Msg) Origin() MsgOrigin {
+func (m *DBMsg) ToMsgIn() *MsgIn {
+	if m.Direction != DirectionIn {
+		panic("can only be called on an inbound message")
+	}
+
+	return NewMsgIn(m.ID, m.Text, m.CreatedOn)
+}
+
+func (m *DBMsg) ToMsgOut(ctx context.Context, store Store) (*MsgOut, error) {
+	if m.Direction != DirectionOut {
+		panic("can only be called on an outbound message")
+	}
+
+	var user *User
+	var err error
+	if m.CreatedByID != NilUserID {
+		user, err = store.GetUser(ctx, m.CreatedByID)
+		if err != nil {
+			return nil, fmt.Errorf("error fetching user: %w", err)
+		}
+	}
+
+	return NewMsgOut(m.ID, m.Text, m.Attachments, m.origin(), user, m.CreatedOn), nil
+}
+
+func (m *DBMsg) origin() MsgOrigin {
 	if m.FlowID != NilFlowID {
 		return MsgOriginFlow
 	} else if m.BroadcastID != NilBroadcastID {
@@ -80,17 +106,17 @@ SELECT row_to_json(r) FROM (
      LIMIT $3
 ) r`
 
-func LoadContactMessages(ctx context.Context, rt *runtime.Runtime, contactID ContactID, before time.Time, limit int) ([]*Msg, error) {
+func LoadContactMessages(ctx context.Context, rt *runtime.Runtime, contactID ContactID, before time.Time, limit int) ([]*DBMsg, error) {
 	rows, err := rt.DB.QueryContext(ctx, sqlSelectContactMessages, contactID, before, limit)
 	if err != nil {
 		return nil, fmt.Errorf("error querying contact messages: %w", err)
 	}
 	defer rows.Close()
 
-	msgs := make([]*Msg, 0)
+	msgs := make([]*DBMsg, 0)
 
 	for rows.Next() {
-		msg := &Msg{}
+		msg := &DBMsg{}
 		if err := dbutil.ScanJSON(rows, msg); err != nil {
 			return nil, fmt.Errorf("error scanning msg row: %w", err)
 		}

core/models/msg_test.go

@@ -7,6 +7,7 @@ import (
 	"github.com/nyaruka/chip/core/models"
 	"github.com/nyaruka/chip/testsuite"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestLoadContactMessages(t *testing.T) {
@@ -67,3 +68,36 @@ func TestLoadContactMessages(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Len(t, msgs, 0)
 }
+
+func TestDMMsgToMsgInAndOut(t *testing.T) {
+	ctx, rt := testsuite.Runtime()
+
+	defer testsuite.ResetDB()
+
+	orgID := testsuite.InsertOrg(rt, "Nyaruka")
+	chanID := testsuite.InsertChannel(rt, "8291264a-4581-4d12-96e5-e9fcfa6e68d9", orgID, "CHP", "WebChat", "123", []string{"webchat"})
+	bobID := testsuite.InsertContact(rt, orgID, "Bob")
+	bobURNID := testsuite.InsertURN(rt, orgID, bobID, "webchat:65vbbDAQCdPdEWlEhDGy4utO")
+
+	msg1ID := testsuite.InsertIncomingMsg(rt, orgID, chanID, bobID, bobURNID, "Hello", time.Now())
+	msg2ID := testsuite.InsertOutgoingMsg(rt, orgID, chanID, bobID, bobURNID, "There", time.Now())
+	msgs, err := models.LoadContactMessages(ctx, rt, bobID, time.Now(), 10)
+	require.NoError(t, err)
+	msg1 := msgs[1]
+	msg2 := msgs[0]
+
+	store := models.NewStore(rt)
+	store.Start()
+	defer store.Stop()
+
+	msg1In := msg1.ToMsgIn()
+	assert.Equal(t, models.NewMsgIn(msg1ID, "Hello", msg1.CreatedOn), msg1In)
+
+	msg2Out, err := msg2.ToMsgOut(ctx, store)
+	assert.NoError(t, err)
+	assert.Equal(t, models.NewMsgOut(msg2ID, "There", nil, "chat", nil, msg2.CreatedOn), msg2Out)
+
+	// can't call ToMsgIn on an outbound message and vice versa
+	assert.Panics(t, func() { msg2.ToMsgIn() })
+	assert.Panics(t, func() { msg1.ToMsgOut(ctx, store) })
+}

core/models/user.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"database/sql"
 	"fmt"
-	"strings"
 
 	"github.com/nyaruka/chip/runtime"
 	"github.com/nyaruka/gocommon/dbutil"
@@ -16,25 +15,15 @@ type UserID null.Int
 const NilUserID = UserID(0)
 
 type User struct {
-	ID        UserID `json:"id"`
-	Email     string `json:"email"`
-	FirstName string `json:"first_name"`
-	LastName  string `json:"last_name"`
-	Avatar    string `json:"avatar"`
-}
-
-func (u *User) Name() string { return strings.TrimSpace(u.FirstName + " " + u.LastName) }
-
-func (u *User) AvatarURL(cfg *runtime.Config) string {
-	if u.Avatar != "" {
-		return cfg.StorageURL + u.Avatar
-	}
-	return ""
+	ID     UserID `json:"id"`
+	Email  string `json:"email"`
+	Name   string `json:"name"`
+	Avatar string `json:"avatar,omitempty"`
 }
 
 const sqlSelectUser = `
 SELECT row_to_json(r) FROM (
-	SELECT u.id, u.email, u.first_name, u.last_name, s.avatar
+	SELECT u.id, u.email, TRIM(CONCAT(u.first_name, ' ', u.last_name)) AS name, s.avatar
 	FROM auth_user u
 	INNER JOIN orgs_usersettings s ON s.user_id = u.id
 	WHERE u.id = $1 AND u.is_active
@@ -54,5 +43,10 @@ func LoadUser(ctx context.Context, rt *runtime.Runtime, id UserID) (*User, error
 	if err := dbutil.ScanJSON(rows, u); err != nil {
 		return nil, fmt.Errorf("error scanning user: %w", err)
 	}
+
+	if u.Avatar != "" {
+		u.Avatar = rt.Config.StorageURL + u.Avatar
+	}
+
 	return u, nil
 }

core/models/user_test.go

@@ -22,7 +22,6 @@ func TestLoadUser(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Equal(t, bobID, u.ID)
 	assert.Equal(t, "[email protected]", u.Email)
-	assert.Equal(t, "Bob McFlows", u.Name())
-	assert.Equal(t, "avatars/1234/1234567890.webp", u.Avatar)
-	assert.Equal(t, "http://localhost/media/avatars/1234/1234567890.webp", u.AvatarURL(rt.Config))
+	assert.Equal(t, "Bob McFlows", u.Name)
+	assert.Equal(t, "http://localhost/media/avatars/1234/1234567890.webp", u.Avatar)
 }

core/queue/lua/outbox_read_ready.lua

@@ -1,15 +1,16 @@
-local queuesKey, readyKey, queuePrefix = KEYS[1], KEYS[2], ARGV[1]
+local queuesKey, readyKey, keyBase = KEYS[1], KEYS[2], ARGV[1]
 
-local chatIDs = redis.call("ZINTER", 2, queuesKey, readyKey)
+local queueIDs = redis.call("ZINTER", 2, queuesKey, readyKey)
 
-local msgs = {}
+local result = {} -- pairs of queue IDs and items
 
-for i, chatID in ipairs(chatIDs) do
-    local chatMsg = redis.call("LINDEX", queuePrefix .. chatID, 0)
+for i, queueID in ipairs(queueIDs) do
+    local item = redis.call("LINDEX", keyBase .. ":queue:" .. queueID, 0)
 
-    msgs[i] = chatMsg
+    table.insert(result, queueID)
+    table.insert(result, item)
 
-    redis.call("SREM", readyKey, chatID)
+    redis.call("SREM", readyKey, queueID)
 end
 
-return msgs
+return result

core/queue/lua/outbox_record_sent.lua

@@ -1,4 +1,4 @@
-local queuesKey, queueKey, readyKey, chatID, msgID = KEYS[1], KEYS[2], KEYS[3], ARGV[1], tonumber(ARGV[2])
+local queuesKey, queueKey, readyKey, queueID, msgID = KEYS[1], KEYS[2], KEYS[3], ARGV[1], tonumber(ARGV[2])
 
 local thisItem = redis.call("LINDEX", queueKey, 0)
 if thisItem == false then
@@ -20,15 +20,15 @@ local hasMore = false
 
 if nextItem == false then
     -- nothing more in the queue for this chat so take it out of the master set
-    redis.call("ZREM", queuesKey, chatID)
+    redis.call("ZREM", queuesKey, queueID)
 else
     -- update the score of this queue to the timestamp of its new oldest message
     local msg = cjson.decode(nextItem)
-    redis.call("ZADD", queuesKey, msg["_ts"], chatID)
+    redis.call("ZADD", queuesKey, msg["_ts"], queueID)
     hasMore = true
 end
 
 -- put this chat back in the ready set
-redis.call("SADD", readyKey, chatID)
+redis.call("SADD", readyKey, queueID)
 
 return {"success", tostring(hasMore)}