Add add{File|Doc}WithoutUpdatingHistory methods to allow importing OL projects with existing history

services/web/app/coffee/Features/History/HistoryController.coffee

@@ -5,29 +5,6 @@ AuthenticationController = require "../Authentication/AuthenticationController"
 ProjectDetailsHandler = require "../Project/ProjectDetailsHandler"
 
 module.exports = HistoryController =
-	initializeProject: (callback = (error, history_id) ->) ->
-		return callback() if !settings.apis.project_history?.initializeHistoryForNewProjects
-		request.post {
-			url: "#{settings.apis.project_history.url}/project"
-		}, (error, res, body)->
-			return callback(error) if error?
-
-			if res.statusCode >= 200 and res.statusCode < 300
-				try
-					project = JSON.parse(body)
-				catch error
-					return callback(error)
-
-				overleaf_id = project?.project?.id
-				if !overleaf_id
-					error = new Error("project-history did not provide an id", project)
-					return callback(error)
-
-				callback null, { overleaf_id }
-			else
-				error = new Error("project-history returned a non-success status code: #{res.statusCode}")
-				callback error
-
 	selectHistoryApi: (req, res, next = (error) ->) ->
 		project_id = req.params?.Project_id
 		# find out which type of history service this project uses

services/web/app/coffee/Features/History/HistoryManager.coffee

@@ -0,0 +1,26 @@
+request = require "request"
+settings = require "settings-sharelatex"
+
+module.exports = HistoryManager =
+	initializeProject: (callback = (error, history_id) ->) ->
+		return callback() if !settings.apis.project_history?.initializeHistoryForNewProjects
+		request.post {
+			url: "#{settings.apis.project_history.url}/project"
+		}, (error, res, body)->
+			return callback(error) if error?
+
+			if res.statusCode >= 200 and res.statusCode < 300
+				try
+					project = JSON.parse(body)
+				catch error
+					return callback(error)
+
+				overleaf_id = project?.project?.id
+				if !overleaf_id
+					error = new Error("project-history did not provide an id", project)
+					return callback(error)
+
+				callback null, { overleaf_id }
+			else
+				error = new Error("project-history returned a non-success status code: #{res.statusCode}")
+				callback error

services/web/app/coffee/Features/Project/ProjectCreationHandler.coffee

@@ -7,7 +7,7 @@ Project = require('../../models/Project').Project
 Folder = require('../../models/Folder').Folder
 ProjectEntityHandler = require('./ProjectEntityHandler')
 ProjectDetailsHandler = require('./ProjectDetailsHandler')
-HistoryController = require('../History/HistoryController')
+HistoryManager = require('../History/HistoryManager')
 User = require('../../models/User').User
 fs = require('fs')
 Path = require "path"
@@ -27,7 +27,7 @@ module.exports = ProjectCreationHandler =
 			if projectHistoryId?
 				ProjectCreationHandler._createBlankProject owner_id, projectName, projectHistoryId, callback
 			else
-				HistoryController.initializeProject (error, history) ->
+				HistoryManager.initializeProject (error, history) ->
 					return callback(error) if error?
 					ProjectCreationHandler._createBlankProject owner_id, projectName, history?.overleaf_id, callback
 

services/web/app/coffee/Features/Project/ProjectDuplicator.coffee

@@ -21,7 +21,7 @@ module.exports = ProjectDuplicator =
 				if !doc?._id?
 					return callback()
 				content = docContents[doc._id.toString()]
-				projectEntityHandler.addDocWithProject newProject, desFolder._id, doc.name, content.lines, owner_id, (err, newDoc)->
+				projectEntityHandler.addDoc newProject, desFolder._id, doc.name, content.lines, owner_id, (err, newDoc)->
 					if err?
 						logger.err err:err, "error copying doc"
 						return callback(err)

services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee

@@ -149,14 +149,35 @@ module.exports = ProjectEntityHandler =
 		else
 			DocstoreManager.getDoc project_id, doc_id, options, callback
 
