Merge pull request #3943 from overleaf/revert-3735-as-chat-reducer

Revert "Refactor chat store to use React state"

GitOrigin-RevId: e75e2d56a80c3741415bea3941a26f7dd8f505d3

services/web/frontend/js/features/chat/components/chat-pane.js

@@ -1,13 +1,11 @@
-import React, { useEffect } from 'react'
+import React, { useEffect, useState } from 'react'
 import PropTypes from 'prop-types'
-import { useTranslation } from 'react-i18next'
-
 import MessageList from './message-list'
 import MessageInput from './message-input'
 import InfiniteScroll from './infinite-scroll'
 import Icon from '../../../shared/components/icon'
+import { useTranslation } from 'react-i18next'
 import { useLayoutContext } from '../../../shared/context/layout-context'
-import { useApplicationContext } from '../../../shared/context/application-context'
 import withErrorBoundary from '../../../infrastructure/error-boundary'
 import { useChatContext } from '../context/chat-context'
 
@@ -15,26 +13,27 @@ function ChatPane() {
   const { t } = useTranslation()
 
   const { chatIsOpen } = useLayoutContext({ chatIsOpen: PropTypes.bool })
-  const { user } = useApplicationContext()
 
   const {
-    status,
-    messages,
-    initialMessagesLoaded,
+    userId,
     atEnd,
-    loadInitialMessages,
+    loading,
     loadMoreMessages,
+    messages,
     sendMessage,
-    markMessagesAsRead
+    resetUnreadMessageCount
   } = useChatContext()
 
+  const [initialMessagesLoaded, setInitialMessagesLoaded] = useState(false)
+
   useEffect(() => {
     if (chatIsOpen && !initialMessagesLoaded) {
-      loadInitialMessages()
+      loadMoreMessages()
+      setInitialMessagesLoaded(true)
     }
-  }, [chatIsOpen, loadInitialMessages, initialMessagesLoaded])
+  }, [initialMessagesLoaded, loadMoreMessages, chatIsOpen])
 
