Don't allow folders to be moved into their child folders

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

@@ -279,29 +279,44 @@ module.exports = ProjectEntityHandler =
 					else
 						callback()
 
-	moveEntity: (project_id, entity_id, folder_id, entityType, sl_req_id, callback = (error) ->)->
-		{callback, sl_req_id} = slReqIdHelper.getCallbackAndReqId(callback, sl_req_id)
+	moveEntity: (project_id, entity_id, folder_id, entityType, callback = (error) ->)->
 		self = @
 		destinationFolder_id = folder_id
-		logger.log sl_req_id: sl_req_id, entityType:entityType, entity_id:entity_id, project_id:project_id, folder_id:folder_id, "moving entity"
+		logger.log entityType:entityType, entity_id:entity_id, project_id:project_id, folder_id:folder_id, "moving entity"
 		if !entityType?
 			logger.err err: "No entityType set", project_id: project_id, entity_id: entity_id
 			return callback("No entityType set")
 		entityType = entityType.toLowerCase()
 		Project.findById project_id, (err, project)=>
+			return callback(err) if err?
 			projectLocator.findElement {project:project, element_id:entity_id, type:entityType}, (err, entity, path)->
 				return callback(err) if err?
-				self._removeElementFromMongoArray Project, project_id, path.mongo, (err)->
-					return callback(err) if err?
-					Project.putElement project_id, destinationFolder_id, entity, entityType, (err, result)->
+				
+				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) ->
+					return callback(error) if error?
+					self._removeElementFromMongoArray Project, project_id, path.mongo, (err)->
 						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, sl_req_id, callback
+						Project.putElement project_id, destinationFolder_id, entity, entityType, (err, result)->
+							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
 
 	deleteEntity: (project_id, entity_id, entityType, sl_req_id, callback = (error) ->)->
 		{callback, sl_req_id} = slReqIdHelper.getCallbackAndReqId(callback, sl_req_id)

services/web/public/coffee/ide/file-tree/FileTreeManager.coffee

@@ -289,11 +289,20 @@ define [
 			}
 
 		moveEntity: (entity, parent_folder, callback = (error) ->) ->
+			# Abort move if the folder being moved (entity) has the parent_folder as child
+			# since that would break the tree structure.
+			return if @_isChildFolder(entity, parent_folder)
 			@_moveEntityInScope(entity, parent_folder)
 			return @ide.$http.post "/project/#{@ide.project_id}/#{entity.type}/#{entity.id}/move", {
 				folder_id: parent_folder.id
 				_csrf: window.csrfToken
 			}
+			
+		_isChildFolder: (parent_folder, child_folder) ->
+			parent_path = @getEntityPath(parent_folder) or "" # null if root folder
+			child_path = @getEntityPath(child_folder) or "" # null if root folder
+			# is parent path the beginning of child path?
+			return (child_path.slice(0, parent_path.length) == parent_path)
 
 		_deleteEntityFromScope: (entity, options = { moveToDeleted: true }) ->
 			parent_folder = null

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

@@ -217,52 +217,153 @@ describe 'ProjectEntityHandler', ->
 				@ProjectEntityHandler._cleanUpDoc.calledWith(@project, @doc1).should.equal true
 				@ProjectEntityHandler._cleanUpDoc.calledWith(@project, @doc2).should.equal true
 
-	describe 'moving an element', ->
+	describe 'moveEntity', ->
 		beforeEach ->
-			@docId = "4eecaffcbffa66588e000009"
-			@doc = {lines:["1234","312343d"], rev: "1234"}
+			@pathAfterMove = {
+				fileSystem: "/somewhere/else.txt"
+			}
+			@ProjectEntityHandler._removeElementFromMongoArray = sinon.stub().callsArg(3)
+			@ProjectModel.putElement = sinon.stub().callsArgWith(4, null, path: @pathAfterMove)
+			@tpdsUpdateSender.moveEntity = sinon.stub().callsArg(1)
+			
+		describe "moving a doc", ->
+			beforeEach (done) ->
+				@docId = "4eecaffcbffa66588e000009"
+				@doc = {lines:["1234","312343d"], rev: "1234"}
+				@path = {
+					mongo:"folders[0]"
+					fileSystem:"/somewhere.txt"
+				}
+				@projectLocator.findElement = sinon.stub().callsArgWith(1, null, @doc, @path)
+				@ProjectEntityHandler.moveEntity project_id, @docId, folder_id, "docs", done
 
