Refactor getCollaborators http method to use CollaboratorsHandler

services/web/app/coffee/Features/Collaborators/CollaboratorsController.coffee

@@ -8,13 +8,12 @@ mimelib = require("mimelib")
 
 module.exports = CollaboratorsController =
 	getCollaborators: (req, res, next = (error) ->) ->
-		ProjectGetter.getProject req.params.Project_id, { owner_ref: true, collaberator_refs: true, readOnly_refs: true}, (error, project) ->
+		project_id = req.params.Project_id
+		CollaboratorsHandler.getMembersWithPrivilegeLevels project_id, (error, members) ->
 			return next(error) if error?
-			ProjectGetter.populateProjectWithUsers project, (error, project) ->
+			CollaboratorsController._formatCollaborators members, (error, collaborators) ->
 				return next(error) if error?
-				CollaboratorsController._formatCollaborators project, (error, collaborators) ->
-					return next(error) if error?
-					res.send(JSON.stringify(collaborators))
+				res.json(collaborators)
 			
 	addUserToProject: (req, res, next) ->
 		project_id = req.params.Project_id
@@ -59,7 +58,7 @@ module.exports = CollaboratorsController =
 			EditorRealTimeController.emitToRoom(project_id, 'userRemovedFromProject', user_id)
 			callback()
 
-	_formatCollaborators: (project, callback = (error, collaborators) ->) ->
+	_formatCollaborators: (members, callback = (error, collaborators) ->) ->
 		collaborators = []
 
 		pushCollaborator = (user, permissions, owner) ->
@@ -71,16 +70,14 @@ module.exports = CollaboratorsController =
 				permissions: permissions
 				owner: owner
 			}
-			
-		if project.owner_ref?
-			pushCollaborator(project.owner_ref, ["read", "write", "admin"], true)
-
-		if project.collaberator_refs? and project.collaberator_refs.length > 0
-			for user in project.collaberator_refs
+		
+		for member in members
+			{user, privilegeLevel} = member
+			if privilegeLevel == "admin"
+				pushCollaborator(user, ["read", "write", "admin"], true)
+			else if privilegeLevel == "readAndWrite"
 				pushCollaborator(user, ["read", "write"], false)
-
-		if project.readOnly_refs? and project.readOnly_refs.length > 0
-			for user in project.readOnly_refs
+			else
 				pushCollaborator(user, ["read"], false)
 
 		callback null, collaborators

services/web/app/coffee/Features/Collaborators/CollaboratorsHandler.coffee

@@ -10,10 +10,11 @@ async = require "async"
 
 module.exports = CollaboratorsHandler =
 	getMemberIdsWithPrivilegeLevels: (project_id, callback = (error, members) ->) ->
-		Project.findOne { _id: project_id }, { collaberator_refs: 1, readOnly_refs: 1 }, (error, project) ->
+		Project.findOne { _id: project_id }, { owner_ref: 1, collaberator_refs: 1, readOnly_refs: 1 }, (error, project) ->
 			return callback(error) if error?
 			return callback null, null if !project?
 			members = []
+			members.push { id: project.owner_ref.toString(), privilegeLevel: "admin" }
 			for member_id in project.readOnly_refs or []
 				members.push { id: member_id, privilegeLevel: "readOnly" }
 			for member_id in project.collaberator_refs or []
@@ -34,6 +35,11 @@ module.exports = CollaboratorsHandler =
 		CollaboratorsHandler.getMemberIdsWithPrivilegeLevels project_id, (error, members) ->
 			return callback(error) if error?
 			return callback null, (members or []).length
+		
+	getCollaboratorCount: (project_id, callback = (error, count) ->) ->
+		CollaboratorsHandler.getMemberCount project_id, (error, count) ->
+			return callback(error) if error?
+			return callback null, count - 1 # Don't count project owner
 
 	isUserMemberOfProject: (user_id, project_id, callback = (error, isMember, privilegeLevel) ->) ->
 		CollaboratorsHandler.getMemberIdsWithPrivilegeLevels project_id, (error, members) ->

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

