Use Error classes, and ensure invited_emails is cleared on new invite

services/web/app/coffee/Features/Errors/Errors.coffee

@@ -56,7 +56,7 @@ V1HistoryNotSyncedError.prototype.__proto___ = Error.prototype
 
 ProjectHistoryDisabledError = (message) ->
 	error = new Error(message)
-	error.name = "ProjectHistoryDisabledError "
+	error.name = "ProjectHistoryDisabledError"
 	error.__proto__ = ProjectHistoryDisabledError.prototype
 	return error
 ProjectHistoryDisabledError.prototype.__proto___ = Error.prototype

services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee

@@ -11,7 +11,7 @@ module.exports =
 		email   = req.body.email
 
 		TeamInvitesHandler.createInvite teamManagerId, email, (err, invite) ->
-			return handleError(err, req, res, next) if err?
+			return next(err) if err?
 			inviteView = { user:
 				{ email: invite.email, sentAt: invite.sentAt, invite: true }
 			}
@@ -22,13 +22,13 @@ module.exports =
 		userId = AuthenticationController.getLoggedInUserId(req)
 
 		TeamInvitesHandler.getInvite token, (err, invite, teamSubscription) ->
-			return handleError(err, req, res, next) if err?
+			return next(err) if err?
 
 			if !invite
 				return ErrorController.notFound(req, res, next)
 
 			SubscriptionLocator.getUsersSubscription userId, (err, personalSubscription) ->
-				return handleError(err, req, res, next) if err?
+				return next(err) if err?
 
 				res.render "subscriptions/team/invite",
 					inviterName: invite.inviterName
@@ -41,8 +41,7 @@ module.exports =
 		userId = AuthenticationController.getLoggedInUserId(req)
 
 		TeamInvitesHandler.acceptInvite token, userId, (err, results) ->
-			return handleError(err, req, res, next) if err?
-
+			return next(err) if err?
 			res.sendStatus 204
 
 	revokeInvite: (req, res) ->
@@ -50,12 +49,5 @@ module.exports =
 		teamManagerId = AuthenticationController.getLoggedInUserId(req)
 
 		TeamInvitesHandler.revokeInvite teamManagerId, email, (err, results) ->
-			return handleError(err, req, res, next) if err?
-
+			return next(err) if err?
 			res.sendStatus 204
-
-handleError = (err, req, res, next) ->
-	if err.teamNotFound or err.inviteNoLongerValid
-		ErrorController.notFound(req, res, next)
-	else
-		next(err)

services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee

@@ -14,12 +14,15 @@ SubscriptionUpdater = require("./SubscriptionUpdater")
 LimitationsManager = require("./LimitationsManager")
 
 EmailHandler = require("../Email/EmailHandler")
+EmailHelper = require("../Helpers/EmailHelper")
+
+Errors = require "../Errors/Errors"
 
 module.exports = TeamInvitesHandler =
 	getInvite: (token, callback) ->
 		Subscription.findOne 'teamInvites.token': token, (err, subscription) ->
 			return callback(err) if err?
-			return callback(teamNotFound: true) unless subscription?
+			return callback(new Errors.NotFoundError('team not found')) unless subscription?
 
 			invite = subscription.teamInvites.find (i) -> i.token == token
 			return callback(null, invite, subscription)
@@ -37,7 +40,9 @@ module.exports = TeamInvitesHandler =
 				else
 					inviterName = teamManager.email
 
-				createInvite(subscription, email, inviterName, callback)
+				removeLegacyInvite subscription.id, email, (error) ->
+					return callback(error) if error?
+					createInvite(subscription, email, inviterName, callback)
 
 	createDomainInvite: (user, licence, callback) ->
 		logger.log {licence, email: user.email}, "Creating domain team invite"
@@ -51,7 +56,7 @@ module.exports = TeamInvitesHandler =
 		logger.log {userId}, "Accepting invite"
 		TeamInvitesHandler.getInvite token, (err, invite, subscription) ->
 			return callback(err) if err?
