diff --git a/services/web/app/src/Features/History/RestoreManager.js b/services/web/app/src/Features/History/RestoreManager.js index dbde81a452..a2401a5c6e 100644 --- a/services/web/app/src/Features/History/RestoreManager.js +++ b/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( diff --git a/services/web/frontend/js/features/history/services/types/shared.ts b/services/web/frontend/js/features/history/services/types/shared.ts index 47785cd139..90962db115 100644 --- a/services/web/frontend/js/features/history/services/types/shared.ts +++ b/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' diff --git a/services/web/frontend/js/features/ide-react/context/editor-manager-context.tsx b/services/web/frontend/js/features/ide-react/context/editor-manager-context.tsx index bc2ce5a673..f90e7c1961 100644 --- a/services/web/frontend/js/features/ide-react/context/editor-manager-context.tsx +++ b/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') diff --git a/services/web/frontend/js/ide/editor/EditorManager.js b/services/web/frontend/js/ide/editor/EditorManager.js index 49ae7a4517..2f3abb7b24 100644 --- a/services/web/frontend/js/ide/editor/EditorManager.js +++ b/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.' diff --git a/services/web/test/unit/src/History/RestoreManagerTests.js b/services/web/test/unit/src/History/RestoreManagerTests.js index ec7ed71075..96d198a4f0 100644 --- a/services/web/test/unit/src/History/RestoreManagerTests.js +++ b/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 () { - expect( - this.RestoreManager.promises.revertFile( - this.user_id, - this.project_id, - this.version, - this.pathname - ) + 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 ) - .to.eventually.be.rejectedWith('File already exists') - .and.be.instanceOf(Errors.InvalidError) + + expect( + this.DocumentUpdaterHandler.promises.setDocument + ).to.have.been.calledWith( + this.project_id, + 'mock-file-id', + this.user_id, + ['foo', 'bar', 'baz'], + 'file-revert' + ) + 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' }) }) })