Refactor to existing `addUserIdToProject` function

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

@@ -1,9 +1,8 @@
-Project = require("../../models/Project").Project
 ProjectInvite = require("../../models/ProjectInvite").ProjectInvite
 mimelib = require("mimelib")
 logger = require('logger-sharelatex')
-ContactManager = require "../Contacts/ContactManager"
 CollaboratorsEmailHandler = require "./CollaboratorsEmailHandler"
+CollaboratorsHandler = require "./CollaboratorsHandler"
 Async = require "async"
 PrivilegeLevels = require "../Authorization/PrivilegeLevels"
 Errors = require "../Errors/Errors"
@@ -60,59 +59,24 @@ module.exports = CollaboratorsInviteHandler =
 			callback(null, invite)
 
 	acceptInvite: (projectId, inviteId, tokenString, user, callback=(err)->) ->
-		# fetch the target project
-		Project.findOne {_id: projectId}, (err, project) ->
+		logger.log {projectId, inviteId, userId: user._id}, "accepting invite"
+		CollaboratorsInviteHandler.getInviteByToken projectId, tokenString, (err, invite) ->
 			if err?
-				logger.err {err, projectId}, "error finding project"
+				logger.err {err, projectId, inviteId}, "error finding invite"
 				return callback(err)
-			if !project
-				err = new Errors.NotFoundError("no project found for invite")
-				logger.log {err, projectId, inviteId}, "no project found"
+			if !invite
+				err = new Errors.NotFoundError("no matching invite found")
+				logger.log {err, projectId, inviteId, tokenString}, "no matching invite found"
 				return callback(err)
-			# fetch the invite
-			ProjectInvite.findOne {_id: inviteId, projectId: projectId, token: tokenString}, (err, invite) ->
+			inviteId = invite._id
+			CollaboratorsHandler.addUserIdToProject projectId, invite.sendingUserId, user._id, invite.privileges, (err) ->
 				if err?
-					logger.err {err, projectId, inviteId}, "error finding invite"
+					logger.err {err, projectId, inviteId, userId: user._id}, "error adding user to project"
 					return callback(err)
-				if !invite
-					err = new Errors.NotFoundError("no matching invite found")
-					logger.log {err, projectId, inviteId, tokenString}, "no matching invite found"
-					return callback(err)
-
-				# check if user is already a member of this project,
-				# return early if so
-				existing_users = (project.collaberator_refs or [])
-				existing_users = existing_users.concat(project.readOnly_refs or [])
-				existing_users = existing_users.map (u) -> u.toString()
-				if existing_users.indexOf(user._id.toString()) > -1
-					logger.log {projectId, userId: user._id}, "user already member of project, returning"
-					return callback null
-
-				# build an update to be applied with $addToSet, user is added to either
-				# `collaberator_refs` or `readOnly_refs`
-				privilegeLevel = invite.privileges
-				if privilegeLevel == PrivilegeLevels.READ_AND_WRITE
-					level = {"collaberator_refs": user._id}
-					logger.log {privileges: privilegeLevel, user_id: user._id, projectId}, "adding user with read-write access"
-				else if privilegeLevel == PrivilegeLevels.READ_ONLY
-					level = {"readOnly_refs": user._id}
-					logger.log {privileges: privilegeLevel, user_id: user._id, projectId}, "adding user with read-only access"
-				else
-					return callback(new Error("unknown privilegeLevel: #{privilegeLevel}"))
-
-				ContactManager.addContact invite.sendingUserId, user._id
-
-				# Update the project, adding the new member.
-				Project.update { _id: project._id }, { $addToSet: level }, (error) ->
-					return callback(error) if error?
-					# Flush to TPDS in background to add files to collaborator's Dropbox
-					ProjectEntityHandler = require("../Project/ProjectEntityHandler")
-					ProjectEntityHandler.flushProjectToThirdPartyDataStore project._id, (error) ->
-						if error?
-							logger.error {err: error, project_id: project._id, user_id}, "error flushing to TPDS after adding collaborator"
-					# Remove invite
-					ProjectInvite.remove {_id: inviteId}, (err) ->
-						if err?
-							logger.err {err, projectId, inviteId}, "error removing invite"
-							return callback(err)
-						callback()
+				# Remove invite
+				logger.log {projectId, inviteId}, "removing invite"
+				ProjectInvite.remove {_id: inviteId}, (err) ->
+					if err?
+						logger.err {err, projectId, inviteId}, "error removing invite"
+						return callback(err)
+					callback()

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