-			return callback(inviteNoLongerValid: true) unless invite?
+			return callback(new Errors.NotFoundError('invite not found')) unless invite?
 
 			SubscriptionUpdater.addUserToGroup subscription.admin_id, userId, (err) ->
 				return callback(err) if err?
@@ -82,8 +87,7 @@ createInvite = (subscription, email, inviterName, callback) ->
 		return callback(error) if error?
 		return callback(reason) unless possible
 
-		# TODO: use standard way to canonalise email addresses
-		email = email.trim().toLowerCase()
+		email = EmailHelper.parseEmail(email)
 
 		invite = subscription.teamInvites.find (invite) -> invite.email == email
 
@@ -110,18 +114,24 @@ createInvite = (subscription, email, inviterName, callback) ->
 				return callback(error, invite)
 
 removeInviteFromTeam = (subscriptionId, email, callback) ->
-	email = email.trim().toLowerCase()
-
+	email = EmailHelper.parseEmail(email)
 	searchConditions = { _id: new ObjectId(subscriptionId.toString()) }
-
 	removeInvite = { $pull: { teamInvites: { email: email } } }
-	removeLegacyInvite = { $pull: { invited_emails: email } }
+	logger.log {subscriptionId, email, searchConditions, removeInvite}, 'removeInviteFromTeam'
 
 	async.series [
 		(cb) -> Subscription.update(searchConditions, removeInvite, cb),
-		(cb) -> Subscription.update(searchConditions, removeLegacyInvite, cb),
+		(cb) -> removeLegacyInvite(subscriptionId, email, cb),
 	], callback
 
+removeLegacyInvite = (subscriptionId, email, callback) ->
+	Subscription.update({
+		_id: new ObjectId(subscriptionId.toString())
+	}, {
+		$pull: {
+			invited_emails: EmailHelper.parseEmail(email)
+		}
+	}, callback)
 
 checkIfInviteIsPossible = (subscription, email, callback = (error, possible, reason) -> ) ->
 	unless subscription.groupPlan

services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee

@@ -6,6 +6,7 @@ querystring = require 'querystring'
 modulePath = "../../../../app/js/Features/Subscription/TeamInvitesHandler"
 
 ObjectId = require("mongojs").ObjectId
+Errors = require("../../../../app/js/Features/Errors/Errors")
 
 describe "TeamInvitesHandler", ->
 	beforeEach ->
@@ -13,13 +14,13 @@ describe "TeamInvitesHandler", ->
 			id: "666666",
 			first_name: "Daenerys"
 			last_name: "Targaryen"
-			email: "daenerys@motherofdragons.com"
+			email: "daenerys@example.com"
 		}
 
 		@token = "aaaaaaaaaaaaaaaaaaaaaa"
 
 		@teamInvite = {
-			email: "jorah@mormont.org",
+			email: "jorah@example.com",
 			token: @token,
 		}
 
@@ -83,6 +84,7 @@ describe "TeamInvitesHandler", ->
 				"./SubscriptionUpdater": @SubscriptionUpdater
 				"./LimitationsManager": @LimitationsManager
 				"../Email/EmailHandler": @EmailHandler
+				"../Errors/Errors": Errors
 
 	describe "getInvite", ->
 		it "returns the invite if there's one", (done) ->
@@ -96,25 +98,25 @@ describe "TeamInvitesHandler", ->
 			@Subscription.findOne = sinon.stub().yields(null, null)
 
 			@TeamInvitesHandler.getInvite @token, (err, invite, subscription) ->
-				expect(err).to.deep.eq(teamNotFound: true)
+				expect(err).to.be.instanceof(Errors.NotFoundError)
 				done()
 
 	describe "createInvite", ->
 		it "adds the team invite to the subscription", (done) ->
-			@TeamInvitesHandler.createInvite @manager.id, "John.Snow@nightwatch.com", (err, invite) =>
+			@TeamInvitesHandler.createInvite @manager.id, "John.Snow@example.com", (err, invite) =>
 				expect(err).to.eq(null)
 				expect(invite.token).to.eq(@newToken)