-		it 'should find the project then element', (done)->
-			@projectLocator.findElement = (options, callback)=>
-				options.element_id.should.equal @docId
-				options.type.should.equal 'docs'
-				done()
-			@ProjectEntityHandler.moveEntity project_id, @docId, folder_id, "docs", ->
+			it 'should find the project then element', ->
+				@projectLocator.findElement
+					.calledWith({
+						element_id: @docId,
+						type: "docs",
+						project: @project
+					})
+					.should.equal true
 
-		it 'should remove the element then add it back in', (done)->
+			it 'should remove the element from its current position', ->
+				@ProjectEntityHandler._removeElementFromMongoArray
+					.calledWith(
+						@ProjectModel,
+						project_id,
+						@path.mongo
+					)
+					.should.equal true
+					
+			it "should put the element back in the new folder", ->
+				@ProjectModel.putElement
+					.calledWith(
+						project_id,
+						folder_id,
+						@doc,
+						"docs"
+					)
+					.should.equal true
+					
+			it 'should tell the third party data store', ->
+				@tpdsUpdateSender.moveEntity
+					.calledWith({
+						project_id: project_id,
+						startPath: @path.fileSystem
+						endPath: @pathAfterMove.fileSystem
+						project_name: @project.name
+						rev: @doc.rev
+					})
+					.should.equal true
+					
+		describe "moving a folder", ->
+			beforeEach ->
+				@folder_id = "folder-to-move"
+				@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"
+						
+			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 = {mongo:"folders[0]"}
-			@projectLocator.findElement = (opts, callback)=>
-				callback(null, @doc, path)
-			@ProjectEntityHandler._removeElementFromMongoArray = (model, model_id, path, callback)-> callback()
+				it 'should find the project then element', ->
+					@projectLocator.findElement
+						.calledWith({
+							element_id: @folder_id,
+							type: "folder",
+							project: @project
+						})
+						.should.equal true
 
-			@ProjectModel.putElement = (passedProject_id, destinationFolder_id, entity, entityType, callback)=>
-				passedProject_id.should.equal project_id
-				destinationFolder_id.should.equal folder_id
-				entity.should.deep.equal @doc
-				entityType.should.equal 'docs'
-				done()
-			@ProjectEntityHandler.moveEntity project_id, @docId, folder_id, "docs", ->
-
-		it 'should tell the third party data store', (done)->
-			startPath = {fileSystem:"/somewhere.txt"}
-			endPath = {fileSystem:"/somewhere.txt"}
-
-			@projectLocator.findElement = (opts, callback)=>
-				callback(null, @doc, startPath)
-			@ProjectEntityHandler._removeElementFromMongoArray = (model, model_id, path, callback)-> callback()
-			@ProjectModel.putElement = (passedProject_id, destinationFolder_id, entity, entityType, callback)->
-				callback null, path:endPath
-
-			@tpdsUpdateSender.moveEntity = (opts)=>
-				opts.project_id.should.equal project_id
-				opts.startPath.should.equal startPath.fileSystem
-				opts.endPath.should.equal endPath.fileSystem
-				opts.project_name.should.equal @project.name
-				opts.rev.should.equal @doc.rev
-				done()
-			@ProjectEntityHandler.moveEntity project_id, @docId, folder_id, "docs", ->
+				it 'should remove the element from its current position', ->
+					@ProjectEntityHandler._removeElementFromMongoArray
+						.calledWith(
+							@ProjectModel,
+							project_id,
+							@path.mongo
+						)
+						.should.equal true
+						
+				it "should put the element back in the new folder", ->
+					@ProjectModel.putElement
+						.calledWith(
+							project_id,
+							@move_to_folder_id,
+							@folder,
+							"folder"
+						)
+						.should.equal true
+						
+				it 'should tell the third party data store', ->
+					@tpdsUpdateSender.moveEntity
+						.calledWith({
+							project_id: project_id,
+							startPath: @path.fileSystem
+							endPath: @pathAfterMove.fileSystem
+							project_name: @project.name,
+							rev: @folder.rev
+						})
+						.should.equal true
+						
+			describe "when the destination folder is inside the moving folder", ->
+				beforeEach ->
+					@path.fileSystem = "/one/two"
+					@pathToMoveTo.fileSystem = "/one/two/three"
+					@callback = sinon.stub()
+					@ProjectEntityHandler.moveEntity project_id, @folder_id, @move_to_folder_id, "folder", @callback
 
+				it 'should find the folder we are moving to as well element', ->
+					@projectLocator.findElement
+						.calledWith({
+							element_id: @move_to_folder_id,
+							type: "folder",
+							project: @project
+						})
+						.should.equal true
+						
+				it "should return an error", ->
+					@callback
+						.calledWith(new Error("destination folder is a child folder of me"))
+						.should.equal true
 
 	describe 'removing element from mongo array', ->
 		it 'should call update with log the path', (done)->