From f27c072ae1f4a64b280081cdf328ca7acfdc581f Mon Sep 17 00:00:00 2001
From: Henry Oswald <henry.oswald@sharelatex.com>
Date: Wed, 27 May 2015 16:33:47 +0100
Subject: [PATCH] pull logic checking if user is already part of a group out of
 controller into handler

---
 .../SubscriptionGroupController.coffee        | 15 ++++++-------
 .../SubscriptionGroupHandler.coffee           | 11 +++++++++-
 .../Subscription/SubscriptionLocator.coffee   |  3 +++
 .../SubscriptionGroupHandlerTests.coffee      | 21 ++++++++++++++++++-
 4 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionGroupController.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionGroupController.coffee
index 3659f91c8f..2cafcb2a24 100644
--- a/services/web/app/coffee/Features/Subscription/SubscriptionGroupController.coffee
+++ b/services/web/app/coffee/Features/Subscription/SubscriptionGroupController.coffee
@@ -41,15 +41,16 @@ module.exports =
 
 	renderGroupInvitePage: (req, res)->
 		subscription_id = req.params.subscription_id
+		user_id = req.session.user._id
 		licence = SubscriptionDomainAllocator.findDomainLicenceBySubscriptionId(subscription_id)
-		SubscriptionGroupHandler.getPopulatedListOfMembers licence.adminUser_id, (err, users)->
-			userInSubscription = _.find users, (user)-> user._id == req.session.user._id
-			if userInSubscription?
+		SubscriptionGroupHandler.isUserPartOfGroup user_id, licence.subscription_id, (err, partOfGroup)->
+			if partOfGroup
 				return res.redirect("/user/subscription/custom_account")
-			res.render "subscriptions/group/invite",
-				title: "Group Invitation"
-				subscription_id:subscription_id
-				licenceName:licence.name
+			else
+				res.render "subscriptions/group/invite",
+					title: "Group Invitation"
+					subscription_id:subscription_id
+					licenceName:licence.name
 
 	beginJoinGroup: (req, res)->
 		subscription_id = req.params.subscription_id
diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee
index a4907f9544..bc05bd36dd 100644
--- a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee
+++ b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee
@@ -5,7 +5,7 @@ SubscriptionUpdater = require("./SubscriptionUpdater")
 SubscriptionLocator = require("./SubscriptionLocator")
 UserLocator = require("../User/UserLocator")
 LimitationsManager = require("./LimitationsManager")
-
+logger = require("logger-sharelatex")
 
 module.exports = 
 
@@ -37,6 +37,15 @@ module.exports =
 			async.series jobs, (err)->
 				callback(err, users)
 
+	isUserPartOfGroup: (user_id, subscription_id, callback=(err, partOfGroup)->)->
+		SubscriptionLocator.getSubscriptionByMemberIdAndId user_id, subscription_id, (err, subscription)->
+			if subscription?
+				partOfGroup = true
+			else
+				partOfGroup = false
+			logger.log user_id:user_id, subscription_id:subscription_id, partOfGroup:partOfGroup, "checking if user is part of a group"
+			callback(err, partOfGroup)
+
 
 buildUserViewModel = (user)->
 	u = 
diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionLocator.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionLocator.coffee
index 19e701443b..52dbf6b835 100644
--- a/services/web/app/coffee/Features/Subscription/SubscriptionLocator.coffee
+++ b/services/web/app/coffee/Features/Subscription/SubscriptionLocator.coffee
@@ -18,3 +18,6 @@ module.exports =
 
 	getSubscription: (subscription_id, callback)->
 		Subscription.findOne _id:subscription_id, callback
+
+	getSubscriptionByMemberIdAndId: (user_id, subscription_id, callback)->
+		Subscription.findOne member_ids: user_id, _id:subscription_id, callback
diff --git a/services/web/test/UnitTests/coffee/Subscription/SubscriptionGroupHandlerTests.coffee b/services/web/test/UnitTests/coffee/Subscription/SubscriptionGroupHandlerTests.coffee
index b9cff7ef72..ddc51a154a 100644
--- a/services/web/test/UnitTests/coffee/Subscription/SubscriptionGroupHandlerTests.coffee
+++ b/services/web/test/UnitTests/coffee/Subscription/SubscriptionGroupHandlerTests.coffee
@@ -10,10 +10,12 @@ describe "Subscription Group Handler", ->
 	beforeEach ->
 		@adminUser_id = "12321"
 		@newEmail = "bob@smith.com"
-		@user = {_id:"3121321", email:@newEmail}
+		@user_id = "3121321"
+		@user = {_id:@user_id, email:@newEmail}
 
 		@SubscriptionLocator = 
 			getUsersSubscription: sinon.stub()
+			getSubscriptionByMemberIdAndId: sinon.stub()
 
 		@UserCreator = 
 			getUserOrCreateHoldingAccount: sinon.stub().callsArgWith(1, null, @user)
@@ -100,3 +102,20 @@ describe "Subscription Group Handler", ->
 				assert.deepEqual users[2], {_id:@subscription.member_ids[2]}
 				done()
 
+	describe "isUserPartOfGroup", ->
+		beforeEach ->
+			@subscription_id = "123ed13123"
+
+		it "should return true when user is part of subscription", (done)->
+			@SubscriptionLocator.getSubscriptionByMemberIdAndId.callsArgWith(2, null, {_id:@subscription_id})
+			@Handler.isUserPartOfGroup @user_id, @subscription_id, (err, partOfGroup)->
+				partOfGroup.should.equal true
+				done()
+
+		it "should return false when no subscription is found", (done)->
+			@SubscriptionLocator.getSubscriptionByMemberIdAndId.callsArgWith(2, null)
+			@Handler.isUserPartOfGroup @user_id, @subscription_id, (err, partOfGroup)->
+				partOfGroup.should.equal false
+				done()
+
+