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

Refactor chat store to use React state

GitOrigin-RevId: 800a21c3c8a5c3c628c0a13bcb091675d1fb6f25
This commit is contained in:
Alasdair Smith
2021-04-23 02:09:54 +00:00
committed by Copybot
parent 59fe11f0ff
commit bb4523b7f6
11 changed files with 783 additions and 436 deletions
@@ -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 })
@@ -0,0 +1,468 @@
// 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)
}
@@ -1,243 +0,0 @@
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
})
})
})
@@ -2,7 +2,7 @@ import { expect } from 'chai'
import {
appendMessage,
prependMessages
} from '../../../../../frontend/js/features/chat/store/message-list-appender'
} from '../../../../../frontend/js/features/chat/utils/message-list-appender'
const testUser = {
id: '123abc'
@@ -1,19 +1,28 @@
// 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 sinon from 'sinon'
import { ChatProvider } from '../../../frontend/js/features/chat/context/chat-context'
import { LayoutProvider } from '../../../frontend/js/shared/context/layout-context'
import { ChatProvider } from '../../../frontend/js/features/chat/context/chat-context'
export function renderWithEditorContext(
children,
{ user = { id: '123abd' }, projectId = 'project123' } = {}
) {
export function EditorProviders({
user = { id: '123abd' },
projectId = 'project123',
socket = {
on: sinon.stub(),
removeListener: sinon.stub()
},
children
}) {
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: {
@@ -27,12 +36,9 @@ export function renderWithEditorContext(
},
$watch: () => {}
},
socket: {
on: sinon.stub(),
removeListener: sinon.stub()
}
socket
}
return render(
return (
<ApplicationProvider>
<EditorProvider ide={window._ide} settings={{}}>
<LayoutProvider $scope={window._ide.$scope}>{children}</LayoutProvider>
@@ -41,11 +47,20 @@ export function renderWithEditorContext(
)
}
export function renderWithChatContext(children, { user, projectId } = {}) {
return renderWithEditorContext(<ChatProvider>{children}</ChatProvider>, {
user,
projectId
})
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 cleanUpContext() {