-  const shouldDisplayPlaceholder = status !== 'pending' && messages.length === 0
+  const shouldDisplayPlaceholder = !loading && messages.length === 0
 
   const messageContentCount = messages.reduce(
     (acc, { contents }) => acc + contents.length,
@@ -47,22 +46,22 @@ function ChatPane() {
         atEnd={atEnd}
         className="messages"
         fetchData={loadMoreMessages}
-        isLoading={status === 'pending'}
+        isLoading={loading}
         itemCount={messageContentCount}
       >
         <div>
           <h2 className="sr-only">{t('chat')}</h2>
-          {status === 'pending' && <LoadingSpinner />}
+          {loading && <LoadingSpinner />}
           {shouldDisplayPlaceholder && <Placeholder />}
           <MessageList
             messages={messages}
-            userId={user.id}
-            resetUnreadMessages={markMessagesAsRead}
+            userId={userId}
+            resetUnreadMessages={resetUnreadMessageCount}
           />
         </div>
       </InfiniteScroll>
       <MessageInput
-        resetUnreadMessages={markMessagesAsRead}
+        resetUnreadMessages={resetUnreadMessageCount}
         sendMessage={sendMessage}
       />
     </aside>

services/web/frontend/js/features/chat/context/chat-context.js

@@ -2,116 +2,37 @@ import React, {
   createContext,
   useCallback,
   useContext,
-  useEffect,
-  useReducer,
-  useMemo
+  useState,
+  useEffect
 } from 'react'
 import PropTypes from 'prop-types'
-import { v4 as uuid } from 'uuid'
-
 import { useApplicationContext } from '../../../shared/context/application-context'
 import { useEditorContext } from '../../../shared/context/editor-context'
-import { getJSON, postJSON } from '../../../infrastructure/fetch-json'
-import { appendMessage, prependMessages } from '../utils/message-list-appender'
+import { ChatStore } from '../store/chat-store'
 import useBrowserWindow from '../../../infrastructure/browser-window-hook'
 import { useLayoutContext } from '../../../shared/context/layout-context'
 
-const PAGE_SIZE = 50
-
-export function chatReducer(state, action) {
-  switch (action.type) {
-    case 'INITIAL_FETCH_MESSAGES':
-      return {
-        ...state,
-        status: 'pending',
-        initialMessagesLoaded: true
-      }
-
-    case 'FETCH_MESSAGES':
-      return {
-        ...state,
-        status: 'pending'
-      }
-
-    case 'FETCH_MESSAGES_SUCCESS':
-      return {
-        ...state,
-        status: 'idle',
-        messages: prependMessages(state.messages, action.messages),
-        lastTimestamp: action.messages[0] ? action.messages[0].timestamp : null,
-        atEnd: action.messages.length < PAGE_SIZE
-      }
-
-    case 'SEND_MESSAGE':
-      return {
-        ...state,
-        messages: appendMessage(state.messages, {
-          // Messages are sent optimistically, so don't have an id (used for
-          // React keys). The uuid is valid for this session, and ensures all
-          // messages have an id. It will be overwritten by the actual ids on
-          // refresh
-          id: uuid(),
-          user: action.user,
-          content: action.content,
-          timestamp: Date.now()
-        }),
-        messageWasJustSent: true
-      }
-
-    case 'RECEIVE_MESSAGE':
-      return {
-        ...state,
-        messages: appendMessage(state.messages, action.message),
-        messageWasJustSent: false,
-        unreadMessageCount: state.unreadMessageCount + 1
-      }
-
-    case 'MARK_MESSAGES_AS_READ':
-      return {
-        ...state,
-        unreadMessageCount: 0
-      }
-
-    case 'ERROR':
-      return {
-        ...state,
-        status: 'error',
-        error: action.error
-      }
-
-    default:
-      throw new Error('Unknown action')
-  }
-}
-
-const initialState = {
-  status: 'idle',
-  messages: [],
-  initialMessagesLoaded: false,
-  lastTimestamp: null,
-  atEnd: false,
-  messageWasJustSent: false,
-  unreadMessageCount: 0,
-  error: null
-}
-
 export const ChatContext = createContext()
 
 ChatContext.Provider.propTypes = {
   value: PropTypes.shape({
-    status: PropTypes.string.isRequired,
+    userId: PropTypes.string.isRequired,
+    atEnd: PropTypes.bool,
+    loading: PropTypes.bool,
     messages: PropTypes.array.isRequired,
-    initialMessagesLoaded: PropTypes.bool.isRequired,
-    atEnd: PropTypes.bool.isRequired,
     unreadMessageCount: PropTypes.number.isRequired,
-    loadInitialMessages: PropTypes.func.isRequired,
+    resetUnreadMessageCount: PropTypes.func.isRequired,
     loadMoreMessages: PropTypes.func.isRequired,
-    sendMessage: PropTypes.func.isRequired,
-    markMessagesAsRead: PropTypes.func.isRequired
+    sendMessage: PropTypes.func.isRequired
   }).isRequired
 }
 
 export function ChatProvider({ children }) {
+  const {
+    hasFocus: windowHasFocus,
+    flashTitle,
+    stopFlashingTitle
+  } = useBrowserWindow()
   const { user } = useApplicationContext({
     user: PropTypes.shape({ id: PropTypes.string.isRequired }.isRequired)
   })
@@ -121,131 +42,68 @@ export function ChatProvider({ children }) {
 
   const { chatIsOpen } = useLayoutContext({ chatIsOpen: PropTypes.bool })
 
-  const {
-    hasFocus: windowHasFocus,
-    flashTitle,
-    stopFlashingTitle
-  } = useBrowserWindow()
-
-  const [state, dispatch] = useReducer(chatReducer, initialState)
-
-  const { loadInitialMessages, loadMoreMessages } = useMemo(() => {
-    function fetchMessages() {
-      if (state.atEnd) return
-
-      const query = { limit: PAGE_SIZE }
-
-      if (state.lastTimestamp) {
-        query.before = state.lastTimestamp
-      }
-
-      const queryString = new URLSearchParams(query)
-      const url = `/project/${projectId}/messages?${queryString.toString()}`
-
-      getJSON(url).then((messages = []) => {
-        dispatch({
-          type: 'FETCH_MESSAGES_SUCCESS',
-          messages: messages.reverse()
-        })
-      })
-    }
-
-    function loadInitialMessages() {
-      if (state.initialMessagesLoaded) return
-
-      dispatch({ type: 'INITIAL_FETCH_MESSAGES' })
-      fetchMessages()
-    }
-
-    function loadMoreMessages() {
-      dispatch({ type: 'FETCH_MESSAGES' })
-      fetchMessages()
-    }
-
-    return {
-      loadInitialMessages,
-      loadMoreMessages
-    }
-  }, [projectId, state.atEnd, state.initialMessagesLoaded, state.lastTimestamp])
-
-  const sendMessage = useCallback(
-    content => {
-      if (!content) return
-
-      dispatch({
-        type: 'SEND_MESSAGE',
-        user,
-        content
-      })
-
-      const url = `/project/${projectId}/messages`
-      postJSON(url, {
-        body: { content }
-      })
-    },
-    [projectId, user]
-  )
-
-  const markMessagesAsRead = useCallback(() => {
-    dispatch({ type: 'MARK_MESSAGES_AS_READ' })
-  }, [])
-
-  // Handling receiving messages over the socket
-  const socket = window._ide?.socket
-  useEffect(() => {
-    if (!socket) return
+  const [unreadMessageCount, setUnreadMessageCount] = useState(0)
+  function resetUnreadMessageCount() {
+    setUnreadMessageCount(0)
+  }
 
-    function receivedMessage(message) {
-      // If the message is from the current user and they just sent a message,
-      // then we are receiving the sent message back from the socket. Ignore it
-      // to prevent double message
-      const messageIsFromSelf = message?.user?.id === user.id
-      if (messageIsFromSelf && state.messageWasJustSent) return
+  const [atEnd, setAtEnd] = useState(false)
+  const [loading, setLoading] = useState(false)
+  const [messages, setMessages] = useState([])
 
-      dispatch({ type: 'RECEIVE_MESSAGE', message })
-    }
+  const [store] = useState(() => new ChatStore(user, projectId))
 
-    socket.on('new-chat-message', receivedMessage)
-    return () => {
-      if (!socket) return
-
-      socket.removeListener('new-chat-message', receivedMessage)
-    }
-    // We're adding and removing the socket listener every time we send a
-    // message (and messageWasJustSent changes). Not great, but no good way
-    // around it
-  }, [socket, state.messageWasJustSent, state.unreadMessageCount, user.id])
-
-  // Handle unread messages
   useEffect(() => {
     if (windowHasFocus) {
       stopFlashingTitle()
       if (chatIsOpen) {
-        markMessagesAsRead()
+        setUnreadMessageCount(0)
       }
     }
-    if (!windowHasFocus && state.unreadMessageCount > 0) {
+    if (!windowHasFocus && unreadMessageCount > 0) {
       flashTitle('New Message')
     }
   }, [
     windowHasFocus,
     chatIsOpen,
-    state.unreadMessageCount,
+    unreadMessageCount,
     flashTitle,
-    stopFlashingTitle,
-    markMessagesAsRead
+    stopFlashingTitle
+  ])
+
+  useEffect(() => {
+    function updateState() {
+      setAtEnd(store.atEnd)
+      setLoading(store.loading)
+      setMessages(store.messages)
+    }
+
+    function handleNewMessage() {
+      setUnreadMessageCount(prevCount => prevCount + 1)
+    }
+
+    store.on('updated', updateState)
+    store.on('message-received', handleNewMessage)
+
+    updateState()
+
+    return () => store.destroy()
+  }, [store])
+
+  const loadMoreMessages = useCallback(() => store.loadMoreMessages(), [store])
+  const sendMessage = useCallback(message => store.sendMessage(message), [
+    store
   ])
 
   const value = {
-    status: state.status,
-    messages: state.messages,
-    initialMessagesLoaded: state.initialMessagesLoaded,
-    atEnd: state.atEnd,
-    unreadMessageCount: state.unreadMessageCount,
-    loadInitialMessages,
+    userId: user.id,
+    atEnd,
+    loading,
+    messages,
+    unreadMessageCount,
+    resetUnreadMessageCount,
     loadMoreMessages,
-    sendMessage,
-    markMessagesAsRead
+    sendMessage
   }
 
   return <ChatContext.Provider value={value}>{children}</ChatContext.Provider>

services/web/frontend/js/features/chat/store/chat-store.js

@@ -0,0 +1,101 @@
+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
+
+export class ChatStore {
+  constructor(user, projectId) {
+    this.messages = []
+    this.loading = false
+    this.atEnd = false
+
+    this._user = user
+    this._projectId = projectId
+    this._nextBeforeTimestamp = null
+    this._justSent = false
+
+    this._emitter = new EventEmitter()
+
+    this._onNewChatMessage = message => {
+      const messageIsFromSelf =
+        message && message.user && message.user.id === this._user.id
+      if (!messageIsFromSelf || !this._justSent) {
+        this.messages = appendMessage(this.messages, message)
+        this._emitter.emit('updated')
+        this._emitter.emit('message-received', message)
+        window.dispatchEvent(
+          new CustomEvent('Chat.MessageReceived', { detail: { message } })
+        )
+      }
+      this._justSent = false
+    }
+
+    window._ide.socket.on('new-chat-message', this._onNewChatMessage)
+  }
+
+  destroy() {
+    window._ide.socket.removeListener(
+      'new-chat-message',
+      this._onNewChatMessage
+    )
+    this._emitter.off() // removes all listeners
+  }
+
+  on(event, fn) {
+    this._emitter.on(event, fn)
+  }
+
+  off(event, fn) {
+    this._emitter.off(event, fn)
+  }
+
+  loadMoreMessages() {
+    if (this.atEnd) {
+      return
+    }
+
+    this.loading = true
+    this._emitter.emit('updated')
+
+    let url = `/project/${window.project_id}/messages?limit=${MESSAGE_LIMIT}`
+    if (this._nextBeforeTimestamp) {
+      url += `&before=${this._nextBeforeTimestamp}`
+    }
+
+    return getJSON(url).then(response => {
+      const messages = response || []
+      this.loading = false
+      if (messages.length < MESSAGE_LIMIT) {
+        this.atEnd = true
+      }
+      messages.reverse()
+      this.messages = prependMessages(this.messages, messages)
+      this._nextBeforeTimestamp = this.messages[0]
+        ? this.messages[0].timestamp
+        : undefined
+      this._emitter.emit('updated')
+    })
+  }
+
+  sendMessage(message) {
+    if (!message) {
+      return
+    }
+    const body = {
+      content: message,
+      _csrf: window.csrfToken
+    }
+    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()
+    })
+    const url = `/project/${this._projectId}/messages`
+    this._emitter.emit('updated')
+    return postJSON(url, { body })
+  }
+}

services/web/package-lock.json

@@ -5461,46 +5461,6 @@
         }
       }
     },
