From c22e44438e0834ff7162e4d3017b5446dac27c09 Mon Sep 17 00:00:00 2001 From: Domagoj Kriskovic Date: Wed, 1 Oct 2025 10:22:57 +0200 Subject: [PATCH] Support for deleting and editing chat messages (#28204) * Initial server-side delete of chat message plus dropdown * Update chat pane after deleting message * Chat message dropdown styling * Add confirmation dialog for deleting a message * Refactor chat message grouping to allow deletion of individual messages * Delete other user's deleted message from chat pane * Implement message editing * Styling * Make the dropdown appear overlap with the button slightly so that the menu stays visible when the user moves their cursor into the menu when the menu is positioned above the button * Submit edit with Enter key * Add edited indicator to edited chat messages * Add animation to chat message deletion * Tidying, edit chat message textarea improvements * Add types to message-list-utils * update dependencies * edit/delete for ide-redesign * fix type errors in tests * filter deleted messages from group * promisify ChatController * fix tests and translations * add new tests * chat-context tests * fix message-list-appender tests * add new tests for message-list-utils * Update services/web/test/frontend/features/chat/context/chat-context.test.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * preserve original content when canceling edits * update delete message translation * hide dropdown only if not already shown * remove delete animation * fix lint error * fix chat.yaml * hide under feature flag --------- Co-authored-by: Tim Down <158919+timdown@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> GitOrigin-RevId: 12521886a1a59ccd564851df19e5d46c70d328f5 --- .../Messages/MessageHttpController.js | 41 ++ services/chat/chat.yaml | 47 ++ .../app/src/Features/Chat/ChatApiHandler.js | 21 + .../app/src/Features/Chat/ChatController.mjs | 41 ++ .../Features/Project/ProjectController.mjs | 1 + services/web/app/src/router.mjs | 14 + .../web/frontend/extracted-translations.json | 3 + .../js/features/chat/components/chat-pane.tsx | 5 +- .../chat/components/message-and-dropdown.tsx | 40 ++ .../chat/components/message-content.tsx | 81 +++- .../chat/components/message-dropdown.tsx | 59 +++ .../chat/components/message-group.tsx | 62 +++ .../features/chat/components/message-list.tsx | 71 ++- .../js/features/chat/components/message.tsx | 56 --- .../js/features/chat/context/chat-context.tsx | 214 ++++++++- .../chat/utils/message-list-appender.ts | 91 ---- .../features/chat/utils/message-list-utils.ts | 134 ++++++ .../ide-redesign/components/chat/chat.tsx | 7 +- .../components/chat/message-and-dropdown.tsx | 92 ++++ .../components/chat/message-group.tsx | 43 ++ .../ide-redesign/components/chat/message.tsx | 87 ---- .../components/auto-expanding-text-area.tsx | 2 +- .../components/types/dropdown-menu-props.ts | 2 + .../stylesheets/pages/editor/chat.scss | 70 +++ services/web/locales/en.json | 2 + .../chat/components/message-group.test.tsx | 250 ++++++++++ .../chat/components/message-list.test.tsx | 130 +++++- .../features/chat/components/message.test.tsx | 120 ----- .../chat/context/chat-context.test.tsx | 134 ++++-- .../chat/util/message-list-appender.test.tsx | 271 ----------- .../chat/util/message-list-utils.test.tsx | 429 ++++++++++++++++++ 31 files changed, 1918 insertions(+), 702 deletions(-) create mode 100644 services/web/frontend/js/features/chat/components/message-and-dropdown.tsx create mode 100644 services/web/frontend/js/features/chat/components/message-dropdown.tsx create mode 100644 services/web/frontend/js/features/chat/components/message-group.tsx delete mode 100644 services/web/frontend/js/features/chat/components/message.tsx delete mode 100644 services/web/frontend/js/features/chat/utils/message-list-appender.ts create mode 100644 services/web/frontend/js/features/chat/utils/message-list-utils.ts create mode 100644 services/web/frontend/js/features/ide-redesign/components/chat/message-and-dropdown.tsx create mode 100644 services/web/frontend/js/features/ide-redesign/components/chat/message-group.tsx delete mode 100644 services/web/frontend/js/features/ide-redesign/components/chat/message.tsx create mode 100644 services/web/test/frontend/features/chat/components/message-group.test.tsx delete mode 100644 services/web/test/frontend/features/chat/components/message.test.tsx delete mode 100644 services/web/test/frontend/features/chat/util/message-list-appender.test.tsx create mode 100644 services/web/test/frontend/features/chat/util/message-list-utils.test.tsx diff --git a/services/chat/app/js/Features/Messages/MessageHttpController.js b/services/chat/app/js/Features/Messages/MessageHttpController.js index 13e8f3ab61..1a14ae66a0 100644 --- a/services/chat/app/js/Features/Messages/MessageHttpController.js +++ b/services/chat/app/js/Features/Messages/MessageHttpController.js @@ -78,6 +78,10 @@ export async function editMessage(context) { return await callMessageHttpController(context, _editMessage) } +export async function editGlobalMessage(context) { + return await callMessageHttpController(context, _editGlobalMessage) +} + export async function deleteMessage(context) { return await callMessageHttpController(context, _deleteMessage) } @@ -86,6 +90,10 @@ export async function deleteUserMessage(context) { return await callMessageHttpController(context, _deleteUserMessage) } +export async function deleteGlobalMessage(context) { + return await callMessageHttpController(context, _deleteGlobalMessage) +} + export async function getResolvedThreadIds(context) { return await callMessageHttpController(context, _getResolvedThreadIds) } @@ -242,6 +250,28 @@ const _editMessage = async (req, res) => { res.status(204) } +const _editGlobalMessage = async (req, res) => { + const { content, userId } = req.body + const { projectId, messageId } = req.params + logger.debug({ projectId, messageId, content }, 'editing global message') + const room = await ThreadManager.findOrCreateThread( + projectId, + ThreadManager.GLOBAL_THREAD + ) + const found = await MessageManager.updateMessage( + room._id, + messageId, + userId, + content, + Date.now() + ) + if (!found) { + res.status(404) + return + } + res.status(204) +} + const _deleteMessage = async (req, res) => { const { projectId, threadId, messageId } = req.params logger.debug({ projectId, threadId, messageId }, 'deleting message') @@ -257,6 +287,16 @@ const _deleteUserMessage = async (req, res) => { res.status(204) } +const _deleteGlobalMessage = async (req, res) => { + const { projectId, messageId } = req.params + const room = await ThreadManager.findOrCreateThread( + projectId, + ThreadManager.GLOBAL_THREAD + ) + await MessageManager.deleteMessage(room._id, messageId) + res.status(204) +} + const _getResolvedThreadIds = async (req, res) => { const { projectId } = req.params const resolvedThreadIds = await ThreadManager.getResolvedThreadIds(projectId) @@ -307,6 +347,7 @@ async function _sendMessage(userId, projectId, content, clientThreadId, res) { ) message = MessageFormatter.formatMessageForClientSide(message) message.room_id = projectId + res.status(201).setBody(message) } diff --git a/services/chat/chat.yaml b/services/chat/chat.yaml index 455921a0b4..8e13031ff3 100644 --- a/services/chat/chat.yaml +++ b/services/chat/chat.yaml @@ -82,6 +82,13 @@ paths: description: Message not found operationId: getGlobalMessage description: Get a single global message by message ID for the project with Project ID provided + delete: + summary: Delete global message + operationId: deleteGlobalMessage + responses: + '204': + description: No Content + description: 'Delete global message' '/project/{projectId}/thread/{threadId}/messages': parameters: - schema: @@ -179,6 +186,46 @@ paths: - user_id: Id of the user (optional) description: | Update message with Message ID provided from the Thread ID and Project ID provided + '/project/{projectId}/messages/{messageId}/edit': + parameters: + - schema: + type: string + name: projectId + in: path + required: true + - schema: + type: string + name: messageId + in: path + required: true + post: + summary: Edit global message + operationId: editGlobalMessage + responses: + '204': + description: No Content + '404': + description: Not Found + requestBody: + content: + application/json: + schema: + type: object + properties: + content: + type: string + user_id: + type: string + readOnly: true + required: + - content + examples: {} + description: |- + JSON object with : + - content: Content of the message to edit + - user_id: Id of the user (optional) + description: | + Update global message with Message ID provided from the Project ID provided '/project/{projectId}/thread/{threadId}/messages/{messageId}': parameters: - schema: diff --git a/services/web/app/src/Features/Chat/ChatApiHandler.js b/services/web/app/src/Features/Chat/ChatApiHandler.js index 7a4f4df31c..c3be2746bc 100644 --- a/services/web/app/src/Features/Chat/ChatApiHandler.js +++ b/services/web/app/src/Features/Chat/ChatApiHandler.js @@ -91,6 +91,16 @@ async function editMessage(projectId, threadId, messageId, userId, content) { ) } +async function editGlobalMessage(projectId, messageId, userId, content) { + await fetchNothing( + chatApiUrl(`/project/${projectId}/messages/${messageId}/edit`), + { + method: 'POST', + json: { content, userId }, + } + ) +} + async function deleteMessage(projectId, threadId, messageId) { await fetchNothing( chatApiUrl( @@ -109,6 +119,13 @@ async function deleteUserMessage(projectId, threadId, userId, messageId) { ) } +async function deleteGlobalMessage(projectId, messageId) { + await fetchNothing( + chatApiUrl(`/project/${projectId}/messages/${messageId}`), + { method: 'DELETE' } + ) +} + async function getResolvedThreadIds(projectId) { const body = await fetchJson( chatApiUrl(`/project/${projectId}/resolved-thread-ids`) @@ -154,8 +171,10 @@ module.exports = { reopenThread: callbackify(reopenThread), deleteThread: callbackify(deleteThread), editMessage: callbackify(editMessage), + editGlobalMessage: callbackify(editGlobalMessage), deleteMessage: callbackify(deleteMessage), deleteUserMessage: callbackify(deleteUserMessage), + deleteGlobalMessage: callbackify(deleteGlobalMessage), getResolvedThreadIds: callbackify(getResolvedThreadIds), duplicateCommentThreads: callbackify(duplicateCommentThreads), generateThreadData: callbackify(generateThreadData), @@ -171,8 +190,10 @@ module.exports = { reopenThread, deleteThread, editMessage, + editGlobalMessage, deleteMessage, deleteUserMessage, + deleteGlobalMessage, getResolvedThreadIds, duplicateCommentThreads, generateThreadData, diff --git a/services/web/app/src/Features/Chat/ChatController.mjs b/services/web/app/src/Features/Chat/ChatController.mjs index 9470bf2b32..1db9cef89f 100644 --- a/services/web/app/src/Features/Chat/ChatController.mjs +++ b/services/web/app/src/Features/Chat/ChatController.mjs @@ -48,7 +48,48 @@ async function getMessages(req, res) { res.json(messages) } +async function deleteMessage(req, res) { + const { project_id: projectId, message_id: messageId } = req.params + const userId = SessionManager.getLoggedInUserId(req.session) + if (userId == null) { + throw new Error('no logged-in user') + } + + await ChatApiHandler.promises.deleteGlobalMessage(projectId, messageId) + + EditorRealTimeController.emitToRoom(projectId, 'delete-global-message', { + messageId, + userId, + }) + res.sendStatus(204) +} + +async function editMessage(req, res, next) { + const { project_id: projectId, message_id: messageId } = req.params + const { content } = req.body + const userId = SessionManager.getLoggedInUserId(req.session) + if (userId == null) { + throw new Error('no logged-in user') + } + + await ChatApiHandler.promises.editGlobalMessage( + projectId, + messageId, + userId, + content + ) + + EditorRealTimeController.emitToRoom(projectId, 'edit-global-message', { + messageId, + userId, + content, + }) + res.sendStatus(204) +} + export default { sendMessage: expressify(sendMessage), getMessages: expressify(getMessages), + deleteMessage: expressify(deleteMessage), + editMessage: expressify(editMessage), } diff --git a/services/web/app/src/Features/Project/ProjectController.mjs b/services/web/app/src/Features/Project/ProjectController.mjs index e17edd1512..4d9adf4151 100644 --- a/services/web/app/src/Features/Project/ProjectController.mjs +++ b/services/web/app/src/Features/Project/ProjectController.mjs @@ -401,6 +401,7 @@ const _ProjectController = { 'client-side-references', 'editor-redesign-new-users', 'writefull-frontend-migration', + 'chat-edit-delete', ].filter(Boolean) const getUserValues = async userId => diff --git a/services/web/app/src/router.mjs b/services/web/app/src/router.mjs index 65cedfca79..a1b6f9abc5 100644 --- a/services/web/app/src/router.mjs +++ b/services/web/app/src/router.mjs @@ -979,6 +979,20 @@ async function initialize(webRouter, privateApiRouter, publicApiRouter) { RateLimiterMiddleware.rateLimit(rateLimiters.sendChatMessage), ChatController.sendMessage ) + webRouter.delete( + '/project/:project_id/messages/:message_id', + AuthorizationMiddleware.blockRestrictedUserFromProject, + AuthorizationMiddleware.ensureUserCanReadProject, + PermissionsController.requirePermission('chat'), + ChatController.deleteMessage + ) + webRouter.post( + '/project/:project_id/messages/:message_id/edit', + AuthorizationMiddleware.blockRestrictedUserFromProject, + AuthorizationMiddleware.ensureUserCanReadProject, + PermissionsController.requirePermission('chat'), + ChatController.editMessage + ) } webRouter.post( diff --git a/services/web/frontend/extracted-translations.json b/services/web/frontend/extracted-translations.json index 9e6b2ca6b9..6e773780f0 100644 --- a/services/web/frontend/extracted-translations.json +++ b/services/web/frontend/extracted-translations.json @@ -397,6 +397,8 @@ "delete_comment_thread": "", "delete_comment_thread_message": "", "delete_figure": "", + "delete_message": "", + "delete_message_confirmation": "", "delete_projects": "", "delete_row_or_column": "", "delete_sso_config": "", @@ -503,6 +505,7 @@ "edit_tag": "", "edit_tag_name": "", "edit_your_custom_dictionary": "", + "edited": "", "editing": "", "editing_captions": "", "editing_tools": "", diff --git a/services/web/frontend/js/features/chat/components/chat-pane.tsx b/services/web/frontend/js/features/chat/components/chat-pane.tsx index 48ae6b6f6e..3397e1a722 100644 --- a/services/web/frontend/js/features/chat/components/chat-pane.tsx +++ b/services/web/frontend/js/features/chat/components/chat-pane.tsx @@ -43,10 +43,7 @@ const ChatPane = React.memo(function ChatPane() { const shouldDisplayPlaceholder = status !== 'pending' && messages.length === 0 - const messageContentCount = messages.reduce( - (acc, { contents }) => acc + contents.length, - 0 - ) + const messageContentCount = messages.length // Keep the chat pane in the DOM to avoid resetting the form input and re-rendering MathJax content. const [chatOpenedOnce, setChatOpenedOnce] = useState(chatIsOpen) diff --git a/services/web/frontend/js/features/chat/components/message-and-dropdown.tsx b/services/web/frontend/js/features/chat/components/message-and-dropdown.tsx new file mode 100644 index 0000000000..889a207bb5 --- /dev/null +++ b/services/web/frontend/js/features/chat/components/message-and-dropdown.tsx @@ -0,0 +1,40 @@ +import { + Message as MessageType, + useChatContext, +} from '@/features/chat/context/chat-context' +import classNames from 'classnames' +import MessageDropdown from '@/features/chat/components/message-dropdown' +import MessageContent from '@/features/chat/components/message-content' +import { useFeatureFlag } from '@/shared/context/split-test-context' + +export function MessageAndDropdown({ + message, + fromSelf, +}: { + message: MessageType + fromSelf: boolean +}) { + const { idOfMessageBeingEdited } = useChatContext() + const hasChatEditDelete = useFeatureFlag('chat-edit-delete') + + const editing = idOfMessageBeingEdited === message.id + + return ( +
+ {hasChatEditDelete && fromSelf && !message.pending && !editing ? ( + + ) : null} +
+ +
+
+ ) +} diff --git a/services/web/frontend/js/features/chat/components/message-content.tsx b/services/web/frontend/js/features/chat/components/message-content.tsx index 3d86b4be68..4e60ec06dd 100644 --- a/services/web/frontend/js/features/chat/components/message-content.tsx +++ b/services/web/frontend/js/features/chat/components/message-content.tsx @@ -1,12 +1,26 @@ -import { useRef, useEffect, type FC } from 'react' +import { useRef, useEffect, type FC, useCallback, useState } from 'react' import Linkify from 'react-linkify' import useIsMounted from '../../../shared/hooks/use-is-mounted' import { loadMathJax } from '../../mathjax/load-mathjax' import { debugConsole } from '@/utils/debugging' +import { Message, useChatContext } from '@/features/chat/context/chat-context' +import OLButton from '@/shared/components/ol/ol-button' +import { useTranslation } from 'react-i18next' +import AutoExpandingTextArea from '@/shared/components/auto-expanding-text-area' -const MessageContent: FC<{ content: string }> = ({ content }) => { +const MessageContent: FC<{ + content: Message['content'] + messageId: Message['id'] + edited: Message['edited'] +}> = ({ content, messageId, edited }) => { + const { t } = useTranslation() const root = useRef(null) const mounted = useIsMounted() + const { idOfMessageBeingEdited, cancelMessageEdit, editMessage } = + useChatContext() + const [editedContent, setEditedContent] = useState(content) + + const editing = idOfMessageBeingEdited === messageId useEffect(() => { if (root.current) { @@ -33,9 +47,70 @@ const MessageContent: FC<{ content: string }> = ({ content }) => { } }, [content, mounted]) - return ( + const completeEdit = useCallback(() => { + editMessage(messageId, editedContent) + }, [editMessage, editedContent, messageId]) + + const handleKeyDown = useCallback( + (e: React.KeyboardEvent) => { + if (e.key === 'Enter') { + e.preventDefault() + completeEdit() + } else if (e.key === 'Escape') { + e.preventDefault() + cancelMessageEdit() + setEditedContent(content) + } + }, + [cancelMessageEdit, completeEdit, content] + ) + + const handleChange = useCallback( + (e: React.ChangeEvent) => { + setEditedContent(e.target.value) + }, + [] + ) + + const handleAutoFocus = useCallback( + (textarea: HTMLTextAreaElement) => textarea.select(), + [] + ) + + return editing ? ( + <> + +
+ { + cancelMessageEdit() + setEditedContent(content) + }} + > + {t('cancel')} + + completeEdit()}> + {t('save')} + + + ) : (

{content} + {edited ? ( + <> + {' '} + ({t('edited')}) + + ) : null}

) } diff --git a/services/web/frontend/js/features/chat/components/message-dropdown.tsx b/services/web/frontend/js/features/chat/components/message-dropdown.tsx new file mode 100644 index 0000000000..505d84922b --- /dev/null +++ b/services/web/frontend/js/features/chat/components/message-dropdown.tsx @@ -0,0 +1,59 @@ +import { + Dropdown, + DropdownItem, + DropdownMenu, + DropdownToggle, +} from '@/shared/components/dropdown/dropdown-menu' +import MaterialIcon from '@/shared/components/material-icon' +import { useTranslation } from 'react-i18next' +import DropdownListItem from '@/shared/components/dropdown/dropdown-list-item' +import { Message, useChatContext } from '@/features/chat/context/chat-context' +import { useModalsContext } from '@/features/ide-react/context/modals-context' +import { useCallback } from 'react' + +export default function MessageDropdown({ message }: { message: Message }) { + const { t } = useTranslation() + const { deleteMessage, startedEditingMessage } = useChatContext() + + const { showGenericConfirmModal } = useModalsContext() + + const deleteButtonHandler = useCallback(() => { + showGenericConfirmModal({ + title: t('delete_message'), + message: t('delete_message_confirmation'), + onConfirm: () => { + deleteMessage(message.id) + }, + }) + }, [deleteMessage, message.id, showGenericConfirmModal, t]) + + const editButtonHandler = useCallback(() => { + startedEditingMessage(message.id) + }, [message.id, startedEditingMessage]) + + return ( + + + + + + + + {t('edit')} + + + {t('delete')} + + + + + ) +} diff --git a/services/web/frontend/js/features/chat/components/message-group.tsx b/services/web/frontend/js/features/chat/components/message-group.tsx new file mode 100644 index 0000000000..a48acddcba --- /dev/null +++ b/services/web/frontend/js/features/chat/components/message-group.tsx @@ -0,0 +1,62 @@ +import { getHueForUserId } from '@/shared/utils/colors' +import type { Message as MessageType } from '@/features/chat/context/chat-context' +import { User } from '../../../../../types/user' +import classNames from 'classnames' +import { MessageAndDropdown } from '@/features/chat/components/message-and-dropdown' +import { useTranslation } from 'react-i18next' + +export interface MessageGroupProps { + messages: MessageType[] + user?: User + fromSelf: boolean +} + +function hue(user?: User) { + return user ? getHueForUserId(user.id) : 0 +} + +function getMessageStyle(user?: User) { + return { + borderColor: `hsl(${hue(user)}, 85%, 40%)`, + backgroundColor: `hsl(${hue(user)}, 85%, 40%`, + } +} + +function getArrowStyle(user?: User) { + return { + borderColor: `hsl(${hue(user)}, 85%, 40%)`, + } +} + +function MessageGroup({ messages, user, fromSelf }: MessageGroupProps) { + const { t } = useTranslation() + + return ( +
+ {!fromSelf && ( +
+ + {user ? user.first_name || user.email : t('deleted_user')} + +
+ )} +
+ {!fromSelf &&
} + + {messages.map(message => ( + + ))} +
+
+ ) +} + +export default MessageGroup diff --git a/services/web/frontend/js/features/chat/components/message-list.tsx b/services/web/frontend/js/features/chat/components/message-list.tsx index a87618d3c0..d541a3c4f3 100644 --- a/services/web/frontend/js/features/chat/components/message-list.tsx +++ b/services/web/frontend/js/features/chat/components/message-list.tsx @@ -1,10 +1,12 @@ import moment from 'moment' -import Message from './message' import type { Message as MessageType } from '@/features/chat/context/chat-context' -import MessageRedesign from '@/features/ide-redesign/components/chat/message' import { useUserContext } from '@/shared/context/user-context' +import { User } from '../../../../../types/user' +import MessageGroup from '@/features/chat/components/message-group' +import MessageGroupRedesign from '@/features/ide-redesign/components/chat/message-group' const FIVE_MINUTES = 5 * 60 * 1000 +const TIMESTAMP_GROUP_SIZE = FIVE_MINUTES function formatTimestamp(date: moment.MomentInput) { if (!date) { @@ -20,13 +22,56 @@ interface MessageListProps { newDesign?: boolean } +type MessageGroupType = { + messages: MessageType[] + id: string + user?: User +} + +// Group messages by the same author that were sent within 5 minutes of each +// other +function groupMessages(messages: MessageType[]) { + const groups: MessageGroupType[] = [] + let currentGroup: MessageGroupType | null = null + let previousMessage: MessageType | null = null + + for (const message of messages) { + if (message.deleted) { + continue + } + if ( + currentGroup && + previousMessage && + !message.pending && + message.user && + message.user.id && + message.user.id === previousMessage.user?.id && + message.timestamp - previousMessage.timestamp < TIMESTAMP_GROUP_SIZE + ) { + currentGroup.messages.push(message) + } else { + currentGroup = { + messages: [message], + id: String(message.timestamp), + user: message.user, + } + groups.push(currentGroup) + } + previousMessage = message + } + + return groups +} + function MessageList({ messages, resetUnreadMessages, newDesign, }: MessageListProps) { const user = useUserContext() - const MessageComponent = newDesign ? MessageRedesign : Message + + const MessageGroupComponent = newDesign ? MessageGroupRedesign : MessageGroup + function shouldRenderDate(messageIndex: number) { if (messageIndex === 0) { return true @@ -41,6 +86,8 @@ function MessageList({ } } + const messageGroups = groupMessages(messages) + return ( // eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions
    - {messages.map((message, index) => ( - // new messages are added to the beginning of the list, so we use a reversed index -
  • + {messageGroups.map((group, index) => ( +
  • {shouldRenderDate(index) && (
    )} -
    • ))} diff --git a/services/web/frontend/js/features/chat/components/message.tsx b/services/web/frontend/js/features/chat/components/message.tsx deleted file mode 100644 index dd5aeccb24..0000000000 --- a/services/web/frontend/js/features/chat/components/message.tsx +++ /dev/null @@ -1,56 +0,0 @@ -import { getHueForUserId } from '@/shared/utils/colors' -import MessageContent from './message-content' -import type { Message as MessageType } from '@/features/chat/context/chat-context' -import { User } from '../../../../../types/user' -import { useTranslation } from 'react-i18next' - -export interface MessageProps { - message: MessageType - fromSelf: boolean -} - -function hue(user?: User) { - return user ? getHueForUserId(user.id) : 0 -} - -function getMessageStyle(user?: User) { - return { - borderColor: `hsl(${hue(user)}, 85%, 40%)`, - backgroundColor: `hsl(${hue(user)}, 85%, 40%`, - } -} - -function getArrowStyle(user?: User) { - return { - borderColor: `hsl(${hue(user)}, 85%, 40%)`, - } -} - -function Message({ message, fromSelf }: MessageProps) { - const { t } = useTranslation() - return ( -
    - {!fromSelf && ( -
    - - {message.user - ? message.user.first_name || message.user.email - : t('deleted_user')} - -
    - )} -
    - {!fromSelf && ( -
    - )} -
    - {message.contents.map((content, index) => ( - - ))} -
    -
    -
    - ) -} - -export default Message diff --git a/services/web/frontend/js/features/chat/context/chat-context.tsx b/services/web/frontend/js/features/chat/context/chat-context.tsx index 4b437378ba..dc57cee8af 100644 --- a/services/web/frontend/js/features/chat/context/chat-context.tsx +++ b/services/web/frontend/js/features/chat/context/chat-context.tsx @@ -11,8 +11,18 @@ import { import clientIdGenerator from '@/utils/client-id' import { useUserContext } from '../../../shared/context/user-context' import { useProjectContext } from '../../../shared/context/project-context' -import { getJSON, postJSON } from '../../../infrastructure/fetch-json' -import { appendMessage, prependMessages } from '../utils/message-list-appender' +import { + deleteJSON, + getJSON, + postJSON, +} from '../../../infrastructure/fetch-json' +import { + appendMessage, + confirmMessage, + deleteMessage, + editMessage, + prependMessages, +} from '../utils/message-list-utils' import useBrowserWindow from '../../../shared/hooks/use-browser-window' import { useLayoutContext } from '../../../shared/context/layout-context' import { useIdeContext } from '@/shared/context/ide-context' @@ -27,12 +37,16 @@ const PAGE_SIZE = 50 export type Message = { id: string timestamp: number - contents: string[] + content: string user?: User + edited?: boolean + deleted?: boolean + pending?: boolean } -export type ServerMessageEntry = Omit & { +export type ServerMessageEntry = Message & { content: string + edited_at?: number } type State = { @@ -44,6 +58,7 @@ type State = { unreadMessageCount: number error?: Error | null uniqueMessageIds: string[] + idOfMessageBeingEdited: Message['id'] | null } type Action = @@ -66,9 +81,29 @@ type Action = type: 'RECEIVE_MESSAGE' message: ServerMessageEntry } + | { + type: 'RECEIVE_OWN_MESSAGE' + message: any + } | { type: 'MARK_MESSAGES_AS_READ' } + | { + type: 'DELETE_MESSAGE' + messageId: Message['id'] + } + | { + type: 'START_EDITING_MESSAGE' + messageId: Message['id'] + } + | { + type: 'CANCEL_MESSAGE_EDIT' + } + | { + type: 'RECEIVE_MESSAGE_EDIT' + messageId: Message['id'] + content: string + } | { type: 'CLEAR' } @@ -125,6 +160,7 @@ function chatReducer(state: State, action: Action): State { user: action.user, content: action.content, timestamp: Date.now(), + pending: true, }, state.uniqueMessageIds ), @@ -141,6 +177,36 @@ function chatReducer(state: State, action: Action): State { unreadMessageCount: state.unreadMessageCount + 1, } + case 'RECEIVE_OWN_MESSAGE': + return { + ...state, + ...confirmMessage(action.message, state.messages), + } + + case 'DELETE_MESSAGE': + return { + ...state, + ...deleteMessage(action.messageId, state.messages), + } + + case 'START_EDITING_MESSAGE': + return { + ...state, + idOfMessageBeingEdited: action.messageId, + } + + case 'CANCEL_MESSAGE_EDIT': + return { + ...state, + idOfMessageBeingEdited: null, + } + + case 'RECEIVE_MESSAGE_EDIT': + return { + ...state, + ...editMessage(action.messageId, action.content, state.messages), + } + case 'MARK_MESSAGES_AS_READ': return { ...state, @@ -171,6 +237,7 @@ const initialState: State = { unreadMessageCount: 0, error: null, uniqueMessageIds: [], + idOfMessageBeingEdited: null, } export const ChatContext = createContext< @@ -180,10 +247,15 @@ export const ChatContext = createContext< initialMessagesLoaded: boolean atEnd: boolean unreadMessageCount: number + idOfMessageBeingEdited: State['idOfMessageBeingEdited'] loadInitialMessages: () => void loadMoreMessages: () => void sendMessage: (message: any) => void markMessagesAsRead: () => void + deleteMessage: (messageId: Message['id']) => void + startedEditingMessage: (messageId: Message['id']) => void + cancelMessageEdit: () => void + editMessage: (messageId: Message['id'], content: string) => void reset: () => void error?: Error | null } @@ -314,6 +386,32 @@ export const ChatProvider: FC = ({ children }) => { [chatEnabled, projectId, user] ) + const startedEditingMessage = useCallback( + (messageId: Message['id']) => { + if (!chatEnabled) { + debugConsole.warn(`chat is disabled, won't send message`) + return + } + + dispatch({ + type: 'START_EDITING_MESSAGE', + messageId, + }) + }, + [chatEnabled] + ) + + const cancelMessageEdit = useCallback(() => { + if (!chatEnabled) { + debugConsole.warn(`chat is disabled, won't cancel message edit`) + return + } + + dispatch({ + type: 'CANCEL_MESSAGE_EDIT', + }) + }, [chatEnabled]) + const markMessagesAsRead = useCallback(() => { if (!chatEnabled) { debugConsole.warn(`chat is disabled, won't mark messages as read`) @@ -322,26 +420,114 @@ export const ChatProvider: FC = ({ children }) => { dispatch({ type: 'MARK_MESSAGES_AS_READ' }) }, [chatEnabled]) - // Handling receiving messages over the socket + const deleteMessage = useCallback( + (messageId: Message['id']) => { + if (!chatEnabled) { + debugConsole.warn(`chat is disabled, won't delete message`) + return + } + if (!messageId) return + + dispatch({ + type: 'DELETE_MESSAGE', + messageId, + }) + + deleteJSON(`/project/${projectId}/messages/${messageId}`).catch(error => { + dispatch({ + type: 'ERROR', + error, + }) + }) + }, + [chatEnabled, projectId] + ) + + const editMessage = useCallback( + (messageId: Message['id'], content: string) => { + if (!chatEnabled) { + debugConsole.warn(`chat is disabled, won't edit message`) + return + } + if (!messageId || !content) return + + dispatch({ + type: 'RECEIVE_MESSAGE_EDIT', + messageId, + content, + }) + + dispatch({ + type: 'CANCEL_MESSAGE_EDIT', + }) + + postJSON(`/project/${projectId}/messages/${messageId}/edit`, { + body: { content }, + }).catch(error => { + dispatch({ + type: 'ERROR', + error, + }) + }) + }, + [chatEnabled, projectId] + ) + + // Handling receiving and deleting messages over the socket const { socket } = useIdeContext() useEffect(() => { if (!chatEnabled || !socket) return function receivedMessage(message: any) { - // If the message is from the current client id, then we are receiving the sent message back from the socket. + // If the message is from the current client id, then we are receiving the + // sent message back from the socket. In this case, we want to update the + // message in our local state with the ID of the message on the server. // Ignore it to prevent double message. - if (message.clientId === clientId.current) return + if (message.clientId === clientId.current) { + dispatch({ type: 'RECEIVE_OWN_MESSAGE', message }) + } else { + dispatch({ type: 'RECEIVE_MESSAGE', message }) + } + } - dispatch({ type: 'RECEIVE_MESSAGE', message }) + function deletedMessage(message: { + messageId: Message['id'] + userId: User['id'] + }) { + if (message.userId === user.id) return + + dispatch({ + type: 'DELETE_MESSAGE', + messageId: message.messageId, + }) + } + + function editedMessage(message: { + messageId: Message['id'] + userId: User['id'] + content: string + }) { + if (message.userId === user.id) return + + dispatch({ + type: 'RECEIVE_MESSAGE_EDIT', + messageId: message.messageId, + content: message.content, + }) } socket.on('new-chat-message', receivedMessage) + socket.on('delete-global-message', deletedMessage) + socket.on('edit-global-message', editedMessage) + return () => { if (!socket) return socket.removeListener('new-chat-message', receivedMessage) + socket.removeListener('delete-global-message', deletedMessage) + socket.removeListener('edit-global-message', editedMessage) } - }, [chatEnabled, socket]) + }, [chatEnabled, socket, user.id]) // Handle unread messages useEffect(() => { @@ -370,17 +556,26 @@ export const ChatProvider: FC = ({ children }) => { initialMessagesLoaded: state.initialMessagesLoaded, atEnd: state.atEnd, unreadMessageCount: state.unreadMessageCount, + idOfMessageBeingEdited: state.idOfMessageBeingEdited, loadInitialMessages, loadMoreMessages, reset, sendMessage, markMessagesAsRead, + deleteMessage, + startedEditingMessage, + cancelMessageEdit, + editMessage, error: state.error, }), [ loadInitialMessages, loadMoreMessages, markMessagesAsRead, + deleteMessage, + startedEditingMessage, + cancelMessageEdit, + editMessage, reset, sendMessage, state.atEnd, @@ -389,6 +584,7 @@ export const ChatProvider: FC = ({ children }) => { state.messages, state.status, state.unreadMessageCount, + state.idOfMessageBeingEdited, ] ) diff --git a/services/web/frontend/js/features/chat/utils/message-list-appender.ts b/services/web/frontend/js/features/chat/utils/message-list-appender.ts deleted file mode 100644 index 26a18150c8..0000000000 --- a/services/web/frontend/js/features/chat/utils/message-list-appender.ts +++ /dev/null @@ -1,91 +0,0 @@ -import { Message, ServerMessageEntry } from '../context/chat-context' - -const TIMESTAMP_GROUP_SIZE = 5 * 60 * 1000 // 5 minutes - -export function appendMessage( - messageList: Message[], - message: ServerMessageEntry, - uniqueMessageIds: string[] -) { - if (uniqueMessageIds.includes(message.id)) { - return { messages: messageList, uniqueMessageIds } - } - - uniqueMessageIds = uniqueMessageIds.slice(0) - - uniqueMessageIds.push(message.id) - - const lastMessage = messageList[messageList.length - 1] - - const shouldGroup = - lastMessage && - message && - message.user && - lastMessage.user && - message.user.id && - message.user.id === lastMessage.user.id && - message.timestamp - lastMessage.timestamp < TIMESTAMP_GROUP_SIZE - - if (shouldGroup) { - messageList = messageList.slice(0, messageList.length - 1).concat({ - ...lastMessage, - // the `id` is updated to the latest received content when a new - // message is appended or prepended - id: message.id, - timestamp: message.timestamp, - contents: lastMessage.contents.concat(message.content), - }) - } else { - messageList = messageList.slice(0).concat({ - id: message.id, - user: message.user, - timestamp: message.timestamp, - contents: [message.content], - }) - } - - return { messages: messageList, uniqueMessageIds } -} - -export function prependMessages( - messageList: Message[], - messages: ServerMessageEntry[], - uniqueMessageIds: string[] -) { - const listCopy = messageList.slice(0) - - uniqueMessageIds = uniqueMessageIds.slice(0) - - messages - .slice(0) - .reverse() - .forEach(message => { - if (uniqueMessageIds.includes(message.id)) { - return - } - uniqueMessageIds.push(message.id) - const firstMessage = listCopy[0] - const shouldGroup = - firstMessage && - message && - firstMessage.user && - message.user && - message.user.id === firstMessage.user.id && - firstMessage.timestamp - message.timestamp < TIMESTAMP_GROUP_SIZE - - if (shouldGroup) { - firstMessage.id = message.id - firstMessage.timestamp = message.timestamp - firstMessage.contents = [message.content].concat(firstMessage.contents) - } else { - listCopy.unshift({ - id: message.id, - user: message.user, - timestamp: message.timestamp, - contents: [message.content], - }) - } - }) - - return { messages: listCopy, uniqueMessageIds } -} diff --git a/services/web/frontend/js/features/chat/utils/message-list-utils.ts b/services/web/frontend/js/features/chat/utils/message-list-utils.ts new file mode 100644 index 0000000000..eecf2eb034 --- /dev/null +++ b/services/web/frontend/js/features/chat/utils/message-list-utils.ts @@ -0,0 +1,134 @@ +import { Message, ServerMessageEntry } from '../context/chat-context' + +export function appendMessage( + messageList: Message[], + message: ServerMessageEntry, + uniqueMessageIds: string[] +) { + if (uniqueMessageIds.includes(message.id)) { + return { messages: messageList, uniqueMessageIds } + } + + uniqueMessageIds = uniqueMessageIds.slice(0) + + uniqueMessageIds.push(message.id) + + messageList = messageList.slice(0).concat({ + id: message.id, + user: message.user, + timestamp: message.timestamp, + content: message.content, + pending: message.pending, + edited: Boolean(message.edited_at), + }) + + return { messages: messageList, uniqueMessageIds } +} + +export function prependMessages( + messageList: Message[], + messages: ServerMessageEntry[], + uniqueMessageIds: string[] +) { + const listCopy = messageList.slice(0) + + uniqueMessageIds = uniqueMessageIds.slice(0) + + messages + .slice(0) + .reverse() + .forEach(message => { + if (uniqueMessageIds.includes(message.id)) { + return + } + uniqueMessageIds.push(message.id) + + listCopy.unshift({ + id: message.id, + user: message.user, + timestamp: message.timestamp, + content: message.content, + edited: Boolean(message.edited_at), + }) + }) + + return { messages: listCopy, uniqueMessageIds } +} + +export function confirmMessage( + updatedMessage: Message, + messageList: Message[] +) { + // Find our message and change its ID from the temporary one we generated + // on creation to the ID assigned to it by the server. This is so that the + // message can be deleted later, for which we need the server ID. + const ownMessageIndex = messageList.findIndex( + message => message.pending && message.content === updatedMessage.content + ) + if (ownMessageIndex === -1) { + throw new Error("Couldn't find own message in local state") + } + const messageWithOldId = messageList[ownMessageIndex] + + const newMessageList = [...messageList] + newMessageList.splice(ownMessageIndex, 1, { + ...messageWithOldId, + pending: false, + id: updatedMessage.id, + user: updatedMessage.user, + timestamp: updatedMessage.timestamp, + content: updatedMessage.content, + }) + + return { + messages: newMessageList, + uniqueMessageIds: Array.from( + new Set(newMessageList.map(message => message.id)) + ), + } +} + +export function deleteMessage(messageId: string, messageList: Message[]) { + const messageIndex = messageList.findIndex( + message => message.id === messageId + ) + if (messageIndex === -1) { + throw new Error(`Message with id ${messageId} not found in message list`) + } + + const newMessageList = [...messageList] + const message = newMessageList[messageIndex] + newMessageList.splice(messageIndex, 1, { + ...message, + deleted: true, + }) + + return { + messages: newMessageList, + } +} + +export function editMessage( + messageId: string, + content: string, + messageList: Message[] +) { + const messageIndex = messageList.findIndex( + message => message.id === messageId + ) + if (messageIndex === -1) { + throw new Error(`Message with id ${messageId} not found in message list`) + } + + const newMessageList = [...messageList] + const message = newMessageList[messageIndex] + newMessageList.splice(messageIndex, 1, { + ...message, + content, + edited: true, + }) + + return { + messages: newMessageList, + } +} diff --git a/services/web/frontend/js/features/ide-redesign/components/chat/chat.tsx b/services/web/frontend/js/features/ide-redesign/components/chat/chat.tsx index 6bc8629982..0567792940 100644 --- a/services/web/frontend/js/features/ide-redesign/components/chat/chat.tsx +++ b/services/web/frontend/js/features/ide-redesign/components/chat/chat.tsx @@ -48,11 +48,6 @@ export const ChatPane = () => { const shouldDisplayPlaceholder = status !== 'pending' && messages.length === 0 - const messageContentCount = messages.reduce( - (acc, { contents }) => acc + contents.length, - 0 - ) - if (error) { // let user try recover from fetch errors if (error instanceof FetchError) { @@ -75,7 +70,7 @@ export const ChatPane = () => { className="messages" fetchData={loadMoreMessages} isLoading={status === 'pending'} - itemCount={messageContentCount} + itemCount={messages.length} >

    {t('chat')}

    diff --git a/services/web/frontend/js/features/ide-redesign/components/chat/message-and-dropdown.tsx b/services/web/frontend/js/features/ide-redesign/components/chat/message-and-dropdown.tsx new file mode 100644 index 0000000000..8f95e01ac3 --- /dev/null +++ b/services/web/frontend/js/features/ide-redesign/components/chat/message-and-dropdown.tsx @@ -0,0 +1,92 @@ +import type { Message } from '@/features/chat/context/chat-context' +import { User } from '../../../../../../types/user' +import { + getBackgroundColorForUserId, + hslStringToLuminance, +} from '@/shared/utils/colors' +import MessageContent from '@/features/chat/components/message-content' +import classNames from 'classnames' +import MaterialIcon from '@/shared/components/material-icon' +import MessageDropdown from '@/features/chat/components/message-dropdown' +import { useFeatureFlag } from '@/shared/context/split-test-context' + +function getAvatarStyle(user?: User) { + if (!user?.id) { + // Deleted user + return { + backgroundColor: 'var(--bg-light-disabled)', + borderColor: 'var(--bg-light-disabled)', + color: 'var(--content-disabled)', + } + } + + const backgroundColor = getBackgroundColorForUserId(user.id) + + return { + borderColor: backgroundColor, + backgroundColor, + color: + hslStringToLuminance(backgroundColor) < 0.5 + ? 'var(--content-primary-dark)' + : 'var(--content-primary)', + } +} + +export function MessageAndDropdown({ + message, + fromSelf, + isLast, + isFirst, +}: { + message: Message + fromSelf: boolean + isLast: boolean + isFirst: boolean +}) { + const hasChatEditDelete = useFeatureFlag('chat-edit-delete') + + return ( +
    + <> + {!fromSelf && isLast ? ( +
    +
    + {message.user?.id && message.user.email ? ( + message.user.first_name?.charAt(0) || + message.user.email.charAt(0) + ) : ( + + )} +
    +
    + ) : ( +
    + )} +
    +
    + {hasChatEditDelete && fromSelf ? ( + + ) : null} +
    +
    + +
    +
    + +
    + ) +} diff --git a/services/web/frontend/js/features/ide-redesign/components/chat/message-group.tsx b/services/web/frontend/js/features/ide-redesign/components/chat/message-group.tsx new file mode 100644 index 0000000000..6fd71f3ffb --- /dev/null +++ b/services/web/frontend/js/features/ide-redesign/components/chat/message-group.tsx @@ -0,0 +1,43 @@ +import { MessageGroupProps } from '@/features/chat/components/message-group' +import { MessageAndDropdown } from './message-and-dropdown' +import { useTranslation } from 'react-i18next' + +function MessageGroup({ messages, user, fromSelf }: MessageGroupProps) { + const { t } = useTranslation() + + return ( +
    +
    +
    +
    + {!fromSelf && ( +
    + + {user?.id && user.email + ? user.first_name || user.email + : t('deleted_user')} + +
    + )} +
    +
    + {messages.map((message, index) => { + const nonDeletedMessages = messages.filter(m => !m.deleted) + const nonDeletedIndex = nonDeletedMessages.findIndex( + m => m.id === message.id + ) + return ( + + ) + })} +
    + ) +} + +export default MessageGroup diff --git a/services/web/frontend/js/features/ide-redesign/components/chat/message.tsx b/services/web/frontend/js/features/ide-redesign/components/chat/message.tsx deleted file mode 100644 index 6822db39da..0000000000 --- a/services/web/frontend/js/features/ide-redesign/components/chat/message.tsx +++ /dev/null @@ -1,87 +0,0 @@ -import { MessageProps } from '@/features/chat/components/message' -import { User } from '../../../../../../types/user' -import { - getBackgroundColorForUserId, - hslStringToLuminance, -} from '@/shared/utils/colors' -import MessageContent from '@/features/chat/components/message-content' -import classNames from 'classnames' -import MaterialIcon from '@/shared/components/material-icon' -import { t } from 'i18next' - -function getAvatarStyle(user?: User) { - if (!user?.id) { - // Deleted user - return { - backgroundColor: 'var(--bg-light-disabled)', - borderColor: 'var(--bg-light-disabled)', - color: 'var(--content-disabled)', - } - } - - const backgroundColor = getBackgroundColorForUserId(user.id) - - return { - borderColor: backgroundColor, - backgroundColor, - color: - hslStringToLuminance(backgroundColor) < 0.5 - ? 'var(--content-primary-dark)' - : 'var(--content-primary)', - } -} - -function Message({ message, fromSelf }: MessageProps) { - return ( -
    -
    -
    - {!fromSelf && ( -
    - - {message.user?.id && message.user.email - ? message.user.first_name || message.user.email - : t('deleted_user')} - -
    - )} -
    - {message.contents.map((content, index) => ( -
    - <> - {!fromSelf && index === message.contents.length - 1 ? ( -
    -
    - {message.user?.id && message.user.email ? ( - message.user.first_name?.charAt(0) || - message.user.email.charAt(0) - ) : ( - - )} -
    -
    - ) : ( -
    - )} -
    -
    - -
    -
    - -
    - ))} -
    - ) -} - -export default Message diff --git a/services/web/frontend/js/shared/components/auto-expanding-text-area.tsx b/services/web/frontend/js/shared/components/auto-expanding-text-area.tsx index 09634a909c..0dafb68561 100644 --- a/services/web/frontend/js/shared/components/auto-expanding-text-area.tsx +++ b/services/web/frontend/js/shared/components/auto-expanding-text-area.tsx @@ -105,7 +105,7 @@ function AutoExpandingTextArea({ } }, [onResize]) - // Maintain a copy onAutoFocus in a ref for use in the autofocus effect + // Maintain a copy of onAutoFocus in a ref for use in the autofocus effect // below so that the effect doesn't run when onAutoFocus changes const onAutoFocusRef = useRef(onAutoFocus) useEffect(() => { diff --git a/services/web/frontend/js/shared/components/types/dropdown-menu-props.ts b/services/web/frontend/js/shared/components/types/dropdown-menu-props.ts index 64b8b0b10a..2664e8914f 100644 --- a/services/web/frontend/js/shared/components/types/dropdown-menu-props.ts +++ b/services/web/frontend/js/shared/components/types/dropdown-menu-props.ts @@ -1,5 +1,6 @@ import type { ElementType, ReactNode, PropsWithChildren } from 'react' import type { ButtonProps } from '@/shared/components/types/button-props' +import type { DropdownMenuProps as BS5DropdownMenuProps } from 'react-bootstrap' type SplitButtonVariants = Extract< ButtonProps['variant'], @@ -71,6 +72,7 @@ export type DropdownMenuProps = PropsWithChildren<{ flip?: boolean id?: string renderOnMount?: boolean + popperConfig?: BS5DropdownMenuProps['popperConfig'] }> export type DropdownDividerProps = PropsWithChildren<{ diff --git a/services/web/frontend/stylesheets/pages/editor/chat.scss b/services/web/frontend/stylesheets/pages/editor/chat.scss index 5d39e6b414..97daccae70 100644 --- a/services/web/frontend/stylesheets/pages/editor/chat.scss +++ b/services/web/frontend/stylesheets/pages/editor/chat.scss @@ -109,6 +109,18 @@ border-radius: var(--border-radius-base); position: relative; + .message-and-dropdown { + clear: both; + + &.pending-message { + opacity: 0.6; + } + } + + .message-dropdown:not(.show) { + visibility: hidden; + } + .message-content { padding: var(--spacing-03) var(--spacing-05); overflow-x: auto; @@ -118,6 +130,12 @@ a { color: var(--white); } + + .message-edited { + @include body-xs; + + font-weight: normal; + } } .arrow { @@ -134,6 +152,28 @@ border-bottom-color: transparent !important; border-width: 10px; } + + .message-dropdown-menu { + min-width: var(--bs-dropdown-min-width); + } + + .message-dropdown-menu-btn { + @include reset-button; + @include action-button; + + color: var(--white); + padding: 0; + width: 30px; + height: 30px; + } + } + + &.own-message-wrapper .message-and-dropdown:hover { + background-color: rgb($neutral-90, 0.08); + + .message-dropdown { + visibility: visible; + } } p { @@ -273,6 +313,30 @@ .message-container { display: flex; justify-content: flex-start; + + .message-dropdown:not(.show) { + visibility: hidden; + } + + &.pending-message { + opacity: 0.6; + } + + &:hover { + .message-dropdown { + visibility: visible; + } + } + + .message-dropdown-menu-btn { + @include reset-button; + @include action-button; + + color: var(--content-primary-themed); + padding: 0; + width: 30px; + height: 30px; + } } .message-author { @@ -292,6 +356,12 @@ p { margin: 0; } + + .message-edited { + @include body-xs; + + font-weight: normal; + } } .message-container.message-from-self { diff --git a/services/web/locales/en.json b/services/web/locales/en.json index c5c46deccd..531c3bf272 100644 --- a/services/web/locales/en.json +++ b/services/web/locales/en.json @@ -520,6 +520,8 @@ "delete_comment_thread": "Delete comment thread", "delete_comment_thread_message": "This will delete the whole comment thread. You cannot undo this action.", "delete_figure": "Delete figure", + "delete_message": "Delete message", + "delete_message_confirmation": "Are you sure you want to delete this message? This can’t be undone.", "delete_projects": "Delete Projects", "delete_row_or_column": "Delete row or column", "delete_sso_config": "Delete SSO configuration", diff --git a/services/web/test/frontend/features/chat/components/message-group.test.tsx b/services/web/test/frontend/features/chat/components/message-group.test.tsx new file mode 100644 index 0000000000..a015e38b07 --- /dev/null +++ b/services/web/test/frontend/features/chat/components/message-group.test.tsx @@ -0,0 +1,250 @@ +import { expect } from 'chai' +import { render, screen } from '@testing-library/react' + +import MessageGroup from '../../../../../frontend/js/features/chat/components/message-group' +import { stubMathJax, tearDownMathJaxStubs } from './stubs' +import { User, UserId } from '@ol-types/user' +import { + ChatContext, + Message as MessageType, +} from '@/features/chat/context/chat-context' +import { UserProvider } from '@/shared/context/user-context' +import { ModalsContextProvider } from '@/features/ide-react/context/modals-context' +import { SplitTestProvider } from '@/shared/context/split-test-context' + +describe('', function () { + function ChatProviders({ + children, + idOfMessageBeingEdited = null, + }: { + children: React.ReactNode + idOfMessageBeingEdited?: string | null + }) { + const mockContextValue = { + idOfMessageBeingEdited, + cancelMessageEdit: () => {}, + editMessage: () => {}, + } + + return ( + + + + + {children} + + + + + ) + } + + const currentUser: User = { + id: 'fake_user' as UserId, + first_name: 'fake_user_first_name', + email: 'fake@example.com', + } + + beforeEach(function () { + window.metaAttributesCache.set('ol-user', currentUser) + stubMathJax() + }) + + afterEach(function () { + tearDownMathJaxStubs() + }) + + it('renders a basic message', function () { + const message: MessageType = { + content: 'a message', + user: currentUser, + id: 'msg_1', + timestamp: new Date('2025-01-01T00:00:00.000Z').getTime(), + } + + render( + + + + ) + + screen.getByText('a message') + }) + + it('renders a message with multiple contents', function () { + const messages: MessageType[] = [ + { + content: 'a message', + user: currentUser, + id: 'msg_1', + timestamp: new Date('2025-01-01T00:00:00.000Z').getTime(), + }, + { + content: 'another message', + user: currentUser, + id: 'msg_2', + timestamp: new Date('2025-01-01T00:00:00.000Z').getTime(), + }, + ] + + render( + + + + ) + screen.getByText('a message') + screen.getByText('another message') + }) + + it('renders HTML links within messages', function () { + const message: MessageType = { + content: + 'a message with a link to Overleaf', + user: currentUser, + id: 'msg_1', + timestamp: new Date('2025-01-01T00:00:00.000Z').getTime(), + } + + render( + + + + ) + + screen.getByRole('link', { name: 'https://overleaf.com' }) + }) + + it('renders edited message with "(edited)" indicator', function () { + const editedMessage: MessageType = { + content: 'this message has been edited', + user: currentUser, + id: 'msg_1', + timestamp: new Date('2025-01-01T00:00:00.000Z').getTime(), + edited: true, + } + + render( + + + + ) + + screen.getByText('this message has been edited') + screen.getByText('(edited)') + }) + + it('does not render "(edited)" indicator for non-edited message', function () { + const message: MessageType = { + content: 'this message was not edited', + user: currentUser, + id: 'msg_1', + timestamp: new Date('2025-01-01T00:00:00.000Z').getTime(), + edited: false, + } + + render( + + + + ) + + screen.getByText('this message was not edited') + expect(screen.queryByText('(edited)')).to.not.exist + }) + + it('renders message being edited with textarea and action buttons', function () { + const messageBeingEdited: MessageType = { + content: 'original message content', + user: currentUser, + id: 'msg_being_edited', + timestamp: new Date('2025-01-01T00:00:00.000Z').getTime(), + } + + render( + + + + ) + + const textarea = screen.getByDisplayValue('original message content') + expect(textarea.tagName.toLowerCase()).to.equal('textarea') + + screen.getByRole('button', { name: 'Cancel' }) + screen.getByRole('button', { name: 'Save' }) + + const paragraphs = screen.queryAllByText('original message content', { + selector: 'p', + }) + expect(paragraphs).to.have.length(0) + }) + + describe('when the message is from the user themselves', function () { + const message: MessageType = { + content: 'a message', + user: currentUser, + id: 'msg_1', + timestamp: new Date('2025-01-01T00:00:00.000Z').getTime(), + } + + it('does not render the user name nor the email', function () { + render( + + + + ) + + expect(screen.queryByText(currentUser.first_name!)).to.not.exist + expect(screen.queryByText(currentUser.email)).to.not.exist + }) + }) + + describe('when the message is from other user', function () { + const otherUser: User = { + id: 'other_user' as UserId, + first_name: 'other_user_first_name', + email: 'other@example.com', + } + + const message: MessageType = { + content: 'a message', + user: otherUser, + id: 'msg_1', + timestamp: new Date('2025-01-01T00:00:00.000Z').getTime(), + } + + it('should render the other user name', function () { + render( + + + + ) + + screen.getByText(otherUser.first_name!) + }) + + it('should render the other user email when their name is not available', function () { + const msg: MessageType = { + content: message.content, + user: { + id: otherUser.id, + email: 'other@example.com', + }, + id: 'msg_1', + timestamp: new Date('2025-01-01T00:00:00.000Z').getTime(), + } + + render( + + + + ) + + expect(screen.queryByText(otherUser.first_name!)).to.not.exist + screen.getByText(msg.user!.email) + }) + }) +}) diff --git a/services/web/test/frontend/features/chat/components/message-list.test.tsx b/services/web/test/frontend/features/chat/components/message-list.test.tsx index f8cc896443..3057252f8e 100644 --- a/services/web/test/frontend/features/chat/components/message-list.test.tsx +++ b/services/web/test/frontend/features/chat/components/message-list.test.tsx @@ -1,14 +1,34 @@ import sinon from 'sinon' import { expect } from 'chai' import { screen, render, fireEvent } from '@testing-library/react' - +import React from 'react' import MessageList from '../../../../../frontend/js/features/chat/components/message-list' import { stubMathJax, tearDownMathJaxStubs } from './stubs' import { UserProvider } from '@/shared/context/user-context' import { User, UserId } from '@ol-types/user' -import { Message } from '@/features/chat/context/chat-context' +import { Message, ChatContext } from '@/features/chat/context/chat-context' +import { ModalsContextProvider } from '@/features/ide-react/context/modals-context' +import { SplitTestProvider } from '@/shared/context/split-test-context' describe('', function () { + function ChatProviders({ children }: { children: React.ReactNode }) { + const mockContextValue = { + idOfMessageBeingEdited: null, + } + + return ( + + + + + {children} + + + + + ) + } + const currentUser: User = { id: 'fake_user' as UserId, first_name: 'fake_user_first_name', @@ -19,13 +39,13 @@ describe('', function () { return [ { id: '1', - contents: ['a message'], + content: 'a message', user: currentUser, timestamp: new Date().getTime(), }, { id: '2', - contents: ['another message'], + content: 'another message', user: currentUser, timestamp: new Date().getTime(), }, @@ -52,12 +72,12 @@ describe('', function () { it('renders multiple messages', function () { render( - + {}} /> - + ) screen.getByText('a message') @@ -70,9 +90,9 @@ describe('', function () { msgs[1].timestamp = new Date(2019, 6, 3, 4, 27).getTime() render( - + {}} /> - + ) screen.getByText('4:23 am Wed, 3rd Jul 19') @@ -85,9 +105,9 @@ describe('', function () { msgs[1].timestamp = new Date(2019, 6, 3, 4, 31).getTime() render( - + {}} /> - + ) screen.getByText('4:23 am Wed, 3rd Jul 19') @@ -97,15 +117,101 @@ describe('', function () { it('resets the number of unread messages after clicking on the input', function () { const resetUnreadMessages = sinon.stub() render( - + - + ) fireEvent.click(screen.getByRole('list')) expect(resetUnreadMessages).to.be.calledOnce }) + + it('groups messages from different users separately', function () { + const anotherUser: User = { + id: 'another_user' as UserId, + first_name: 'another_user_first_name', + email: 'another@example.com', + } + + const messages: Message[] = [ + { + id: '1', + content: 'first message from current user', + user: currentUser, + timestamp: new Date('2025-09-01 4:20:10').getTime(), + }, + { + id: '2', + content: 'second message from current user', + user: currentUser, + timestamp: new Date('2025-09-01 4:20:11').getTime(), + }, + { + id: '3', + content: 'first message from another user', + user: anotherUser, + timestamp: new Date('2025-09-01 4:20:12').getTime(), + }, + { + id: '4', + content: 'second message from another user', + user: anotherUser, + timestamp: new Date('2025-09-01 4:20:13').getTime(), + }, + ] + + render( + + {}} /> + + ) + + const messageGroups = screen.getAllByRole('listitem') + + // Should have 2 message groups + expect(messageGroups).to.have.length(2) + + screen.getByText('first message from current user') + screen.getByText('second message from current user') + screen.getByText('first message from another user') + screen.getByText('second message from another user') + }) + + it('does not show deleted messages', function () { + const messages: Message[] = [ + { + id: '1', + content: 'visible message', + user: currentUser, + timestamp: new Date().getTime(), + }, + { + id: '2', + content: 'deleted message', + user: currentUser, + timestamp: new Date().getTime() + 1000, + deleted: true, + }, + { + id: '3', + content: 'another visible message', + user: currentUser, + timestamp: new Date().getTime() + 2000, + }, + ] + + render( + + {}} /> + + ) + + screen.getByText('visible message') + screen.getByText('another visible message') + + expect(screen.queryByText('deleted message')).to.not.exist + }) }) diff --git a/services/web/test/frontend/features/chat/components/message.test.tsx b/services/web/test/frontend/features/chat/components/message.test.tsx deleted file mode 100644 index 2106c74241..0000000000 --- a/services/web/test/frontend/features/chat/components/message.test.tsx +++ /dev/null @@ -1,120 +0,0 @@ -import { expect } from 'chai' -import { render, screen } from '@testing-library/react' - -import Message from '../../../../../frontend/js/features/chat/components/message' -import { stubMathJax, tearDownMathJaxStubs } from './stubs' -import { User, UserId } from '@ol-types/user' -import { Message as MessageType } from '@/features/chat/context/chat-context' - -describe('', function () { - const currentUser: User = { - id: 'fake_user' as UserId, - first_name: 'fake_user_first_name', - email: 'fake@example.com', - } - - beforeEach(function () { - window.metaAttributesCache.set('ol-user', currentUser) - stubMathJax() - }) - - afterEach(function () { - tearDownMathJaxStubs() - }) - - it('renders a basic message', function () { - const message: MessageType = { - contents: ['a message'], - user: currentUser, - id: 'msg_1', - timestamp: new Date('2025-01-01T00:00:00.000Z').getTime(), - } - - render() - - screen.getByText('a message') - }) - - it('renders a message with multiple contents', function () { - const message: MessageType = { - contents: ['a message', 'another message'], - user: currentUser, - id: 'msg_1', - timestamp: new Date('2025-01-01T00:00:00.000Z').getTime(), - } - - render() - - screen.getByText('a message') - screen.getByText('another message') - }) - - it('renders HTML links within messages', function () { - const message: MessageType = { - contents: [ - 'a message with a link to Overleaf', - ], - user: currentUser, - id: 'msg_1', - timestamp: new Date('2025-01-01T00:00:00.000Z').getTime(), - } - - render() - - screen.getByRole('link', { name: 'https://overleaf.com' }) - }) - - describe('when the message is from the user themselves', function () { - const message: MessageType = { - contents: ['a message'], - user: currentUser, - id: 'msg_1', - timestamp: new Date('2025-01-01T00:00:00.000Z').getTime(), - } - - it('does not render the user name nor the email', function () { - render() - - expect(screen.queryByText(currentUser.first_name!)).to.not.exist - expect(screen.queryByText(currentUser.email)).to.not.exist - }) - }) - - describe('when the message is from other user', function () { - const otherUser: User = { - id: 'other_user' as UserId, - first_name: 'other_user_first_name', - email: 'other@example.com', - } - - const message: MessageType = { - contents: ['a message'], - user: otherUser, - id: 'msg_1', - timestamp: new Date('2025-01-01T00:00:00.000Z').getTime(), - } - - it('should render the other user name', function () { - render() - - screen.getByText(otherUser.first_name!) - }) - - it('should render the other user email when their name is not available', function () { - const msg: MessageType = { - contents: message.contents, - user: { - id: otherUser.id, - email: 'other@example.com', - }, - id: 'msg_1', - timestamp: new Date('2025-01-01T00:00:00.000Z').getTime(), - } - - render() - - expect(screen.queryByText(otherUser.first_name!)).to.not.exist - screen.getByText(msg.user!.email) - }) - }) -}) diff --git a/services/web/test/frontend/features/chat/context/chat-context.test.tsx b/services/web/test/frontend/features/chat/context/chat-context.test.tsx index f392842baa..6693c5b465 100644 --- a/services/web/test/frontend/features/chat/context/chat-context.test.tsx +++ b/services/web/test/frontend/features/chat/context/chat-context.test.tsx @@ -109,7 +109,7 @@ describe('ChatContext', function () { await waitFor(() => { const message = result.current.messages[0] expect(message.id).to.equal('msg_1') - expect(message.contents).to.deep.equal(['new message']) + expect(message.content).to.deep.equal('new message') }) }) @@ -161,7 +161,7 @@ describe('ChatContext', function () { const message = result.current.messages[0] expect(message.id).to.equal('msg_1') - expect(message.contents).to.deep.equal(['new message']) + expect(message.content).to.deep.equal('new message') }) it('deduplicate messages from websocket', async function () { @@ -209,7 +209,7 @@ describe('ChatContext', function () { const message = result.current.messages[0] expect(message.id).to.equal('msg_1') - expect(message.contents).to.deep.equal(['new message']) + expect(message.content).to.deep.equal('new message') }) it("doesn't add received messages from the current user if a message was just sent", async function () { @@ -225,14 +225,14 @@ describe('ChatContext', function () { ) // Send a message from the current user - const sentMsg = 'sent message' - result.current.sendMessage(sentMsg) + const content = 'sent message' + result.current.sendMessage(content) act(() => { // Receive a message from the current user socket.emitToClient('new-chat-message', { id: 'msg_1', - content: 'received message', + content, timestamp: Date.now(), user, clientId: uuidValue, @@ -243,7 +243,7 @@ describe('ChatContext', function () { const [message] = result.current.messages - expect(message.contents).to.deep.equal([sentMsg]) + expect(message.content).to.deep.equal(content) }) it('adds the new message from the current user if another message was received after sending', async function () { @@ -259,13 +259,13 @@ describe('ChatContext', function () { ) // Send a message from the current user - const sentMsg = 'sent message from current user' + const content = 'sent message from current user' act(() => { - result.current.sendMessage(sentMsg) + result.current.sendMessage(content) }) const [sentMessageFromCurrentUser] = result.current.messages - expect(sentMessageFromCurrentUser.contents).to.deep.equal([sentMsg]) + expect(sentMessageFromCurrentUser.content).to.deep.equal(content) const otherMsg = 'new message from other user' @@ -285,25 +285,105 @@ describe('ChatContext', function () { }) const [, messageFromOtherUser] = result.current.messages - expect(messageFromOtherUser.contents).to.deep.equal([otherMsg]) + expect(messageFromOtherUser.content).to.deep.equal(otherMsg) + const receivedMessageTimestamp = Date.now() act(() => { // Receive a message from the current user socket.emitToClient('new-chat-message', { id: 'msg_2', - content: 'received message from current user', - timestamp: Date.now(), + content, + timestamp: receivedMessageTimestamp, user, clientId: uuidValue, }) }) - // Since the current user didn't just send a message, it is now shown + // Since this message has the same clientId, it should update the pending message + const updatedSentMessage = { + ...sentMessageFromCurrentUser, + id: 'msg_2', + content, + pending: false, + user, + timestamp: receivedMessageTimestamp, + } expect(result.current.messages).to.deep.equal([ - sentMessageFromCurrentUser, + updatedSentMessage, messageFromOtherUser, ]) }) + + it('handles multiple pending messages correctly when confirmed out of order', async function () { + const socket = new SocketIOMock() + const { result } = renderChatContextHook({ + socket: socket as any as Socket, + }) + + // Wait until initial messages have loaded + result.current.loadInitialMessages() + await waitFor( + () => expect(result.current.initialMessagesLoaded).to.be.true + ) + + // Send first message + act(() => { + result.current.sendMessage('first message') + }) + + // Send second message quickly + act(() => { + result.current.sendMessage('second message') + }) + + // At this point we should have 2 pending messages + expect(result.current.messages).to.have.length(2) + expect(result.current.messages[0].content).to.equal('first message') + expect(result.current.messages[0].pending).to.be.true + expect(result.current.messages[1].content).to.equal('second message') + expect(result.current.messages[1].pending).to.be.true + + // Server confirms the second message first + const secondMessageTimestamp = Date.now() + act(() => { + socket.emitToClient('new-chat-message', { + id: 'server-id-2', + content: 'second message', + user, + timestamp: secondMessageTimestamp, + clientId: uuidValue, + }) + }) + + await waitFor(() => { + expect(result.current.messages[0].content).to.equal('first message') + expect(result.current.messages[0].pending).to.be.true // Still pending + expect(result.current.messages[1].content).to.equal('second message') + expect(result.current.messages[1].pending).to.be.false // Confirmed + expect(result.current.messages[1].id).to.equal('server-id-2') + }) + + // Server confirms the first message + const firstMessageTimestamp = Date.now() + act(() => { + socket.emitToClient('new-chat-message', { + id: 'server-id-1', + content: 'first message', + user, + timestamp: firstMessageTimestamp, + clientId: uuidValue, + }) + }) + + await waitFor(() => { + expect(result.current.messages[0].content).to.equal('first message') + expect(result.current.messages[0].pending).to.be.false // Now confirmed + expect(result.current.messages[0].id).to.equal('server-id-1') + expect(result.current.messages[1].content).to.equal('second message') + expect(result.current.messages[1].pending).to.be.false // Still confirmed + expect(result.current.messages[1].id).to.equal('server-id-2') + }) + }) }) describe('loadInitialMessages', function () { @@ -323,7 +403,7 @@ describe('ChatContext', function () { result.current.loadInitialMessages() await waitFor(() => - expect(result.current.messages[0].contents).to.deep.equal(['a message']) + expect(result.current.messages[0].content).to.deep.equal('a message') ) }) @@ -374,9 +454,9 @@ describe('ChatContext', function () { result.current.loadMoreMessages() await waitFor(() => - expect(result.current.messages[0].contents).to.deep.equal([ - 'first message', - ]) + expect(result.current.messages[0].content).to.deep.equal( + 'first message' + ) ) // The before query param is not set @@ -403,9 +483,7 @@ describe('ChatContext', function () { const { result } = renderChatContextHook({}) result.current.loadMoreMessages() - await waitFor(() => - expect(result.current.messages[0].contents).to.have.length(50) - ) + await waitFor(() => expect(result.current.messages).to.have.length(50)) // Call a second time result.current.loadMoreMessages() @@ -414,7 +492,7 @@ describe('ChatContext', function () { // Since both messages from the same user, they are collapsed into the // same "message" await waitFor(() => - expect(result.current.messages[0].contents).to.include( + expect(result.current.messages[0].content).to.include( 'message from second page' ) ) @@ -439,9 +517,7 @@ describe('ChatContext', function () { const { result } = renderChatContextHook({}) result.current.loadMoreMessages() - await waitFor(() => - expect(result.current.messages[0].contents).to.have.length(49) - ) + await waitFor(() => expect(result.current.messages).to.have.length(49)) result.current.loadMoreMessages() @@ -497,7 +573,7 @@ describe('ChatContext', function () { // 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] + ({ content }) => content ) expect(messageContents).to.deep.equal([ 'loaded message', @@ -532,9 +608,7 @@ describe('ChatContext', function () { result.current.sendMessage('sent message') await waitFor(() => - expect(result.current.messages[0].contents).to.deep.equal([ - 'sent message', - ]) + expect(result.current.messages[0].content).to.deep.equal('sent message') ) }) diff --git a/services/web/test/frontend/features/chat/util/message-list-appender.test.tsx b/services/web/test/frontend/features/chat/util/message-list-appender.test.tsx deleted file mode 100644 index 894dab8639..0000000000 --- a/services/web/test/frontend/features/chat/util/message-list-appender.test.tsx +++ /dev/null @@ -1,271 +0,0 @@ -import { expect } from 'chai' -import { - appendMessage, - prependMessages, -} from '../../../../../frontend/js/features/chat/utils/message-list-appender' -import { User, UserId } from '@ol-types/user' -import { - Message, - ServerMessageEntry, -} from '@/features/chat/context/chat-context' - -const testUser: User = { - id: '123abc' as UserId, - email: 'test-user@example.com', -} - -const otherUser: User = { - id: '234other' as UserId, - email: 'other-user@example.com', -} - -function createTestMessageList(): Message[] { - return [ - { - id: 'msg_1', - contents: ['hello', 'world'], - timestamp: new Date().getTime(), - user: otherUser, - }, - { - id: 'msg_2', - contents: ['foo'], - timestamp: new Date().getTime(), - user: testUser, - }, - ] -} - -describe('prependMessages()', function () { - function createTestMessages(): ServerMessageEntry[] { - const message1 = { - id: 'prepended_message', - content: 'hello', - timestamp: new Date().getTime(), - user: testUser, - } - const message2 = { ...message1, id: 'prepended_message_2' } - return [message1, message2] - } - - it('to an empty list', function () { - const messages = createTestMessages() - const uniqueMessageIds: string[] = [] - - expect( - prependMessages([], messages, uniqueMessageIds).messages - ).to.deep.equal([ - { - id: messages[0].id, - timestamp: messages[0].timestamp, - user: messages[0].user, - contents: [messages[0].content, messages[1].content], - }, - ]) - }) - - describe('when the messages to prepend are from the same user', function () { - let list, messages: ServerMessageEntry[], uniqueMessageIds: string[] - - beforeEach(function () { - list = createTestMessageList() - messages = createTestMessages() - messages[0].user = testUser // makes all the messages have the same author - uniqueMessageIds = [] - }) - - it('when the prepended messages are close in time, contents should be merged into the same message', function () { - const result = prependMessages( - createTestMessageList(), - messages, - uniqueMessageIds - ).messages - expect(result.length).to.equal(list.length + 1) - expect(result[0]).to.deep.equal({ - id: messages[0].id, - timestamp: messages[0].timestamp, - user: messages[0].user, - contents: [messages[0].content, messages[1].content], - }) - }) - - it('when the prepended messages are separated in time, each message is prepended', function () { - messages[0].timestamp = messages[1].timestamp - 6 * 60 * 1000 // 6 minutes before the next message - const result = prependMessages( - createTestMessageList(), - messages, - uniqueMessageIds - ).messages - expect(result.length).to.equal(list.length + 2) - expect(result[0]).to.deep.equal({ - id: messages[0].id, - timestamp: messages[0].timestamp, - user: messages[0].user, - contents: [messages[0].content], - }) - expect(result[1]).to.deep.equal({ - id: messages[1].id, - timestamp: messages[1].timestamp, - user: messages[1].user, - contents: [messages[1].content], - }) - }) - }) - - describe('when the messages to prepend are from different users', function () { - let list, messages: ServerMessageEntry[], uniqueMessageIds: string[] - - beforeEach(function () { - list = createTestMessageList() - messages = createTestMessages() - uniqueMessageIds = [] - }) - - it('should prepend separate messages to the list', function () { - messages[0].user = otherUser - const result = prependMessages( - createTestMessageList(), - messages, - uniqueMessageIds - ).messages - expect(result.length).to.equal(list.length + 2) - expect(result[0]).to.deep.equal({ - id: messages[0].id, - timestamp: messages[0].timestamp, - user: messages[0].user, - contents: [messages[0].content], - }) - expect(result[1]).to.deep.equal({ - id: messages[1].id, - timestamp: messages[1].timestamp, - user: messages[1].user, - contents: [messages[1].content], - }) - }) - }) - - it('should merge the prepended messages into the first existing one when user is same user and are close in time', function () { - const list = createTestMessageList() - const messages = createTestMessages() - messages[0].user = messages[1].user = list[0].user - const uniqueMessageIds: string[] = [] - - const result = prependMessages( - createTestMessageList(), - messages, - uniqueMessageIds - ).messages - expect(result.length).to.equal(list.length) - expect(result[0]).to.deep.equal({ - id: messages[0].id, - timestamp: messages[0].timestamp, - user: messages[0].user, - contents: [messages[0].content, messages[1].content, ...list[0].contents], - }) - }) -}) - -describe('appendMessage()', function () { - function createTestMessage() { - return { - id: 'appended_message', - content: 'hi!', - timestamp: new Date().getTime(), - user: testUser, - } - } - - it('to an empty list', function () { - const testMessage = createTestMessage() - const uniqueMessageIds: string[] = [] - - expect( - appendMessage([], testMessage, uniqueMessageIds).messages - ).to.deep.equal([ - { - id: 'appended_message', - timestamp: testMessage.timestamp, - user: testMessage.user, - contents: [testMessage.content], - }, - ]) - }) - - describe('messages appended shortly after the last message on the list', function () { - let list: Message[], message: ServerMessageEntry, uniqueMessageIds: string[] - - beforeEach(function () { - list = createTestMessageList() - message = createTestMessage() - message.timestamp = list[1].timestamp + 6 * 1000 // 6 seconds after the last message in the list - uniqueMessageIds = [] - }) - - describe('when the author is the same as the last message', function () { - it('should append the content to the last message', function () { - const result = appendMessage(list, message, uniqueMessageIds).messages - expect(result.length).to.equal(list.length) - expect(result[1].contents).to.deep.equal( - list[1].contents.concat(message.content) - ) - }) - - it('should update the last message timestamp', function () { - const result = appendMessage(list, message, uniqueMessageIds).messages - expect(result[1].timestamp).to.equal(message.timestamp) - }) - }) - - describe('when the author is different than the last message', function () { - beforeEach(function () { - message.user = otherUser - }) - - it('should append the new message to the list', function () { - const result = appendMessage(list, message, uniqueMessageIds).messages - expect(result.length).to.equal(list.length + 1) - expect(result[2]).to.deep.equal({ - id: 'appended_message', - timestamp: message.timestamp, - user: message.user, - contents: [message.content], - }) - }) - }) - }) - - describe('messages appended later after the last message on the list', function () { - let list: Message[], message: ServerMessageEntry, uniqueMessageIds: string[] - - beforeEach(function () { - list = createTestMessageList() - message = createTestMessage() - message.timestamp = list[1].timestamp + 6 * 60 * 1000 // 6 minutes after the last message in the list - uniqueMessageIds = [] - }) - - it('when the author is the same as the last message, should be appended as new message', function () { - const result = appendMessage(list, message, uniqueMessageIds).messages - expect(result.length).to.equal(3) - expect(result[2]).to.deep.equal({ - id: 'appended_message', - timestamp: message.timestamp, - user: message.user, - contents: [message.content], - }) - }) - - it('when the author is the different than the last message, should be appended as new message', function () { - message.user = otherUser - - const result = appendMessage(list, message, uniqueMessageIds).messages - expect(result.length).to.equal(3) - expect(result[2]).to.deep.equal({ - id: 'appended_message', - timestamp: message.timestamp, - user: message.user, - contents: [message.content], - }) - }) - }) -}) diff --git a/services/web/test/frontend/features/chat/util/message-list-utils.test.tsx b/services/web/test/frontend/features/chat/util/message-list-utils.test.tsx new file mode 100644 index 0000000000..11ae92c049 --- /dev/null +++ b/services/web/test/frontend/features/chat/util/message-list-utils.test.tsx @@ -0,0 +1,429 @@ +import { expect } from 'chai' +import { + appendMessage, + prependMessages, + editMessage, + deleteMessage, + confirmMessage, +} from '../../../../../frontend/js/features/chat/utils/message-list-utils' +import { User, UserId } from '@ol-types/user' +import { + Message, + ServerMessageEntry, +} from '@/features/chat/context/chat-context' + +const testUser: User = { + id: '123abc' as UserId, + email: 'test-user@example.com', +} + +const otherUser: User = { + id: '234other' as UserId, + email: 'other-user@example.com', +} + +function createTestMessageList(): Message[] { + return [ + { + id: 'msg_1', + content: 'hello world', + timestamp: new Date().getTime(), + user: otherUser, + }, + { + id: 'msg_2', + content: 'foo', + timestamp: new Date().getTime(), + user: testUser, + }, + ] +} +describe('message-list-utils', function () { + describe('prependMessages()', function () { + function createTestMessages(): ServerMessageEntry[] { + const message1 = { + id: 'prepended_message', + content: 'hello', + timestamp: new Date().getTime(), + user: testUser, + } + const message2 = { ...message1, id: 'prepended_message_2' } + return [message1, message2] + } + + it('to an empty list', function () { + const messages = createTestMessages() + const uniqueMessageIds: string[] = [] + + expect( + prependMessages([], messages, uniqueMessageIds).messages + ).to.deep.equal([ + { + id: messages[0].id, + timestamp: messages[0].timestamp, + user: messages[0].user, + content: messages[0].content, + edited: false, + }, + { + id: messages[1].id, + timestamp: messages[1].timestamp, + user: messages[1].user, + content: messages[1].content, + edited: false, + }, + ]) + }) + + describe('when the messages to prepend are from the same user', function () { + let list, messages: ServerMessageEntry[], uniqueMessageIds: string[] + + beforeEach(function () { + list = createTestMessageList() + messages = createTestMessages() + messages[0].user = testUser // makes all the messages have the same author + uniqueMessageIds = [] + }) + + it('when the prepended messages are close in time, contents should be merged into the same message', function () { + const result = prependMessages( + createTestMessageList(), + messages, + uniqueMessageIds + ).messages + expect(result.length).to.equal(list.length + 2) + expect(result[0]).to.deep.equal({ + id: messages[0].id, + timestamp: messages[0].timestamp, + user: messages[0].user, + content: messages[0].content, + edited: false, + }) + expect(result[1]).to.deep.equal({ + id: messages[1].id, + timestamp: messages[1].timestamp, + user: messages[1].user, + content: messages[1].content, + edited: false, + }) + }) + + it('when the prepended messages are separated in time, each message is prepended', function () { + messages[0].timestamp = messages[1].timestamp - 6 * 60 * 1000 // 6 minutes before the next message + const result = prependMessages( + createTestMessageList(), + messages, + uniqueMessageIds + ).messages + expect(result.length).to.equal(list.length + 2) + expect(result[0]).to.deep.equal({ + id: messages[0].id, + timestamp: messages[0].timestamp, + user: messages[0].user, + content: messages[0].content, + edited: false, + }) + expect(result[1]).to.deep.equal({ + id: messages[1].id, + timestamp: messages[1].timestamp, + user: messages[1].user, + content: messages[1].content, + edited: false, + }) + }) + }) + + describe('when the messages to prepend are from different users', function () { + let list, messages: ServerMessageEntry[], uniqueMessageIds: string[] + + beforeEach(function () { + list = createTestMessageList() + messages = createTestMessages() + uniqueMessageIds = [] + }) + + it('should prepend separate messages to the list', function () { + messages[0].user = otherUser + const result = prependMessages( + createTestMessageList(), + messages, + uniqueMessageIds + ).messages + expect(result.length).to.equal(list.length + 2) + expect(result[0]).to.deep.equal({ + id: messages[0].id, + timestamp: messages[0].timestamp, + user: messages[0].user, + content: messages[0].content, + edited: false, + }) + expect(result[1]).to.deep.equal({ + id: messages[1].id, + timestamp: messages[1].timestamp, + user: messages[1].user, + content: messages[1].content, + edited: false, + }) + }) + }) + + it('should merge the prepended messages into the first existing one when user is same user and are close in time', function () { + const list = createTestMessageList() + const messages = createTestMessages() + messages[0].user = messages[1].user = list[0].user + const uniqueMessageIds: string[] = [] + + const result = prependMessages( + createTestMessageList(), + messages, + uniqueMessageIds + ).messages + expect(result.length).to.equal(list.length + 2) + expect(result[0]).to.deep.equal({ + id: messages[0].id, + timestamp: messages[0].timestamp, + user: messages[0].user, + content: messages[0].content, + edited: false, + }) + }) + }) + + describe('appendMessage()', function () { + function createTestMessage() { + return { + id: 'appended_message', + content: 'hi!', + timestamp: new Date().getTime(), + user: testUser, + pending: false, + } + } + + it('to an empty list', function () { + const testMessage = createTestMessage() + const uniqueMessageIds: string[] = [] + + expect( + appendMessage([], testMessage, uniqueMessageIds).messages + ).to.deep.equal([ + { + id: 'appended_message', + timestamp: testMessage.timestamp, + user: testMessage.user, + content: testMessage.content, + pending: false, + edited: false, + }, + ]) + }) + + describe('messages appended shortly after the last message on the list', function () { + let list: Message[], + message: ServerMessageEntry, + uniqueMessageIds: string[] + + beforeEach(function () { + list = createTestMessageList() + message = createTestMessage() + message.timestamp = list[1].timestamp + 6 * 1000 // 6 seconds after the last message in the list + uniqueMessageIds = [] + }) + + describe('when the author is the same as the last message', function () { + it('should append the content to the last message', function () { + const result = appendMessage(list, message, uniqueMessageIds).messages + expect(result.length).to.equal(list.length + 1) + expect(result[2]).to.deep.equal({ + id: message.id, + timestamp: message.timestamp, + user: message.user, + content: message.content, + pending: false, + edited: false, + }) + }) + + it('should update the last message timestamp', function () { + const result = appendMessage(list, message, uniqueMessageIds).messages + expect(result[2].timestamp).to.equal(message.timestamp) + }) + }) + + describe('when the author is different than the last message', function () { + beforeEach(function () { + message.user = otherUser + }) + + it('should append the new message to the list', function () { + const result = appendMessage(list, message, uniqueMessageIds).messages + expect(result.length).to.equal(list.length + 1) + expect(result[2]).to.deep.equal({ + id: 'appended_message', + timestamp: message.timestamp, + user: message.user, + content: message.content, + pending: false, + edited: false, + }) + }) + }) + }) + + describe('messages appended later after the last message on the list', function () { + let list: Message[], + message: ServerMessageEntry, + uniqueMessageIds: string[] + + beforeEach(function () { + list = createTestMessageList() + message = createTestMessage() + message.timestamp = list[1].timestamp + 6 * 60 * 1000 // 6 minutes after the last message in the list + uniqueMessageIds = [] + }) + + it('when the author is the same as the last message, should be appended as new message', function () { + const result = appendMessage(list, message, uniqueMessageIds).messages + expect(result.length).to.equal(3) + expect(result[2]).to.deep.equal({ + id: 'appended_message', + timestamp: message.timestamp, + user: message.user, + content: message.content, + pending: false, + edited: false, + }) + }) + + it('when the author is the different than the last message, should be appended as new message', function () { + message.user = otherUser + + const result = appendMessage(list, message, uniqueMessageIds).messages + expect(result.length).to.equal(3) + expect(result[2]).to.deep.equal({ + id: 'appended_message', + timestamp: message.timestamp, + user: message.user, + content: message.content, + pending: false, + edited: false, + }) + }) + }) + }) + + describe('editMessage()', function () { + it('should edit an existing message', function () { + const list = createTestMessageList() + const messageId = 'msg_1' + const newContent = 'edited content' + + const result = editMessage(messageId, newContent, list) + + expect(result.messages.length).to.equal(list.length) + expect(result.messages[0]).to.deep.equal({ + id: messageId, + content: newContent, + timestamp: list[0].timestamp, + user: list[0].user, + edited: true, + }) + expect(result.messages[1]).to.deep.equal(list[1]) + }) + + it('should throw an error if message is not found', function () { + const list = createTestMessageList() + const nonExistentId = 'non_existent_id' + const newContent = 'edited content' + + expect(() => { + editMessage(nonExistentId, newContent, list) + }).to.throw(`Message with id ${nonExistentId} not found in message list`) + }) + }) + + describe('deleteMessage()', function () { + it('should mark an existing message as deleted', function () { + const list = createTestMessageList() + const messageId = 'msg_1' + + const result = deleteMessage(messageId, list) + + expect(result.messages.length).to.equal(list.length) + expect(result.messages[0]).to.deep.equal({ + id: messageId, + content: list[0].content, + timestamp: list[0].timestamp, + user: list[0].user, + deleted: true, + }) + expect(result.messages[1]).to.deep.equal(list[1]) + }) + + it('should throw an error if message is not found', function () { + const list = createTestMessageList() + const nonExistentId = 'non_existent_id' + + expect(() => { + deleteMessage(nonExistentId, list) + }).to.throw(`Message with id ${nonExistentId} not found in message list`) + }) + }) + + describe('confirmMessage()', function () { + function createMessageListWithPendingMessage(): Message[] { + return [ + { + id: 'msg_1', + content: 'hello world', + timestamp: new Date().getTime(), + user: otherUser, + }, + { + id: 'temp_id', + content: 'pending message', + timestamp: new Date().getTime(), + user: testUser, + pending: true, + }, + ] + } + + it('should confirm a pending message and update its ID', function () { + const list = createMessageListWithPendingMessage() + const updatedMessage: Message = { + id: 'server_id', + content: 'pending message', + timestamp: new Date().getTime() + 1000, + user: testUser, + } + + const result = confirmMessage(updatedMessage, list) + + expect(result.messages.length).to.equal(list.length) + expect(result.messages[0]).to.deep.equal(list[0]) + expect(result.messages[1]).to.deep.equal({ + id: 'server_id', + content: 'pending message', + timestamp: updatedMessage.timestamp, + user: testUser, + pending: false, + }) + expect(result.uniqueMessageIds).to.deep.equal(['msg_1', 'server_id']) + }) + + it('should throw an error if pending message is not found', function () { + const list = createTestMessageList() // No pending messages + const updatedMessage: Message = { + id: 'server_id', + content: 'non-existent pending message', + timestamp: new Date().getTime(), + user: testUser, + } + + expect(() => { + confirmMessage(updatedMessage, list) + }).to.throw("Couldn't find own message in local state") + }) + }) +})