@@ -20,20 +20,13 @@ describe "CollaboratorsInviteHandler", ->
 			@findOne: sinon.stub()
 			@find: sinon.stub()
 			@remove: sinon.stub()
-		@Project = class Project
-			constructor: () ->
-				this
-			@findOne: sinon.stub()
-			@update: sinon.stub()
 		@Crypto = Crypto
 		@CollaboratorsInviteHandler = SandboxedModule.require modulePath, requires:
 			'settings-sharelatex': @settings = {}
 			'logger-sharelatex': @logger = {err: sinon.stub(), error: sinon.stub(), log: sinon.stub()}
 			'./CollaboratorsEmailHandler': @CollaboratorsEmailHandler = {}
-			'../Contacts/ContactManager': @ContactManager = {}
-			'../../models/Project': {Project: @Project}
+			"./CollaboratorsHandler": @CollaboratorsHandler = {addUserIdToProject: sinon.stub()}
 			'../../models/ProjectInvite': {ProjectInvite: @ProjectInvite}
-			"../Project/ProjectEntityHandler": @ProjectEntityHandler = {}
 			'crypto': @Crypto
 
 		@projectId = ObjectId()
@@ -249,15 +242,16 @@ describe "CollaboratorsInviteHandler", ->
 				_id: @projectId
 				collaberator_refs: []
 				readOnly_refs: []
-			@Project.findOne.callsArgWith(1, null, @fakeProject)
-			@ProjectInvite.findOne.callsArgWith(1, null, @fakeInvite)
-			@ContactManager.addContact = sinon.stub()
-			@Project.update.callsArgWith(2, null)
-			@ProjectEntityHandler.flushProjectToThirdPartyDataStore = sinon.stub().callsArgWith(1, null)
+			@CollaboratorsHandler.addUserIdToProject.callsArgWith(4, null)
+			@_getInviteByToken = sinon.stub(@CollaboratorsInviteHandler, 'getInviteByToken')
+			@_getInviteByToken.callsArgWith(2, null, @fakeInvite)
 			@ProjectInvite.remove.callsArgWith(1, null)
 			@call = (callback) =>
 				@CollaboratorsInviteHandler.acceptInvite @projectId, @inviteId, @token, @user, callback
 
+		afterEach ->
+			@_getInviteByToken.restore()
+
 		describe 'when all goes well', ->
 
 			beforeEach ->
@@ -268,34 +262,16 @@ describe "CollaboratorsInviteHandler", ->
 					expect(err).to.be.oneOf [null, undefined]
 					done()
 
-			it 'should have called Project.findOne', (done) ->
-				@call (err) =>
-					@Project.findOne.callCount.should.equal 1
-					@Project.findOne.calledWith({_id: @projectId}).should.equal true
-					done()
-
-			it 'should have called ProjectInvite.findOne', (done) ->
-				@call (err) =>
-					@ProjectInvite.findOne.callCount.should.equal 1
-					@ProjectInvite.findOne.calledWith({_id: @inviteId, projectId: @projectId, token: @token}).should.equal true
-					done()
-
-			it 'should have called ContactManager.addContact', (done) ->
+			it 'should have called getInviteByToken', (done) ->
 				@call (err) =>
-					@ContactManager.addContact.callCount.should.equal 1
-					@ContactManager.addContact.calledWith(@sendingUserId, @userId).should.equal true
+					@_getInviteByToken.callCount.should.equal 1
+					@_getInviteByToken.calledWith(@projectId, @token).should.equal true
 					done()
 
-			it 'should have called Project.update, adding the user to collaberator_refs', (done) ->
+			it 'should have called CollaboratorsHandler.addUserIdToProject', (done) ->
 				@call (err) =>
-					@Project.update.callCount.should.equal 1
-					@Project.update.calledWith({_id: @projectId}, {$addToSet: {"collaberator_refs": @userId}}).should.equal true
-					done()
-
-			it 'should have called ProjectEntityHandler.flushProjectToThirdPartyDataStore', (done) ->
-				@call (err) =>
-					@ProjectEntityHandler.flushProjectToThirdPartyDataStore.callCount.should.equal 1
-					@ProjectEntityHandler.flushProjectToThirdPartyDataStore.calledWith(@projectId).should.equal true
+					@CollaboratorsHandler.addUserIdToProject.callCount.should.equal 1
+					@CollaboratorsHandler.addUserIdToProject.calledWith(@projectId, @sendingUserId, @userId, @fakeInvite.privileges).should.equal true
 					done()
 
 			it 'should have called ProjectInvite.remove', (done) ->