-    "@testing-library/react-hooks": {
-      "version": "5.1.1",
-      "resolved": "https://registry.npmjs.org/@testing-library/react-hooks/-/react-hooks-5.1.1.tgz",
-      "integrity": "sha512-52D2XnpelFDefnWpy/V6z2qGNj8JLIvW5DjYtelMvFXdEyWiykSaI7IXHwFy4ICoqXJDmmwHAiFRiFboub/U5g==",
-      "dev": true,
-      "requires": {
-        "@babel/runtime": "^7.12.5",
-        "@types/react": ">=16.9.0",
-        "@types/react-dom": ">=16.9.0",
-        "@types/react-test-renderer": ">=16.9.0",
-        "filter-console": "^0.1.1",
-        "react-error-boundary": "^3.1.0"
-      },
-      "dependencies": {
-        "@babel/runtime": {
-          "version": "7.13.10",
-          "resolved": "https://registry.npmjs.org/@babel/runtime/-/runtime-7.13.10.tgz",
-          "integrity": "sha512-4QPkjJq6Ns3V/RgpEahRk+AGfL0eO6RHHtTWoNNr5mO49G6B5+X6d6THgWEAvTrznU5xYpbAlVKRYcsCgh/Akw==",
-          "dev": true,
-          "requires": {
-            "regenerator-runtime": "^0.13.4"
-          }
-        },
-        "react-error-boundary": {
-          "version": "3.1.1",
-          "resolved": "https://registry.npmjs.org/react-error-boundary/-/react-error-boundary-3.1.1.tgz",
-          "integrity": "sha512-W3xCd9zXnanqrTUeViceufD3mIW8Ut29BUD+S2f0eO2XCOU8b6UrJfY46RDGe5lxCJzfe4j0yvIfh0RbTZhKJw==",
-          "dev": true,
-          "requires": {
-            "@babel/runtime": "^7.12.5"
-          }
-        },
-        "regenerator-runtime": {
-          "version": "0.13.7",
-          "resolved": "https://registry.npmjs.org/regenerator-runtime/-/regenerator-runtime-0.13.7.tgz",
-          "integrity": "sha512-a54FxoJDIr27pgf7IgeQGxmqUNYrcV338lf/6gH456HZ/PhX+5BcwHXG9ajESmwe6WRO0tAzRUrRmNONWgkrew==",
-          "dev": true
-        }
-      }
-    },
     "@tootallnate/once": {
       "version": "1.1.2",
       "resolved": "https://registry.npmjs.org/@tootallnate/once/-/once-1.1.2.tgz",
@@ -5793,15 +5753,6 @@
         "@types/reactcss": "*"
       }
     },