-				expect(invite.email).to.eq("john.snow@nightwatch.com")
-				expect(invite.inviterName).to.eq("Daenerys Targaryen (daenerys@motherofdragons.com)")
+				expect(invite.email).to.eq("john.snow@example.com")
+				expect(invite.inviterName).to.eq("Daenerys Targaryen (daenerys@example.com)")
 				expect(@subscription.teamInvites).to.deep.include(invite)
 				done()
 
 		it "sends an email", (done) ->
-			@TeamInvitesHandler.createInvite @manager.id, "John.Snow@nightwatch.com", (err, invite) =>
+			@TeamInvitesHandler.createInvite @manager.id, "John.Snow@example.com", (err, invite) =>
 				@EmailHandler.sendEmail.calledWith("verifyEmailToJoinTeam",
 					sinon.match({
-						to: "john.snow@nightwatch.com",
-						inviterName: "Daenerys Targaryen (daenerys@motherofdragons.com)",
+						to: "john.snow@example.com",
+						inviterName: "Daenerys Targaryen (daenerys@example.com)",
 						acceptInviteUrl: "http://example.com/subscription/invites/#{@newToken}/"
 					})
 				).should.equal true
@@ -136,6 +138,14 @@ describe "TeamInvitesHandler", ->
 
 				done()
 
+		it "removes any legacy invite from the subscription", (done) ->
+			@TeamInvitesHandler.createInvite @manager.id, "John.Snow@example.com", (err, invite) =>
+				@Subscription.update.calledWith(
+					{ _id: new ObjectId("55153a8014829a865bbf700d") },
+					{ '$pull': { invited_emails: "john.snow@example.com" } }
+				).should.eq true
+				done()
+
 	describe "createDomainInvite", ->
 		beforeEach ->
 			@licence =
@@ -143,13 +153,13 @@ describe "TeamInvitesHandler", ->
 				name: "Team Daenerys"
 
 			@user =
-				email: "John.Snow@nightwatch.com"
+				email: "John.Snow@example.com"
 
 		it "adds the team invite to the subscription", (done) ->
 			@TeamInvitesHandler.createDomainInvite @user, @licence, (err, invite) =>
 				expect(err).to.eq(null)
 				expect(invite.token).to.eq(@newToken)
-				expect(invite.email).to.eq("john.snow@nightwatch.com")
+				expect(invite.email).to.eq("john.snow@example.com")
 				expect(invite.inviterName).to.eq("Team Daenerys")
 				expect(@subscription.teamInvites).to.deep.include(invite)
 				done()
@@ -158,7 +168,7 @@ describe "TeamInvitesHandler", ->
 			@TeamInvitesHandler.createDomainInvite @user, @licence, (err, invite) =>
 				@EmailHandler.sendEmail.calledWith("verifyEmailToJoinTeam",
 					sinon.match({
-						to: "john.snow@nightwatch.com"
+						to: "john.snow@example.com"
 						inviterName: "Team Daenerys"
 						acceptInviteUrl: "http://example.com/subscription/invites/#{@newToken}/"
 					})
@@ -171,15 +181,15 @@ describe "TeamInvitesHandler", ->
 				id: "123456789",
 				first_name: "Tyrion",
 				last_name: "Lannister",
-				email: "tyrion@lannister.com"
+				email: "tyrion@example.com"
 			}
 
 			@UserGetter.getUserByAnyEmail.withArgs(@user.email).yields(null, @user)
 
 			@subscription.teamInvites.push({
-				email: "john.snow@nightwatch.com",
+				email: "john.snow@example.com",
 				token: "dddddddd",
-				inviterName: "Daenerys Targaryen (daenerys@motherofdragons.com)"
+				inviterName: "Daenerys Targaryen (daenerys@example.com)"
 			})
 
 		it "adds the user to the team", (done) ->