-	addDoc: (project_id, folder_id, docName, docLines, userId, callback = (error, doc, folder_id) ->)=>
+	addDoc: (project_or_id, folder_id, docName, docLines, userId, callback = (error, doc, folder_id) ->)=>
-		ProjectGetter.getProjectWithOnlyFolders project_id, (err, project) ->
+		ProjectEntityHandler.addDocWithoutUpdatingHistory project_or_id, folder_id, docName, docLines, userId, (error, doc, folder_id, result) ->
+			return callback(error) if error?
+			newDocs = [
+				doc: doc
+				path: result?.path?.fileSystem
+				docLines: docLines.join('\n')
+			]
+			project_id = project_or_id._id or project_or_id
+			DocumentUpdaterHandler.updateProjectStructure project_id, userId, {newDocs}, (error) ->
+				return callback(error) if error?
+				callback null, doc, folder_id
+
+	addDocWithoutUpdatingHistory: (project_or_id, folder_id, docName, docLines, userId, callback = (error, doc, folder_id) ->)=>
+		# This method should never be called directly, except when importing a project
+		# from Overleaf. It skips sending updates to the project history, which will break
+		# the history unless you are making sure it is updated in some other way.
+		getProject = (cb) ->
+			if project_or_id._id? # project
+				return cb(null, project_or_id)
+			else # id
+				return ProjectGetter.getProjectWithOnlyFolders project_or_id, cb
+		getProject (error, project) ->
 			if err?
 				logger.err project_id:project_id, err:err, "error getting project for add doc"
 				return callback(err)
-			ProjectEntityHandler.addDocWithProject project, folder_id, docName, docLines, userId, callback
+			ProjectEntityHandler._addDocWithProject project, folder_id, docName, docLines, userId, callback
 
-	addDocWithProject: (project, folder_id, docName, docLines, userId, callback = (error, doc, folder_id) ->)=>
+	_addDocWithProject: (project, folder_id, docName, docLines, userId, callback = (error, doc, folder_id, result) ->)=>
 		project_id = project._id
 		logger.log project_id: project_id, folder_id: folder_id, doc_name: docName, "adding doc to project with project"
 		confirmFolder project, folder_id, (folder_id)=>
@@ -176,14 +197,7 @@ module.exports = ProjectEntityHandler =
 						rev:          0
 					}, (err) ->
 						return callback(err) if err?
-						newDocs = [
+						callback(null, doc, folder_id, result)
-							doc: doc
-							path: result?.path?.fileSystem
-							docLines: docLines.join('\n')
-						]
-						DocumentUpdaterHandler.updateProjectStructure project_id, userId, {newDocs}, (error) ->
-							return callback(error) if error?
-							callback null, doc, folder_id
 
 	restoreDoc: (project_id, doc_id, name, callback = (error, doc, folder_id) ->) ->
 		# getDoc will return the deleted doc's lines, but we don't actually remove
@@ -192,37 +206,37 @@ module.exports = ProjectEntityHandler =
 			return callback(error) if error?
 			ProjectEntityHandler.addDoc project_id, null, name, lines, callback
 
-	addFile: (project_id, folder_id, fileName, path, userId, callback = (error, fileRef, folder_id) ->)->
+	addFileWithoutUpdatingHistory: (project_id, folder_id, fileName, path, userId, callback = (error, fileRef, folder_id, result, fileStoreUrl) ->)->
 		ProjectGetter.getProjectWithOnlyFolders project_id, (err, project) ->
 			if err?
 				logger.err project_id:project_id, err:err, "error getting project for add file"
 				return callback(err)
-			ProjectEntityHandler.addFileWithProject project, folder_id, fileName, path, userId, callback
+			logger.log project_id: project._id, folder_id: folder_id, file_name: fileName, path:path, "adding file"
-
+			return callback(err) if err?
-	addFileWithProject: (project, folder_id, fileName, path, userId, callback = (error, fileRef, folder_id) ->)->
+			confirmFolder project, folder_id, (folder_id)->
-		project_id = project._id
+				fileRef = new File name : fileName
-		logger.log project_id: project._id, folder_id: folder_id, file_name: fileName, path:path, "adding file"
+				FileStoreHandler.uploadFileFromDisk project._id, fileRef._id, path, (err, fileStoreUrl)->
-		return callback(err) if err?
-		confirmFolder project, folder_id, (folder_id)->
-			fileRef = new File name : fileName
-			FileStoreHandler.uploadFileFromDisk project._id, fileRef._id, path, (err, fileStoreUrl)->
-				if err?
-					logger.err err:err, project_id: project._id, folder_id: folder_id, file_name: fileName, fileRef:fileRef, "error uploading image to s3"
-					return callback(err)
-				ProjectEntityHandler._putElement project, folder_id, fileRef, "file", (err, result)=>
 					if err?