-    "@types/react-dom": {
-      "version": "17.0.3",
-      "resolved": "https://registry.npmjs.org/@types/react-dom/-/react-dom-17.0.3.tgz",
-      "integrity": "sha512-4NnJbCeWE+8YBzupn/YrJxZ8VnjcJq5iR1laqQ1vkpQgBiA7bwk0Rp24fxsdNinzJY2U+HHS4dJJDPdoMjdJ7w==",
-      "dev": true,
-      "requires": {
-        "@types/react": "*"
-      }
-    },
     "@types/react-syntax-highlighter": {
       "version": "11.0.4",
       "resolved": "https://registry.npmjs.org/@types/react-syntax-highlighter/-/react-syntax-highlighter-11.0.4.tgz",
@@ -5811,15 +5762,6 @@
         "@types/react": "*"
       }
     },
-    "@types/react-test-renderer": {
-      "version": "17.0.1",
-      "resolved": "https://registry.npmjs.org/@types/react-test-renderer/-/react-test-renderer-17.0.1.tgz",
-      "integrity": "sha512-3Fi2O6Zzq/f3QR9dRnlnHso9bMl7weKCviFmfF6B4LS1Uat6Hkm15k0ZAQuDz+UBq6B3+g+NM6IT2nr5QgPzCw==",
-      "dev": true,
-      "requires": {
-        "@types/react": "*"
-      }
-    },
     "@types/reactcss": {
       "version": "1.2.3",
       "resolved": "https://registry.npmjs.org/@types/reactcss/-/reactcss-1.2.3.tgz",
@@ -14925,12 +14867,6 @@
         }
       }
     },