@@ -191,37 +201,37 @@ describe "TeamInvitesHandler", ->
 			@TeamInvitesHandler.acceptInvite "dddddddd", @user.id, =>
 				@Subscription.update.calledWith(
 					{ _id: new ObjectId("55153a8014829a865bbf700d") },
-					{ '$pull': { teamInvites: { email: 'john.snow@nightwatch.com' } } }
+					{ '$pull': { teamInvites: { email: 'john.snow@example.com' } } }
 				).should.eq true
 				done()
 
 	describe "revokeInvite", ->
 		it "removes the team invite from the subscription", (done) ->
-			@TeamInvitesHandler.revokeInvite @manager.id, "jorah@mormont.org", =>
+			@TeamInvitesHandler.revokeInvite @manager.id, "jorah@example.com", =>
 				@Subscription.update.calledWith(
 					{ _id: new ObjectId("55153a8014829a865bbf700d") },
-					{ '$pull': { teamInvites: { email: "jorah@mormont.org" } } }
+					{ '$pull': { teamInvites: { email: "jorah@example.com" } } }
 				).should.eq true
 
 				@Subscription.update.calledWith(
 					{ _id: new ObjectId("55153a8014829a865bbf700d") },
-					{ '$pull': { invited_emails: "jorah@mormont.org" } }
+					{ '$pull': { invited_emails: "jorah@example.com" } }
 				).should.eq true
 				done()
 
 	describe "createTeamInvitesForLegacyInvitedEmail", (done) ->
 		beforeEach ->
-			@subscription.invited_emails = ["eddard@stark.com", "robert@stark.com"]
+			@subscription.invited_emails = ["eddard@example.com", "robert@example.com"]
 			@TeamInvitesHandler.createInvite = sinon.stub().yields(null)
 			@SubscriptionLocator.getGroupsWithEmailInvite = sinon.stub().yields(null, [@subscription])
 
 		it "sends an invitation email to addresses in the legacy invited_emails field", (done) ->
-			@TeamInvitesHandler.createTeamInvitesForLegacyInvitedEmail "eddard@stark.com", (err, invite) =>
+			@TeamInvitesHandler.createTeamInvitesForLegacyInvitedEmail "eddard@example.com", (err, invite) =>
 				expect(err).not.to.exist
 
 				@TeamInvitesHandler.createInvite.calledWith(
 					@subscription.admin_id,
-					"eddard@stark.com"
+					"eddard@example.com"
 				).should.eq true
 
 				@TeamInvitesHandler.createInvite.callCount.should.eq 1
@@ -231,13 +241,13 @@ describe "TeamInvitesHandler", ->
 	describe "validation", ->
 		it "doesn't create an invite if the team limit has been reached", (done) ->
 			@LimitationsManager.teamHasReachedMemberLimit = sinon.stub().returns(true)
-			@TeamInvitesHandler.createInvite @manager.id, "John.Snow@nightwatch.com", (err, invite) =>
+			@TeamInvitesHandler.createInvite @manager.id, "John.Snow@example.com", (err, invite) =>
 				expect(err).to.deep.equal(limitReached: true)
 				done()
 
 		it "doesn't create an invite if the subscription is not in a group plan", (done) ->
 			@subscription.groupPlan = false
-			@TeamInvitesHandler.createInvite @manager.id, "John.Snow@nightwatch.com", (err, invite) =>
+			@TeamInvitesHandler.createInvite @manager.id, "John.Snow@example.com", (err, invite) =>
 				expect(err).to.deep.equal(wrongPlan: true)
 				done()
 
@@ -245,13 +255,13 @@ describe "TeamInvitesHandler", ->
 			member = {
 				id: "1a2b",
 				_id: "1a2b",
-				email: "tyrion@lannister.com"
+				email: "tyrion@example.com"
 			}
 
 			@subscription.member_ids = [member.id]
 			@UserGetter.getUserByAnyEmail.withArgs(member.email).yields(null, member)
 
-			@TeamInvitesHandler.createInvite @manager.id, "tyrion@lannister.com", (err, invite) =>
+			@TeamInvitesHandler.createInvite @manager.id, "tyrion@example.com", (err, invite) =>
 				expect(err).to.deep.equal(alreadyInTeam: true)
 				expect(invite).not.to.exist
 				done()