@@ -3,7 +3,6 @@ db = mongojs.db
 ObjectId = mongojs.ObjectId
 async = require "async"
 Project = require("../../models/Project").Project
-CollaboratorsHandler = require "../Collaborators/CollaboratorsHandler"
 
 module.exports = ProjectGetter =
 	EXCLUDE_DEPTH: 8
@@ -31,6 +30,7 @@ module.exports = ProjectGetter =
 		db.projects.findOne query, projection, callback
 	
 	findAllUsersProjects: (user_id, fields, callback = (error, ownedProjects, readAndWriteProjects, readOnlyProjects) ->) ->
+		CollaboratorsHandler = require "../Collaborators/CollaboratorsHandler"
 		Project.find {owner_ref: user_id}, fields, (error, projects) ->
 			return callback(error) if error?
 			CollaboratorsHandler.getProjectsUserIsMemberOf user_id, fields, (error, readAndWriteProjects, readOnlyProjects) ->

services/web/test/UnitTests/coffee/Collaborators/CollaboratorsControllerTests.coffee

@@ -26,32 +26,49 @@ describe "CollaboratorsController", ->
 
 	describe "getCollaborators", ->
 		beforeEach ->
-			@project =
-				_id: @project_id = "project-id-123"
-			@collaborators = ["array of collaborators"]
-			@req.params = Project_id: @project_id
-			@ProjectGetter.getProject = sinon.stub().callsArgWith(2, null, @project)
-			@ProjectGetter.populateProjectWithUsers = sinon.stub().callsArgWith(1, null, @project)
-			@CollaboratorsController._formatCollaborators = sinon.stub().callsArgWith(1, null, @collaborators)
+			@req.params =
+				Project_id: @project_id
+			@members = [
+				{
+					user: { _id: "admin-id", email: "admin@example.com", first_name: "Joe", last_name: "Admin", foo: "bar" }
+					privilegeLevel: "admin"
+				},
+				{
+					user: { _id: "rw-id", email: "rw@example.com", first_name: "Jane", last_name: "Write", foo: "bar" }
+					privilegeLevel: "readAndWrite"
+				},
+				{
+					user: { _id: "ro-id", email: "ro@example.com", first_name: "Joe", last_name: "Read", foo: "bar" }
+					privilegeLevel: "readOnly"
+				}
+			]
+			@CollaboratorsHandler.getMembersWithPrivilegeLevels = sinon.stub()
+			@CollaboratorsHandler.getMembersWithPrivilegeLevels
+				.withArgs(@project_id)
+				.yields(null, @members)
+			@res.json = sinon.stub()
 			@CollaboratorsController.getCollaborators(@req, @res)
 
-		it "should get the project", ->
-			@ProjectGetter.getProject
-				.calledWith(@project_id, { owner_ref: true, collaberator_refs: true, readOnly_refs: true })
-				.should.equal true
-
-		it "should populate the users in the project", ->
-			@ProjectGetter.populateProjectWithUsers
-				.calledWith(@project)
-				.should.equal true
-
-		it "should format the collaborators", ->
-			@CollaboratorsController._formatCollaborators
-				.calledWith(@project)
-				.should.equal true
-
 		it "should return the formatted collaborators", ->
-			@res.body.should.equal JSON.stringify(@collaborators)
+			@res.json
+				.calledWith([
+					{
+						id: "admin-id", email: "admin@example.com", first_name: "Joe", last_name: "Admin"
+						permissions: ["read", "write", "admin"]
+						owner: true
+					}
+					{
+						id: "rw-id", email: "rw@example.com", first_name: "Jane", last_name: "Write"
+						permissions: ["read", "write"]
+						owner: false
+					}
+					{
+						id: "ro-id", email: "ro@example.com", first_name: "Joe", last_name: "Read"
+						permissions: ["read"]
+						owner: false
+					}
+				])
+				.should.equal true
 
 	describe "addUserToProject", ->
 		beforeEach ->
@@ -179,85 +196,3 @@ describe "CollaboratorsController", ->
 		it "should return a success code", ->
 			@res.sendStatus.calledWith(204).should.equal true
 
