If user is member of project, redirect to project.

Leave invite in place to expire naturally.

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

@@ -55,6 +55,7 @@ module.exports = CollaboratorsInviteController =
 		token = req.params.token
 		currentUser = req.session.user
 		_renderInvalidPage = () ->
+			logger.log {projectId, token}, "invite not valid, rendering not-valid page"
 			res.render "project/invite/not-valid"
 		# get the target project
 		Project.findOne {_id: projectId}, {owner_ref: 1, name: 1, collaberator_refs: 1, readOnly_refs: 1}, (err, project) ->
@@ -64,21 +65,20 @@ module.exports = CollaboratorsInviteController =
 			if !project
 				logger.log {projectId}, "no project found"
 				return _renderInvalidPage()
+			# check if user is already a member of the project, redirect to project if so
+			allMembers = (project.collaberator_refs || []).concat(project.readOnly_refs || []).map((oid) -> oid.toString())
+			if currentUser._id in allMembers
+				logger.log {projectId, userId: currentUser._id}, "user is already a member of this project, redirecting"
+				return res.redirect "/project/#{projectId}"
 			# get the invite
 			CollaboratorsInviteHandler.getInviteByToken projectId, token, (err, invite) ->
 				if err?
 					logger.err {projectId, token}, "error getting invite by token"
 					return next(err)
-				# check if invite is gone
+				# check if invite is gone, or otherwise non-existent
 				if !invite
 					logger.log {projectId, token}, "no invite found for this token"
-					# check if user is already a member of the project, redirect to project if so
-					allMembers = (project.collaberator_refs || []).concat(project.readOnly_refs || []).map((oid) -> oid.toString())
-					if currentUser._id in allMembers
-						logger.log {projectId, userId: currentUser._id}, "user is already a member of this project, redirecting"
-						return res.redirect "/project/#{projectId}"
-					else
-						return _renderInvalidPage()
+					return _renderInvalidPage()
 				# check the user who sent the invite exists
 				User.findOne {_id: invite.sendingUserId}, {email: 1, first_name: 1, last_name: 1}, (err, owner) ->
 					if err?

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

@@ -79,6 +79,15 @@ module.exports = CollaboratorsInviteHandler =
 					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
@@ -93,9 +102,7 @@ module.exports = CollaboratorsInviteHandler =
 
 				ContactManager.addContact invite.sendingUserId, user._id
 
-				# Update the project, adding the new member. We don't check if the user is already a member of the project,
-				# because even if they are we still want to have them 'accept' the invite and go through the usual process,
-				# despite the $addToSet operation having no meaningful effect
+				# 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

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

@@ -329,8 +329,8 @@ describe "CollaboratorsInviteController", ->
 				it 'should call Project.findOne', ->
 					@Project.findOne.callCount.should.equal 1
 
-				it 'should call getInviteByToken', ->
-					@CollaboratorsInviteHandler.getInviteByToken.callCount.should.equal 1
+				it 'should not call getInviteByToken', ->
+					@CollaboratorsInviteHandler.getInviteByToken.callCount.should.equal 0
 
 				it 'should not call User.findOne', ->
 					@User.findOne.callCount.should.equal 0

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

@@ -343,6 +343,41 @@ describe "CollaboratorsInviteHandler", ->
 					@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) ->
+				@call (err) =>
+					@ProjectInvite.findOne.callCount.should.equal 1
+					@ProjectInvite.findOne.calledWith({_id: @inviteId, projectId: @projectId, token: @token}).should.equal true
+					done()
+
+			it 'should have returned early, 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 ProjectInvite.findOne does not find an invite', ->
 
 			beforeEach ->