-						logger.err err:err, project_id: project._id, folder_id: folder_id, file_name: fileName, fileRef:fileRef, "error adding file with project"
+						logger.err err:err, project_id: project._id, folder_id: folder_id, file_name: fileName, fileRef:fileRef, "error uploading image to s3"
 						return callback(err)
-					tpdsUpdateSender.addFile {project_id:project._id, file_id:fileRef._id, path:result?.path?.fileSystem, project_name:project.name, rev:fileRef.rev}, (err) ->
+					ProjectEntityHandler._putElement project, folder_id, fileRef, "file", (err, result)=>
-						return callback(err) if err?
+						if err?
-						newFiles = [
+							logger.err err:err, project_id: project._id, folder_id: folder_id, file_name: fileName, fileRef:fileRef, "error adding file with project"
-							file: fileRef
+							return callback(err)
-							path: result?.path?.fileSystem
+						tpdsUpdateSender.addFile {project_id:project._id, file_id:fileRef._id, path:result?.path?.fileSystem, project_name:project.name, rev:fileRef.rev}, (err) ->
-							url: fileStoreUrl
+							return callback(err) if err?
-						]
+							callback(null, fileRef, folder_id, result, fileStoreUrl)
-						DocumentUpdaterHandler.updateProjectStructure project_id, userId, {newFiles}, (error) ->
+
-							return callback(error) if error?
+	addFile:  (project_id, folder_id, fileName, path, userId, callback = (error, fileRef, folder_id) ->)->
-							callback null, fileRef, folder_id
+		ProjectEntityHandler.addFileWithoutUpdatingHistory project_id, folder_id, fileName, path, userId, (error, fileRef, folder_id, result, fileStoreUrl) ->
+			newFiles = [
+				file: fileRef
+				path: result?.path?.fileSystem
+				url: fileStoreUrl
+			]
+			DocumentUpdaterHandler.updateProjectStructure project_id, userId, {newFiles}, (error) ->
+				return callback(error) if error?
+				callback null, fileRef, folder_id
 
 	replaceFile: (project_id, file_id, fsPath, userId, callback)->
 		self = ProjectEntityHandler

services/web/app/coffee/models/Project.coffee

@@ -56,6 +56,7 @@ ProjectSchema = new Schema
 		read_token      : { type: String }
 		history         :
 			id            : { type: Number }
+			display       : { type: Boolean }
 
 ProjectSchema.statics.getProject = (project_or_id, fields, callback)->
 	if project_or_id._id?

services/web/test/acceptance/coffee/ProjectStructureTests.coffee

@@ -59,7 +59,7 @@ describe "ProjectStructureChanges", ->
 				@dup_project_id = body.project_id
 				done()
 
-		it "should version the dosc created", ->
+		it "should version the docs created", ->
 			updates = MockDocUpdaterApi.getProjectStructureUpdates(@dup_project_id).docUpdates
 			expect(updates.length).to.equal(2)
 			_.each updates, (update) =>

services/web/test/unit/coffee/History/HistoryControllerTests.coffee

@@ -114,68 +114,3 @@ describe "HistoryController", ->
 
 			it "should pass the error up the call chain", ->
 				@next.calledWith(@error).should.equal true