-    "filter-console": {
-      "version": "0.1.1",
-      "resolved": "https://registry.npmjs.org/filter-console/-/filter-console-0.1.1.tgz",
-      "integrity": "sha512-zrXoV1Uaz52DqPs+qEwNJWJFAWZpYJ47UNmpN9q4j+/EYsz85uV0DC9k8tRND5kYmoVzL0W+Y75q4Rg8sRJCdg==",
-      "dev": true
-    },
     "finalhandler": {
       "version": "1.1.2",
       "resolved": "https://registry.npmjs.org/finalhandler/-/finalhandler-1.1.2.tgz",

services/web/package.json

@@ -174,7 +174,6 @@
     "@storybook/react": "^6.1.10",
     "@testing-library/dom": "^7.29.4",
     "@testing-library/react": "^11.2.3",
-    "@testing-library/react-hooks": "^5.1.0",
     "acorn": "^7.1.1",
     "acorn-walk": "^7.1.1",
     "angular-mocks": "~1.8.0",

services/web/test/frontend/features/chat/components/chat-pane.test.js

@@ -52,7 +52,7 @@ describe('<ChatPane />', function () {
     await screen.findByText('another message')
   })
 
-  it('a loading spinner is rendered while the messages are loading, then disappears', async function () {
+  it('A loading spinner is rendered while the messages are loading, then disappears', async function () {
     fetchMock.get(/messages/, [])
 
     renderWithChatContext(<ChatPane />, { user })

services/web/test/frontend/features/chat/context/chat-context.test.js

@@ -1,468 +0,0 @@
-// Disable prop type checks for test harnesses
-/* eslint-disable react/prop-types */
-
-import React from 'react'
-import { renderHook, act } from '@testing-library/react-hooks/dom'
-import { expect } from 'chai'
-import fetchMock from 'fetch-mock'
-import EventEmitter from 'events'
-
-import { useChatContext } from '../../../../../frontend/js/features/chat/context/chat-context'
-import {
-  ChatProviders,
-  cleanUpContext
-} from '../../../helpers/render-with-context'
-import { stubMathJax, tearDownMathJaxStubs } from '../components/stubs'
-
-describe('ChatContext', function () {
-  const user = {
-    id: 'fake_user',
-    first_name: 'fake_user_first_name',
-    email: 'fake@example.com'
-  }
-
-  beforeEach(function () {
-    fetchMock.reset()
-    cleanUpContext()
-
-    stubMathJax()
-  })
-
-  afterEach(function () {
-    tearDownMathJaxStubs()
-  })
-
-  describe('socket connection', function () {
-    beforeEach(function () {
-      // Mock GET messages to return no messages
-      fetchMock.get('express:/project/:projectId/messages', [])
-
-      // Mock POST new message to return 200
-      fetchMock.post('express:/project/:projectId/messages', 200)
-    })
-
-    it('subscribes when mounted', function () {
-      const socket = new EventEmitter()
-      renderChatContextHook({ user, socket })
-
-      // Assert that there is 1 listener
-      expect(socket.rawListeners('new-chat-message').length).to.equal(1)
-    })
-
-    it('unsubscribes when unmounted', function () {
-      const socket = new EventEmitter()
-      const { unmount } = renderChatContextHook({ user, socket })
-
-      unmount()
-
-      // Assert that there is 0 listeners
-      expect(socket.rawListeners('new-chat-message').length).to.equal(0)
-    })
-
-    it('adds received messages to the list', async function () {
-      // Mock socket: we only need to emit events, not mock actual connections
-      const socket = new EventEmitter()
-      const { result, waitForNextUpdate } = renderChatContextHook({
-        user,
-        socket
-      })
-
-      // Wait until initial messages have loaded
-      result.current.loadInitialMessages()
-      await waitForNextUpdate()
-
-      // No messages shown at first
-      expect(result.current.messages).to.deep.equal([])
-
-      // Mock message being received from another user
-      socket.emit('new-chat-message', {
-        id: 'msg_1',
-        content: 'new message',
-        timestamp: Date.now(),
-        user: {
-          id: 'another_fake_user',
-          first_name: 'another_fake_user_first_name',
-          email: 'another_fake@example.com'
-        }
-      })
-
-      const message = result.current.messages[0]
-      expect(message.id).to.equal('msg_1')
-      expect(message.contents).to.deep.equal(['new message'])
-    })
-
-    it("doesn't add received messages from the current user if a message was just sent", async function () {
-      const socket = new EventEmitter()
-      const { result, waitForNextUpdate } = renderChatContextHook({
-        user,
-        socket
-      })
-
-      // Wait until initial messages have loaded
-      result.current.loadInitialMessages()
-      await waitForNextUpdate()
-
-      // Send a message from the current user
-      result.current.sendMessage('sent message')
-
-      // Receive a message from the current user
-      socket.emit('new-chat-message', {
-        id: 'msg_1',
-        content: 'received message',
-        timestamp: Date.now(),
-        user
-      })
-
-      // Expect that the sent message is shown, but the new message is not
-      const messageContents = result.current.messages.map(
-        ({ contents }) => contents[0]
-      )
-      expect(messageContents).to.include('sent message')
-      expect(messageContents).to.not.include('received message')
-    })
-
-    it('adds the new message from the current user if another message was received after sending', async function () {
-      const socket = new EventEmitter()
-      const { result, waitForNextUpdate } = renderChatContextHook({
-        user,
-        socket
-      })
-
-      // Wait until initial messages have loaded
-      result.current.loadInitialMessages()
-      await waitForNextUpdate()
-
-      // Send a message from the current user
-      result.current.sendMessage('sent message from current user')
-
-      const [sentMessageFromCurrentUser] = result.current.messages
-      expect(sentMessageFromCurrentUser.contents).to.deep.equal([
-        'sent message from current user'
-      ])
-
-      act(() => {
-        // Receive a message from another user.
-        socket.emit('new-chat-message', {
-          id: 'msg_1',
-          content: 'new message from other user',
-          timestamp: Date.now(),
-          user: {
-            id: 'another_fake_user',
-            first_name: 'another_fake_user_first_name',
-            email: 'another_fake@example.com'
-          }
-        })
-      })
-
-      const [, messageFromOtherUser] = result.current.messages
-      expect(messageFromOtherUser.contents).to.deep.equal([
-        'new message from other user'
-      ])
-
-      // Receive a message from the current user
-      socket.emit('new-chat-message', {
-        id: 'msg_2',
-        content: 'received message from current user',
-        timestamp: Date.now(),
-        user
-      })
-
-      // Since the current user didn't just send a message, it is now shown
-      const [, , receivedMessageFromCurrentUser] = result.current.messages
-      expect(receivedMessageFromCurrentUser.contents).to.deep.equal([
-        'received message from current user'
-      ])
-    })
-  })
-
-  describe('loadInitialMessages', function () {
-    beforeEach(function () {
-      fetchMock.get('express:/project/:projectId/messages', [
-        {
-          id: 'msg_1',
-          content: 'a message',
-          user,
-          timestamp: Date.now()
-        }
-      ])
-    })
-
-    it('adds messages to the list', async function () {
-      const { result, waitForNextUpdate } = renderChatContextHook({ user })
-
-      result.current.loadInitialMessages()
-      await waitForNextUpdate()
-
-      expect(result.current.messages[0].contents).to.deep.equal(['a message'])
-    })
-
-    it("won't load messages a second time", async function () {
-      const { result, waitForNextUpdate } = renderChatContextHook({ user })
-
-      result.current.loadInitialMessages()
-      await waitForNextUpdate()
-
-      expect(result.current.initialMessagesLoaded).to.equal(true)
-
-      // Calling a second time won't do anything
-      result.current.loadInitialMessages()
-      expect(fetchMock.calls()).to.have.lengthOf(1)
-    })
-  })
-
-  describe('loadMoreMessages', function () {
-    it('adds messages to the list', async function () {
-      // Mock a GET request for an initial message
-      fetchMock.getOnce('express:/project/:projectId/messages', [
-        {
-          id: 'msg_1',
-          content: 'first message',
-          user,
-          timestamp: new Date('2021-03-04T10:00:00').getTime()
-        }
-      ])
-
-      const { result, waitForNextUpdate } = renderChatContextHook({ user })
-
-      result.current.loadMoreMessages()
-      await waitForNextUpdate()
-
-      expect(result.current.messages[0].contents).to.deep.equal([
-        'first message'
-      ])
-
-      // The before query param is not set
-      expect(getLastFetchMockQueryParam('before')).to.be.null
-    })
-
-    it('adds more messages if called a second time', async function () {
-      // Mock 2 GET requests, with different content
-      fetchMock
-        .getOnce(
-          'express:/project/:projectId/messages',
-          // Resolve a full "page" of messages (50)
-          createMessages(50, user, new Date('2021-03-04T10:00:00').getTime())
-        )
-        .getOnce(
-          'express:/project/:projectId/messages',
-          [
-            {
-              id: 'msg_51',
-              content: 'message from second page',
-              user,
-              timestamp: new Date('2021-03-04T11:00:00').getTime()
-            }
-          ],
-          { overwriteRoutes: false }
-        )
-
-      const { result, waitForNextUpdate } = renderChatContextHook({ user })
-
-      result.current.loadMoreMessages()
-      await waitForNextUpdate()
-
-      // Call a second time
-      result.current.loadMoreMessages()
-      await waitForNextUpdate()
-
-      // The second request is added to the list
-      // Since both messages from the same user, they are collapsed into the
-      // same "message"
-      expect(result.current.messages[0].contents).to.include(
-        'message from second page'
-      )
-
-      // The before query param for the second request matches the timestamp
-      // of the first message
-      const beforeParam = parseInt(getLastFetchMockQueryParam('before'), 10)
-      expect(beforeParam).to.equal(new Date('2021-03-04T10:00:00').getTime())
-    })
-
-    it("won't load more messages if there are no more messages", async function () {
-      // Mock a GET request for 49 messages. This is less the the full page size
-      // (50 messages), meaning that there are no further messages to be loaded
-      fetchMock.getOnce(
-        'express:/project/:projectId/messages',
-        createMessages(49, user)
-      )
-
-      const { result, waitForNextUpdate } = renderChatContextHook({ user })
-
-      result.current.loadMoreMessages()
-      await waitForNextUpdate()
-
-      expect(result.current.messages[0].contents).to.have.length(49)
-
-      result.current.loadMoreMessages()
-
-      expect(result.current.atEnd).to.be.true
-      expect(fetchMock.calls()).to.have.lengthOf(1)
-    })
-
-    it('handles socket messages while loading', async function () {
-      // Mock GET messages so that we can control when the promise is resolved
-      let resolveLoadingMessages
-      fetchMock.get(
-        'express:/project/:projectId/messages',
-        new Promise(resolve => {
-          resolveLoadingMessages = resolve
-        })
-      )
-
-      const socket = new EventEmitter()
-      const { result, waitForNextUpdate } = renderChatContextHook({
-        user,
-        socket
-      })
-
-      // Start loading messages
-      result.current.loadMoreMessages()
-
-      // Mock message being received from the socket while the request is in
-      // flight
-      socket.emit('new-chat-message', {
-        id: 'socket_msg',
-        content: 'socket message',
-        timestamp: Date.now(),
-        user: {
-          id: 'another_fake_user',
-          first_name: 'another_fake_user_first_name',
-          email: 'another_fake@example.com'
-        }
-      })
-
-      // Resolve messages being loaded
-      resolveLoadingMessages([
-        {
-          id: 'fetched_msg',
-          content: 'loaded message',
-          user,
-          timestamp: Date.now()
-        }
-      ])
-      await waitForNextUpdate()
-
-      // Although the loaded message was resolved last, it appears first (since
-      // requested messages must have come first)
-      const messageContents = result.current.messages.map(
-        ({ contents }) => contents[0]
-      )
-      expect(messageContents).to.deep.equal([
-        'loaded message',
-        'socket message'
-      ])
-    })
-  })
-
-  describe('sendMessage', function () {
-    beforeEach(function () {
-      // Mock GET messages to return no messages and POST new message to be
-      // successful
-      fetchMock
-        .get('express:/project/:projectId/messages', [])
-        .postOnce('express:/project/:projectId/messages', 200)
-    })
-
-    it('optimistically adds the message to the list', function () {
-      const { result } = renderChatContextHook({ user })
-
-      result.current.sendMessage('sent message')
-
-      expect(result.current.messages[0].contents).to.deep.equal([
-        'sent message'
-      ])
-    })
-
-    it('POSTs the message to the backend', function () {
-      const { result } = renderChatContextHook({ user })
-
-      result.current.sendMessage('sent message')
-
-      const [, { body }] = fetchMock.lastCall(
-        'express:/project/:projectId/messages',
-        'POST'
-      )
-      expect(JSON.parse(body)).to.deep.equal({ content: 'sent message' })
-    })
-
-    it("doesn't send if the content is empty", function () {
-      const { result } = renderChatContextHook({ user })
-
-      result.current.sendMessage('')
-
-      expect(result.current.messages).to.be.empty
-      expect(
-        fetchMock.called('express:/project/:projectId/messages', {
-          method: 'post'
-        })
-      ).to.be.false
-    })
-  })
-
-  describe('unread messages', function () {
-    beforeEach(function () {
-      // Mock GET messages to return no messages
-      fetchMock.get('express:/project/:projectId/messages', [])
-    })
-
-    it('increments unreadMessageCount when a new message is received', function () {
-      const socket = new EventEmitter()
-      const { result } = renderChatContextHook({ user, socket })
-
-      // Receive a new message from the socket
-      socket.emit('new-chat-message', {
-        id: 'msg_1',
-        content: 'new message',
-        timestamp: Date.now(),
-        user
-      })
-
-      expect(result.current.unreadMessageCount).to.equal(1)
-    })
-
-    it('resets unreadMessageCount when markMessagesAsRead is called', function () {
-      const socket = new EventEmitter()
-      const { result } = renderChatContextHook({ user, socket })
-
-      // Receive a new message from the socket, incrementing unreadMessageCount
-      // by 1
-      socket.emit('new-chat-message', {
-        id: 'msg_1',
-        content: 'new message',
-        timestamp: Date.now(),
-        user
-      })
-
-      result.current.markMessagesAsRead()
-
-      expect(result.current.unreadMessageCount).to.equal(0)
-    })
-  })
-})
-
-function renderChatContextHook(props) {
-  return renderHook(() => useChatContext(), {
-    // Wrap with ChatContext.Provider (and the other editor context providers)
-    wrapper: ({ children }) => (
-      <ChatProviders {...props}>{children}</ChatProviders>
-    )
-  })
-}
-
-function createMessages(number, user, timestamp = Date.now()) {
-  return Array.from({ length: number }, (_m, idx) => ({
-    id: `msg_${idx + 1}`,
-    content: `message ${idx + 1}`,
-    user,
-    timestamp
-  }))
-}
-
-/*
- * Get query param by key from the last fetchMock response
- */
-function getLastFetchMockQueryParam(key) {
-  const { url } = fetchMock.lastResponse()
-  const { searchParams } = new URL(url, 'https://www.overleaf.com')
-  return searchParams.get(key)
-}

services/web/test/frontend/features/chat/store/chat-store.test.js

@@ -0,0 +1,243 @@
+import { expect } from 'chai'
+import sinon from 'sinon'
+import fetchMock from 'fetch-mock'
+import {
+  ChatStore,
+  MESSAGE_LIMIT
+} from '../../../../../frontend/js/features/chat/store/chat-store'
+
+describe('ChatStore', function () {
+  let store, socket, mockSocketMessage
+
+  const user = {
+    id: '123abc'
+  }
+
+  const testProjectId = 'project-123'
+
+  const testMessage = {
+    id: 'msg_1',
+    content: 'hello',
+    timestamp: new Date().getTime(),
+    user
+  }
+
+  beforeEach(function () {
+    fetchMock.reset()
+
+    window.csrfToken = 'csrf_tok'
+
+    socket = { on: sinon.stub(), removeListener: sinon.stub() }
+    window._ide = { socket }
+    mockSocketMessage = message => socket.on.getCall(0).args[1](message)
+
+    store = new ChatStore(user, testProjectId)
+  })
+
+  afterEach(function () {
+    fetchMock.restore()
+    delete window._ide
+    delete window.csrfToken
+    delete window.user
+    delete window.project_id
+  })
+
+  describe('new message events', function () {
+    it('subscribes to the socket for new message events', function () {
+      expect(socket.on).to.be.calledWith('new-chat-message')
+    })
+
+    it('notifies an update event after new messages are received', function () {
+      const subscriber = sinon.stub()
+      store.on('updated', subscriber)
+      mockSocketMessage(testMessage)
+      expect(subscriber).to.be.calledOnce
+    })
+
+    it('can unsubscribe from events', function () {
+      const subscriber = sinon.stub()
+      store.on('updated', subscriber)
+      store.off('updated', subscriber)
+      mockSocketMessage(testMessage)
+      expect(subscriber).not.to.be.called
+    })
+
+    it('when the message is from other user, it is added to the messages list', function () {
+      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]
+      })
+    })
+
+    describe('messages sent by the user', function () {
+      beforeEach(function () {
+        fetchMock.post(/messages/, 204)
+      })
+
+      it('are not added to the message list', async function () {
+        await store.sendMessage(testMessage.content)
+        const originalMessageList = store.messages.slice(0)
+        mockSocketMessage(testMessage)
+        expect(originalMessageList).to.deep.equal(store.messages)
+
+        // 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]
+        })
+      })
+
+      it("don't notify an update event after new messages are received", async function () {
+        await store.sendMessage(testMessage.content)
+
+        const subscriber = sinon.stub()
+        store.on('updated', subscriber)
+        mockSocketMessage(testMessage)
+
+        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')
+      })
+    })
+  })
+
+  describe('loadMoreMessages()', function () {
+    it('aborts the request when the entire message list is loaded', async function () {
+      store.atEnd = true
+      await store.loadMoreMessages()
+      expect(fetchMock.calls().length).to.equal(0)
+      expect(store.loading).to.equal(false)
+    })
+
+    it('updates the list of messages', async function () {
+      const originalMessageList = store.messages.slice(0)
+      fetchMock.get(/messages/, [testMessage])
+      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]
+      })
+    })
+
+    it('notifies an update event for when the loading starts, and a second one once data is available', async function () {
+      const subscriber = sinon.stub()
+      store.on('updated', subscriber)
+      fetchMock.get(/messages/, [testMessage])
+      await store.loadMoreMessages()
+      expect(subscriber).to.be.calledTwice
+    })
+
+    it('marks `atEnd` flag to true when there are no more messages to retrieve', async function () {
+      expect(store.atEnd).to.equal(false)
+      fetchMock.get(/messages/, [testMessage])
+      await store.loadMoreMessages()
+      expect(store.atEnd).to.equal(true)
+    })
+
+    it('marks `atEnd` flag to false when there are still messages to retrieve', async function () {
+      const messages = []
+      for (let i = 0; i < MESSAGE_LIMIT; i++) {
+        messages.push({ ...testMessage, content: `message #${i}` })
+      }
+      expect(store.atEnd).to.equal(false)
+      fetchMock.get(/messages/, messages)
+      await store.loadMoreMessages()
+      expect(store.atEnd).to.equal(false)
+    })
+
+    it('subsequent requests for new messages start at the timestamp of the latest message', async function () {
+      const messages = []
+      for (let i = 0; i < MESSAGE_LIMIT - 1; i++) {
+        // sending enough messages so it doesn't mark `atEnd === true`
+        messages.push({ ...testMessage, content: `message #${i}` })
+      }
+
+      const timestamp = new Date().getTime()
+      messages.push({ ...testMessage, timestamp })
+
+      fetchMock.get(/messages/, messages)
+      await store.loadMoreMessages()
+
+      fetchMock.get(/messages/, [])
+      await store.loadMoreMessages()
+
+      expect(fetchMock.calls().length).to.equal(2)
+      const url = fetchMock.lastCall()[0]
+      expect(url).to.match(new RegExp(`&before=${timestamp}`))
+    })
+  })
+
+  describe('sendMessage()', function () {
+    beforeEach(function () {
+      fetchMock.post(/messages/, 204)
+    })
+
+    it('appends the message to the list', async function () {
+      const originalMessageList = store.messages.slice(0)
+      await store.sendMessage('a message')
+      expect(store.messages.length).to.equal(originalMessageList.length + 1)
+      const lastMessage = store.messages[store.messages.length - 1]
+      expect(lastMessage.contents).to.deep.equal(['a message'])
+      expect(lastMessage.user).to.deep.equal(user)
+      expect(lastMessage.timestamp).to.be.greaterThan(0)
+    })
+
+    it('notifies an update event', async function () {
+      const subscriber = sinon.stub()
+      store.on('updated', subscriber)
+      await store.sendMessage('a message')
+      expect(subscriber).to.be.calledOnce
+    })
+
+    it('sends an http POST request to the server', async function () {
+      await store.sendMessage('a message')
+      expect(fetchMock.calls().length).to.equal(1)
+      const body = fetchMock.lastCall()[1].body
+      expect(JSON.parse(body)).to.deep.equal({
+        content: 'a message',
+        _csrf: 'csrf_tok'
+      })
+    })
+
+    it('ignores empty messages', async function () {
+      const subscriber = sinon.stub()
+      store.on('updated', subscriber)
+      await store.sendMessage('')
+      await store.sendMessage(null)
+      expect(subscriber).not.to.be.called
+    })
+  })
+
+  describe('destroy', function () {
+    beforeEach(function () {
+      fetchMock.post(/messages/, 204)
+    })
+
+    it('removes event listeners', async function () {
+      const subscriber = sinon.stub()
+      store.on('updated', subscriber)
+      store.destroy()
+      await store.sendMessage('a message')
+      expect(subscriber).not.to.be.called
+    })
+  })
+})

