Moved register function into user registration handler and new user

controller

services/web/app/coffee/Features/User/UserController.coffee

@@ -3,9 +3,11 @@ UserLocator = require("./UserLocator")
 User = require("../../models/User").User
 newsLetterManager = require('../Newsletter/NewsletterManager')
 sanitize = require('sanitizer')
+UserRegistrationHandler = require("./UserRegistrationHandler")
 logger = require("logger-sharelatex")
 metrics = require("../../infrastructure/Metrics")
-
+Url = require("url")
+AuthenticationController = require("../Authentication/AuthenticationController")
 module.exports =
 
 	deleteUser: (req, res)->
@@ -45,4 +47,25 @@ module.exports =
 				logger.err err: err, 'error destorying session'
 			res.redirect '/login'
 
+	register : (req, res, next = (error) ->)->
+		logger.log email: req.body.email, "attempted register"
+		redir = Url.parse(req.body.redir or "/project").path
+		UserRegistrationHandler.registerNewUser req.body, (err, user)->
+			console.log err
+			if err == "EmailAlreadyRegisterd"
+				return AuthenticationController.login req, res
+			else if err?
+				next(err)
+			else
+				metrics.inc "user.register.success"
+				req.session.user = user
+				req.session.justRegistered = true
+				res.send
+					redir:redir
+					id:user._id.toString()
+					first_name: user.first_name
+					last_name: user.last_name
+					email: user.email
+					created: Date.now()
+
 

services/web/app/coffee/Features/User/UserCreator.coffee

@@ -15,5 +15,14 @@ module.exports =
 		user = new User()
 		user.email = opts.email
 		user.holdingAccount = opts.holdingAccount
+
+		username = opts.email.match(/^[^@]*/)
+		if username?
+			user.first_name = username[0]
+		else
+			user.first_name = ""
+		user.last_name = ""
+
+
 		user.save (err)->
 			callback(err, user)

services/web/app/coffee/Features/User/UserRegistrationHandler.coffee

@@ -1,4 +1,11 @@
 sanitize = require('sanitizer')
+User = require("../../models/User").User
+UserCreator = require("./UserCreator")
+AuthenticationManager = require("../Authentication/AuthenticationManager")
+NewsLetterManager = require("../NewsLetter/NewsLetterManager")
+async = require("async")
+EmailHandler = require("../Email/EmailHandler")
+logger = require("logger-sharelatex")
 
 module.exports =
 	validateEmail : (email) ->
@@ -12,25 +19,50 @@ module.exports =
 				hasZeroLength = true
 		return hasZeroLength
 
-	validateRegisterRequest : (req, callback)->
-		email = sanitize.escape(req.body.email).trim().toLowerCase()
-		password = req.body.password
+	_registrationRequestIsValid : (body, callback)->
+		email = sanitize.escape(body.email).trim().toLowerCase()
+		password = body.password
 		username = email.match(/^[^@]*/)
-		if username?
-			first_name = username[0]
-		else
-			first_name = ""
-		last_name = ""
-
 		if @hasZeroLengths([password, email])
-			callback('please fill in all the fields', null)
+			return false
 		else if !@validateEmail(email)
-			callback('not valid email', null)
+			return false
 		else
-			callback(null, {first_name:first_name, last_name:last_name, email:email, password:password})
-
+			return true
 
+	_createNewUserIfRequired: (user, userDetails, callback)->
+		if !user?
+			UserCreator.createNewUser {holdingAccount:false, email:userDetails.email}, callback
+		else
+			callback null, user
 
 	registerNewUser: (userDetails, callback)->
+		self = @
+		requestIsValid = @_registrationRequestIsValid userDetails
+		if !requestIsValid
+			return callback("request is not valid")
+		userDetails.email = userDetails.email?.trim()?.toLowerCase()
+		User.findOne email:userDetails.email, (err, user)->
+			if err?
+				return callback err
+			if user?.holdingAccount == false
+				return callback("EmailAlreadyRegisterd")
+			self._createNewUserIfRequired user, userDetails, (err, user)->
+				if err?
+					return callback(err)
+				async.series [
+					(cb)-> User.update {_id: user._id}, {"$set":{holdingAccount:false}}, cb
+					(cb)-> AuthenticationManager.setUserPassword user._id, userDetails.password, cb
+					(cb)-> NewsLetterManager.subscribe user, cb
+					(cb)-> 
+						emailOpts =
+							first_name:user.first_name
+							to: user.email
+						EmailHandler.sendEmail "welcome", emailOpts, cb
+				], (err)->
+					logger.log user: user, "registered"
+					callback(err, user)
+
+
+
 