-
-	describe "initializeProject", ->
-		describe "with project history enabled", ->
-			beforeEach ->
-				@settings.apis.project_history.initializeHistoryForNewProjects = true
-
-			describe "project history returns a successful response", ->
-				beforeEach ->
-					@overleaf_id = 1234
-					@res = statusCode: 200
-					@body = JSON.stringify(project: id: @overleaf_id)
-					@request.post = sinon.stub().callsArgWith(1, null, @res, @body)
-
-					@HistoryController.initializeProject @callback
-
-				it "should call the project history api", ->
-					@request.post.calledWith(
-						url: "#{@settings.apis.project_history.url}/project"
-					).should.equal true
-
-				it "should return the callback with the overleaf id", ->
-					@callback.calledWithExactly(null, { @overleaf_id }).should.equal true
-
-			describe "project history returns a response without the project id", ->
-				beforeEach ->
-					@res = statusCode: 200
-					@body = JSON.stringify(project: {})
-					@request.post = sinon.stub().callsArgWith(1, null, @res, @body)
-
-					@HistoryController.initializeProject @callback
-
-				it "should return the callback with an error", ->
-					@callback
-						.calledWith(sinon.match.has("message", "project-history did not provide an id"))
-						.should.equal true
-
-			describe "project history returns a unsuccessful response", ->
-				beforeEach ->
-					@res = statusCode: 404
-					@request.post = sinon.stub().callsArgWith(1, null, @res)
-
-					@HistoryController.initializeProject @callback
-
-				it "should return the callback with an error", ->
-					@callback
-						.calledWith(sinon.match.has("message", "project-history returned a non-success status code: 404"))
-						.should.equal true
-
-			describe "project history errors", ->
-				beforeEach ->
-					@error = sinon.stub()
-					@request.post = sinon.stub().callsArgWith(1, @error)
-
-					@HistoryController.initializeProject @callback
-
-				it "should return the callback with the error", ->
-					@callback.calledWithExactly(@error).should.equal true
-
-		describe "with project history disabled", ->
-			beforeEach ->
-				@settings.apis.project_history.initializeHistoryForNewProjects = false
-				@HistoryController.initializeProject @callback
-
-			it "should return the callback", ->
-				@callback.calledWithExactly().should.equal true

services/web/test/unit/coffee/History/HistoryManagerTests.coffee

@@ -0,0 +1,86 @@
+chai = require('chai')
+chai.should()
+sinon = require("sinon")
+modulePath = "../../../../app/js/Features/History/HistoryManager"
+SandboxedModule = require('sandboxed-module')
+
+describe "HistoryManager", ->
+	beforeEach ->
+		@callback = sinon.stub()
+		@user_id = "user-id-123"
+		@AuthenticationController =
+			getLoggedInUserId: sinon.stub().returns(@user_id)
+		@HistoryManager = SandboxedModule.require modulePath, requires:
+			"request" : @request = sinon.stub()
+			"settings-sharelatex": @settings = {}
+		@settings.apis =
+			trackchanges:
+				enabled: false
+				url: "http://trackchanges.example.com"
+			project_history:
+				url: "http://project_history.example.com"
+
+	describe "initializeProject", ->
+		describe "with project history enabled", ->
+			beforeEach ->
+				@settings.apis.project_history.initializeHistoryForNewProjects = true
+
+			describe "project history returns a successful response", ->
+				beforeEach ->
+					@overleaf_id = 1234
+					@res = statusCode: 200
+					@body = JSON.stringify(project: id: @overleaf_id)
+					@request.post = sinon.stub().callsArgWith(1, null, @res, @body)
+
+					@HistoryManager.initializeProject @callback
+
+				it "should call the project history api", ->
+					@request.post.calledWith(
+						url: "#{@settings.apis.project_history.url}/project"
+					).should.equal true
+
+				it "should return the callback with the overleaf id", ->
+					@callback.calledWithExactly(null, { @overleaf_id }).should.equal true
+
+			describe "project history returns a response without the project id", ->
+				beforeEach ->
+					@res = statusCode: 200
+					@body = JSON.stringify(project: {})
+					@request.post = sinon.stub().callsArgWith(1, null, @res, @body)
+
+					@HistoryManager.initializeProject @callback
+
+				it "should return the callback with an error", ->
+					@callback
+						.calledWith(sinon.match.has("message", "project-history did not provide an id"))
+						.should.equal true
+
+			describe "project history returns a unsuccessful response", ->
+				beforeEach ->
+					@res = statusCode: 404
+					@request.post = sinon.stub().callsArgWith(1, null, @res)
+
+					@HistoryManager.initializeProject @callback
+
+				it "should return the callback with an error", ->
+					@callback
+						.calledWith(sinon.match.has("message", "project-history returned a non-success status code: 404"))
+						.should.equal true
+
+			describe "project history errors", ->
+				beforeEach ->
+					@error = sinon.stub()
+					@request.post = sinon.stub().callsArgWith(1, @error)
+
+					@HistoryManager.initializeProject @callback
+
+				it "should return the callback with the error", ->
+					@callback.calledWithExactly(@error).should.equal true
+
+		describe "with project history disabled", ->
+			beforeEach ->
+				@settings.apis.project_history.initializeHistoryForNewProjects = false
+				@HistoryManager.initializeProject @callback
+
+			it "should return the callback", ->
+				@callback.calledWithExactly().should.equal true