services/web/test/frontend/features/chat/util/message-list-appender.test.js → services/web/test/frontend/features/chat/store/message-list-appender.test.js

@@ -2,7 +2,7 @@ import { expect } from 'chai'
 import {
   appendMessage,
   prependMessages
-} from '../../../../../frontend/js/features/chat/utils/message-list-appender'
+} from '../../../../../frontend/js/features/chat/store/message-list-appender'
 
 const testUser = {
   id: '123abc'

services/web/test/frontend/helpers/render-with-context.js

@@ -1,28 +1,19 @@
-// Disable prop type checks for test harnesses
-/* eslint-disable react/prop-types */
-
 import React from 'react'
 import { render } from '@testing-library/react'
-import sinon from 'sinon'
 import { ApplicationProvider } from '../../../frontend/js/shared/context/application-context'
 import { EditorProvider } from '../../../frontend/js/shared/context/editor-context'
-import { LayoutProvider } from '../../../frontend/js/shared/context/layout-context'
+import sinon from 'sinon'
 import { ChatProvider } from '../../../frontend/js/features/chat/context/chat-context'
+import { LayoutProvider } from '../../../frontend/js/shared/context/layout-context'
 
-export function EditorProviders({
-  user = { id: '123abd' },
-  projectId = 'project123',
-  socket = {
-    on: sinon.stub(),
-    removeListener: sinon.stub()
-  },
-  children
-}) {
+export function renderWithEditorContext(
+  children,
+  { user = { id: '123abd' }, projectId = 'project123' } = {}
+) {
   window.user = user || window.user
   window.ExposedSettings.appName = 'test'
   window.gitBridgePublicBaseUrl = 'git.overleaf.test'
   window.project_id = projectId != null ? projectId : window.project_id
-
   window._ide = {
     $scope: {
       project: {
@@ -36,9 +27,12 @@ export function EditorProviders({
       },
       $watch: () => {}
     },
-    socket
+    socket: {
+      on: sinon.stub(),
+      removeListener: sinon.stub()
+    }
   }
-  return (
+  return render(
     <ApplicationProvider>
       <EditorProvider ide={window._ide} settings={{}}>
         <LayoutProvider $scope={window._ide.$scope}>{children}</LayoutProvider>
@@ -47,20 +41,11 @@ export function EditorProviders({
   )
 }
 
-export function renderWithEditorContext(children, props) {
-  return render(<EditorProviders {...props}>{children}</EditorProviders>)
-}
-
-export function ChatProviders({ children, ...props }) {
-  return (
-    <EditorProviders {...props}>
-      <ChatProvider>{children}</ChatProvider>
-    </EditorProviders>
-  )
-}
-
-export function renderWithChatContext(children, props) {
-  return render(<ChatProviders {...props}>{children}</ChatProviders>)
+export function renderWithChatContext(children, { user, projectId } = {}) {
+  return renderWithEditorContext(<ChatProvider>{children}</ChatProvider>, {
+    user,
+    projectId
+  })
 }
 
 export function cleanUpContext() {