@@ -308,7 +284,7 @@ describe "CollaboratorsInviteHandler", ->
 
 			beforeEach ->
 				@fakeInvite.privileges = 'readOnly'
-				@ProjectInvite.findOne.callsArgWith(1, null, @fakeInvite)
+				@_getInviteByToken.callsArgWith(2, null, @fakeInvite)
 
 			it 'should not produce an error', (done) ->
 				@call (err) =>
@@ -316,60 +292,32 @@ describe "CollaboratorsInviteHandler", ->
 					expect(err).to.be.oneOf [null, undefined]
 					done()
 
-			it 'should have called Project.update, adding the user to readOnly_refs', (done) ->
+			it 'should have called CollaboratorsHandler.addUserIdToProject', (done) ->
 				@call (err) =>
-					@Project.update.callCount.should.equal 1
-					@Project.update.calledWith({_id: @projectId}, {$addToSet: {"readOnly_refs": @userId}}).should.equal true
+					@CollaboratorsHandler.addUserIdToProject.callCount.should.equal 1
+					@CollaboratorsHandler.addUserIdToProject.calledWith(@projectId, @sendingUserId, @userId, @fakeInvite.privileges).should.equal true
 					done()
 
-		describe 'when the invite is for an unknown access level', ->
+		describe 'when getInviteByToken does not find an invite', ->
 
 			beforeEach ->
-				@fakeInvite.privileges = 'some_crazy_permission'
-				@ProjectInvite.findOne.callsArgWith(1, null, @fakeInvite)
+				@_getInviteByToken.callsArgWith(2, null, null)
 
 			it 'should produce an error', (done) ->
 				@call (err) =>
 					expect(err).to.be.instanceof Error
+					expect(err.name).to.equal "NotFoundError"
 					done()
 
-			it 'should not have called Project.update', (done) ->
-				@call (err) =>
-					@Project.update.callCount.should.equal 0
-					done()
-
-			it 'should not have called ProjectInvite.remove', (done) ->
-				@call (err) =>
-					@ProjectInvite.remove.callCount.should.equal 0
-					done()
-
-		describe 'when user is already a member of project', ->
-
-			beforeEach ->
-				@fakeProject.collaberator_refs = [ObjectId(), @user._id, ObjectId(), ObjectId()]
-				@Project.findOne.callsArgWith(1, null, @fakeProject)
-
-			it 'should not produce an error', (done) ->
-				@call (err) =>
-					expect(err).to.not.be.instanceof Error
-					expect(err).to.be.oneOf [null, undefined]
-					done()
-
-			it 'should have called Project.findOne', (done) ->
-				@call (err) =>
-					@Project.findOne.callCount.should.equal 1
-					@Project.findOne.calledWith({_id: @projectId}).should.equal true
-					done()
-
-			it 'should have called ProjectInvite.findOne', (done) ->
+			it 'should have called getInviteByToken', (done) ->
 				@call (err) =>
-					@ProjectInvite.findOne.callCount.should.equal 1
-					@ProjectInvite.findOne.calledWith({_id: @inviteId, projectId: @projectId, token: @token}).should.equal true
+					@_getInviteByToken.callCount.should.equal 1
+					@_getInviteByToken.calledWith(@projectId, @token).should.equal true
 					done()
 
-			it 'should have returned early, not have called Project.update', (done) ->
+			it 'should not have called CollaboratorsHandler.addUserIdToProject', (done) ->
 				@call (err) =>
-					@Project.update.callCount.should.equal 0
+					@CollaboratorsHandler.addUserIdToProject.callCount.should.equal 0
 					done()
 
 			it 'should not have called ProjectInvite.remove', (done) ->
@@ -377,41 +325,25 @@ describe "CollaboratorsInviteHandler", ->
 					@ProjectInvite.remove.callCount.should.equal 0
 					done()
 
-
-		describe 'when ProjectInvite.findOne does not find an invite', ->
+		describe 'when getInviteByToken produces an error', ->
 
 			beforeEach ->
