From bedc8a049210b9b51732f57665fa06f4670731ca Mon Sep 17 00:00:00 2001 From: James Allen Date: Mon, 7 Mar 2016 15:25:10 +0000 Subject: [PATCH] Remove ProjectGetter.populateProjectWithUsers --- .../Editor/EditorHttpController.coffee | 5 +- .../Project/ProjectEditorHandler.coffee | 69 +++++++++--------- .../Features/Project/ProjectGetter.coffee | 40 ---------- .../Editor/EditorHttpControllerTests.coffee | 8 +- .../Project/ProjectEditorHandlerTests.coffee | 73 +++++++++---------- .../coffee/Project/ProjectGetterTests.coffee | 35 --------- 6 files changed, 76 insertions(+), 154 deletions(-) diff --git a/services/web/app/coffee/Features/Editor/EditorHttpController.coffee b/services/web/app/coffee/Features/Editor/EditorHttpController.coffee index 2619316e6d..18e8f7d308 100644 --- a/services/web/app/coffee/Features/Editor/EditorHttpController.coffee +++ b/services/web/app/coffee/Features/Editor/EditorHttpController.coffee @@ -8,6 +8,7 @@ UserGetter = require('../User/UserGetter') AuthorizationManager = require("../Security/AuthorizationManager") ProjectEditorHandler = require('../Project/ProjectEditorHandler') Metrics = require('../../infrastructure/Metrics') +CollaboratorsHandler = require("../Collaborators/CollaboratorsHandler") module.exports = EditorHttpController = joinProject: (req, res, next) -> @@ -29,7 +30,7 @@ module.exports = EditorHttpController = ProjectGetter.getProjectWithoutDocLines project_id, (error, project) -> return callback(error) if error? return callback(new Error("not found")) if !project? - ProjectGetter.populateProjectWithUsers project, (error, project) -> + CollaboratorsHandler.getMembersWithPrivilegeLevels project, (error, members) -> return callback(error) if error? UserGetter.getUser user_id, { isAdmin: true }, (error, user) -> return callback(error) if error? @@ -39,7 +40,7 @@ module.exports = EditorHttpController = callback null, null, false else callback(null, - ProjectEditorHandler.buildProjectModelView(project), + ProjectEditorHandler.buildProjectModelView(project, members), privilegeLevel ) diff --git a/services/web/app/coffee/Features/Project/ProjectEditorHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEditorHandler.coffee index f12a7d548e..5b3143d765 100644 --- a/services/web/app/coffee/Features/Project/ProjectEditorHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEditorHandler.coffee @@ -1,11 +1,7 @@ _ = require("underscore") module.exports = ProjectEditorHandler = - buildProjectModelView: (project, options) -> - options ||= {} - if !options.includeUsers? - options.includeUsers = true - + buildProjectModelView: (project, members) -> result = _id : project._id name : project.name @@ -18,40 +14,41 @@ module.exports = ProjectEditorHandler = spellCheckLanguage: project.spellCheckLanguage deletedByExternalDataSource : project.deletedByExternalDataSource || false deletedDocs: project.deletedDocs + members: [] - if options.includeUsers - result.features = - collaborators: -1 # Infinite - versioning: false - dropbox:false - compileTimeout: 60 - compileGroup:"standard" - templates: false - references: false + result.features = # defaults + collaborators: -1 # Infinite + versioning: false + dropbox:false + compileTimeout: 60 + compileGroup:"standard" + templates: false + references: false + + owner = null + for member in members + if member.privilegeLevel == "admin" + owner = member.user + else + result.members.push @buildUserModelView member.user, member.privilegeLevel + result.owner = @buildUserModelView owner, "owner" - if project.owner_ref.features? - if project.owner_ref.features.collaborators? - result.features.collaborators = project.owner_ref.features.collaborators - if project.owner_ref.features.versioning? - result.features.versioning = project.owner_ref.features.versioning - if project.owner_ref.features.dropbox? - result.features.dropbox = project.owner_ref.features.dropbox - if project.owner_ref.features.compileTimeout? - result.features.compileTimeout = project.owner_ref.features.compileTimeout - if project.owner_ref.features.compileGroup? - result.features.compileGroup = project.owner_ref.features.compileGroup - if project.owner_ref.features.templates? - result.features.templates = project.owner_ref.features.templates - if project.owner_ref.features.references? - result.features.references = project.owner_ref.features.references + if owner?.features? + if owner.features.collaborators? + result.features.collaborators = owner.features.collaborators + if owner.features.versioning? + result.features.versioning = owner.features.versioning + if owner.features.dropbox? + result.features.dropbox = owner.features.dropbox + if owner.features.compileTimeout? + result.features.compileTimeout = owner.features.compileTimeout + if owner.features.compileGroup? + result.features.compileGroup = owner.features.compileGroup + if owner.features.templates? + result.features.templates = owner.features.templates + if owner.features.references? + result.features.references = owner.features.references - - result.owner = @buildUserModelView project.owner_ref, "owner" - result.members = [] - for ref in project.readOnly_refs - result.members.push @buildUserModelView ref, "readOnly" - for ref in project.collaberator_refs - result.members.push @buildUserModelView ref, "readAndWrite" return result buildUserModelView: (user, privileges) -> diff --git a/services/web/app/coffee/Features/Project/ProjectGetter.coffee b/services/web/app/coffee/Features/Project/ProjectGetter.coffee index 13fa409e99..7d9fd13bb6 100644 --- a/services/web/app/coffee/Features/Project/ProjectGetter.coffee +++ b/services/web/app/coffee/Features/Project/ProjectGetter.coffee @@ -36,43 +36,3 @@ module.exports = ProjectGetter = CollaboratorsHandler.getProjectsUserIsMemberOf user_id, fields, (error, readAndWriteProjects, readOnlyProjects) -> return callback(error) if error? callback null, projects, readAndWriteProjects, readOnlyProjects - - populateProjectWithUsers: (project, callback=(error, project) ->) -> - # eventually this should be in a UserGetter.getUser module - getUser = (user_id, callback=(error, user) ->) -> - unless user_id instanceof ObjectId - user_id = ObjectId(user_id) - db.users.find _id: user_id, (error, users = []) -> - callback error, users[0] - - jobs = [] - jobs.push (callback) -> - getUser project.owner_ref, (error, user) -> - return callback(error) if error? - if user? - project.owner_ref = user - callback null, project - - readOnly_refs = project.readOnly_refs - project.readOnly_refs = [] - for readOnly_ref in readOnly_refs - do (readOnly_ref) -> - jobs.push (callback) -> - getUser readOnly_ref, (error, user) -> - return callback(error) if error? - if user? - project.readOnly_refs.push user - callback null, project - - collaberator_refs = project.collaberator_refs - project.collaberator_refs = [] - for collaberator_ref in collaberator_refs - do (collaberator_ref) -> - jobs.push (callback) -> - getUser collaberator_ref, (error, user) -> - return callback(error) if error? - if user? - project.collaberator_refs.push user - callback null, project - - async.parallelLimit jobs, 3, (error) -> callback error, project diff --git a/services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee b/services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee index ab003208c2..21c8a195e4 100644 --- a/services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee @@ -16,6 +16,7 @@ describe "EditorHttpController", -> "logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub() } "./EditorController": @EditorController = {} '../../infrastructure/Metrics': @Metrics = {inc: sinon.stub()} + "../Collaborators/CollaboratorsHandler": @CollaboratorsHandler = {} @project_id = "mock-project-id" @doc_id = "mock-doc-id" @@ -85,13 +86,14 @@ describe "EditorHttpController", -> @user = _id: @user_id = "user-id" projects: {} + @members = ["members", "mock"] @projectModelView = _id: @project_id owner:{_id:"something"} view: true @ProjectEditorHandler.buildProjectModelView = sinon.stub().returns(@projectModelView) @ProjectGetter.getProjectWithoutDocLines = sinon.stub().callsArgWith(1, null, @project) - @ProjectGetter.populateProjectWithUsers = sinon.stub().callsArgWith(1, null, @project) + @CollaboratorsHandler.getMembersWithPrivilegeLevels = sinon.stub().callsArgWith(1, null, @members) @UserGetter.getUser = sinon.stub().callsArgWith(2, null, @user) describe "when authorized", -> @@ -105,8 +107,8 @@ describe "EditorHttpController", -> .calledWith(@project_id) .should.equal true - it "should populate the user references in the project", -> - @ProjectGetter.populateProjectWithUsers + it "should get the list of users in the project", -> + @CollaboratorsHandler.getMembersWithPrivilegeLevels .calledWith(@project) .should.equal true diff --git a/services/web/test/UnitTests/coffee/Project/ProjectEditorHandlerTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectEditorHandlerTests.coffee index 52c871669a..7d71f6c89f 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectEditorHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectEditorHandlerTests.coffee @@ -38,33 +38,41 @@ describe "ProjectEditorHandler", -> folders : [] }] }] - owner_ref : - _id: "owner-id" - first_name : "Owner" - last_name : "ShareLaTeX" - email : "owner@sharelatex.com" - readOnly_refs: [{ - _id: "read-only-id" - first_name : "Read" - last_name : "Only" - email : "read-only@sharelatex.com" - }] - collaberator_refs: [{ - _id: "read-write-id" - first_name : "Read" - last_name : "Write" - email : "read-write@sharelatex.com" - }] deletedDocs: [{ _id: "deleted-doc-id" name: "main.tex" }] + @members = [{ + user: @owner = { + _id: "owner-id" + first_name : "Owner" + last_name : "ShareLaTeX" + email : "owner@sharelatex.com" + }, + privilegeLevel: "admin" + },{ + user: { + _id: "read-only-id" + first_name : "Read" + last_name : "Only" + email : "read-only@sharelatex.com" + }, + privilegeLevel: "readOnly" + },{ + user: { + _id: "read-write-id" + first_name : "Read" + last_name : "Write" + email : "read-write@sharelatex.com" + }, + privilegeLevel: "readAndWrite" + }] @handler = SandboxedModule.require modulePath describe "buildProjectModelView", -> describe "with owner and members included", -> beforeEach -> - @result = @handler.buildProjectModelView @project + @result = @handler.buildProjectModelView @project, @members it "should include the id", -> should.exist @result._id @@ -140,41 +148,30 @@ describe "ProjectEditorHandler", -> it "should set the deletedByExternalDataSource flag to false when it is not there", -> delete @project.deletedByExternalDataSource - result = @handler.buildProjectModelView @project + result = @handler.buildProjectModelView @project, @members result.deletedByExternalDataSource.should.equal false it "should set the deletedByExternalDataSource flag to false when it is false", -> - result = @handler.buildProjectModelView @project + result = @handler.buildProjectModelView @project, @members result.deletedByExternalDataSource.should.equal false it "should set the deletedByExternalDataSource flag to true when it is true", -> @project.deletedByExternalDataSource = true - result = @handler.buildProjectModelView @project + result = @handler.buildProjectModelView @project, @members result.deletedByExternalDataSource.should.equal true describe "features", -> beforeEach -> - @project.owner_ref.features = + @owner.features = versioning: true collaborators: 3 compileGroup:"priority" compileTimeout: 96 - @result = @handler.buildProjectModelView @project + @result = @handler.buildProjectModelView @project, @members it "should copy the owner features to the project", -> - @result.features.versioning.should.equal @project.owner_ref.features.versioning - @result.features.collaborators.should.equal @project.owner_ref.features.collaborators - @result.features.compileGroup.should.equal @project.owner_ref.features.compileGroup - @result.features.compileTimeout.should.equal @project.owner_ref.features.compileTimeout + @result.features.versioning.should.equal @owner.features.versioning + @result.features.collaborators.should.equal @owner.features.collaborators + @result.features.compileGroup.should.equal @owner.features.compileGroup + @result.features.compileTimeout.should.equal @owner.features.compileTimeout - - describe "without owners and members", -> - beforeEach -> - @result = @handler.buildProjectModelView @project, includeUsers: false - - it "should not include the owner", -> - should.not.exist @result.owner - - it "should not include the members", -> - should.not.exist @result.members - diff --git a/services/web/test/UnitTests/coffee/Project/ProjectGetterTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectGetterTests.coffee index a107031c0b..da9b9765e3 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectGetterTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectGetterTests.coffee @@ -79,41 +79,6 @@ describe "ProjectGetter", -> it "should call the callback with the project", -> @callback.calledWith(null, @project).should.equal true - - describe "populateProjectWithUsers", -> - beforeEach -> - @users = [] - @user_lookup = {} - for i in [0..4] - @users[i] = _id: ObjectId.createPk() - @user_lookup[@users[i]._id.toString()] = @users[i] - @project = - _id: ObjectId.createPk() - owner_ref: @users[0]._id - readOnly_refs: [@users[1]._id, @users[2]._id] - collaberator_refs: [@users[3]._id, @users[4]._id] - @db.users.find = (query, callback) => - callback null, [@user_lookup[query._id.toString()]] - sinon.spy @db.users, "find" - @ProjectGetter.populateProjectWithUsers @project, (err, project)=> - @callback err, project - - it "should look up each user", -> - for user in @users - @db.users.find.calledWith(_id: user._id).should.equal true - - it "should set the owner_ref to the owner", -> - @project.owner_ref.should.equal @users[0] - - it "should set the readOnly_refs to the read only users", -> - expect(@project.readOnly_refs).to.deep.equal [@users[1], @users[2]] - - it "should set the collaberator_refs to the collaborators", -> - expect(@project.collaberator_refs).to.deep.equal [@users[3], @users[4]] - - it "should call the callback", -> - assert.deepEqual @callback.args[0][1], @project - describe "findAllUsersProjects", -> beforeEach -> @fields = {"mock": "fields"}