-		callback()

services/web/app/coffee/controllers/UserController.coffee

@@ -19,53 +19,8 @@ uuid = require("node-uuid")
 
 module.exports =
 
-	apiRegister : (req, res, next = (error) ->)->
-		logger.log email: req.body.email, "attempted register"
-		redir = Url.parse(req.body.redir or "/project").path
-		userRegistrationHandler.validateRegisterRequest req, (err, data)->
-			if err?
-				logger.log validation_error: err, "user validation error"
-				metrics.inc "user.register.validation-error"
-				res.send message:
-					 text:err
-					 type:'error'
-			else
-				User.findOne {email:data.email}, (err, foundUser)->
-					if foundUser? && foundUser.holdingAccount == false
-						AuthenticationController.login req, res
-						logger.log email: data.email, "email already registered"
-						metrics.inc "user.register.already-registered"
-						return AuthenticationController.login req, res
-					else if foundUser? && foundUser.holdingAccount == true #someone put them in as a collaberator
-						user = foundUser
-						user.holdingAccount = false
-					else
-						user = new User email: data.email
-					d = new Date()
-					user.first_name = data.first_name
-					user.last_name = data.last_name
-					user.signUpDate = new Date()
-					metrics.inc "user.register.success"
-					user.save (err)->
-						req.session.user = user
-						req.session.justRegistered = true
-						logger.log user: user, "registered"
-						AuthenticationManager.setUserPassword user._id, data.password, (error) ->
-							return next(error) if error?
-							res.send
-								redir:redir
-								id:user._id.toString()
-								first_name: user.first_name
-								last_name: user.last_name
-								email: user.email
-								created: Date.now()
-					#things that can be fired and forgot.
-					newsLetterManager.subscribe user
-					ReferalAllocator.allocate req.session.referal_id, user._id, req.session.referal_source, req.session.referal_medium
-					emailOpts =
-						first_name:user.first_name
-						to: user.email
-					EmailHandler.sendEmail "welcome", emailOpts
+	
+				
 
 
 

services/web/app/coffee/router.coffee

@@ -67,7 +67,7 @@ module.exports = class Router
 		app.get '/dropbox', InfoController.dropbox
 
 		app.get  '/register', UserPagesController.registerPage
-		app.post '/register', UserController.apiRegister
+		app.post '/register', UserController_new.register
 
 		SubscriptionRouter.apply(app)
 		UploadsRouter.apply(app)

services/web/test/UnitTests/coffee/User/UserControllerTests.coffee

@@ -8,6 +8,7 @@ events = require "events"
 MockResponse = require "../helpers/MockResponse"
 MockRequest = require "../helpers/MockRequest"
 ObjectId = require("mongojs").ObjectId
+assert = require("assert")
 
 describe "UserController", ->
 	beforeEach ->
@@ -24,11 +25,17 @@ describe "UserController", ->
 			findById: sinon.stub().callsArgWith(1, null, @user)
 		@NewsLetterManager =
 			unsubscribe: sinon.stub().callsArgWith(1)
+		@UserRegistrationHandler =
+			registerNewUser: sinon.stub()
+		@AuthenticationController = {}
+
 		@UserController = SandboxedModule.require modulePath, requires:
 			"./UserLocator": @UserLocator
 			"./UserDeleter": @UserDeleter
 			"../../models/User": User:@User
 			'../Newsletter/NewsletterManager':@NewsLetterManager
+			"./UserRegistrationHandler":@UserRegistrationHandler
+			"../Authentication/AuthenticationController": @AuthenticationController
 			"logger-sharelatex": {log:->}
 
 
@@ -97,3 +104,44 @@ describe "UserController", ->
 			@UserController.logout @req, @res
 
 
