Merge pull request #18778 from overleaf/mj-web-chat-restore-tests

[chat+web] Add tests for revert functionality

GitOrigin-RevId: f10a5589d8ee1299949ef3decd8325d8fa1f7d41

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)
+    })
+  })
 })

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,
+    },
+  })
+}

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)
+    })
+  })
 })

services/web/test/unit/src/History/RestoreManagerTests.js

@@ -225,100 +225,8 @@ 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
-          .stub()
-          .resolves({ type: 'doc' })
-        this.ProjectLocator.promises.findElementByPath = sinon
-          .stub()
-          .resolves({ type: 'doc', element: { _id: 'mock-file-id' } })
-        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()
-        this.EditorController.promises.addDocWithRanges = sinon
-          .stub()
-          .resolves()
-      })
-
-      it('should delete the existing document', async function () {
-        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
-        )
-      })
-    })
-
-    describe('when reverting a binary file', function () {
-      beforeEach(async function () {
-        this.pathname = 'foo.png'
-        this.FileSystemImportManager.promises.importFile = sinon
-          .stub()
-          .resolves({ type: 'file' })
-        this.EditorController.promises.upsertFile = sinon
-          .stub()
-          .resolves({ _id: 'mock-file-id', type: 'file' })
-      })
-
-      it('should return the created entity if file exists', async function () {
-        this.ProjectLocator.promises.findElementByPath = sinon
-          .stub()
-          .resolves({ type: 'file' })
-
-        const revertRes = await this.RestoreManager.promises.revertFile(
-          this.user_id,
-          this.project_id,
-          this.version,
-          this.pathname
-        )
-
-        expect(revertRes).to.deep.equal({ _id: 'mock-file-id', type: 'file' })
-      })
-
-      it('should return the created entity if file does not exists', async function () {
-        this.ProjectLocator.promises.findElementByPath = sinon.stub().rejects()
-
-        const revertRes = await this.RestoreManager.promises.revertFile(
-          this.user_id,
-          this.project_id,
-          this.version,
-          this.pathname
-        )
-
-        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' } }),
@@ -341,12 +249,23 @@ describe('RestoreManager', function () {
               },
             },
           })
-        this.ChatApiHandler.promises.generateThreadData = sinon
-          .stub()
-          .resolves({})
+        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()
+          .resolves(this.threadData)
+
         this.EditorRealTimeController.emitToRoom = sinon.stub()
         this.tracked_changes = [
           {
@@ -363,40 +282,170 @@ describe('RestoreManager', function () {
           .resolves({ type: 'doc', lines: ['foo', 'bar', 'baz'] })
         this.RestoreManager.promises._getRangesFromHistory = sinon
           .stub()
-          .resolves({ changes: this.tracked_changes, comments: this.comments })
+          .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(
+      })
+
+      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
+          )
+        })
+
+        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
+          )
+        })
+      })
+    })
+
+    describe('when reverting a binary file', function () {
+      beforeEach(async function () {
+        this.pathname = 'foo.png'
+        this.FileSystemImportManager.promises.importFile = sinon
+          .stub()
+          .resolves({ type: 'file' })
+        this.EditorController.promises.upsertFile = sinon
+          .stub()
+          .resolves({ _id: 'mock-file-id', type: 'file' })
+      })
+
+      it('should return the created entity if file exists', async function () {
+        this.ProjectLocator.promises.findElementByPath = sinon
+          .stub()
+          .resolves({ type: 'file' })
+
+        const revertRes = await this.RestoreManager.promises.revertFile(
           this.user_id,
           this.project_id,
           this.version,
           this.pathname
         )
+
+        expect(revertRes).to.deep.equal({ _id: 'mock-file-id', type: 'file' })
       })
 
-      it('should import the file', function () {
-        expect(
-          this.EditorController.promises.addDocWithRanges
-        ).to.have.been.calledWith(
+      it('should return the created entity if file does not exists', async function () {
+        this.ProjectLocator.promises.findElementByPath = sinon.stub().rejects()
+
+        const revertRes = await this.RestoreManager.promises.revertFile(
+          this.user_id,
           this.project_id,
-          this.folder_id,
-          'foo.tex',
-          ['foo', 'bar', 'baz'],
-          { changes: this.tracked_changes, comments: this.remappedComments }
+          this.version,
+          this.pathname
         )
-      })
-
-      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)
+        expect(revertRes).to.deep.equal({ _id: 'mock-file-id', type: 'file' })
       })
     })
   })