From 9b70fe731839b327859f981cd21139e7ef1bb87e Mon Sep 17 00:00:00 2001
From: Hugh O'Brien <hugh.obrien@overleaf.com>
Date: Wed, 3 Feb 2021 10:23:37 +0000
Subject: [PATCH] Merge pull request #3614 from
 overleaf/msm-chat-add-message-id

[Chat] Added `id` to message groups

GitOrigin-RevId: 45e73ad236984bfbac45c837e603c6ae04382972
---
 .../js/features/chat/components/message-list.js | 14 +++++++-------
 .../js/features/chat/store/chat-store.js        |  2 ++
 .../chat/store/message-list-appender.js         |  6 ++++++
 .../features/chat/components/chat-pane.test.js  |  2 ++
 .../chat/components/message-list.test.js        |  2 ++
 .../features/chat/store/chat-store.test.js      | 12 +++++++++++-
 .../chat/store/message-list-appender.test.js    | 17 ++++++++++++++++-
 7 files changed, 46 insertions(+), 9 deletions(-)

diff --git a/services/web/frontend/js/features/chat/components/message-list.js b/services/web/frontend/js/features/chat/components/message-list.js
index 310d7fd34a..19c085272d 100644
--- a/services/web/frontend/js/features/chat/components/message-list.js
+++ b/services/web/frontend/js/features/chat/components/message-list.js
@@ -13,10 +13,6 @@ function formatTimestamp(date) {
   }
 }
 