+	describe "register", ->
+
+		it "should ask the UserRegistrationHandler to register user", (done)->
+			@UserRegistrationHandler.registerNewUser.callsArgWith(1, null, @user)
+			@res.send = =>
+				@UserRegistrationHandler.registerNewUser.calledWith(@req.body).should.equal true
+				done()
+			@UserController.register @req, @res
+
+		it "should try and log the user in if there is an EmailAlreadyRegisterd error", (done)->
+
+			@UserRegistrationHandler.registerNewUser.callsArgWith(1, message:"EmailAlreadyRegisterd")
+			@AuthenticationController.login = (req, res)=>
+				assert.deepEqual req, @req
+				assert.deepEqual res, @res
+				done()
+			@UserController.register @req, @res
+
+		it "should put the user on the session and mark them as justRegistered", (done)->
+			@UserRegistrationHandler.registerNewUser.callsArgWith(1, null, @user)
+			@res.send = =>
+				assert.deepEqual @user, @req.session.user
+				assert.equal @req.session.justRegistered, true
+				done()
+			@UserController.register @req, @res
+
+		it "should redirect to project page", (done)->
+			@UserRegistrationHandler.registerNewUser.callsArgWith(1, null, @user)
+			@res.send = (opts)=>
+				opts.redir.should.equal "/project"
+				done()
+			@UserController.register @req, @res			
+
+
+		it "should redirect passed redir if it exists", (done)->
+			@UserRegistrationHandler.registerNewUser.callsArgWith(1, null, @user)
+			@req.body.redir = "/somewhere"
+			@res.send = (opts)=>
+				opts.redir.should.equal "/somewhere"
+				done()
+			@UserController.register @req, @res		

services/web/test/UnitTests/coffee/User/UserCreatorTests.coffee

@@ -50,7 +50,7 @@ describe "UserCreator", ->
 			@UserCreator.createNewUser opts, (err, user)=>
 				assert.equal user.email, @email
 				assert.equal user.holdingAccount, true
-
+				assert.equal user.first_name, "bob.oswald"
 				done()
 
 

services/web/test/UnitTests/coffee/User/UserRegistrationHandlerTests.coffee

@@ -3,68 +3,135 @@ SandboxedModule = require('sandboxed-module')
 assert = require('assert')
 path = require('path')
 modulePath = path.join __dirname, '../../../../app/js/Features/User/UserRegistrationHandler'
+sinon = require("sinon")
+expect = require("chai").expect
 
 describe "UserRegistrationHandler", ->
-	
+
 	beforeEach ->
-		@handler = SandboxedModule.require modulePath, requires:{}
-		@passingRequest = {body:{email:"something@email.com", password:"123"}}
+		@user =
+			_id: "31j2lk21kjl"
+		@User = 
+			findOne:sinon.stub()
+			update: sinon.stub().callsArgWith(2)
+		@UserCreator = 
+			createNewUser:sinon.stub().callsArgWith(1, null, @user)
+		@AuthenticationManager =
+			setUserPassword: sinon.stub().callsArgWith(2)
+		@NewsLetterManager =
+			subscribe: sinon.stub().callsArgWith(1)
+		@EmailHandler =
+			sendEmail:sinon.stub().callsArgWith(2)
+		@handler = SandboxedModule.require modulePath, requires:
+			"../../models/User": {User:@User}
+			"./UserCreator": @UserCreator
+			"../Authentication/AuthenticationManager":@AuthenticationManager
+			"../NewsLetter/NewsLetterManager":@NewsLetterManager
+			"../Email/EmailHandler": @EmailHandler
+
+		@passingRequest = {email:"something@email.com", password:"123"}
 
 
 	describe 'validate Register Request', ->
 
 
 		it 'allow working account through', ->
-			@handler.validateRegisterRequest @passingRequest, (err, data)=>
-				should.not.exist(err)
-				data.first_name.should.equal "something"
-				data.last_name.should.equal ""
-				data.email.should.equal @passingRequest.body.email
-				data.password.should.equal @passingRequest.body.password
-		
-		it 'not allow not valid email through ', (done)->
-			@passingRequest.body.email = "notemail"
-			@handler.validateRegisterRequest @passingRequest, (err, data)->
-				should.exist(err)
-				err.should.equal "not valid email"
-				done()
+			result = @handler._registrationRequestIsValid @passingRequest
+			result.should.equal true
 		
+		it 'not allow not valid email through ', ()->
+			@passingRequest.email = "notemail"
+			result = @handler._registrationRequestIsValid @passingRequest
+			result.should.equal false
+
 		it 'not allow no email through ', ->
-			@passingRequest.body.email = ""
-			@handler.validateRegisterRequest @passingRequest, (err, data)->
-				should.exist(err)
-				err.should.equal "please fill in all the fields"
+			@passingRequest.email = ""
+			result = @handler._registrationRequestIsValid @passingRequest
+			result.should.equal false
 		
