From 06116dc956989ac8466ffa8121d38a6a75b15d8b Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Thu, 2 Nov 2017 09:44:23 +0000 Subject: [PATCH] version moving entities --- .../Features/Editor/EditorController.coffee | 4 +- .../Editor/EditorHttpController.coffee | 3 +- .../Project/ProjectEntityHandler.coffee | 62 +++++++------- .../Editor/EditorControllerTests.coffee | 14 ++-- .../Editor/EditorHttpControllerTests.coffee | 12 +-- .../Project/ProjectEntityHandlerTests.coffee | 82 +++++++++++-------- 6 files changed, 94 insertions(+), 83 deletions(-) diff --git a/services/web/app/coffee/Features/Editor/EditorController.coffee b/services/web/app/coffee/Features/Editor/EditorController.coffee index 5026f62595..a43d6f0447 100644 --- a/services/web/app/coffee/Features/Editor/EditorController.coffee +++ b/services/web/app/coffee/Features/Editor/EditorController.coffee @@ -162,13 +162,13 @@ module.exports = EditorController = EditorRealTimeController.emitToRoom project_id, 'reciveEntityRename', entity_id, newName callback?() - moveEntity: (project_id, entity_id, folder_id, entityType, callback)-> + moveEntity: (project_id, entity_id, folder_id, entityType, userId, callback)-> Metrics.inc "editor.move-entity" LockManager.getLock project_id, (err)-> if err? logger.err err:err, project_id:project_id, "could not get lock for move entity" return callback(err) - ProjectEntityHandler.moveEntity project_id, entity_id, folder_id, entityType, => + ProjectEntityHandler.moveEntity project_id, entity_id, folder_id, entityType, userId, => if err? logger.err err:err, project_id:project_id, entity_id:entity_id, folder_id:folder_id, "error moving entity" return callback(err) diff --git a/services/web/app/coffee/Features/Editor/EditorHttpController.coffee b/services/web/app/coffee/Features/Editor/EditorHttpController.coffee index 63eeef70e9..e9bac7300d 100644 --- a/services/web/app/coffee/Features/Editor/EditorHttpController.coffee +++ b/services/web/app/coffee/Features/Editor/EditorHttpController.coffee @@ -125,7 +125,8 @@ module.exports = EditorHttpController = entity_id = req.params.entity_id entity_type = req.params.entity_type folder_id = req.body.folder_id - EditorController.moveEntity project_id, entity_id, folder_id, entity_type, (error) -> + user_id = AuthenticationController.getLoggedInUserId(req) + EditorController.moveEntity project_id, entity_id, folder_id, entity_type, user_id, (error) -> return next(error) if error? res.sendStatus 204 diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 2eb39d2f95..e519140b35 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -355,47 +355,49 @@ module.exports = ProjectEntityHandler = else callback() - moveEntity: (project_id, entity_id, folder_id, entityType, callback = (error) ->)-> + moveEntity: (project_id, entity_id, destFolderId, entityType, userId, callback = (error) ->)-> self = @ - destinationFolder_id = folder_id - logger.log entityType:entityType, entity_id:entity_id, project_id:project_id, folder_id:folder_id, "moving entity" + logger.log {entityType, entity_id, project_id, destFolderId}, "moving entity" if !entityType? - logger.err err: "No entityType set", project_id: project_id, entity_id: entity_id + logger.err {err: "No entityType set", project_id, entity_id} return callback("No entityType set") entityType = entityType.toLowerCase() ProjectGetter.getProject project_id, {rootFolder:true, name:true}, (err, project)=> return callback(err) if err? - projectLocator.findElement {project:project, element_id:entity_id, type:entityType}, (err, entity, path)-> + projectLocator.findElement {project, element_id: entity_id, type: entityType}, (err, entity, entityPath)-> return callback(err) if err? - - if entityType.match(/folder/) - ensureFolderIsNotMovedIntoChild = (callback = (error) ->) -> - projectLocator.findElement {project: project, element_id: folder_id, type:"folder"}, (err, destEntity, destPath) -> - logger.log destPath: destPath.fileSystem, folderPath: path.fileSystem, "checking folder is not moving into child folder" - if (destPath.fileSystem.slice(0, path.fileSystem.length) == path.fileSystem) - logger.log "destination is a child folder, aborting" - callback(new Error("destination folder is a child folder of me")) - else - callback() - else - ensureFolderIsNotMovedIntoChild = (callback = () ->) -> callback() - - ensureFolderIsNotMovedIntoChild (error) -> + self._checkValidMove project, entityType, entityPath, destFolderId, (error) -> return callback(error) if error? - self._removeElementFromMongoArray Project, project_id, path.mongo, (err)-> - return callback(err) if err? - # We've updated the project structure by removing the element, so must refresh it. - ProjectGetter.getProject project_id, {rootFolder:true, name:true}, (err, project)=> + ProjectEntityHandler.getAllEntitiesFromProject project, (error, oldDocs) => + return callback(error) if error? + self._removeElementFromMongoArray Project, project_id, entityPath.mongo, (err, newProject)-> return callback(err) if err? - ProjectEntityHandler._putElement project, destinationFolder_id, entity, entityType, (err, result)-> + ProjectEntityHandler._putElement newProject, destFolderId, entity, entityType, (err, result, newProject)-> return callback(err) if err? opts = - project_id:project_id - project_name:project.name - startPath:path.fileSystem - endPath:result.path.fileSystem, - rev:entity.rev - tpdsUpdateSender.moveEntity opts, callback + project_id: project_id + project_name: project.name + startPath: entityPath.fileSystem + endPath: result.path.fileSystem, + rev: entity.rev + tpdsUpdateSender.moveEntity opts + ProjectEntityHandler.getAllEntitiesFromProject newProject, (error, newDocs) => + return callback(error) if error? + documentUpdaterHandler = require('../../Features/DocumentUpdater/DocumentUpdaterHandler') + documentUpdaterHandler.updateProjectStructure project_id, userId, oldDocs, newDocs, callback + + _checkValidMove: (project, entityType, entityPath, destFolderId, callback = (error) ->) -> + return callback() if !entityType.match(/folder/) + + projectLocator.findElement { project, element_id: destFolderId, type:"folder"}, (err, destEntity, destFolderPath) -> + return callback(err) if err? + logger.log destFolderPath: destFolderPath.fileSystem, folderPath: entityPath.fileSystem, "checking folder is not moving into child folder" + isNestedFolder = destFolderPath.fileSystem.slice(0, entityPath.fileSystem.length) == entityPath.fileSystem + if isNestedFolder + callback(new Error("destination folder is a child folder of me")) + else + callback() + deleteEntity: (project_id, entity_id, entityType, callback = (error) ->)-> self = @ diff --git a/services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee b/services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee index 2d9e344c8e..ae5e8d79e3 100644 --- a/services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee @@ -508,34 +508,32 @@ describe "EditorController", -> done() describe "moveEntity", -> - beforeEach -> - @err = "errro" @entity_id = "entity_id_here" @entityType = "doc" @folder_id = "313dasd21dasdsa" - @ProjectEntityHandler.moveEntity = sinon.stub().callsArgWith(4, @err) + @ProjectEntityHandler.moveEntity = sinon.stub().callsArg(5) @EditorRealTimeController.emitToRoom = sinon.stub() @LockManager.releaseLock.callsArgWith(1) @LockManager.getLock.callsArgWith(1) it "should call the ProjectEntityHandler", (done)-> - @EditorController.moveEntity @project_id, @entity_id, @folder_id, @entityType, => - @ProjectEntityHandler.moveEntity.calledWith(@project_id, @entity_id, @folder_id, @entityType).should.equal true + @EditorController.moveEntity @project_id, @entity_id, @folder_id, @entityType, @user_id, => + @ProjectEntityHandler.moveEntity.calledWith(@project_id, @entity_id, @folder_id, @entityType, @user_id).should.equal true done() it "should take the lock", (done)-> - @EditorController.moveEntity @project_id, @entity_id, @folder_id, @entityType, => + @EditorController.moveEntity @project_id, @entity_id, @folder_id, @entityType, @user_id, => @LockManager.getLock.calledWith(@project_id).should.equal true done() it "should release the lock", (done)-> - @EditorController.moveEntity @project_id, @entity_id, @folder_id, @entityType, => + @EditorController.moveEntity @project_id, @entity_id, @folder_id, @entityType, @user_id, => @LockManager.releaseLock.calledWith(@project_id).should.equal true done() it "should emit the update to the room", (done)-> - @EditorController.moveEntity @project_id, @entity_id, @folder_id, @entityType, => + @EditorController.moveEntity @project_id, @entity_id, @folder_id, @entityType, @user_id, => @EditorRealTimeController.emitToRoom.calledWith(@project_id, 'reciveEntityMove', @entity_id, @folder_id).should.equal true done() diff --git a/services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee b/services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee index 2567230a36..2df4af72ec 100644 --- a/services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee @@ -25,6 +25,8 @@ describe "EditorHttpController", -> @doc_id = "mock-doc-id" @user_id = "mock-user-id" @parent_folder_id = "mock-folder-id" + @userId = 1234 + @AuthenticationController.getLoggedInUserId = sinon.stub().returns(@userId) @req = {} @res = send: sinon.stub() @@ -265,8 +267,6 @@ describe "EditorHttpController", -> entity_type: @entity_type = "entity-type" @req.body = name: @name = "new-name" - @userId = 1234 - @AuthenticationController.getLoggedInUserId = sinon.stub().returns(@userId) @EditorController.renameEntity = sinon.stub().callsArg(5) @EditorHttpController.renameEntity @req, @res @@ -286,8 +286,6 @@ describe "EditorHttpController", -> entity_type: @entity_type = "entity-type" @req.body = name: @name = "EDMUBEEBKBXUUUZERMNSXFFWIBHGSDAWGMRIQWJBXGWSBVWSIKLFPRBYSJEKMFHTRZBHVKJSRGKTBHMJRXPHORFHAKRNPZGGYIOTEDMUBEEBKBXUUUZERMNSXFFWIBHGSDAWGMRIQWJBXGWSBVWSIKLFPRBYSJEKMFHTRZBHVKJSRGKTBHMJRXPHORFHAKRNPZGGYIOT" - @userId = 1234 - @AuthenticationController.getLoggedInUserId = sinon.stub().returns(@userId) @EditorController.renameEntity = sinon.stub().callsArg(4) @EditorHttpController.renameEntity @req, @res @@ -302,8 +300,6 @@ describe "EditorHttpController", -> entity_type: @entity_type = "entity-type" @req.body = name: @name = "" - @userId = 1234 - @AuthenticationController.getLoggedInUserId = sinon.stub().returns(@userId) @EditorController.renameEntity = sinon.stub().callsArg(4) @EditorHttpController.renameEntity @req, @res @@ -318,12 +314,12 @@ describe "EditorHttpController", -> entity_type: @entity_type = "entity-type" @req.body = folder_id: @folder_id = "folder-id-123" - @EditorController.moveEntity = sinon.stub().callsArg(4) + @EditorController.moveEntity = sinon.stub().callsArg(5) @EditorHttpController.moveEntity @req, @res it "should call EditorController.moveEntity", -> @EditorController.moveEntity - .calledWith(@project_id, @entity_id, @folder_id, @entity_type) + .calledWith(@project_id, @entity_id, @folder_id, @entity_type, @userId) .should.equal true it "should send back a success response", -> diff --git a/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee index fa6fba40a7..c4902f2ff0 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee @@ -14,6 +14,7 @@ describe 'ProjectEntityHandler', -> doc_id = '4eecb1c1bffa66588e0000a2' folder_id = "4eecaffcbffa66588e000008" rootFolderId = "4eecaffcbffa66588e000007" + userId = 1234 beforeEach -> @FileStoreHandler = @@ -235,10 +236,16 @@ describe 'ProjectEntityHandler', -> @pathAfterMove = { fileSystem: "/somewhere/else.txt" } - @ProjectEntityHandler._removeElementFromMongoArray = sinon.stub().callsArg(3) + @ProjectEntityHandler._removeElementFromMongoArray = sinon.stub().callsArgWith(3, null, @project) @ProjectEntityHandler._putElement = sinon.stub().callsArgWith(4, null, path: @pathAfterMove) @ProjectGetter.getProject.callsArgWith(2, null, @project) - @tpdsUpdateSender.moveEntity = sinon.stub().callsArg(1) + @tpdsUpdateSender.moveEntity = sinon.stub() + @documentUpdaterHandler.updateProjectStructure = sinon.stub().callsArg(4) + @ProjectEntityHandler.getAllEntitiesFromProject = sinon.stub() + @ProjectEntityHandler.getAllEntitiesFromProject + .onFirstCall().callsArgWith(1, null, @oldDocs = []) + @ProjectEntityHandler.getAllEntitiesFromProject + .onSecondCall().callsArgWith(1, null, @newDocs = []) describe "moving a doc", -> beforeEach (done) -> @@ -246,14 +253,26 @@ describe 'ProjectEntityHandler', -> @doc = {lines:["1234","312343d"], rev: "1234"} @path = { mongo:"folders[0]" - fileSystem:"/somewhere.txt" + fileSystem:"/old_folder/somewhere.txt" } - @projectLocator.findElement = sinon.stub().callsArgWith(1, null, @doc, @path) - @ProjectEntityHandler.moveEntity project_id, @docId, folder_id, "docs", done + @destFolder = { name: "folder" } + @destFolderPath = { + mongo: "folders[0]" + fileSystem: "/dest_folder" + } + @projectLocator.findElement = sinon.stub() + @projectLocator.findElement.withArgs({project: @project, element_id: @docId, type: 'docs'}) + .callsArgWith(1, null, @doc, @path) + @ProjectEntityHandler.moveEntity project_id, @docId, folder_id, "docs", userId, done - it 'should find the project then element', -> + it 'should find the doc to move', -> @projectLocator.findElement.calledWith({element_id: @docId, type: "docs", project: @project }).should.equal true + it "should should send the update to the doc updater", -> + @documentUpdaterHandler.updateProjectStructure + .calledWith(project_id, userId, @oldDocs, @newDocs) + .should.equal true + it 'should remove the element from its current position', -> @ProjectEntityHandler._removeElementFromMongoArray .calledWith(@ProjectModel, project_id, @path.mongo ).should.equal true @@ -278,28 +297,19 @@ describe 'ProjectEntityHandler', -> @move_to_folder_id = "folder-to-move-to" @folder = { name: "folder" } @folder_to_move_to = { name: "folder to move to" } - @path = { - mongo: "folders[0]" - fileSystem: "/somewhere.txt" - } - @pathToMoveTo = { - mongo: "folders[0]" - fileSystem: "/somewhere.txt" - } - @projectLocator.findElement = (options, callback) => - if options.element_id == @folder_id - callback(null, @folder, @path) - else if options.element_id == @move_to_folder_id - callback(null, @folder_to_move_to, @pathToMoveTo) - else - console.log "UNKNOWN ID", options - sinon.spy @projectLocator, "findElement" + @path = { mongo:"folders[0]" } + @pathToMoveTo = { mongo: "folders[0]" } + @projectLocator.findElement = sinon.stub() + @projectLocator.findElement.withArgs({project: @project, element_id: @folder_id, type: 'folder'}) + .callsArgWith(1, null, @folder, @path) + @projectLocator.findElement.withArgs({project: @project, element_id: @move_to_folder_id, type: 'folder'}) + .callsArgWith(1, null, @folder_to_move_to, @pathToMoveTo) describe "when the destination folder is outside the moving folder", -> beforeEach (done) -> - @path.fileSystem = "/one/directory" - @pathToMoveTo.fileSystem = "/another/directory" - @ProjectEntityHandler.moveEntity project_id, @folder_id, @move_to_folder_id, "folder", done + @path.fileSystem = "/one/src_dir" + @pathToMoveTo.fileSystem = "/two/dest_dir" + @ProjectEntityHandler.moveEntity project_id, @folder_id, @move_to_folder_id, "folder", userId, done it 'should find the project then element', -> @projectLocator.findElement @@ -310,6 +320,11 @@ describe 'ProjectEntityHandler', -> }) .should.equal true + it "should should send the update to the doc updater", -> + @documentUpdaterHandler.updateProjectStructure + .calledWith(project_id, userId, @oldDocs, @newDocs) + .should.equal true + it 'should remove the element from its current position', -> @ProjectEntityHandler._removeElementFromMongoArray .calledWith( @@ -345,9 +360,9 @@ describe 'ProjectEntityHandler', -> @path.fileSystem = "/one/two" @pathToMoveTo.fileSystem = "/one/two/three" @callback = sinon.stub() - @ProjectEntityHandler.moveEntity project_id, @folder_id, @move_to_folder_id, "folder", @callback + @ProjectEntityHandler.moveEntity project_id, @folder_id, @move_to_folder_id, "folder", userId, @callback - it 'should find the folder we are moving to as well element', -> + it 'should find the folder we are moving to element', -> @projectLocator.findElement .calledWith({ element_id: @move_to_folder_id, @@ -982,7 +997,6 @@ describe 'ProjectEntityHandler', -> @entityType = "doc" @newName = "new.tex" @path = mongo: "mongo.path", fileSystem: "/file/system/old.tex" - @userId = 1234 @ProjectGetter.getProject.callsArgWith(2, null, @project) @ProjectEntityHandler.getAllEntitiesFromProject = sinon.stub() @@ -997,20 +1011,20 @@ describe 'ProjectEntityHandler', -> @documentUpdaterHandler.updateProjectStructure = sinon.stub().callsArg(4) it "should should send the old and new project structure to the doc updater", (done) -> - @ProjectEntityHandler.renameEntity @project_id, @entity_id, @entityType, @newName, @userId, => + @ProjectEntityHandler.renameEntity project_id, @entity_id, @entityType, @newName, userId, => @documentUpdaterHandler.updateProjectStructure.calledWith( - @project_id, @userId, @oldDocs, @newDocs, + project_id, userId, @oldDocs, @newDocs, ).should.equal true done() it "should update the name in mongo", (done)-> - @ProjectEntityHandler.renameEntity @project_id, @entity_id, @entityType, @newName, @userId, => - @ProjectModel.findOneAndUpdate.calledWith({_id: @project_id}, {"$set":{"mongo.path.name": @newName}}, {"new": true}).should.equal true + @ProjectEntityHandler.renameEntity project_id, @entity_id, @entityType, @newName, userId, => + @ProjectModel.findOneAndUpdate.calledWith({_id: project_id}, {"$set":{"mongo.path.name": @newName}}, {"new": true}).should.equal true done() it "should send the update to the tpds", (done)-> - @ProjectEntityHandler.renameEntity @project_id, @entity_id, @entityType, @newName, @userId, => - @tpdsUpdateSender.moveEntity.calledWith({project_id:@project_id, startPath:@path.fileSystem, endPath:"/file/system/new.tex", project_name:@project.name, rev:4}).should.equal true + @ProjectEntityHandler.renameEntity project_id, @entity_id, @entityType, @newName, userId, => + @tpdsUpdateSender.moveEntity.calledWith({project_id:project_id, startPath:@path.fileSystem, endPath:"/file/system/new.tex", project_name:@project.name, rev:4}).should.equal true done() describe "_insertDeletedDocReference", ->