services/web/test/unit/coffee/Project/ProjectCreationHandlerTests.coffee

@@ -38,7 +38,7 @@ describe 'ProjectCreationHandler', ->
 			setRootDoc: sinon.stub().callsArg(2)
 		@ProjectDetailsHandler =
 			validateProjectName: sinon.stub().yields()
-		@HistoryController =
+		@HistoryManager =
 			initializeProject: sinon.stub().callsArg(0)
 
 		@user =
@@ -53,7 +53,7 @@ describe 'ProjectCreationHandler', ->
 			'../../models/User': User:@User
 			'../../models/Project':{Project:@ProjectModel}
 			'../../models/Folder':{Folder:@FolderModel}
-			'../History/HistoryController': @HistoryController
+			'../History/HistoryManager': @HistoryManager
 			'./ProjectEntityHandler':@ProjectEntityHandler
 			"./ProjectDetailsHandler":@ProjectDetailsHandler
 			"settings-sharelatex": @Settings = {}
@@ -66,7 +66,7 @@ describe 'ProjectCreationHandler', ->
 	describe 'Creating a Blank project', ->
 		beforeEach ->
 			@overleaf_id = 1234
-			@HistoryController.initializeProject = sinon.stub().callsArgWith(0, null, { @overleaf_id })
+			@HistoryManager.initializeProject = sinon.stub().callsArgWith(0, null, { @overleaf_id })
 			@ProjectModel::save = sinon.stub().callsArg(0)
 
 		describe "successfully", ->
@@ -83,7 +83,7 @@ describe 'ProjectCreationHandler', ->
 
 			it "should initialize the project overleaf if history id not provided", (done)->
 				@handler.createBlankProject ownerId, projectName, done
-				@HistoryController.initializeProject.calledWith().should.equal true
+				@HistoryManager.initializeProject.calledWith().should.equal true
 
 			it "should set the overleaf id if overleaf id not provided", (done)->
 				@handler.createBlankProject ownerId, projectName, (err, project)=>

services/web/test/unit/coffee/Project/ProjectDuplicatorTests.coffee

@@ -64,7 +64,7 @@ describe 'ProjectDuplicator', ->
 		@projectOptionsHandler =
 			setCompiler : sinon.stub()								
 		@entityHandler =
-			addDocWithProject: sinon.stub().callsArgWith(5, null, {name:"somDoc"})
+			addDoc: sinon.stub().callsArgWith(5, null, {name:"somDoc"})
 			copyFileFromExistingProjectWithProject: sinon.stub().callsArgWith(5)
 			setRootDoc: sinon.stub()
 			addFolderWithProject: sinon.stub().callsArgWith(3, null, @newFolder)
@@ -112,13 +112,13 @@ describe 'ProjectDuplicator', ->
 			done()
 
 	it 'should use the same compiler', (done)->
-		@entityHandler.addDocWithProject.callsArgWith(5, null, @rootFolder.docs[0], @owner._id)
+		@entityHandler.addDoc.callsArgWith(5, null, @rootFolder.docs[0], @owner._id)
 		@duplicator.duplicate @owner, @old_project_id, "", (err, newProject)=>
 			@projectOptionsHandler.setCompiler.calledWith(@stubbedNewProject._id, @project.compiler).should.equal true
 			done()
 	
 	it 'should use the same root doc', (done)->