-		it 'not allow no password through ', (done)->
-			@passingRequest.body.password= ""
-			@handler.validateRegisterRequest @passingRequest, (err, data)->
-				should.exist(err)
-				err.should.equal "please fill in all the fields"
-				done()
-
-		it 'trim white space from email', (done)->
-			@passingRequest.body.email = " some@email.com "
-			@handler.validateRegisterRequest @passingRequest, (err, data)->
-				should.not.exist(err)
-				data.email.should.equal "some@email.com"
-				done()
-
-		it 'lower case email', (done)->
-			@passingRequest.body.email = "soMe@eMail.cOm"
-			@handler.validateRegisterRequest @passingRequest, (err, data)->
-				should.not.exist(err)
-				data.email.should.equal "some@email.com"
-				done()
-
-		it 'should allow a short registeration request through', (done) ->
-			@handler.validateRegisterRequest body: {
-				email: "user_name@example.com"
-				password: @passingRequest.body.password
-			}, (err, data) =>
-				should.not.exist(err)
-				data.email.should.equal "user_name@example.com"
-				data.password.should.equal @passingRequest.body.password
-				data.first_name.should.equal "user_name"
-				done()
+		it 'not allow no password through ', ()->
+			@passingRequest.password= ""
+			result = @handler._registrationRequestIsValid @passingRequest
+			result.should.equal false
+
+
+
+	describe "registerNewUser", ->
+
+		describe "holdingAccount", (done)->
+
+			beforeEach ->
+				@user.holdingAccount = true
+				@handler._registrationRequestIsValid = sinon.stub().returns true
+				@User.findOne.callsArgWith(1, null, @user)
+
+			it "should not create a new user if there is a holding account there", (done)->
+				@handler.registerNewUser @passingRequest, (err)=>
+					@UserCreator.createNewUser.called.should.equal false
+					done()
+
+			it "should set holding account to false", (done)->
+				@handler.registerNewUser @passingRequest, (err)=>
+					update = @User.update.args[0]
+					assert.deepEqual update[0], {_id:@user._id}
+					assert.deepEqual update[1], {"$set":{holdingAccount:false}}
+					done()
+
+		describe "invalidRequest", ->
+
+			it "should not create a new user if the the request is not valid", (done)->
+				@handler._registrationRequestIsValid = sinon.stub().returns false
+				@handler.registerNewUser @passingRequest, (err)=>
+					expect(err).to.exist
+					@UserCreator.createNewUser.called.should.equal false
+					done()
+
+			it "should return email registered in the error if there is a non holdingAccount there", (done)->
+				@User.findOne.callsArgWith(1, null, {holdingAccount:false})
+				@handler.registerNewUser @passingRequest, (err)=>
+					err.should.equal "EmailAlreadyRegisterd"
+					done()
+
+		describe "validRequest", ->
+			beforeEach ->
+				@handler._registrationRequestIsValid = sinon.stub().returns true
+				@User.findOne.callsArgWith 1
+
+			it "should create a new user", (done)->
+				@handler.registerNewUser @passingRequest, (err)=>
+					@UserCreator.createNewUser.calledWith({email:@passingRequest.email, holdingAccount:false}).should.equal true
+					done()
+
+			it 'lower case email', (done)->
+				@passingRequest.email = "soMe@eMail.cOm"
+				@handler.registerNewUser @passingRequest, (err)=>
+					@UserCreator.createNewUser.calledWith({email:@passingRequest.email.toLowerCase(), holdingAccount:false}).should.equal true
+					done()
+
+			it 'trim white space from email', (done)->
+				@passingRequest.email = " some@email.com "
+				@handler.registerNewUser @passingRequest, (err)=>
+					@UserCreator.createNewUser.calledWith({email:"some@email.com", holdingAccount:false}).should.equal true
+					done()
+
+
+			it "should set the password", (done)->
+				@handler.registerNewUser @passingRequest, (err)=>
+					@AuthenticationManager.setUserPassword.calledWith(@user._id, @passingRequest.password).should.equal true
+					done()			
+
+			it "should add the user to the news letter manager", (done)->
+				@handler.registerNewUser @passingRequest, (err)=>
+					@NewsLetterManager.subscribe.calledWith(@user).should.equal true
+					done()
+
+			it "should send a welcome email", (done)->
+				@handler.registerNewUser @passingRequest, (err)=>
+					@EmailHandler.sendEmail.calledWith("welcome").should.equal true
+					done()
+
+
+		it "should call the ReferalAllocator", (done)->
+			done()