diff --git a/services/web/frontend/js/features/chat/components/chat-pane.js b/services/web/frontend/js/features/chat/components/chat-pane.js index 51bd7d2526..4cb830e6c0 100644 --- a/services/web/frontend/js/features/chat/components/chat-pane.js +++ b/services/web/frontend/js/features/chat/components/chat-pane.js @@ -1,11 +1,13 @@ -import React, { useEffect, useState } from 'react' +import React, { useEffect } 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' @@ -13,27 +15,26 @@ function ChatPane() { const { t } = useTranslation() const { chatIsOpen } = useLayoutContext({ chatIsOpen: PropTypes.bool }) + const { user } = useApplicationContext() const { - userId, - atEnd, - loading, - loadMoreMessages, + status, messages, + initialMessagesLoaded, + atEnd, + loadInitialMessages, + loadMoreMessages, sendMessage, - resetUnreadMessageCount + markMessagesAsRead } = useChatContext() - const [initialMessagesLoaded, setInitialMessagesLoaded] = useState(false) - useEffect(() => { if (chatIsOpen && !initialMessagesLoaded) { - loadMoreMessages() - setInitialMessagesLoaded(true) + loadInitialMessages() } - }, [initialMessagesLoaded, loadMoreMessages, chatIsOpen]) + }, [chatIsOpen, loadInitialMessages, initialMessagesLoaded]) - const shouldDisplayPlaceholder = !loading && messages.length === 0 + const shouldDisplayPlaceholder = status !== 'pending' && messages.length === 0 const messageContentCount = messages.reduce( (acc, { contents }) => acc + contents.length, @@ -46,22 +47,22 @@ function ChatPane() { atEnd={atEnd} className="messages" fetchData={loadMoreMessages} - isLoading={loading} + isLoading={status === 'pending'} itemCount={messageContentCount} >

{t('chat')}

