From 8c6c67564fa4ac820007c5bab5201b6ea46717a5 Mon Sep 17 00:00:00 2001 From: Mathias Jakobsen Date: Mon, 10 Jun 2024 13:02:56 +0100 Subject: [PATCH] Merge pull request #18778 from overleaf/mj-web-chat-restore-tests [chat+web] Add tests for revert functionality GitOrigin-RevId: f10a5589d8ee1299949ef3decd8325d8fa1f7d41 --- .../acceptance/js/GettingMessagesTests.js | 47 +++ .../test/acceptance/js/helpers/ChatClient.js | 10 + .../test/unit/src/Chat/ChatApiHandlerTests.js | 99 ++++++ .../unit/src/History/RestoreManagerTests.js | 285 ++++++++++-------- 4 files changed, 323 insertions(+), 118 deletions(-) diff --git a/services/chat/test/acceptance/js/GettingMessagesTests.js b/services/chat/test/acceptance/js/GettingMessagesTests.js index af6a0624a9..4fb20da2a1 100644 --- a/services/chat/test/acceptance/js/GettingMessagesTests.js +++ b/services/chat/test/acceptance/js/GettingMessagesTests.js @@ -114,4 +114,51 @@ describe('Getting messages', async function () { expect(thread2.messages[1].user_id).to.equal(userId2) }) }) + + describe('from a list of threads', function () { + const projectId = new ObjectId().toString() + const threadId1 = new ObjectId().toString() + const threadId2 = new ObjectId().toString() + const threadId3 = new ObjectId().toString() + + before(async function () { + const { response } = await ChatClient.sendMessage( + projectId, + threadId1, + userId1, + 'one' + ) + expect(response.statusCode).to.equal(201) + const { response: response2 } = await ChatClient.sendMessage( + projectId, + threadId2, + userId2, + 'two' + ) + expect(response2.statusCode).to.equal(201) + const { response: response3 } = await ChatClient.sendMessage( + projectId, + threadId1, + userId1, + 'three' + ) + expect(response3.statusCode).to.equal(201) + }) + + it('should contain a dictionary of threads with messages with populated users', async function () { + const { response, body: threads } = await ChatClient.generateThreadData( + projectId, + [threadId1, threadId3] + ) + expect(response.statusCode).to.equal(200) + expect(Object.keys(threads).length).to.equal(1) + const thread1 = threads[threadId1] + expect(thread1.messages.length).to.equal(2) + + expect(thread1.messages[0].content).to.equal('one') + expect(thread1.messages[0].user_id).to.equal(userId1) + expect(thread1.messages[1].content).to.equal('three') + expect(thread1.messages[1].user_id).to.equal(userId1) + }) + }) }) diff --git a/services/chat/test/acceptance/js/helpers/ChatClient.js b/services/chat/test/acceptance/js/helpers/ChatClient.js index 857c3f7fc8..6a8196dd02 100644 --- a/services/chat/test/acceptance/js/helpers/ChatClient.js +++ b/services/chat/test/acceptance/js/helpers/ChatClient.js @@ -154,3 +154,13 @@ export async function duplicateCommentThreads(projectId, threads) { }, }) } + +export async function generateThreadData(projectId, threads) { + return await asyncRequest({ + method: 'post', + url: `/project/${projectId}/generate-thread-data`, + json: { + threads, + }, + }) +} diff --git a/services/web/test/unit/src/Chat/ChatApiHandlerTests.js b/services/web/test/unit/src/Chat/ChatApiHandlerTests.js index 560e9cf023..561e6f27bb 100644 --- a/services/web/test/unit/src/Chat/ChatApiHandlerTests.js +++ b/services/web/test/unit/src/Chat/ChatApiHandlerTests.js @@ -128,4 +128,103 @@ describe('ChatApiHandler', function () { }) }) }) + + describe('duplicateCommentThreads', function () { + beforeEach(async function () { + this.FetchUtils.fetchJson.resolves( + (this.mapping = { + 'comment-thread-1': 'comment-thread-1-dup', + 'comment-thread-2': 'comment-thread-2-dup', + 'comment-thread-3': 'comment-thread-3-dup', + }) + ) + this.threads = [ + 'comment-thread-1', + 'comment-thread-2', + 'comment-thread-3', + ] + this.result = await this.ChatApiHandler.promises.duplicateCommentThreads( + this.project_id, + this.threads + ) + }) + + it('should make a post request to the chat api', function () { + expect(this.FetchUtils.fetchJson).to.have.been.calledWith( + sinon.match( + url => + url.toString() === + `${this.settings.apis.chat.internal_url}/project/${this.project_id}/duplicate-comment-threads` + ), + { + method: 'POST', + json: { + threads: this.threads, + }, + } + ) + }) + + it('should return the thread mapping', function () { + expect(this.result).to.deep.equal(this.mapping) + }) + }) + + describe('generateThreadData', async function () { + beforeEach(async function () { + this.FetchUtils.fetchJson.resolves( + (this.chatResponse = { + 'comment-thread-1': { + messages: [ + { + content: 'message 1', + timestamp: '2024-01-01T00:00:00.000Z', + user_id: 'user-1', + }, + ], + }, + 'comment-thread-2': { + messages: [ + { + content: 'message 2', + timestamp: '2024-01-01T00:00:00.000Z', + user_id: 'user-2', + }, + ], + }, + }) + ) + // Chat won't return threads that couldn't be found, so response can have + // fewer threads + this.threads = [ + 'comment-thread-1', + 'comment-thread-2', + 'comment-thread-3', + ] + this.result = await this.ChatApiHandler.promises.generateThreadData( + this.project_id, + this.threads + ) + }) + + it('should make a post request to the chat api', function () { + expect(this.FetchUtils.fetchJson).to.have.been.calledWith( + sinon.match( + url => + url.toString() === + `${this.settings.apis.chat.internal_url}/project/${this.project_id}/generate-thread-data` + ), + { + method: 'POST', + json: { + threads: this.threads, + }, + } + ) + }) + + it('should return the thread data', function () { + expect(this.result).to.deep.equal(this.chatResponse) + }) + }) }) diff --git a/services/web/test/unit/src/History/RestoreManagerTests.js b/services/web/test/unit/src/History/RestoreManagerTests.js index 603d484b93..11d34fc213 100644 --- a/services/web/test/unit/src/History/RestoreManagerTests.js +++ b/services/web/test/unit/src/History/RestoreManagerTests.js @@ -225,55 +225,187 @@ describe('RestoreManager', function () { .rejects() }) - describe('with an existing file in the current project', function () { + describe('reverting a document', function () { beforeEach(function () { this.pathname = 'foo.tex' - this.FileSystemImportManager.promises.importFile = sinon + 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({ type: 'doc' }) - this.ProjectLocator.promises.findElementByPath = sinon + .resolves({ + newThreads: { + 'comment-1': { + duplicateId: 'comment-2', + }, + }, + }) + this.ChatApiHandler.promises.generateThreadData = sinon.stub().resolves( + (this.threadData = { + 'comment-2': { + messages: [ + { + content: 'message-content', + timestamp: '2024-01-01T00:00:00.000Z', + user_id: 'user-1', + }, + ], + }, + }) + ) + this.ChatManager.promises.injectUserInfoIntoThreads = sinon .stub() - .resolves({ type: 'doc', element: { _id: 'mock-file-id' } }) + .resolves(this.threadData) + + this.EditorRealTimeController.emitToRoom = sinon.stub() + this.tracked_changes = [ + { + op: { pos: 4, i: 'bar' }, + metadata: { ts: '2024-01-01T00:00:00.000Z', user_id: 'user-1' }, + }, + { + op: { pos: 8, d: 'qux' }, + metadata: { ts: '2024-01-01T00:00:00.000Z', user_id: 'user-2' }, + }, + ] this.FileSystemImportManager.promises.importFile = sinon .stub() .resolves({ type: 'doc', lines: ['foo', 'bar', 'baz'] }) - 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() + .resolves({ + changes: this.tracked_changes, + comments: this.comments, + }) this.EditorController.promises.addDocWithRanges = sinon .stub() - .resolves() + .resolves( + (this.addedFile = { doc: 'mock-doc', folderId: 'mock-folder' }) + ) }) - it('should delete the existing document', async function () { - await this.RestoreManager.promises.revertFile( - this.user_id, - this.project_id, - this.version, - this.pathname - ) + describe("when reverting a file that doesn't current exist", function () { + beforeEach(async function () { + this.data = await this.RestoreManager.promises.revertFile( + this.user_id, + this.project_id, + this.version, + this.pathname + ) + }) - expect( - this.EditorController.promises.deleteEntity - ).to.have.been.calledWith( - this.project_id, - 'mock-file-id', - 'doc', - 'revert', - this.user_id - ) + it('should flush the document before fetching ranges', function () { + expect( + this.DocumentUpdaterHandler.promises.flushProjectToMongo + ).to.have.been.calledBefore( + this.DocstoreManager.promises.getAllRanges + ) + }) + + it('should import the file', function () { + expect( + this.EditorController.promises.addDocWithRanges + ).to.have.been.calledWith( + this.project_id, + this.folder_id, + 'foo.tex', + ['foo', 'bar', 'baz'], + { changes: this.tracked_changes, comments: this.remappedComments } + ) + }) + + it('should return the created entity', function () { + expect(this.data).to.equal(this.addedFile) + }) + + it('should look up ranges', function () { + expect( + this.RestoreManager.promises._getRangesFromHistory + ).to.have.been.calledWith( + this.project_id, + this.version, + this.pathname + ) + }) + }) + + describe('with an existing file in the current project', function () { + beforeEach(async function () { + this.ProjectLocator.promises.findElementByPath = sinon + .stub() + .resolves({ type: 'doc', element: { _id: 'mock-file-id' } }) + this.EditorController.promises.deleteEntity = sinon.stub().resolves() + + this.data = await this.RestoreManager.promises.revertFile( + this.user_id, + this.project_id, + this.version, + this.pathname + ) + }) + + it('should delete the existing document', async function () { + expect( + this.EditorController.promises.deleteEntity + ).to.have.been.calledWith( + this.project_id, + 'mock-file-id', + 'doc', + 'revert', + this.user_id + ) + }) + + it('should delete the document before flushing', function () { + expect( + this.EditorController.promises.deleteEntity + ).to.have.been.calledBefore( + this.DocumentUpdaterHandler.promises.flushProjectToMongo + ) + }) + + it('should flush the document before fetching ranges', function () { + expect( + this.DocumentUpdaterHandler.promises.flushProjectToMongo + ).to.have.been.calledBefore( + this.DocstoreManager.promises.getAllRanges + ) + }) + + it('should import the file', function () { + expect( + this.EditorController.promises.addDocWithRanges + ).to.have.been.calledWith( + this.project_id, + this.folder_id, + 'foo.tex', + ['foo', 'bar', 'baz'], + { changes: this.tracked_changes, comments: this.remappedComments } + ) + }) + + it('should return the created entity', function () { + expect(this.data).to.equal(this.addedFile) + }) + + it('should look up ranges', function () { + expect( + this.RestoreManager.promises._getRangesFromHistory + ).to.have.been.calledWith( + this.project_id, + this.version, + this.pathname + ) + }) }) }) @@ -316,88 +448,5 @@ describe('RestoreManager', function () { expect(revertRes).to.deep.equal({ _id: 'mock-file-id', type: 'file' }) }) }) - - 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' }, - metadata: { ts: '2024-01-01T00:00:00.000Z', user_id: 'user-1' }, - }, - { - op: { pos: 8, d: 'qux' }, - metadata: { ts: '2024-01-01T00:00:00.000Z', user_id: 'user-2' }, - }, - ] - this.FileSystemImportManager.promises.importFile = sinon - .stub() - .resolves({ type: 'doc', lines: ['foo', 'bar', 'baz'] }) - this.RestoreManager.promises._getRangesFromHistory = sinon - .stub() - .resolves({ changes: this.tracked_changes, comments: this.comments }) - this.EditorController.promises.addDocWithRanges = sinon - .stub() - .resolves( - (this.addedFile = { doc: 'mock-doc', folderId: 'mock-folder' }) - ) - this.data = await this.RestoreManager.promises.revertFile( - this.user_id, - this.project_id, - this.version, - this.pathname - ) - }) - - it('should import the file', function () { - expect( - this.EditorController.promises.addDocWithRanges - ).to.have.been.calledWith( - this.project_id, - this.folder_id, - 'foo.tex', - ['foo', 'bar', 'baz'], - { changes: this.tracked_changes, comments: this.remappedComments } - ) - }) - - it('should return the created entity', function () { - expect(this.data).to.equal(this.addedFile) - }) - - it('should look up ranges', function () { - expect( - this.RestoreManager.promises._getRangesFromHistory - ).to.have.been.calledWith(this.project_id, this.version, this.pathname) - }) - }) }) })