diff --git a/services/chat/app/js/Features/Messages/MessageHttpController.js b/services/chat/app/js/Features/Messages/MessageHttpController.js index ab6129a623..a20d005864 100644 --- a/services/chat/app/js/Features/Messages/MessageHttpController.js +++ b/services/chat/app/js/Features/Messages/MessageHttpController.js @@ -82,6 +82,14 @@ export async function destroyProject(context) { return await callMessageHttpController(context, _destroyProject) } +export async function duplicateCommentThreads(context) { + return await callMessageHttpController(context, _duplicateCommentThreads) +} + +export async function generateThreadData(context) { + return await callMessageHttpController(context, _generateThreadData) +} + export async function getStatus(context) { const message = 'chat is alive' context.res.status(200).setBody(message) @@ -120,6 +128,18 @@ const _getAllThreads = async (req, res) => { res.json(threads) } +const _generateThreadData = async (req, res) => { + const { projectId } = req.params + const { threads } = req.body + logger.debug({ projectId }, 'getting all threads') + const rooms = await ThreadManager.findThreadsById(projectId, threads) + const roomIds = rooms.map(r => r._id) + const messages = await MessageManager.findAllMessagesInRooms(roomIds) + logger.debug({ rooms, messages }, 'looked up messages in the rooms') + const threadData = MessageFormatter.groupMessagesByThreads(rooms, messages) + res.json(threadData) +} + const _resolveThread = async (req, res) => { const { projectId, threadId } = req.params const { user_id: userId } = req.body @@ -254,3 +274,29 @@ async function _getMessages(clientThreadId, req, res) { logger.debug({ projectId, messages }, 'got messages') res.status(200).setBody(messages) } + +async function _duplicateCommentThreads(req, res) { + const { projectId } = req.params + const { threads } = req.body + const result = {} + for (const id of threads) { + logger.debug({ projectId, thread: id }, 'duplicating thread') + try { + const { oldRoom, newRoom } = await ThreadManager.duplicateThread( + projectId, + id + ) + await MessageManager.duplicateRoomToOtherRoom(oldRoom._id, newRoom._id) + result[id] = { duplicateId: newRoom.thread_id } + } catch (error) { + if (error instanceof ThreadManager.MissingThreadError) { + // Expected error when the comment has been deleted prior to duplication + result[id] = { error: 'not found' } + } else { + logger.err({ error }, 'error duplicating thread') + result[id] = { error: 'unknown' } + } + } + } + res.json({ newThreads: result }) +} diff --git a/services/chat/app/js/Features/Messages/MessageManager.js b/services/chat/app/js/Features/Messages/MessageManager.js index 77103f118a..cb8818e3b6 100644 --- a/services/chat/app/js/Features/Messages/MessageManager.js +++ b/services/chat/app/js/Features/Messages/MessageManager.js @@ -89,3 +89,16 @@ function _ensureIdsAreObjectIds(query) { } return query } + +export async function duplicateRoomToOtherRoom(sourceRoomId, targetRoomId) { + const sourceMessages = await findAllMessagesInRooms([sourceRoomId]) + const targetMessages = sourceMessages.map(comment => { + return _ensureIdsAreObjectIds({ + room_id: targetRoomId, + content: comment.content, + timestamp: comment.timestamp, + user_id: comment.user_id, + }) + }) + await db.messages.insertMany(targetMessages) +} diff --git a/services/chat/app/js/Features/Threads/ThreadManager.js b/services/chat/app/js/Features/Threads/ThreadManager.js index efda558a54..5697b39393 100644 --- a/services/chat/app/js/Features/Threads/ThreadManager.js +++ b/services/chat/app/js/Features/Threads/ThreadManager.js @@ -1,5 +1,7 @@ import { db, ObjectId } from '../../mongodb.js' +export class MissingThreadError extends Error {} + export const GLOBAL_THREAD = 'GLOBAL' export async function findOrCreateThread(projectId, threadId) { @@ -124,3 +126,32 @@ export async function getResolvedThreadIds(projectId) { .toArray() return resolvedThreadIds } + +export async function duplicateThread(projectId, threadId) { + const room = await db.rooms.findOne({ + project_id: new ObjectId(projectId), + thread_id: new ObjectId(threadId), + }) + if (!room) { + throw new MissingThreadError('Trying to duplicate a non-existent thread') + } + const newRoom = { + project_id: room.project_id, + thread_id: new ObjectId(), + } + if (room.resolved) { + newRoom.resolved = room.resolved + } + const confirmation = await db.rooms.insertOne(newRoom) + newRoom._id = confirmation.insertedId + return { oldRoom: room, newRoom } +} + +export async function findThreadsById(projectId, threadIds) { + return await db.rooms + .find({ + project_id: new ObjectId(projectId), + thread_id: { $in: threadIds.map(id => new ObjectId(id)) }, + }) + .toArray() +} diff --git a/services/chat/chat.yaml b/services/chat/chat.yaml index b328baeced..3ccdf9bc30 100644 --- a/services/chat/chat.yaml +++ b/services/chat/chat.yaml @@ -303,6 +303,64 @@ paths: description: chat is alive operationId: getStatus description: Check that the Chat service is alive + '/project/{projectId}/duplicate-comment-threads': + parameters: + - schema: + type: string + name: projectId + in: path + required: true + post: + summary: Duplicate comment threads + operationId: duplicateCommentThreads + requestBody: + content: + application/json: + schema: + type: object + properties: + threads: + type: array + items: + type: string + responses: + '200': + content: + application/json: + schema: + type: object + properties: + newThreads: + type: object + description: Mapping of old thread ids to their duplicated thread ids + description: Duplicate a list of comment threads + '/project/{projectId}/generate-thread-data': + parameters: + - schema: + type: string + name: projectId + in: path + required: true + post: + summary: Generate thread data to load into the frontend + operationId: generateThreadData + requestBody: + content: + application/json: + schema: + type: object + properties: + threads: + type: array + items: + type: string + responses: + '200': + content: + application/json: + schema: + type: object + description: Load threads and generate a json blob containing all messages in all the threads components: schemas: Message: diff --git a/services/chat/test/acceptance/js/CloningCommentThreadsTests.js b/services/chat/test/acceptance/js/CloningCommentThreadsTests.js new file mode 100644 index 0000000000..f4adde1a52 --- /dev/null +++ b/services/chat/test/acceptance/js/CloningCommentThreadsTests.js @@ -0,0 +1,93 @@ +import { ObjectId } from '../../../app/js/mongodb.js' +import { expect } from 'chai' + +import * as ChatClient from './helpers/ChatClient.js' +import * as ChatApp from './helpers/ChatApp.js' + +const user1Id = new ObjectId().toString() +const user2Id = new ObjectId().toString() + +async function createCommentThread(projectId, threadId = new ObjectId()) { + const { response: response1 } = await ChatClient.sendMessage( + projectId, + threadId.toString(), + user1Id, + 'message 1' + ) + expect(response1.statusCode).to.equal(201) + const { response: response2 } = await ChatClient.sendMessage( + projectId, + threadId, + user2Id, + 'message 2' + ) + expect(response2.statusCode).to.equal(201) + return threadId.toString() +} + +describe('Cloning comment threads', async function () { + const projectId = new ObjectId().toString() + + before(async function () { + await ChatApp.ensureRunning() + this.thread1Id = await createCommentThread(projectId) + this.thread2Id = await createCommentThread(projectId) + this.thread3Id = await createCommentThread(projectId) + }) + + describe('with non-orphaned threads', async function () { + before(async function () { + const { + response: { body: result, statusCode }, + } = await ChatClient.duplicateCommentThreads(projectId, [this.thread3Id]) + this.result = result + expect(statusCode).to.equal(200) + expect(this.result).to.have.property('newThreads') + this.newThreadId = this.result.newThreads[this.thread3Id].duplicateId + }) + + it('should duplicate threads', function () { + expect(this.result.newThreads).to.have.property(this.thread3Id) + expect(this.result.newThreads[this.thread3Id]).to.have.property( + 'duplicateId' + ) + expect(this.result.newThreads[this.thread3Id].duplicateId).to.not.equal( + this.thread3Id + ) + }) + + it('should not duplicate other threads threads', function () { + expect(this.result.newThreads).to.not.have.property(this.thread1Id) + expect(this.result.newThreads).to.not.have.property(this.thread2Id) + }) + + it('should duplicate the messages in the thread', async function () { + const { + response: { body: threads }, + } = await ChatClient.getThreads(projectId) + function ignoreId(comment) { + return { + ...comment, + id: undefined, + } + } + expect(threads[this.thread3Id].messages.map(ignoreId)).to.deep.equal( + threads[this.newThreadId].messages.map(ignoreId) + ) + }) + + it('should have two separate unlinked threads', async function () { + await ChatClient.sendMessage( + projectId, + this.newThreadId, + user1Id, + 'third message' + ) + const { + response: { body: threads }, + } = await ChatClient.getThreads(projectId) + expect(threads[this.thread3Id].messages.length).to.equal(2) + expect(threads[this.newThreadId].messages.length).to.equal(3) + }) + }) +}) diff --git a/services/chat/test/acceptance/js/helpers/ChatClient.js b/services/chat/test/acceptance/js/helpers/ChatClient.js index 43be545ebc..857c3f7fc8 100644 --- a/services/chat/test/acceptance/js/helpers/ChatClient.js +++ b/services/chat/test/acceptance/js/helpers/ChatClient.js @@ -144,3 +144,13 @@ export async function destroyProject(projectId) { url: `/project/${projectId}`, }) } + +export async function duplicateCommentThreads(projectId, threads) { + return await asyncRequest({ + method: 'post', + url: `/project/${projectId}/duplicate-comment-threads`, + json: { + threads, + }, + }) +} diff --git a/services/web/app/src/Features/Chat/ChatApiHandler.js b/services/web/app/src/Features/Chat/ChatApiHandler.js index 76c58d7e49..d581827a89 100644 --- a/services/web/app/src/Features/Chat/ChatApiHandler.js +++ b/services/web/app/src/Features/Chat/ChatApiHandler.js @@ -97,6 +97,28 @@ async function getResolvedThreadIds(projectId) { return body.resolvedThreadIds } +async function duplicateCommentThreads(projectId, threads) { + return await fetchJson( + chatApiUrl(`/project/${projectId}/duplicate-comment-threads`), + { + method: 'POST', + json: { + threads, + }, + } + ) +} + +async function generateThreadData(projectId, threads) { + return await fetchJson( + chatApiUrl(`/project/${projectId}/generate-thread-data`), + { + method: 'POST', + json: { threads }, + } + ) +} + function chatApiUrl(path) { return new URL(path, settings.apis.chat.internal_url) } @@ -113,6 +135,8 @@ module.exports = { editMessage: callbackify(editMessage), deleteMessage: callbackify(deleteMessage), getResolvedThreadIds: callbackify(getResolvedThreadIds), + duplicateCommentThreads: callbackify(duplicateCommentThreads), + generateThreadData: callbackify(generateThreadData), promises: { getThreads, destroyProject, @@ -125,5 +149,7 @@ module.exports = { editMessage, deleteMessage, getResolvedThreadIds, + duplicateCommentThreads, + generateThreadData, }, } diff --git a/services/web/app/src/Features/Chat/ChatController.js b/services/web/app/src/Features/Chat/ChatController.js index 6050d8e22c..51d217ed9e 100644 --- a/services/web/app/src/Features/Chat/ChatController.js +++ b/services/web/app/src/Features/Chat/ChatController.js @@ -18,7 +18,8 @@ const EditorRealTimeController = require('../Editor/EditorRealTimeController') const SessionManager = require('../Authentication/SessionManager') const UserInfoManager = require('../User/UserInfoManager') const UserInfoController = require('../User/UserInfoController') -const async = require('async') +const ChatManager = require('./ChatManager') +const logger = require('@overleaf/logger') module.exports = ChatController = { sendMessage(req, res, next) { @@ -68,7 +69,7 @@ module.exports = ChatController = { if (err != null) { return next(err) } - return ChatController._injectUserInfoIntoThreads( + return ChatManager.injectUserInfoIntoThreads( { global: { messages } }, function (err) { if (err != null) { @@ -80,55 +81,4 @@ module.exports = ChatController = { } ) }, - - _injectUserInfoIntoThreads(threads, callback) { - // There will be a lot of repitition of user_ids, so first build a list - // of unique ones to perform db look ups on, then use these to populate the - // user fields - let message, thread, threadId, userId - if (callback == null) { - callback = function () {} - } - const userIds = {} - for (threadId in threads) { - thread = threads[threadId] - if (thread.resolved) { - userIds[thread.resolved_by_user_id] = true - } - for (message of Array.from(thread.messages)) { - userIds[message.user_id] = true - } - } - - const jobs = [] - const users = {} - for (userId in userIds) { - const _ = userIds[userId] - ;(userId => - jobs.push(cb => - UserInfoManager.getPersonalInfo(userId, function (error, user) { - if (error != null) return cb(error) - user = UserInfoController.formatPersonalInfo(user) - users[userId] = user - cb() - }) - ))(userId) - } - - return async.series(jobs, function (error) { - if (error != null) { - return callback(error) - } - for (threadId in threads) { - thread = threads[threadId] - if (thread.resolved) { - thread.resolved_by_user = users[thread.resolved_by_user_id] - } - for (message of Array.from(thread.messages)) { - message.user = users[message.user_id] - } - } - return callback(null, threads) - }) - }, } diff --git a/services/web/app/src/Features/Chat/ChatManager.js b/services/web/app/src/Features/Chat/ChatManager.js new file mode 100644 index 0000000000..9625881dd8 --- /dev/null +++ b/services/web/app/src/Features/Chat/ChatManager.js @@ -0,0 +1,61 @@ +const async = require('async') +const UserInfoManager = require('../User/UserInfoManager') +const UserInfoController = require('../User/UserInfoController') +const { promisify } = require('@overleaf/promise-utils') + +function injectUserInfoIntoThreads(threads, callback) { + // There will be a lot of repitition of user_ids, so first build a list + // of unique ones to perform db look ups on, then use these to populate the + // user fields + let message, thread, threadId, userId + if (callback == null) { + callback = function () {} + } + const userIds = {} + for (threadId in threads) { + thread = threads[threadId] + if (thread.resolved) { + userIds[thread.resolved_by_user_id] = true + } + for (message of Array.from(thread.messages)) { + userIds[message.user_id] = true + } + } + + const jobs = [] + const users = {} + for (userId in userIds) { + ;(userId => + jobs.push(cb => + UserInfoManager.getPersonalInfo(userId, function (error, user) { + if (error != null) return cb(error) + user = UserInfoController.formatPersonalInfo(user) + users[userId] = user + cb() + }) + ))(userId) + } + + return async.series(jobs, function (error) { + if (error != null) { + return callback(error) + } + for (threadId in threads) { + thread = threads[threadId] + if (thread.resolved) { + thread.resolved_by_user = users[thread.resolved_by_user_id] + } + for (message of Array.from(thread.messages)) { + message.user = users[message.user_id] + } + } + return callback(null, threads) + }) +} + +module.exports = { + injectUserInfoIntoThreads, + promises: { + injectUserInfoIntoThreads: promisify(injectUserInfoIntoThreads), + }, +} diff --git a/services/web/app/src/Features/History/RestoreManager.js b/services/web/app/src/Features/History/RestoreManager.js index a2401a5c6e..5f2c8244e6 100644 --- a/services/web/app/src/Features/History/RestoreManager.js +++ b/services/web/app/src/Features/History/RestoreManager.js @@ -9,6 +9,11 @@ const { callbackifyAll } = require('@overleaf/promise-utils') const { fetchJson } = require('@overleaf/fetch-utils') const ProjectLocator = require('../Project/ProjectLocator') const DocumentUpdaterHandler = require('../DocumentUpdater/DocumentUpdaterHandler') +const ChatApiHandler = require('../Chat/ChatApiHandler') +const DocstoreManager = require('../Docstore/DocstoreManager') +const logger = require('@overleaf/logger') +const EditorRealTimeController = require('../Editor/EditorRealTimeController') +const ChatManager = require('../Chat/ChatManager') const RestoreManager = { async restoreFileFromV2(userId, projectId, version, pathname) { @@ -88,17 +93,17 @@ const RestoreManager = { } if (file) { - await DocumentUpdaterHandler.promises.setDocument( + logger.debug( + { projectId, fileId: file.element._id, type: importInfo.type }, + 'deleting entity before reverting it' + ) + await EditorController.promises.deleteEntity( projectId, file.element._id, - userId, - importInfo.lines, - source + importInfo.type, + 'revert', + userId ) - return { - _id: file.element._id, - type: importInfo.type, - } } const ranges = await RestoreManager._getRangesFromHistory( @@ -107,12 +112,73 @@ const RestoreManager = { pathname ) + const documentCommentIds = new Set( + ranges.comments?.map(({ op: { t } }) => t) + ) + + await DocumentUpdaterHandler.promises.flushProjectToMongo(projectId) + + const docsWithRanges = + await DocstoreManager.promises.getAllRanges(projectId) + + const nonOrphanedThreadIds = new Set() + for (const { ranges } of docsWithRanges) { + for (const comment of ranges.comments ?? []) { + nonOrphanedThreadIds.add(comment.op.t) + } + } + + const commentIdsToDuplicate = Array.from(documentCommentIds).filter(id => + nonOrphanedThreadIds.has(id) + ) + + const newRanges = { changes: ranges.changes, comments: [] } + + if (commentIdsToDuplicate.length > 0) { + const { newThreads: newCommentIds } = + await ChatApiHandler.promises.duplicateCommentThreads( + projectId, + commentIdsToDuplicate + ) + + logger.debug({ mapping: newCommentIds }, 'replacing comment threads') + + for (const comment of ranges.comments ?? []) { + if (Object.prototype.hasOwnProperty.call(newCommentIds, comment.op.t)) { + const result = newCommentIds[comment.op.t] + if (result.error) { + // We couldn't duplicate the thread, so we need to delete it from + // the resulting ranges. + continue + } + // We have a new id for this comment thread + comment.op.t = result.duplicateId + newRanges.comments.push(comment) + } + } + } else { + newRanges.comments = ranges.comments + } + + const newCommentThreadData = + await ChatApiHandler.promises.generateThreadData( + projectId, + newRanges.comments.map(({ op: { t } }) => t) + ) + await ChatManager.promises.injectUserInfoIntoThreads(newCommentThreadData) + logger.debug({ newCommentThreadData }, 'emitting new comment threads') + EditorRealTimeController.emitToRoom( + projectId, + 'new-comment-threads', + newCommentThreadData + ) + return await EditorController.promises.addDocWithRanges( projectId, parentFolderId, basename, importInfo.lines, - ranges, + newRanges, 'revert', userId ) diff --git a/services/web/frontend/js/features/ide-react/context/review-panel/hooks/use-review-panel-state.ts b/services/web/frontend/js/features/ide-react/context/review-panel/hooks/use-review-panel-state.ts index 3fdfb8c624..1e7b981597 100644 --- a/services/web/frontend/js/features/ide-react/context/review-panel/hooks/use-review-panel-state.ts +++ b/services/web/frontend/js/features/ide-react/context/review-panel/hooks/use-review-panel-state.ts @@ -1452,6 +1452,27 @@ function useReviewPanelState(): ReviewPanel.ReviewPanelState { [getThread] ) ) + useSocketListener( + socket, + 'new-comment-threads', + useCallback( + (threads: ReviewPanelCommentThreadsApi) => { + setCommentThreads(prevState => { + const newThreads = { ...prevState } + for (const threadIdString of Object.keys(threads)) { + const threadId = threadIdString as ThreadId + const { submitting: _, ...thread } = getThread(threadId) + // Replace already loaded messages with the server provided ones + thread.messages = threads[threadId].messages.map(formatComment) + newThreads[threadId] = thread + } + return newThreads + }) + handleLayoutChange({ async: true }) + }, + [getThread] + ) + ) const openSubView = useRef('cur_file') useEffect(() => { diff --git a/services/web/frontend/js/features/source-editor/languages/latex/latex-language.ts b/services/web/frontend/js/features/source-editor/languages/latex/latex-language.ts index 509a604c93..e669913c87 100644 --- a/services/web/frontend/js/features/source-editor/languages/latex/latex-language.ts +++ b/services/web/frontend/js/features/source-editor/languages/latex/latex-language.ts @@ -152,8 +152,7 @@ export const LaTeXLanguage = LRLanguage.define({ 'HrefCommand/ShortTextArgument/ShortArg/...': t.link, 'HrefCommand/UrlArgument/...': t.monospace, 'CtrlSeq Csname': t.tagName, - 'DocumentClass/OptionalArgument/ShortOptionalArg/Normal': - t.attributeValue, + 'DocumentClass/OptionalArgument/ShortOptionalArg/...': t.attributeValue, 'DocumentClass/ShortTextArgument/ShortArg/Normal': t.typeName, 'ListEnvironment/BeginEnv/OptionalArgument/...': t.monospace, Number: t.number, diff --git a/services/web/frontend/js/features/source-editor/languages/latex/linter/latex-linter.worker.js b/services/web/frontend/js/features/source-editor/languages/latex/linter/latex-linter.worker.js index 19b9532297..6ae97d474b 100644 --- a/services/web/frontend/js/features/source-editor/languages/latex/linter/latex-linter.worker.js +++ b/services/web/frontend/js/features/source-editor/languages/latex/linter/latex-linter.worker.js @@ -322,7 +322,7 @@ const read1filename = function (TokeniseResult, k) { } } -const readOptionalLabel = function (TokeniseResult, k) { +const readOptionalArgumentWithUnderscores = function (TokeniseResult, k) { // read a label my_label:text.. const Tokens = TokeniseResult.tokens const text = TokeniseResult.text @@ -348,6 +348,7 @@ const readOptionalLabel = function (TokeniseResult, k) { label = label + str if (str.match(/\]/)) { // breaking due to ] + j++ break } } else if (tok[1] === '_') { @@ -800,6 +801,31 @@ const InterpretTokens = function (TokeniseResult, ErrorReporter) { ) { // Environments.push({command: "end", name: "user-defined-equation", token: token}); seenUserDefinedEndEquation = true + } else if (seq === 'documentclass') { + // try to read any optional params [LABEL].... allowing for + // underscores, advance if found + let newPos = readOptionalArgumentWithUnderscores(TokeniseResult, i) + if (newPos instanceof Error) { + TokenErrorFromTo( + Tokens[i + 1], + Tokens[Math.min(newPos.pos, len - 1)], + 'invalid documentclass option' + ) + i = newPos.pos + } else if (newPos == null) { + /* do nothing */ + } else { + i = newPos + } + // Read parameter {....}, ignore if missing + newPos = readDefinition(TokeniseResult, i) + if (newPos === null) { + // NOTE: We could choose to throw an error here, as the argument is + // required. However, that would show errors as you are typing. So + // maybe it's better to be lenient. + } else { + i = newPos + } } else if ( seq === 'newcommand' || seq === 'renewcommand' || @@ -1038,7 +1064,7 @@ const InterpretTokens = function (TokeniseResult, ErrorReporter) { } else if (seq === 'hyperref') { // try to read any optional params [LABEL].... allowing for // underscores, advance if found - let newPos = readOptionalLabel(TokeniseResult, i) + let newPos = readOptionalArgumentWithUnderscores(TokeniseResult, i) if (newPos instanceof Error) { TokenErrorFromTo( Tokens[i + 1], diff --git a/services/web/frontend/js/features/source-editor/lezer-latex/latex.grammar b/services/web/frontend/js/features/source-editor/lezer-latex/latex.grammar index 01a5aa8f1f..a05ec70fe7 100644 --- a/services/web/frontend/js/features/source-editor/lezer-latex/latex.grammar +++ b/services/web/frontend/js/features/source-editor/lezer-latex/latex.grammar @@ -484,6 +484,7 @@ ShortOptionalArg { ( textBase | NonEmptyGroup | "#" // macro character + | "_" // underscore is used in some parameter names )* } diff --git a/services/web/frontend/stylesheets/app/editor/left-menu.less b/services/web/frontend/stylesheets/app/editor/left-menu.less index 2af6f1dfaf..ae8dd373d3 100644 --- a/services/web/frontend/stylesheets/app/editor/left-menu.less +++ b/services/web/frontend/stylesheets/app/editor/left-menu.less @@ -191,11 +191,18 @@ transition: opacity 0.5s; } +// We want to be able to stack modals on top of the left-side menu. So we make +// it a little lower than the normal modal backdrops. We don't want to go too +// low, to avoid conflicting with dropdowns etc. +@left-menu-z-index-backdrop: @zindex-modal-background - 5; +@left-menu-z-index: @zindex-modal-background - 2; + // Make the Bootstrap Modal behavior as a left sidebar #left-menu-modal { opacity: 1; overflow-y: hidden; padding-left: 0 !important; // bootstrap modal may randomly give padding-left when zooming in / out in chrome + z-index: @left-menu-z-index; .modal-dialog { height: 100%; margin: 0; @@ -219,5 +226,6 @@ // Don't disable backdrop that allows closing the Modal when clicking outside of it, // But match its background color with the original mask background color. .left-menu-modal-backdrop { + z-index: @left-menu-z-index-backdrop; background-color: transparent; } diff --git a/services/web/test/frontend/features/source-editor/languages/latex/latex-linter.test.ts b/services/web/test/frontend/features/source-editor/languages/latex/latex-linter.test.ts index cbfce9e145..f5c374077f 100644 --- a/services/web/test/frontend/features/source-editor/languages/latex/latex-linter.test.ts +++ b/services/web/test/frontend/features/source-editor/languages/latex/latex-linter.test.ts @@ -494,6 +494,23 @@ describe('LatexLinter', function () { assert.equal(errors[0].text, 'unclosed group {') }) + it('should accept documentclass with no options', function () { + const { errors } = Parse('\\documentclass{article}') + assert.equal(errors.length, 0) + }) + + it('should accept documentclass with options', function () { + const { errors } = Parse('\\documentclass[a4paper]{article}') + assert.equal(errors.length, 0) + }) + + it('should accept documentclass with underscore in options', function () { + const { errors } = Parse( + '\\documentclass[my_custom_document_class_option]{my-custom-class}' + ) + assert.equal(errors.length, 0) + }) + // %novalidate // %begin novalidate // %end novalidate diff --git a/services/web/test/unit/src/Chat/ChatControllerTests.js b/services/web/test/unit/src/Chat/ChatControllerTests.js index ca7a5e18d9..12cd2d81d8 100644 --- a/services/web/test/unit/src/Chat/ChatControllerTests.js +++ b/services/web/test/unit/src/Chat/ChatControllerTests.js @@ -19,13 +19,13 @@ const modulePath = path.join( __dirname, '../../../../app/src/Features/Chat/ChatController' ) -const { expect } = require('chai') describe('ChatController', function () { beforeEach(function () { this.user_id = 'mock-user-id' this.settings = {} this.ChatApiHandler = {} + this.ChatManager = {} this.EditorRealTimeController = { emitToRoom: sinon.stub() } this.SessionManager = { getLoggedInUserId: sinon.stub().returns(this.user_id), @@ -34,6 +34,7 @@ describe('ChatController', function () { requires: { '@overleaf/settings': this.settings, './ChatApiHandler': this.ChatApiHandler, + './ChatManager': this.ChatManager, '../Editor/EditorRealTimeController': this.EditorRealTimeController, '../Authentication/SessionManager': this.SessionManager, '../User/UserInfoManager': (this.UserInfoManager = {}), @@ -106,7 +107,7 @@ describe('ChatController', function () { limit: (this.limit = '30'), before: (this.before = '12345'), } - this.ChatController._injectUserInfoIntoThreads = sinon.stub().yields() + this.ChatManager.injectUserInfoIntoThreads = sinon.stub().yields() this.ChatApiHandler.getGlobalMessages = sinon .stub() .yields(null, (this.messages = ['mock', 'messages'])) @@ -123,107 +124,4 @@ describe('ChatController', function () { return this.res.json.calledWith(this.messages).should.equal(true) }) }) - - describe('_injectUserInfoIntoThreads', function () { - beforeEach(function () { - this.users = { - user_id_1: { - mock: 'user_1', - }, - user_id_2: { - mock: 'user_2', - }, - } - this.UserInfoManager.getPersonalInfo = (userId, callback) => { - return callback(null, this.users[userId]) - } - sinon.spy(this.UserInfoManager, 'getPersonalInfo') - return (this.UserInfoController.formatPersonalInfo = user => ({ - formatted: user.mock, - })) - }) - - it('should inject a user object into messaged and resolved data', function (done) { - return this.ChatController._injectUserInfoIntoThreads( - { - thread1: { - resolved: true, - resolved_by_user_id: 'user_id_1', - messages: [ - { - user_id: 'user_id_1', - content: 'foo', - }, - { - user_id: 'user_id_2', - content: 'bar', - }, - ], - }, - thread2: { - messages: [ - { - user_id: 'user_id_1', - content: 'baz', - }, - ], - }, - }, - (error, threads) => { - expect(threads).to.deep.equal({ - thread1: { - resolved: true, - resolved_by_user_id: 'user_id_1', - resolved_by_user: { formatted: 'user_1' }, - messages: [ - { - user_id: 'user_id_1', - user: { formatted: 'user_1' }, - content: 'foo', - }, - { - user_id: 'user_id_2', - user: { formatted: 'user_2' }, - content: 'bar', - }, - ], - }, - thread2: { - messages: [ - { - user_id: 'user_id_1', - user: { formatted: 'user_1' }, - content: 'baz', - }, - ], - }, - }) - return done() - } - ) - }) - - it('should only need to look up each user once', function (done) { - return this.ChatController._injectUserInfoIntoThreads( - [ - { - messages: [ - { - user_id: 'user_id_1', - content: 'foo', - }, - { - user_id: 'user_id_1', - content: 'bar', - }, - ], - }, - ], - (error, threads) => { - this.UserInfoManager.getPersonalInfo.calledOnce.should.equal(true) - return done() - } - ) - }) - }) }) diff --git a/services/web/test/unit/src/Chat/ChatManagerTests.js b/services/web/test/unit/src/Chat/ChatManagerTests.js new file mode 100644 index 0000000000..76d9b79c4b --- /dev/null +++ b/services/web/test/unit/src/Chat/ChatManagerTests.js @@ -0,0 +1,135 @@ +const SandboxedModule = require('sandboxed-module') +const path = require('path') +const sinon = require('sinon') +const modulePath = path.join( + __dirname, + '../../../../app/src/Features/Chat/ChatManager' +) +const { expect } = require('chai') + +describe('ChatManager', function () { + beforeEach(function () { + this.user_id = 'mock-user-id' + this.ChatManager = SandboxedModule.require(modulePath, { + requires: { + '../User/UserInfoManager': (this.UserInfoManager = {}), + '../User/UserInfoController': (this.UserInfoController = {}), + }, + }) + this.req = { + params: { + project_id: this.project_id, + }, + } + this.res = { + json: sinon.stub(), + send: sinon.stub(), + sendStatus: sinon.stub(), + } + }) + + describe('injectUserInfoIntoThreads', function () { + beforeEach(function () { + this.users = { + user_id_1: { + mock: 'user_1', + }, + user_id_2: { + mock: 'user_2', + }, + } + this.UserInfoManager.getPersonalInfo = (userId, callback) => { + return callback(null, this.users[userId]) + } + sinon.spy(this.UserInfoManager, 'getPersonalInfo') + return (this.UserInfoController.formatPersonalInfo = user => ({ + formatted: user.mock, + })) + }) + + it('should inject a user object into messaged and resolved data', function (done) { + return this.ChatManager.injectUserInfoIntoThreads( + { + thread1: { + resolved: true, + resolved_by_user_id: 'user_id_1', + messages: [ + { + user_id: 'user_id_1', + content: 'foo', + }, + { + user_id: 'user_id_2', + content: 'bar', + }, + ], + }, + thread2: { + messages: [ + { + user_id: 'user_id_1', + content: 'baz', + }, + ], + }, + }, + (error, threads) => { + expect(error).to.be.null + expect(threads).to.deep.equal({ + thread1: { + resolved: true, + resolved_by_user_id: 'user_id_1', + resolved_by_user: { formatted: 'user_1' }, + messages: [ + { + user_id: 'user_id_1', + user: { formatted: 'user_1' }, + content: 'foo', + }, + { + user_id: 'user_id_2', + user: { formatted: 'user_2' }, + content: 'bar', + }, + ], + }, + thread2: { + messages: [ + { + user_id: 'user_id_1', + user: { formatted: 'user_1' }, + content: 'baz', + }, + ], + }, + }) + return done() + } + ) + }) + + it('should only need to look up each user once', function (done) { + return this.ChatManager.injectUserInfoIntoThreads( + [ + { + messages: [ + { + user_id: 'user_id_1', + content: 'foo', + }, + { + user_id: 'user_id_1', + content: 'bar', + }, + ], + }, + ], + (error, threads) => { + expect(error).to.be.null + this.UserInfoManager.getPersonalInfo.calledOnce.should.equal(true) + return done() + } + ) + }) + }) +}) diff --git a/services/web/test/unit/src/History/RestoreManagerTests.js b/services/web/test/unit/src/History/RestoreManagerTests.js index 96d198a4f0..603d484b93 100644 --- a/services/web/test/unit/src/History/RestoreManagerTests.js +++ b/services/web/test/unit/src/History/RestoreManagerTests.js @@ -24,7 +24,16 @@ describe('RestoreManager', function () { }), '../Project/ProjectLocator': (this.ProjectLocator = { promises: {} }), '../DocumentUpdater/DocumentUpdaterHandler': - (this.DocumentUpdaterHandler = { promises: {} }), + (this.DocumentUpdaterHandler = { + promises: { flushProjectToMongo: sinon.stub().resolves() }, + }), + '../Docstore/DocstoreManager': (this.DocstoreManager = { + promises: {}, + }), + '../Chat/ChatApiHandler': (this.ChatApiHandler = { promises: {} }), + '../Chat/ChatManager': (this.ChatManager = { promises: {} }), + '../Editor/EditorRealTimeController': (this.EditorRealTimeController = + {}), }, }) this.user_id = 'mock-user-id' @@ -231,10 +240,25 @@ describe('RestoreManager', function () { this.DocumentUpdaterHandler.promises.setDocument = sinon .stub() .resolves() + this.EditorController.promises.deleteEntity = sinon.stub().resolves() + this.RestoreManager.promises._getRangesFromHistory = sinon + .stub() + .resolves({ changes: [], comments: [] }) + this.DocstoreManager.promises.getAllRanges = sinon.stub().resolves([]) + this.ChatApiHandler.promises.generateThreadData = sinon + .stub() + .resolves({}) + this.ChatManager.promises.injectUserInfoIntoThreads = sinon + .stub() + .resolves() + this.EditorRealTimeController.emitToRoom = sinon.stub() + this.EditorController.promises.addDocWithRanges = sinon + .stub() + .resolves() }) - it('should call setDocument in document updater and revert file', async function () { - const revertRes = await this.RestoreManager.promises.revertFile( + it('should delete the existing document', async function () { + await this.RestoreManager.promises.revertFile( this.user_id, this.project_id, this.version, @@ -242,15 +266,14 @@ describe('RestoreManager', function () { ) expect( - this.DocumentUpdaterHandler.promises.setDocument + this.EditorController.promises.deleteEntity ).to.have.been.calledWith( this.project_id, 'mock-file-id', - this.user_id, - ['foo', 'bar', 'baz'], - 'file-revert' + 'doc', + 'revert', + this.user_id ) - expect(revertRes).to.deep.equal({ _id: 'mock-file-id', type: 'doc' }) }) }) @@ -297,7 +320,34 @@ describe('RestoreManager', function () { describe("when reverting a file that doesn't current exist", function () { beforeEach(async function () { this.pathname = 'foo.tex' + this.comments = [ + (this.comment = { op: { t: 'comment-1', p: 0, c: 'foo' } }), + ] + this.remappedComments = [{ op: { t: 'comment-2', p: 0, c: 'foo' } }] this.ProjectLocator.promises.findElementByPath = sinon.stub().rejects() + this.DocstoreManager.promises.getAllRanges = sinon.stub().resolves([ + { + ranges: { + comments: [this.comment], + }, + }, + ]) + this.ChatApiHandler.promises.duplicateCommentThreads = sinon + .stub() + .resolves({ + newThreads: { + 'comment-1': { + duplicateId: 'comment-2', + }, + }, + }) + this.ChatApiHandler.promises.generateThreadData = sinon + .stub() + .resolves({}) + this.ChatManager.promises.injectUserInfoIntoThreads = sinon + .stub() + .resolves() + this.EditorRealTimeController.emitToRoom = sinon.stub() this.tracked_changes = [ { op: { pos: 4, i: 'bar' }, @@ -308,13 +358,12 @@ describe('RestoreManager', function () { metadata: { ts: '2024-01-01T00:00:00.000Z', user_id: 'user-2' }, }, ] - this.comments = [{ op: { t: 'comment-1', p: 0, c: 'foo' } }] this.FileSystemImportManager.promises.importFile = sinon .stub() .resolves({ type: 'doc', lines: ['foo', 'bar', 'baz'] }) this.RestoreManager.promises._getRangesFromHistory = sinon .stub() - .resolves({ changes: this.tracked_changes, comment: this.comments }) + .resolves({ changes: this.tracked_changes, comments: this.comments }) this.EditorController.promises.addDocWithRanges = sinon .stub() .resolves( @@ -336,7 +385,7 @@ describe('RestoreManager', function () { this.folder_id, 'foo.tex', ['foo', 'bar', 'baz'], - { changes: this.tracked_changes, comment: this.comments } + { changes: this.tracked_changes, comments: this.remappedComments } ) })