Selaa lähdekoodia

[web] support for reverting binary files (#18033)

* [web] revert binary file

* use addEntityWithName if file was deleted

* todo comments

* only show Revert file in ui even if deleted

* use _revertBinaryFile function

* emit new ids when reverting

* format:fix

* await emitToRoom calls

* use EditorController.upsertFile

* remove _revertBinaryFile function

* binary file check

* mock importFile method in tests

* move findElementByPath stub

* debug ci error

* resolve with empty object as file

* fix tests

* remove await before expect()

* format:fix

* test when binary file exists and when it does not

* use "file-revert" for source

* [web] revert existing file without ranges support (#18107)

* [web] revert existing file without ranges support

* ignore document_updated_externally if file-revert

* fix test

GitOrigin-RevId: a5e0c83a7635bc7d934dec9debe916bdd4beb51e
Domagoj Kriskovic 2 vuotta sitten
vanhempi
sitoutus
218a4538c1

+ 37 - 16
services/web/app/src/Features/History/RestoreManager.js

@@ -8,6 +8,7 @@ const moment = require('moment')
 const { callbackifyAll } = require('@overleaf/promise-utils')
 const { fetchJson } = require('@overleaf/fetch-utils')
 const ProjectLocator = require('../Project/ProjectLocator')
+const DocumentUpdaterHandler = require('../DocumentUpdater/DocumentUpdaterHandler')
 
 const RestoreManager = {
   async restoreFileFromV2(userId, projectId, version, pathname) {
@@ -42,6 +43,7 @@ const RestoreManager = {
   },
 
   async revertFile(userId, projectId, version, pathname) {
+    const source = 'file-revert'
     const fsPath = await RestoreManager._writeFileVersionToDisk(
       projectId,
       version,
@@ -50,34 +52,53 @@ const RestoreManager = {
     const basename = Path.basename(pathname)
     let dirname = Path.dirname(pathname)
     if (dirname === '.') {
-      // no directory
-      dirname = ''
+      // root directory
+      dirname = '/'
     }
     const parentFolderId = await RestoreManager._findOrCreateFolder(
       projectId,
       dirname
     )
-    let fileExists = true
-    try {
-      // TODO: Is there a better way of doing this?
-      await ProjectLocator.promises.findElementByPath({
-        projectId,
+    const file = await ProjectLocator.promises
+      .findElementByPath({
+        project_id: projectId,
         path: pathname,
       })
-    } catch (error) {
-      fileExists = false
-    }
-    if (fileExists) {
-      throw new Errors.InvalidError('File already exists')
-    }
+      .catch(() => null)
 
     const importInfo = await FileSystemImportManager.promises.importFile(
       fsPath,
       pathname
     )
-    if (importInfo.type !== 'doc') {
-      // TODO: Handle binary files
-      throw new Errors.InvalidError('File is not editable')
+    if (importInfo.type === 'file') {
+      const newFile = await EditorController.promises.upsertFile(
+        projectId,
+        parentFolderId,
+        basename,
+        fsPath,
+        file?.element?.linkedFileData,
+        source,
+        userId
+      )
+
+      return {
+        _id: newFile._id,
+        type: importInfo.type,
+      }
+    }
+
+    if (file) {
+      await DocumentUpdaterHandler.promises.setDocument(
+        projectId,
+        file.element._id,
+        userId,
+        importInfo.lines,
+        source
+      )
+      return {
+        _id: file.element._id,
+        type: importInfo.type,
+      }
     }
 
     const ranges = await RestoreManager._getRangesFromHistory(

+ 1 - 1
services/web/frontend/js/features/history/services/types/shared.ts

@@ -12,7 +12,7 @@ export interface Meta {
   start_ts: number
   end_ts: number
   type?: 'external' // TODO
-  source?: 'git-bridge' // TODO
+  source?: 'git-bridge' | 'file-revert' // TODO
   origin?: {
     kind:
       | 'dropbox'

+ 3 - 0
services/web/frontend/js/features/ide-react/context/editor-manager-context.tsx

@@ -283,6 +283,9 @@ export const EditorManagerProvider: FC = ({ children }) => {
         ) {
           return
         }
+        if (update.meta.source === 'file-revert') {
+          return
+        }
         showGenericMessageModal(
           t('document_updated_externally'),
           t('document_updated_externally_detail')

+ 3 - 0
services/web/frontend/js/ide/editor/EditorManager.js

@@ -442,6 +442,9 @@ export default EditorManager = (function () {
         ) {
           return
         }
+        if (update?.meta?.source === 'file-revert') {
+          return
+        }
         return this.ide.showGenericMessageModal(
           'Document Updated Externally',
           'This document was just updated externally. Any recent changes you have made may have been overwritten. To see previous versions please look in the history.'

+ 61 - 22
services/web/test/unit/src/History/RestoreManagerTests.js

@@ -23,6 +23,8 @@ describe('RestoreManager', function () {
           promises: {},
         }),
         '../Project/ProjectLocator': (this.ProjectLocator = { promises: {} }),
+        '../DocumentUpdater/DocumentUpdaterHandler':
+          (this.DocumentUpdaterHandler = { promises: {} }),
       },
     })
     this.user_id = 'mock-user-id'
@@ -217,41 +219,78 @@ describe('RestoreManager', function () {
     describe('with an existing file in the current project', function () {
       beforeEach(function () {
         this.pathname = 'foo.tex'
-        this.ProjectLocator.promises.findElementByPath = sinon.stub().resolves()
+        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()
       })
 
-      it('should reject', function () {
+      it('should call setDocument in document updater and revert file', async function () {
+        const revertRes = await this.RestoreManager.promises.revertFile(
+          this.user_id,
+          this.project_id,
+          this.version,
+          this.pathname
+        )
+
         expect(
-          this.RestoreManager.promises.revertFile(
-            this.user_id,
-            this.project_id,
-            this.version,
-            this.pathname
-          )
+          this.DocumentUpdaterHandler.promises.setDocument
+        ).to.have.been.calledWith(
+          this.project_id,
+          'mock-file-id',
+          this.user_id,
+          ['foo', 'bar', 'baz'],
+          'file-revert'
         )
-          .to.eventually.be.rejectedWith('File already exists')
-          .and.be.instanceOf(Errors.InvalidError)
+        expect(revertRes).to.deep.equal({ _id: 'mock-file-id', type: 'doc' })
       })
     })
 
     describe('when reverting a binary file', function () {
-      beforeEach(function () {
+      beforeEach(async function () {
         this.pathname = 'foo.png'
         this.FileSystemImportManager.promises.importFile = sinon
           .stub()
-          .resolves({ type: 'binary' })
+          .resolves({ type: 'file' })
+        this.EditorController.promises.upsertFile = sinon
+          .stub()
+          .resolves({ _id: 'mock-file-id', type: 'file' })
       })
-      it('should reject', function () {
-        expect(
-          this.RestoreManager.promises.revertFile(
-            this.user_id,
-            this.project_id,
-            this.version,
-            this.pathname
-          )
+
+      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
         )
-          .to.eventually.be.rejectedWith('File is not editable')
-          .and.be.instanceOf(Errors.InvalidError)
+
+        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' })
       })
     })