-				@ProjectInvite.findOne.callsArgWith(1, null, null)
+				@_getInviteByToken.callsArgWith(2, new Error('woops'))
 
 			it 'should produce an error', (done) ->
 				@call (err) =>
 					expect(err).to.be.instanceof Error
-					expect(err.name).to.equal "NotFoundError"
-					done()
-
-			it 'should not have called Project.update', (done) ->
-				@call (err) =>
-					@Project.update.callCount.should.equal 0
-					done()
-
-			it 'should not have called ProjectInvite.remove', (done) ->
-				@call (err) =>
-					@ProjectInvite.remove.callCount.should.equal 0
 					done()
 
-		describe 'when Project.findOne produces an error', ->
-
-			beforeEach ->
-				@Project.findOne.callsArgWith(1, new Error('woops'))
-
-			it 'should produce an error', (done) ->
+			it 'should have called getInviteByToken', (done) ->
 				@call (err) =>
-					expect(err).to.be.instanceof Error
+					@_getInviteByToken.callCount.should.equal 1
+					@_getInviteByToken.calledWith(@projectId, @token).should.equal true
 					done()
 
-			it 'should not have called Project.update', (done) ->
+			it 'should not have called CollaboratorsHandler.addUserIdToProject', (done) ->
 				@call (err) =>
-					@Project.update.callCount.should.equal 0
+					@CollaboratorsHandler.addUserIdToProject.callCount.should.equal 0
 					done()
 
 			it 'should not have called ProjectInvite.remove', (done) ->
@@ -419,39 +351,26 @@ describe "CollaboratorsInviteHandler", ->
 					@ProjectInvite.remove.callCount.should.equal 0
 					done()
 
-		describe 'when ProjectInvite.findOne produces an error', ->
+		describe 'when addUserIdToProject produces an error', ->
 
 			beforeEach ->
-				@ProjectInvite.findOne.callsArgWith(1, new Error('woops'))
+				@CollaboratorsHandler.addUserIdToProject.callsArgWith(4, new Error('woops'))
 
 			it 'should produce an error', (done) ->
 				@call (err) =>
 					expect(err).to.be.instanceof Error
 					done()
 
-			it 'should not have called Project.update', (done) ->
+			it 'should have called getInviteByToken', (done) ->
 				@call (err) =>
-					@Project.update.callCount.should.equal 0
+					@_getInviteByToken.callCount.should.equal 1
+					@_getInviteByToken.calledWith(@projectId, @token).should.equal true
 					done()
 
-			it 'should not have called ProjectInvite.remove', (done) ->
+			it 'should have called CollaboratorsHandler.addUserIdToProject', (done) ->
 				@call (err) =>
-					@ProjectInvite.remove.callCount.should.equal 0
-					done()
-
-		describe 'when Project.update produces an error', ->
-
-			beforeEach ->
-				@Project.update.callsArgWith(2, new Error('woops'))
-
-			it 'should produce an error', (done) ->
-				@call (err) =>
-					expect(err).to.be.instanceof Error
-					done()
-
-			it 'should have called Project.update', (done) ->
-				@call (err) =>
-					@Project.update.callCount.should.equal 1
+					@CollaboratorsHandler.addUserIdToProject.callCount.should.equal 1
+					@CollaboratorsHandler.addUserIdToProject.calledWith(@projectId, @sendingUserId, @userId, @fakeInvite.privileges).should.equal true
 					done()
 
 			it 'should not have called ProjectInvite.remove', (done) ->
@@ -469,9 +388,16 @@ describe "CollaboratorsInviteHandler", ->
 					expect(err).to.be.instanceof Error
 					done()
 
-			it 'should have called Project.update', (done) ->
+			it 'should have called getInviteByToken', (done) ->
+				@call (err) =>
+					@_getInviteByToken.callCount.should.equal 1
+					@_getInviteByToken.calledWith(@projectId, @token).should.equal true
+					done()
+
+			it 'should have called CollaboratorsHandler.addUserIdToProject', (done) ->
 				@call (err) =>
-					@Project.update.callCount.should.equal 1
+					@CollaboratorsHandler.addUserIdToProject.callCount.should.equal 1
+					@CollaboratorsHandler.addUserIdToProject.calledWith(@projectId, @sendingUserId, @userId, @fakeInvite.privileges).should.equal true
 					done()
 
 			it 'should have called ProjectInvite.remove', (done) ->