From d7706f642bbc0ea36ce782b66ee3d13e7cf7601a Mon Sep 17 00:00:00 2001 From: James Allen Date: Tue, 23 Jan 2018 13:47:48 +0000 Subject: [PATCH 01/11] Show the creator as the owner if no owner present --- .../Features/Project/ProjectController.coffee | 22 +++++++++++++------ .../main/project-list/project-list.coffee | 4 ++-- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectController.coffee b/services/web/app/coffee/Features/Project/ProjectController.coffee index 722d0b73b4..359d0ddeb4 100644 --- a/services/web/app/coffee/Features/Project/ProjectController.coffee +++ b/services/web/app/coffee/Features/Project/ProjectController.coffee @@ -395,19 +395,27 @@ module.exports = ProjectController = return model _buildV1ProjectViewModel: (project) -> - { + projectViewModel = { id: project.id name: project.title lastUpdated: new Date(project.updated_at * 1000) # Convert from epoch - accessLevel: if project.owner?.user_is_owner then "owner" else "readOnly" archived: project.removed || project.archived - owner: { - # Unlisted V1 projects don't have an owner, so just show N/A - first_name: if project.owner then project.owner.name else 'N/A' - last_name: '' - } isV1Project: true } + if (project.owner? and project.owner.user_is_owner) or (project.creator? and project.creator.user_is_creator) + projectViewModel.accessLevel = "owner" + else + projectViewModel.accessLevel = "readOnly" + if project.owner? + projectViewModel.owner = { + first_name: project.owner.name + } + else if project.creator? + projectViewModel.owner = { + first_name: project.creator.name + } + return projectViewModel + _injectProjectOwners: (projects, callback = (error, projects) ->) -> users = {} diff --git a/services/web/public/coffee/main/project-list/project-list.coffee b/services/web/public/coffee/main/project-list/project-list.coffee index 75d4767ec0..5880129294 100644 --- a/services/web/public/coffee/main/project-list/project-list.coffee +++ b/services/web/public/coffee/main/project-list/project-list.coffee @@ -480,9 +480,9 @@ define [ if $scope.project.accessLevel == "owner" return "You" else if $scope.project.owner? - return "#{$scope.project.owner.first_name} #{$scope.project.owner.last_name}" + return [$scope.project.owner.first_name, $scope.project.owner.last_name].filter((n) -> n?).join(" ") else - return "?" + return "None" $scope.$watch "project.selected", (value) -> if value? From a558d05ac6f92dffcbde43edb90f5a5894270fe5 Mon Sep 17 00:00:00 2001 From: James Allen Date: Tue, 23 Jan 2018 13:48:03 +0000 Subject: [PATCH 02/11] Only show import modal for owned projects --- .../web/app/views/project/list/v1-item.pug | 22 ++++++++++--------- .../public/stylesheets/app/project-list.less | 7 ++++-- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/services/web/app/views/project/list/v1-item.pug b/services/web/app/views/project/list/v1-item.pug index 31fd036f7a..68f7b2af63 100644 --- a/services/web/app/views/project/list/v1-item.pug +++ b/services/web/app/views/project/list/v1-item.pug @@ -1,20 +1,22 @@ .col-xs-6 - span.v1-badge( - aria-label=translate("v1_badge") - tooltip-template="'v1ProjectTooltipTemplate'" - tooltip-append-to-body="true" - ) + .select-item + span.v1-badge( + aria-label=translate("v1_badge") + tooltip-template="'v1ProjectTooltipTemplate'" + tooltip-append-to-body="true" + ) span if settings.overleaf && settings.overleaf.host + button.btn.btn-link.projectName( + ng-click="openV1ImportModal(project)" + stop-propagation="click" + ng-show="project.accessLevel == 'owner'" + ) {{project.name}} a.projectName( href=settings.overleaf.host + "/{{project.id}}" target="_blank" + ng-hide="project.accessLevel == 'owner'" ) {{project.name}} - //- To re-enable the import dialog (may need style changing for padding/line-height): - //- button.btn.btn-link.projectName( - //- ng-click="openV1ImportModal(project)" - //- stop-propagation="click" - //- ) {{project.name}} .col-xs-2 span.owner {{ownerName()}} diff --git a/services/web/public/stylesheets/app/project-list.less b/services/web/public/stylesheets/app/project-list.less index 3a5ea560db..fc6777dea8 100644 --- a/services/web/public/stylesheets/app/project-list.less +++ b/services/web/public/stylesheets/app/project-list.less @@ -336,6 +336,10 @@ ul.project-list { } .projectName { margin-right: @line-height-computed / 4; + padding: 0; + vertical-align: inherit; + white-space: normal; + text-align: left; } .tag-label { @@ -373,8 +377,7 @@ ul.project-list { } .v1-badge { - margin-right: 9px; - margin-left: 7px; + margin-left: -4px; } } i.tablesort { From b537747ccd4cbca4f913bbe8062459a9a2bf1983 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 19 Jan 2018 12:01:23 +0000 Subject: [PATCH 03/11] check for duplicates in putElement --- .../Project/ProjectEntityHandler.coffee | 42 ++++++++++++------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index c4992a29f8..750924beb3 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -170,7 +170,8 @@ module.exports = ProjectEntityHandler = if project_or_id._id? # project return cb(null, project_or_id) else # id - return ProjectGetter.getProjectWithOnlyFolders project_or_id, cb + # need to retrieve full project structure to check for duplicates + return ProjectGetter.getProject project_or_id, {}, cb getProject (error, project) -> if err? logger.err project_id:project_id, err:err, "error getting project for add doc" @@ -207,7 +208,7 @@ module.exports = ProjectEntityHandler = ProjectEntityHandler.addDoc project_id, null, name, lines, callback addFileWithoutUpdatingHistory: (project_id, folder_id, fileName, path, userId, callback = (error, fileRef, folder_id, path, fileStoreUrl) ->)-> - ProjectGetter.getProjectWithOnlyFolders project_id, (err, project) -> + ProjectGetter.getProject project_id, {}, (err, project) -> if err? logger.err project_id:project_id, err:err, "error getting project for add file" return callback(err) @@ -606,19 +607,32 @@ module.exports = ProjectEntityHandler = newPath = fileSystem: "#{path.fileSystem}/#{element.name}" mongo: path.mongo - id = element._id+'' - element._id = require('mongoose').Types.ObjectId(id) - conditions = _id:project._id - mongopath = "#{path.mongo}.#{type}" - update = "$push":{} - update["$push"][mongopath] = element - logger.log project_id: project._id, element_id: element._id, fileType: type, folder_id: folder_id, mongopath:mongopath, "adding element to project" - Project.findOneAndUpdate conditions, update, {"new": true}, (err, project)-> - if err? - logger.err err: err, project_id: project._id, 'error saving in putElement project' - return callback(err) - callback(err, {path:newPath}, project) + ProjectEntityHandler.checkElementName folder, element.name, (err) => + return callback(err) if err? + id = element._id+'' + element._id = require('mongoose').Types.ObjectId(id) + conditions = _id:project._id + mongopath = "#{path.mongo}.#{type}" + update = "$push":{} + update["$push"][mongopath] = element + logger.log project_id: project._id, element_id: element._id, fileType: type, folder_id: folder_id, mongopath:mongopath, "adding element to project" + Project.findOneAndUpdate conditions, update, {"new": true}, (err, project)-> + if err? + logger.err err: err, project_id: project._id, 'error saving in putElement project' + return callback(err) + callback(err, {path:newPath}, project) + checkElementName: (folder, name, callback = (err) ->) -> + # check if the name is already taken by a doc, file or + # folder. If so, return an error "file already exists". + err = new Errors.InvalidNameError("file already exists") + for doc in folder?.docs or [] + return callback(err) if doc.name is name + for file in folder?.fileRefs or [] + return callback(err) if file.name is name + for folder in folder?.folders or [] + return callback(err) if folder.name is name + callback() confirmFolder = (project, folder_id, callback)-> logger.log folder_id:folder_id, project_id:project._id, "confirming folder in project" From c6cca79737f21a1b87182f6a0b385f6073ee0a29 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 19 Jan 2018 12:02:04 +0000 Subject: [PATCH 04/11] check for duplicates in addFolder --- .../Project/ProjectEntityHandler.coffee | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 750924beb3..d606b94c40 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -341,7 +341,7 @@ module.exports = ProjectEntityHandler = callback(null, folders, lastFolder) addFolder: (project_id, parentFolder_id, folderName, callback) -> - ProjectGetter.getProjectWithOnlyFolders project_id, (err, project)=> + ProjectGetter.getProject project_id, {}, (err, project)=> if err? logger.err project_id:project_id, err:err, "error getting project for add folder" return callback(err) @@ -459,17 +459,20 @@ module.exports = ProjectEntityHandler = return callback(error) if error? projectLocator.findElement {project:project, element_id:entity_id, type:entityType}, (error, entity, entPath)=> return callback(error) if error? - endPath = path.join(path.dirname(entPath.fileSystem), newName) - conditions = {_id:project_id} - update = "$set":{} - namePath = entPath.mongo+".name" - update["$set"][namePath] = newName - tpdsUpdateSender.moveEntity({project_id:project_id, startPath:entPath.fileSystem, endPath:endPath, project_name:project.name, rev:entity.rev}) - Project.findOneAndUpdate conditions, update, { "new": true}, (error, newProject) -> - return callback(error) if error? - ProjectEntityHandler.getAllEntitiesFromProject newProject, (error, newDocs, newFiles) => + ProjectEntityHandler.checkElementName entity, newName, (err) => + return callback(err) if err? + endPath = path.join(path.dirname(entPath.fileSystem), newName) + conditions = {_id:project_id} + update = "$set":{} + namePath = entPath.mongo+".name" + update["$set"][namePath] = newName + # FIXME check if this would create a duplicate file! + tpdsUpdateSender.moveEntity({project_id:project_id, startPath:entPath.fileSystem, endPath:endPath, project_name:project.name, rev:entity.rev}) + Project.findOneAndUpdate conditions, update, { "new": true}, (error, newProject) -> return callback(error) if error? - DocumentUpdaterHandler.updateProjectStructure project_id, userId, {oldDocs, newDocs, oldFiles, newFiles}, callback + ProjectEntityHandler.getAllEntitiesFromProject newProject, (error, newDocs, newFiles) => + return callback(error) if error? + DocumentUpdaterHandler.updateProjectStructure project_id, userId, {oldDocs, newDocs, oldFiles, newFiles}, callback _cleanUpEntity: (project, entity, entityType, path, userId, callback = (error) ->) -> if(entityType.indexOf("file") != -1) From 3881eb1d78ceafad08872e48688bb66a587a4f45 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 18 Jan 2018 16:22:38 +0000 Subject: [PATCH 05/11] check for duplicates on rename --- .../coffee/Features/Project/ProjectEntityHandler.coffee | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index d606b94c40..74010fba33 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -457,16 +457,16 @@ module.exports = ProjectEntityHandler = return callback(error) if error? ProjectEntityHandler.getAllEntitiesFromProject project, (error, oldDocs, oldFiles) => return callback(error) if error? - projectLocator.findElement {project:project, element_id:entity_id, type:entityType}, (error, entity, entPath)=> + projectLocator.findElement {project:project, element_id:entity_id, type:entityType}, (error, entity, entPath, parentFolder)=> return callback(error) if error? - ProjectEntityHandler.checkElementName entity, newName, (err) => - return callback(err) if err? + # check if the new name already exists in the current folder + ProjectEntityHandler.checkElementName parentFolder, newName, (error) => + return callback(error) if error? endPath = path.join(path.dirname(entPath.fileSystem), newName) conditions = {_id:project_id} update = "$set":{} namePath = entPath.mongo+".name" update["$set"][namePath] = newName - # FIXME check if this would create a duplicate file! tpdsUpdateSender.moveEntity({project_id:project_id, startPath:entPath.fileSystem, endPath:endPath, project_name:project.name, rev:entity.rev}) Project.findOneAndUpdate conditions, update, { "new": true}, (error, newProject) -> return callback(error) if error? From 4f5a5cb677215dd189e8f346c5d77d4ee425b0f0 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 18 Jan 2018 16:42:26 +0000 Subject: [PATCH 06/11] check for duplicates on move --- .../Project/ProjectEntityHandler.coffee | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 74010fba33..89fc9fb42a 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -395,7 +395,7 @@ module.exports = ProjectEntityHandler = return callback(err) if err? projectLocator.findElement {project, element_id: entity_id, type: entityType}, (err, entity, entityPath)-> return callback(err) if err? - self._checkValidMove project, entityType, entityPath, destFolderId, (error) -> + self._checkValidMove project, entityType, entity, entityPath, destFolderId, (error) -> return callback(error) if error? self.getAllEntitiesFromProject project, (error, oldDocs, oldFiles) => return callback(error) if error? @@ -414,19 +414,20 @@ module.exports = ProjectEntityHandler = return callback(error) if error? DocumentUpdaterHandler.updateProjectStructure project_id, userId, {oldDocs, newDocs, oldFiles, newFiles}, callback - _checkValidMove: (project, entityType, entityPath, destFolderId, callback = (error) ->) -> - return callback() if !entityType.match(/folder/) - + _checkValidMove: (project, entityType, entity, entityPath, destFolderId, callback = (error) ->) -> 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 + # check if there is already a doc/file/folder with the same name + # in the destination folder + ProjectEntityHandler.checkElementName destEntity, entity.name, (err)-> + return callback(err) if err? + if entityType.match(/folder/) + 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 + return callback(new Error("destination folder is a child folder of me")) callback() - deleteEntity: (project_id, entity_id, entityType, userId, callback = (error) ->)-> self = @ logger.log entity_id:entity_id, entityType:entityType, project_id:project_id, "deleting project entity" From 9538df1644b5423f19229fa696514b9509054fef Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 18 Jan 2018 14:28:36 +0000 Subject: [PATCH 07/11] update unit tests for duplicate checks --- .../Project/ProjectEntityHandlerTests.coffee | 140 +++++++++++++++++- 1 file changed, 138 insertions(+), 2 deletions(-) diff --git a/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee b/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee index 3b6a6e8626..39c1eba75d 100644 --- a/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee @@ -62,7 +62,7 @@ describe 'ProjectEntityHandler', -> @ProjectGetter = getProjectWithOnlyFolders : (project_id, callback)=> callback(null, @project) getProjectWithoutDocLines : (project_id, callback)=> callback(null, @project) - getProject:sinon.stub() + getProject: sinon.stub().callsArgWith(2, null, @project) @projectUpdater = markAsUpdated:sinon.stub() @projectLocator = findElement : sinon.stub() @@ -287,6 +287,8 @@ describe 'ProjectEntityHandler', -> @projectLocator.findElement = sinon.stub() @projectLocator.findElement.withArgs({project: @project, element_id: @docId, type: 'docs'}) .callsArgWith(1, null, @doc, @path) + @projectLocator.findElement.withArgs({project: @project, element_id: folder_id, type:"folder"},) + .callsArgWith(1, null, @destFolder, @destFolderPath) @ProjectEntityHandler.moveEntity project_id, @docId, folder_id, "docs", userId, done it 'should find the doc to move', -> @@ -315,6 +317,46 @@ describe 'ProjectEntityHandler', -> }) .should.equal true + describe "moving a doc when another with the same name already exists", -> + beforeEach () -> + @docId = "4eecaffcbffa66588e000009" + @doc = { name: "another-doc.tex", lines:["1234","312343d"], rev: "1234"} + @path = { + mongo:"folders[0]" + fileSystem:"/old_folder/somewhere.txt" + } + @destFolder = { name: "folder", docs: [ {name:"another-doc.tex"} ] } + @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) + @projectLocator.findElement.withArgs({project: @project, element_id: folder_id, type:"folder"},) + .callsArgWith(1, null, @destFolder, @destFolderPath) + @callback = sinon.stub() + @ProjectEntityHandler.moveEntity project_id, @docId, folder_id, "docs", userId, @callback + + it 'should return an error', -> + @callback.calledWith(new Errors.InvalidNameError("file already exists")).should.equal true + + it "should should not send the update to the doc updater", -> + @documentUpdaterHandler.updateProjectStructure + .called.should.equal false + + it 'should not remove the element from its current position', -> + @ProjectEntityHandler._removeElementFromMongoArray + .called.should.equal false + + it "should not put the element back in the new folder", -> + @ProjectEntityHandler._putElement.called.should.equal false + + it 'should not tell the third party data store', -> + @tpdsUpdateSender.moveEntity + .called.should.equal false + + describe "moving a folder", -> beforeEach -> @folder_id = "folder-to-move" @@ -379,10 +421,37 @@ describe 'ProjectEntityHandler', -> }) .should.equal true + describe "when the destination folder contains a file with the same name", -> + beforeEach -> + @path.fileSystem = "/one/src_dir" + @pathToMoveTo.fileSystem = "/two/dest_dir" + @folder_to_move_to = { name: "folder to move to", fileRefs: [ {name: "folder"}] } + @projectLocator.findElement.withArgs({project: @project, element_id: @move_to_folder_id, type: 'folder'}) + .callsArgWith(1, null, @folder_to_move_to, @pathToMoveTo) + @callback = sinon.stub() + @ProjectEntityHandler.moveEntity project_id, @folder_id, @move_to_folder_id, "folder", userId, @callback + + it 'should find the folder we are moving to 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 Errors.InvalidNameError("file already exists")) + .should.equal true + describe "when the destination folder is inside the moving folder", -> beforeEach -> @path.fileSystem = "/one/two" @pathToMoveTo.fileSystem = "/one/two/three" + + @projectLocator.findElement.withArgs({project: @project, element_id: @move_to_folder_id, type: 'folder'}) + .callsArgWith(1, null, @folder_to_move_to, @pathToMoveTo) @callback = sinon.stub() @ProjectEntityHandler.moveEntity project_id, @folder_id, @move_to_folder_id, "folder", userId, @callback @@ -472,6 +541,7 @@ describe 'ProjectEntityHandler', -> @lines = ['1234','abc'] @path = "/path/to/doc" + @ProjectGetter.getProject = sinon.stub().callsArgWith(2, null, @project) @ProjectEntityHandler._putElement = sinon.stub().callsArgWith(4, null, {path:{fileSystem:@path}}) @callback = sinon.stub() @tpdsUpdateSender.addDoc = sinon.stub().callsArg(1) @@ -522,6 +592,7 @@ describe 'ProjectEntityHandler', -> @lines = ['1234','abc'] @path = "/path/to/doc" + @ProjectGetter.getProject = sinon.stub().callsArgWith(2, null, @project) @ProjectEntityHandler._putElement = sinon.stub().callsArgWith(4, null, {path:{fileSystem:@path}}) @callback = sinon.stub() @tpdsUpdateSender.addDoc = sinon.stub().callsArg(1) @@ -1123,6 +1194,20 @@ describe 'ProjectEntityHandler', -> @newName = "new.tex" @path = mongo: "mongo.path", fileSystem: "/oldnamepath/oldname" + @project_id = project_id + @project = + _id: ObjectId(project_id) + rootFolder: [_id:ObjectId()] + @folder = + _id: ObjectId() + name: "someFolder" + docs: [ {name: "another-doc.tex"} ] + fileRefs: [ {name: "another-file.tex"} ] + folders: [ {name: "another-folder"} ] + @doc = + _id: ObjectId() + name: "new.tex" + @ProjectGetter.getProject.callsArgWith(2, null, @project) @ProjectEntityHandler.getAllEntitiesFromProject = sinon.stub() @ProjectEntityHandler.getAllEntitiesFromProject @@ -1132,7 +1217,7 @@ describe 'ProjectEntityHandler', -> .onSecondCall() .callsArgWith(1, null, @newDocs = ['new-doc'], @newFiles = ['new-file']) - @projectLocator.findElement = sinon.stub().callsArgWith(1, null, @entity = { _id: @entity_id, name:"oldname", rev:4 }, @path) + @projectLocator.findElement = sinon.stub().callsArgWith(1, null, @entity = { _id: @entity_id, name:"oldname", rev:4 }, @path, @folder) @tpdsUpdateSender.moveEntity = sinon.stub() @ProjectModel.findOneAndUpdate = sinon.stub().callsArgWith(3, null, @project) @documentUpdaterHandler.updateProjectStructure = sinon.stub().yields() @@ -1154,6 +1239,27 @@ describe 'ProjectEntityHandler', -> @tpdsUpdateSender.moveEntity.calledWith({project_id:project_id, startPath:@path.fileSystem, endPath:"/oldnamepath/new.tex", project_name:@project.name, rev:4}).should.equal true done() + describe "when a document already exists with the same name", -> + beforeEach -> + @project = + _id: ObjectId(project_id) + rootFolder: [_id:ObjectId()] + @folder = + _id: ObjectId() + name: "someFolder" + docs: [ {name: "another-doc.tex"} ] + fileRefs: [ {name: "another-file.tex"} ] + folders: [ {name: "another-folder"} ] + @doc = + _id: ObjectId() + name: "new.tex" + @newName = "another-doc.tex" + + it "should return an error", (done)-> + @ProjectEntityHandler.renameEntity project_id, @entity_id, @entityType, @newName, userId, (err)=> + err.should.deep.equal new Errors.InvalidNameError("file already exists") + done() + describe "_insertDeletedDocReference", -> beforeEach -> @doc = @@ -1248,6 +1354,9 @@ describe 'ProjectEntityHandler', -> @folder = _id: ObjectId() name: "someFolder" + docs: [ {name: "another-doc.tex"} ] + fileRefs: [ {name: "another-file.tex"} ] + folders: [ {name: "another-folder"} ] @doc = _id: ObjectId() name: "new.tex" @@ -1290,6 +1399,33 @@ describe 'ProjectEntityHandler', -> @ProjectModel.findOneAndUpdate.called.should.equal false done() + it "should error if a document already exists with the same name", (done)-> + doc = + _id: ObjectId() + name: "another-doc.tex" + @ProjectEntityHandler._putElement @project, @folder, doc, "doc", (err)=> + @ProjectModel.findOneAndUpdate.called.should.equal false + err.should.deep.equal new Errors.InvalidNameError("file already exists") + done() + + it "should error if a file already exists with the same name", (done)-> + doc = + _id: ObjectId() + name: "another-file.tex" + @ProjectEntityHandler._putElement @project, @folder, doc, "doc", (err)=> + @ProjectModel.findOneAndUpdate.called.should.equal false + err.should.deep.equal new Errors.InvalidNameError("file already exists") + done() + + it "should error if a folder already exists with the same name", (done)-> + doc = + _id: ObjectId() + name: "another-folder" + @ProjectEntityHandler._putElement @project, @folder, doc, "doc", (err)=> + @ProjectModel.findOneAndUpdate.called.should.equal false + err.should.deep.equal new Errors.InvalidNameError("file already exists") + done() + describe "_countElements", -> beforeEach -> From 9b8ce78eb94ed6c6d0916c7e7a7a0ae63e40a021 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 17 Jan 2018 12:11:02 +0000 Subject: [PATCH 08/11] handle errors normally in addFolder modal --- .../web/app/coffee/Features/Editor/EditorHttpController.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/Editor/EditorHttpController.coffee b/services/web/app/coffee/Features/Editor/EditorHttpController.coffee index 16b1a79e31..614781211c 100644 --- a/services/web/app/coffee/Features/Editor/EditorHttpController.coffee +++ b/services/web/app/coffee/Features/Editor/EditorHttpController.coffee @@ -105,7 +105,7 @@ module.exports = EditorHttpController = else if error?.message == 'invalid element name' res.status(400).json(req.i18n.translate('invalid_file_name')) else if error? - res.status(500).json(req.i18n.translate('generic_something_went_wrong')) + next(error) else res.json doc From feb02dacd4d138c0eb724e84ec7aaa60d067e8e6 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 19 Jan 2018 11:27:22 +0000 Subject: [PATCH 09/11] only update client filetree on success --- .../coffee/ide/file-tree/FileTreeManager.coffee | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/services/web/public/coffee/ide/file-tree/FileTreeManager.coffee b/services/web/public/coffee/ide/file-tree/FileTreeManager.coffee index 390b9dba79..d46525bf20 100644 --- a/services/web/public/coffee/ide/file-tree/FileTreeManager.coffee +++ b/services/web/public/coffee/ide/file-tree/FileTreeManager.coffee @@ -341,12 +341,12 @@ define [ renameEntity: (entity, name, callback = (error) ->) -> return if entity.name == name - if name.length < 150 - entity.name = name - return @ide.$http.post "/project/#{@ide.project_id}/#{entity.type}/#{entity.id}/rename", { - name: entity.name, + return if name.length >= 150 + @ide.$http.post("/project/#{@ide.project_id}/#{entity.type}/#{entity.id}/rename", { + name: name, _csrf: window.csrfToken - } + }).then () -> + entity.name = name deleteEntity: (entity, callback = (error) ->) -> # We'll wait for the socket.io notification to @@ -362,11 +362,11 @@ define [ # 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.queuedHttp.post "/project/#{@ide.project_id}/#{entity.type}/#{entity.id}/move", { + @ide.queuedHttp.post("/project/#{@ide.project_id}/#{entity.type}/#{entity.id}/move", { folder_id: parent_folder.id _csrf: window.csrfToken - } + }).then () => + @_moveEntityInScope(entity, parent_folder) _isChildFolder: (parent_folder, child_folder) -> parent_path = @getEntityPath(parent_folder) or "" # null if root folder From 2a0b0d3a870ba334f6a71e3655fc51a4f24aae8c Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 23 Jan 2018 10:21:05 +0000 Subject: [PATCH 10/11] only need to load rootFolder from project --- .../app/coffee/Features/Project/ProjectEntityHandler.coffee | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 89fc9fb42a..470545b8f9 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -171,7 +171,7 @@ module.exports = ProjectEntityHandler = return cb(null, project_or_id) else # id # need to retrieve full project structure to check for duplicates - return ProjectGetter.getProject project_or_id, {}, cb + return ProjectGetter.getProject project_or_id, {rootFolder:true, name:true}, cb getProject (error, project) -> if err? logger.err project_id:project_id, err:err, "error getting project for add doc" @@ -208,7 +208,7 @@ module.exports = ProjectEntityHandler = ProjectEntityHandler.addDoc project_id, null, name, lines, callback addFileWithoutUpdatingHistory: (project_id, folder_id, fileName, path, userId, callback = (error, fileRef, folder_id, path, fileStoreUrl) ->)-> - ProjectGetter.getProject project_id, {}, (err, project) -> + ProjectGetter.getProject project_id, {rootFolder:true, name:true}, (err, project) -> if err? logger.err project_id:project_id, err:err, "error getting project for add file" return callback(err) @@ -341,7 +341,7 @@ module.exports = ProjectEntityHandler = callback(null, folders, lastFolder) addFolder: (project_id, parentFolder_id, folderName, callback) -> - ProjectGetter.getProject project_id, {}, (err, project)=> + ProjectGetter.getProject project_id, {rootFolder:true, name:true}, (err, project)=> if err? logger.err project_id:project_id, err:err, "error getting project for add folder" return callback(err) From ec93cedf27fb315f955b834e28dab04c47bf68f3 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 23 Jan 2018 10:35:38 +0000 Subject: [PATCH 11/11] rename checkElementName to checkValidElementName --- .../coffee/Features/Project/ProjectEntityHandler.coffee | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 470545b8f9..496bb73bd5 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -419,7 +419,7 @@ module.exports = ProjectEntityHandler = return callback(err) if err? # check if there is already a doc/file/folder with the same name # in the destination folder - ProjectEntityHandler.checkElementName destEntity, entity.name, (err)-> + ProjectEntityHandler.checkValidElementName destEntity, entity.name, (err)-> return callback(err) if err? if entityType.match(/folder/) logger.log destFolderPath: destFolderPath.fileSystem, folderPath: entityPath.fileSystem, "checking folder is not moving into child folder" @@ -461,7 +461,7 @@ module.exports = ProjectEntityHandler = projectLocator.findElement {project:project, element_id:entity_id, type:entityType}, (error, entity, entPath, parentFolder)=> return callback(error) if error? # check if the new name already exists in the current folder - ProjectEntityHandler.checkElementName parentFolder, newName, (error) => + ProjectEntityHandler.checkValidElementName parentFolder, newName, (error) => return callback(error) if error? endPath = path.join(path.dirname(entPath.fileSystem), newName) conditions = {_id:project_id} @@ -611,7 +611,7 @@ module.exports = ProjectEntityHandler = newPath = fileSystem: "#{path.fileSystem}/#{element.name}" mongo: path.mongo - ProjectEntityHandler.checkElementName folder, element.name, (err) => + ProjectEntityHandler.checkValidElementName folder, element.name, (err) => return callback(err) if err? id = element._id+'' element._id = require('mongoose').Types.ObjectId(id) @@ -626,7 +626,7 @@ module.exports = ProjectEntityHandler = return callback(err) callback(err, {path:newPath}, project) - checkElementName: (folder, name, callback = (err) ->) -> + checkValidElementName: (folder, name, callback = (err) ->) -> # check if the name is already taken by a doc, file or # folder. If so, return an error "file already exists". err = new Errors.InvalidNameError("file already exists")