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 4cb830e6c0..51bd7d2526 100644 --- a/services/web/frontend/js/features/chat/components/chat-pane.js +++ b/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} >

{t('chat')}

- {status === 'pending' && } + {loading && } {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 66dcd00688..c5fe86dd38 100644 --- a/services/web/frontend/js/features/chat/context/chat-context.js +++ b/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 [unreadMessageCount, setUnreadMessageCount] = useState(0) + function resetUnreadMessageCount() { + setUnreadMessageCount(0) + } - const [state, dispatch] = useReducer(chatReducer, initialState) + const [atEnd, setAtEnd] = useState(false) + const [loading, setLoading] = useState(false) + const [messages, setMessages] = useState([]) - const { loadInitialMessages, loadMoreMessages } = useMemo(() => { - function fetchMessages() { - if (state.atEnd) return + const [store] = useState(() => new ChatStore(user, projectId)) - 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) { - 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 {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 new file mode 100644 index 0000000000..11d1bb778f --- /dev/null +++ b/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 }) + } +} diff --git a/services/web/frontend/js/features/chat/utils/message-list-appender.js b/services/web/frontend/js/features/chat/store/message-list-appender.js similarity index 100% rename from services/web/frontend/js/features/chat/utils/message-list-appender.js rename to services/web/frontend/js/features/chat/store/message-list-appender.js diff --git a/services/web/package-lock.json b/services/web/package-lock.json index 02ea16ffcd..46df6af542 100644 --- a/services/web/package-lock.json +++ b/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", diff --git a/services/web/package.json b/services/web/package.json index 5803405d7c..626a3cc427 100644 --- a/services/web/package.json +++ b/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", 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 0a0fe77ce7..6377d879f5 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 deleted file mode 100644 index 497ec28e93..0000000000 --- a/services/web/test/frontend/features/chat/context/chat-context.test.js +++ /dev/null @@ -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 }) => ( - {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 new file mode 100644 index 0000000000..d3c75e69a3 --- /dev/null +++ b/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 + }) + }) +}) diff --git a/services/web/test/frontend/features/chat/util/message-list-appender.test.js b/services/web/test/frontend/features/chat/store/message-list-appender.test.js similarity index 99% rename from services/web/test/frontend/features/chat/util/message-list-appender.test.js rename to services/web/test/frontend/features/chat/store/message-list-appender.test.js index 31a6a72782..0a41dcf689 100644 --- a/services/web/test/frontend/features/chat/util/message-list-appender.test.js +++ b/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' diff --git a/services/web/test/frontend/helpers/render-with-context.js b/services/web/test/frontend/helpers/render-with-context.js index 3e7952389f..f0d9daa4a8 100644 --- a/services/web/test/frontend/helpers/render-with-context.js +++ b/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( {children} @@ -47,20 +41,11 @@ export function EditorProviders({ ) } -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 renderWithChatContext(children, { user, projectId } = {}) { + return renderWithEditorContext({children}, { + user, + projectId + }) } export function cleanUpContext() {