-		@entityHandler.addDocWithProject.callsArgWith(5, null, @rootFolder.docs[0], @owner._id)
+		@entityHandler.addDoc.callsArgWith(5, null, @rootFolder.docs[0], @owner._id)
 		@duplicator.duplicate @owner, @old_project_id, "", (err, newProject)=>
 			@entityHandler.setRootDoc.calledWith(@stubbedNewProject._id, @rootFolder.docs[0]._id).should.equal true
 			done()
@@ -139,13 +139,13 @@ describe 'ProjectDuplicator', ->
 	it 'should copy all the docs', (done)->
 		@duplicator.duplicate @owner, @old_project_id, "", (err, newProject)=>
 			@DocstoreManager.getAllDocs.calledWith(@old_project_id).should.equal true
-			@entityHandler.addDocWithProject
+			@entityHandler.addDoc
 				.calledWith(@stubbedNewProject, @stubbedNewProject.rootFolder[0]._id, @doc0.name, @doc0_lines, @owner._id)
 				.should.equal true
-			@entityHandler.addDocWithProject
+			@entityHandler.addDoc
 				.calledWith(@stubbedNewProject, @newFolder._id, @doc1.name, @doc1_lines, @owner._id)
 				.should.equal true
-			@entityHandler.addDocWithProject
+			@entityHandler.addDoc
 				.calledWith(@stubbedNewProject, @newFolder._id, @doc2.name, @doc2_lines, @owner._id)
 				.should.equal true
 			done()

services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee

@@ -496,6 +496,51 @@ describe 'ProjectEntityHandler', ->
 				.calledWith(project_id, userId, {newDocs})
 				.should.equal true
 
+	describe 'addDocWithoutUpdatingHistory', ->
+		beforeEach ->
+			@name = "some new doc"
+			@lines = ['1234','abc']
+			@path = "/path/to/doc"
+
+			@ProjectEntityHandler._putElement = sinon.stub().callsArgWith(4, null, {path:{fileSystem:@path}})
+			@callback = sinon.stub()
+			@tpdsUpdateSender.addDoc = sinon.stub().callsArg(1)
+			@DocstoreManager.updateDoc = sinon.stub().yields(null, true, 0)
+
+			@ProjectEntityHandler.addDocWithoutUpdatingHistory project_id, folder_id, @name, @lines, userId, @callback
+
+			# Created doc
+			@doc = @ProjectEntityHandler._putElement.args[0][2]
+			@doc.name.should.equal @name
+			expect(@doc.lines).to.be.undefined
+
+		it 'should call put element', ->
+			@ProjectEntityHandler._putElement.calledWith(@project, folder_id, @doc).should.equal true
+
+		it 'should return doc and parent folder', ->
+			@callback.calledWith(null, @doc, folder_id).should.equal true
+
+		it 'should call third party data store', ->
+			@tpdsUpdateSender.addDoc
+				.calledWith({
+					project_id: project_id
+					doc_id: doc_id
+					path: @path
+					project_name: @project.name
+					rev: 0
+				})
+				.should.equal true
+
+		it "should send the doc lines to the doc store", ->
+			@DocstoreManager.updateDoc
+				.calledWith(project_id, @doc._id.toString(), @lines)
+				.should.equal true
+
+		it "should not should send the change in project structure to the doc updater", () ->
+			@documentUpdaterHandler.updateProjectStructure
+				.called
+				.should.equal false
+
 	describe "restoreDoc", ->
 		beforeEach ->
 			@name = "doc-name"
@@ -584,6 +629,12 @@ describe 'ProjectEntityHandler', ->
 
 			@ProjectEntityHandler.addFile project_id, folder_id, fileName, {}, userId, () ->
 
+		it "should not send the change in project structure to the doc updater when called as addFileWithoutUpdatingHistory", (done) ->
+			@documentUpdaterHandler.updateProjectStructure = sinon.stub().yields()
+			@ProjectEntityHandler.addFileWithoutUpdatingHistory project_id, folder_id, fileName, {}, userId, () =>
+				@documentUpdaterHandler.updateProjectStructure.called.should.equal false
+				done()
+
 	describe 'replaceFile', ->
 		beforeEach ->
 			@projectLocator