-	describe "_formatCollaborators", ->
-		beforeEach ->
-			@owner =
-				_id: ObjectId()
-				first_name: "Lenny"
-				last_name: "Lion"
-				email: "test@sharelatex.com"
-				hashed_password: "password" # should not be included
-
-		describe "formatting the owner", ->
-			beforeEach ->
-				@project =
-					owner_ref: @owner
-					collaberator_refs: []
-				@CollaboratorsController._formatCollaborators(@project, @callback)
-
-			it "should return the owner with read, write and admin permissions", ->
-				@formattedOwner = @callback.args[0][1][0]
-				expect(@formattedOwner).to.deep.equal {
-					id: @owner._id.toString()
-					first_name: @owner.first_name
-					last_name: @owner.last_name
-					email: @owner.email
-					permissions: ["read", "write", "admin"]
-					owner: true
-				}
-
-		describe "formatting a collaborator with write access", ->
-			beforeEach ->
-				@collaborator =
-					_id: ObjectId()
-					first_name: "Douglas"
-					last_name: "Adams"
-					email: "doug@sharelatex.com"
-					hashed_password: "password" # should not be included
-					
-				@project =
-					owner_ref: @owner
-					collaberator_refs: [ @collaborator ]
-				@CollaboratorsController._formatCollaborators(@project, @callback)
-
-			it "should return the collaborator with read and write permissions", ->
-				@formattedCollaborator = @callback.args[0][1][1]
-				expect(@formattedCollaborator).to.deep.equal {
-					id: @collaborator._id.toString()
-					first_name: @collaborator.first_name
-					last_name: @collaborator.last_name
-					email: @collaborator.email
-					permissions: ["read", "write"]
-					owner: false
-				}
-
-		describe "formatting a collaborator with read only access", ->
-			beforeEach ->
-				@collaborator =
-					_id: ObjectId()
-					first_name: "Douglas"
-					last_name: "Adams"
-					email: "doug@sharelatex.com"
-					hashed_password: "password" # should not be included
-					
-				@project =
-					owner_ref: @owner
-					collaberator_refs: []
-					readOnly_refs: [ @collaborator ]
-				@CollaboratorsController._formatCollaborators(@project, @callback)
-
-			it "should return the collaborator with read permissions", ->
-				@formattedCollaborator = @callback.args[0][1][1]
-				expect(@formattedCollaborator).to.deep.equal {
-					id: @collaborator._id.toString()
-					first_name: @collaborator.first_name
-					last_name: @collaborator.last_name
-					email: @collaborator.email
-					permissions: ["read"]
-					owner: false
-				}
-					
-			
-
-		
-				

services/web/test/UnitTests/coffee/Collaborators/CollaboratorsHandlerTests.coffee

@@ -26,7 +26,8 @@ describe "CollaboratorsHandler", ->
 	describe "getMemberIdsWithPrivilegeLevels", ->
 		beforeEach ->
 			@Project.findOne = sinon.stub()
-			@Project.findOne.withArgs({_id: @project_id}, {collaberator_refs: 1, readOnly_refs: 1}).yields(null, @project = {
+			@Project.findOne.withArgs({_id: @project_id}, {owner_ref: 1, collaberator_refs: 1, readOnly_refs: 1}).yields(null, @project = {
+				owner_ref: [ "owner-ref" ]
 				readOnly_refs: [ "read-only-ref-1", "read-only-ref-2" ]
 				collaberator_refs: [ "read-write-ref-1", "read-write-ref-2" ]
 			})
@@ -35,6 +36,7 @@ describe "CollaboratorsHandler", ->
 		it "should return an array of member ids with their privilege levels", ->
 			@callback
 				.calledWith(null, [
+					{ id: "owner-ref", privilegeLevel: "admin" }
 					{ id: "read-only-ref-1", privilegeLevel: "readOnly" }
 					{ id: "read-only-ref-2", privilegeLevel: "readOnly" }
 					{ id: "read-write-ref-1", privilegeLevel: "readAndWrite" }