-function indexFromEnd(list, index) {
-  return list.length - index - 1
-}
-
 function MessageList({ messages, resetUnreadMessages, userId }) {
   function shouldRenderDate(messageIndex) {
     if (messageIndex === 0) {
@@ -37,7 +33,7 @@ function MessageList({ messages, resetUnreadMessages, userId }) {
     >
       {messages.map((message, index) => (
         // new messages are added to the beginning of the list, so we use a reversed index
-        <li key={indexFromEnd(messages, index)} className="message">
+        <li key={message.id} className="message">
           {shouldRenderDate(index) && (
             <div className="date">
               <time
@@ -59,8 +55,12 @@ function MessageList({ messages, resetUnreadMessages, userId }) {
 }
 
 MessageList.propTypes = {
-  messages: PropTypes.arrayOf(PropTypes.shape({ timestamp: PropTypes.number }))
-    .isRequired,
+  messages: PropTypes.arrayOf(
+    PropTypes.shape({
+      id: PropTypes.string.isRequired,
+      timestamp: PropTypes.number
+    })
+  ).isRequired,
   resetUnreadMessages: PropTypes.func.isRequired,
   userId: PropTypes.string.isRequired
 }
diff --git a/services/web/frontend/js/features/chat/store/chat-store.js b/services/web/frontend/js/features/chat/store/chat-store.js
index a50df61262..11d1bb778f 100644
--- a/services/web/frontend/js/features/chat/store/chat-store.js
+++ b/services/web/frontend/js/features/chat/store/chat-store.js
@@ -1,6 +1,7 @@
 import EventEmitter from '../../../utils/EventEmitter'
 import { appendMessage, prependMessages } from './message-list-appender'
 import { getJSON, postJSON } from '../../../infrastructure/fetch-json'
+import { v4 as uuid } from 'uuid'
 
 export const MESSAGE_LIMIT = 50
 
@@ -88,6 +89,7 @@ export class ChatStore {
     }
     this._justSent = true
     this.messages = appendMessage(this.messages, {
+      id: uuid(), // uuid valid for this session, ensures all messages have an identifier
       user: this._user,
       content: message,
       timestamp: Date.now()
diff --git a/services/web/frontend/js/features/chat/store/message-list-appender.js b/services/web/frontend/js/features/chat/store/message-list-appender.js
index b3600f4ff4..d32adb5f56 100644
--- a/services/web/frontend/js/features/chat/store/message-list-appender.js
+++ b/services/web/frontend/js/features/chat/store/message-list-appender.js
@@ -14,11 +14,15 @@ export function appendMessage(messageList, message) {
   if (shouldGroup) {
     return messageList.slice(0, messageList.length - 1).concat({
       ...lastMessage,
+      // the `id` is updated to the latest received content when a new
+      // message is appended or prepended
+      id: message.id,
       timestamp: message.timestamp,
       contents: lastMessage.contents.concat(message.content)
     })
   } else {
     return messageList.slice(0).concat({
+      id: message.id,
       user: message.user,
       timestamp: message.timestamp,
       contents: [message.content]
@@ -41,10 +45,12 @@ export function prependMessages(messageList, messages) {
         firstMessage.timestamp - message.timestamp < TIMESTAMP_GROUP_SIZE
 
       if (shouldGroup) {
+        firstMessage.id = message.id
         firstMessage.timestamp = message.timestamp
         firstMessage.contents = [message.content].concat(firstMessage.contents)
       } else {
         listCopy.unshift({
+          id: message.id,
           user: message.user,
           timestamp: message.timestamp,
           contents: [message.content]
diff --git a/services/web/test/frontend/features/chat/components/chat-pane.test.js b/services/web/test/frontend/features/chat/components/chat-pane.test.js
index 2fc670688f..d84d58fef4 100644
--- a/services/web/test/frontend/features/chat/components/chat-pane.test.js
+++ b/services/web/test/frontend/features/chat/components/chat-pane.test.js
@@ -23,11 +23,13 @@ describe('<ChatPane />', function() {
 
   const testMessages = [
     {
+      id: 'msg_1',
       content: 'a message',
       user: currentUser,
       timestamp: new Date().getTime()
     },
     {
+      id: 'msg_2',
       content: 'another message',
       user: currentUser,
       timestamp: new Date().getTime()
diff --git a/services/web/test/frontend/features/chat/components/message-list.test.js b/services/web/test/frontend/features/chat/components/message-list.test.js
index 0efc5aef9d..f61ad0ebfb 100644
--- a/services/web/test/frontend/features/chat/components/message-list.test.js
+++ b/services/web/test/frontend/features/chat/components/message-list.test.js
@@ -23,11 +23,13 @@ describe('<MessageList />', function() {
   function createMessages() {
     return [
       {
+        id: 'test_msg_1',
         contents: ['a message'],
         user: currentUser,
         timestamp: new Date().getTime()
       },
       {
+        id: 'test_msg_2',
         contents: ['another message'],
         user: currentUser,
         timestamp: new Date().getTime()
diff --git a/services/web/test/frontend/features/chat/store/chat-store.test.js b/services/web/test/frontend/features/chat/store/chat-store.test.js
index bc125a11f0..ceb14c290d 100644
--- a/services/web/test/frontend/features/chat/store/chat-store.test.js
+++ b/services/web/test/frontend/features/chat/store/chat-store.test.js
@@ -16,6 +16,7 @@ describe('ChatStore', function() {
   const testProjectId = 'project-123'
 
   const testMessage = {
+    id: 'msg_1',
     content: 'hello',
     timestamp: new Date().getTime(),
     user
@@ -62,8 +63,9 @@ describe('ChatStore', function() {
     })
 
     it('when the message is from other user, it is added to the messages list', function() {
-      mockSocketMessage({ ...testMessage, id: 'other_user' })
+      mockSocketMessage({ ...testMessage, id: 'other_user_msg' })
       expect(store.messages[store.messages.length - 1]).to.deep.equal({
+        id: 'other_user_msg',
         user: testMessage.user,
         timestamp: testMessage.timestamp,
         contents: [testMessage.content]
@@ -84,12 +86,14 @@ describe('ChatStore', function() {
         // next message by a different user is added normally
         const otherMessage = {
           ...testMessage,
+          id: 'other_user_msg',
           user: { id: 'other_user' },
           content: 'other'
         }
         mockSocketMessage(otherMessage)
         expect(store.messages.length).to.equal(originalMessageList.length + 1)
         expect(store.messages[store.messages.length - 1]).to.deep.equal({
+          id: otherMessage.id,
           user: otherMessage.user,
           timestamp: otherMessage.timestamp,
           contents: [otherMessage.content]
@@ -105,6 +109,11 @@ describe('ChatStore', function() {
 
         expect(subscriber).not.to.be.called
       })
+
+      it("have an 'id' property", async function() {
+        await store.sendMessage(testMessage.content)
+        expect(typeof store.messages[0].id).to.equal('string')
+      })
     })
   })
 
@@ -122,6 +131,7 @@ describe('ChatStore', function() {
       await store.loadMoreMessages()
       expect(store.messages.length).to.equal(originalMessageList.length + 1)
       expect(store.messages[store.messages.length - 1]).to.deep.equal({
+        id: testMessage.id,
         user: testMessage.user,
         timestamp: testMessage.timestamp,
         contents: [testMessage.content]
diff --git a/services/web/test/frontend/features/chat/store/message-list-appender.test.js b/services/web/test/frontend/features/chat/store/message-list-appender.test.js
index b8abff5c5e..8b7f347f14 100644
--- a/services/web/test/frontend/features/chat/store/message-list-appender.test.js
+++ b/services/web/test/frontend/features/chat/store/message-list-appender.test.js
@@ -15,11 +15,13 @@ const otherUser = {
 function createTestMessageList() {
   return [
     {
+      id: 'msg_1',
       contents: ['hello', 'world'],
       timestamp: new Date().getTime(),
       user: otherUser
     },
     {
+      id: 'msg_2',
       contents: ['foo'],
       timestamp: new Date().getTime(),
       user: testUser
@@ -30,11 +32,12 @@ function createTestMessageList() {
 describe('prependMessages()', function() {
   function createTestMessages() {
     const message1 = {
+      id: 'prepended_message',
       content: 'hello',
       timestamp: new Date().getTime(),
       user: testUser
     }
-    const message2 = { ...message1 }
+    const message2 = { ...message1, id: 'prepended_message_2' }
     return [message1, message2]
   }
 
@@ -42,6 +45,7 @@ describe('prependMessages()', function() {
     const messages = createTestMessages()
     expect(prependMessages([], messages)).to.deep.equal([
       {
+        id: messages[0].id,
         timestamp: messages[0].timestamp,
         user: messages[0].user,
         contents: [messages[0].content, messages[1].content]
@@ -62,6 +66,7 @@ describe('prependMessages()', function() {
       const result = prependMessages(createTestMessageList(), messages)
       expect(result.length).to.equal(list.length + 1)
       expect(result[0]).to.deep.equal({
+        id: messages[0].id,
         timestamp: messages[0].timestamp,
         user: messages[0].user,
         contents: [messages[0].content, messages[1].content]
@@ -73,11 +78,13 @@ describe('prependMessages()', function() {
       const result = prependMessages(createTestMessageList(), messages)
       expect(result.length).to.equal(list.length + 2)
       expect(result[0]).to.deep.equal({
+        id: messages[0].id,
         timestamp: messages[0].timestamp,
         user: messages[0].user,
         contents: [messages[0].content]
       })
       expect(result[1]).to.deep.equal({
+        id: messages[1].id,
         timestamp: messages[1].timestamp,
         user: messages[1].user,
         contents: [messages[1].content]
@@ -98,11 +105,13 @@ describe('prependMessages()', function() {
       const result = prependMessages(createTestMessageList(), messages)
       expect(result.length).to.equal(list.length + 2)
       expect(result[0]).to.deep.equal({
+        id: messages[0].id,
         timestamp: messages[0].timestamp,
         user: messages[0].user,
         contents: [messages[0].content]
       })
       expect(result[1]).to.deep.equal({
+        id: messages[1].id,
         timestamp: messages[1].timestamp,
         user: messages[1].user,
         contents: [messages[1].content]
@@ -118,6 +127,7 @@ describe('prependMessages()', function() {
     const result = prependMessages(createTestMessageList(), messages)
     expect(result.length).to.equal(list.length)
     expect(result[0]).to.deep.equal({
+      id: messages[0].id,
       timestamp: messages[0].timestamp,
       user: messages[0].user,
       contents: [messages[0].content, messages[1].content, ...list[0].contents]
@@ -128,6 +138,7 @@ describe('prependMessages()', function() {
 describe('appendMessage()', function() {
   function createTestMessage() {
     return {
+      id: 'appended_message',
       content: 'hi!',
       timestamp: new Date().getTime(),
       user: testUser
@@ -138,6 +149,7 @@ describe('appendMessage()', function() {
     const testMessage = createTestMessage()
     expect(appendMessage([], testMessage)).to.deep.equal([
       {
+        id: 'appended_message',
         timestamp: testMessage.timestamp,
         user: testMessage.user,
         contents: [testMessage.content]
@@ -178,6 +190,7 @@ describe('appendMessage()', function() {
         const result = appendMessage(list, message)
         expect(result.length).to.equal(list.length + 1)
         expect(result[2]).to.deep.equal({
+          id: 'appended_message',
           timestamp: message.timestamp,
           user: message.user,
           contents: [message.content]
@@ -199,6 +212,7 @@ describe('appendMessage()', function() {
       const result = appendMessage(list, message)
       expect(result.length).to.equal(3)
       expect(result[2]).to.deep.equal({
+        id: 'appended_message',
         timestamp: message.timestamp,
         user: message.user,
         contents: [message.content]
@@ -211,6 +225,7 @@ describe('appendMessage()', function() {
       const result = appendMessage(list, message)
       expect(result.length).to.equal(3)
       expect(result[2]).to.deep.equal({
+        id: 'appended_message',
         timestamp: message.timestamp,
         user: message.user,
         contents: [message.content]