- {loading && } + {status === 'pending' && } {shouldDisplayPlaceholder && }
diff --git a/services/web/frontend/js/features/chat/context/chat-context.js b/services/web/frontend/js/features/chat/context/chat-context.js index c5fe86dd38..66dcd00688 100644 --- a/services/web/frontend/js/features/chat/context/chat-context.js +++ b/services/web/frontend/js/features/chat/context/chat-context.js @@ -2,37 +2,116 @@ import React, { createContext, useCallback, useContext, - useState, - useEffect + useEffect, + useReducer, + useMemo } 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 { ChatStore } from '../store/chat-store' +import { getJSON, postJSON } from '../../../infrastructure/fetch-json' +import { appendMessage, prependMessages } from '../utils/message-list-appender' 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({ - userId: PropTypes.string.isRequired, - atEnd: PropTypes.bool, - loading: PropTypes.bool, + status: PropTypes.string.isRequired, messages: PropTypes.array.isRequired, + initialMessagesLoaded: PropTypes.bool.isRequired, + atEnd: PropTypes.bool.isRequired, unreadMessageCount: PropTypes.number.isRequired, - resetUnreadMessageCount: PropTypes.func.isRequired, + loadInitialMessages: PropTypes.func.isRequired, loadMoreMessages: PropTypes.func.isRequired, - sendMessage: PropTypes.func.isRequired + sendMessage: PropTypes.func.isRequired, + markMessagesAsRead: 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) }) @@ -42,68 +121,131 @@ export function ChatProvider({ children }) { const { chatIsOpen } = useLayoutContext({ chatIsOpen: PropTypes.bool }) - const [unreadMessageCount, setUnreadMessageCount] = useState(0) - function resetUnreadMessageCount() { - setUnreadMessageCount(0) - } + const { + hasFocus: windowHasFocus, + flashTitle, + stopFlashingTitle + } = useBrowserWindow() - const [atEnd, setAtEnd] = useState(false) - const [loading, setLoading] = useState(false) - const [messages, setMessages] = useState([]) + const [state, dispatch] = useReducer(chatReducer, initialState) - const [store] = useState(() => new ChatStore(user, projectId)) + 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 + + 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 + + dispatch({ type: 'RECEIVE_MESSAGE', message }) + } + + 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) { - setUnreadMessageCount(0) + markMessagesAsRead() } } - if (!windowHasFocus && unreadMessageCount > 0) { + if (!windowHasFocus && state.unreadMessageCount > 0) { flashTitle('New Message') } }, [ windowHasFocus, chatIsOpen, - unreadMessageCount, + state.unreadMessageCount, flashTitle, - 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 + stopFlashingTitle, + markMessagesAsRead ]) const value = { - userId: user.id, - atEnd, - loading, - messages, - unreadMessageCount, - resetUnreadMessageCount, + status: state.status, + messages: state.messages, + initialMessagesLoaded: state.initialMessagesLoaded, + atEnd: state.atEnd, + unreadMessageCount: state.unreadMessageCount, + loadInitialMessages, loadMoreMessages, - sendMessage + sendMessage, + markMessagesAsRead } return {children} diff --git a/services/web/frontend/js/features/chat/store/chat-store.js b/services/web/frontend/js/features/chat/store/chat-store.js deleted file mode 100644 index 11d1bb778f..0000000000 --- a/services/web/frontend/js/features/chat/store/chat-store.js +++ /dev/null @@ -1,101 +0,0 @@ -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 }) - } -} diff --git a/services/web/frontend/js/features/chat/store/message-list-appender.js b/services/web/frontend/js/features/chat/utils/message-list-appender.js similarity index 100% rename from services/web/frontend/js/features/chat/store/message-list-appender.js rename to services/web/frontend/js/features/chat/utils/message-list-appender.js diff --git a/services/web/package-lock.json b/services/web/package-lock.json index 46df6af542..02ea16ffcd 100644 --- a/services/web/package-lock.json +++ b/services/web/package-lock.json @@ -5461,6 +5461,46 @@ } } }, + "@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", @@ -5753,6 +5793,15 @@ "@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", @@ -5762,6 +5811,15 @@ "@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", @@ -14867,6 +14925,12 @@ } } }, + "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", diff --git a/services/web/package.json b/services/web/package.json index 626a3cc427..5803405d7c 100644 --- a/services/web/package.json +++ b/services/web/package.json @@ -174,6 +174,7 @@ "@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", 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 6377d879f5..0a0fe77ce7 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 @@ -52,7 +52,7 @@ describe('', 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(, { user }) diff --git a/services/web/test/frontend/features/chat/context/chat-context.test.js b/services/web/test/frontend/features/chat/context/chat-context.test.js new file mode 100644 index 0000000000..497ec28e93 --- /dev/null +++ b/services/web/test/frontend/features/chat/context/chat-context.test.js @@ -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 }) => ( + {children} + ) + }) +} + +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) +} 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 deleted file mode 100644 index d3c75e69a3..0000000000 --- a/services/web/test/frontend/features/chat/store/chat-store.test.js +++ /dev/null @@ -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 - }) - }) -}) diff --git a/services/web/test/frontend/features/chat/store/message-list-appender.test.js b/services/web/test/frontend/features/chat/util/message-list-appender.test.js similarity index 99% rename from services/web/test/frontend/features/chat/store/message-list-appender.test.js rename to services/web/test/frontend/features/chat/util/message-list-appender.test.js index 0a41dcf689..31a6a72782 100644 --- a/services/web/test/frontend/features/chat/store/message-list-appender.test.js +++ b/services/web/test/frontend/features/chat/util/message-list-appender.test.js @@ -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' diff --git a/services/web/test/frontend/helpers/render-with-context.js b/services/web/test/frontend/helpers/render-with-context.js index f0d9daa4a8..3e7952389f 100644 --- a/services/web/test/frontend/helpers/render-with-context.js +++ b/services/web/test/frontend/helpers/render-with-context.js @@ -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 ( {children} @@ -41,11 +47,20 @@ export function renderWithEditorContext( ) } -export function renderWithChatContext(children, { user, projectId } = {}) { - return renderWithEditorContext({children}, { - user, - projectId - }) +export function renderWithEditorContext(children, props) { + return render({children}) +} + +export function ChatProviders({ children, ...props }) { + return ( + + {children} + + ) +} + +export function renderWithChatContext(children, props) { + return render({children}) } export function cleanUpContext() {