From f1d07811df149e727859bd5673e52aebdeea3840 Mon Sep 17 00:00:00 2001
From: Henry Oswald <henry.oswald@sharelatex.com>
Date: Wed, 7 Oct 2015 12:42:34 +0100
Subject: [PATCH] update the entire users features on assign bonus bu don't
 update the user features if there is nothing to update when assigning bonus

---
 .../Features/Referal/ReferalAllocator.coffee  | 21 +++++++++--------
 .../Referal/ReferalAllocatorTests.coffee      | 23 +++++++++++++++++--
 2 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/services/web/app/coffee/Features/Referal/ReferalAllocator.coffee b/services/web/app/coffee/Features/Referal/ReferalAllocator.coffee
index 0ea66e62b7..0127e5681e 100644
--- a/services/web/app/coffee/Features/Referal/ReferalAllocator.coffee
+++ b/services/web/app/coffee/Features/Referal/ReferalAllocator.coffee
@@ -38,28 +38,29 @@ module.exports = ReferalAllocator =
 			return callback(new Error("user not found")) if !user?
 			logger.log user_id: user_id, refered_user_count: user.refered_user_count, "assigning bonus"
 			if user.refered_user_count? and user.refered_user_count > 0
-				newFeatures = ReferalAllocator._calculateBonuses(user)
+				newFeatures = ReferalAllocator._calculateFeatures(user)
+				if _.isEqual newFeatures, user.features
+					return callback()
 				User.update query, { $set: features: newFeatures }, callback
-
 			else
 				callback()
-		
-	_calculateBonuses : (user)->
-		bonusLevel = ReferalAllocator._getBonusLevel(user)
 
-		newFeatures = {}
+	_calculateFeatures : (user)->
+		bonusLevel = ReferalAllocator._getBonusLevel(user)
+		currentFeatures = _.clone(user.features) #need to clone because we exend with underscore later
+		betterBonusFeatures = {}
 		_.each Settings.bonus_features["#{bonusLevel}"], (bonusLevel, key)->
 			currentLevel = user?.features?[key]
 			if _.isBoolean(currentLevel) and currentLevel == false
-				newFeatures[key] = bonusLevel
+				betterBonusFeatures[key] = bonusLevel
 
-			if _.isNumber(currentLevel) 
+			if _.isNumber(currentLevel)
 				if currentLevel == -1
 					return
 				bonusIsGreaterThanCurrent = currentLevel < bonusLevel
 				if bonusIsGreaterThanCurrent or bonusLevel == -1
-					newFeatures[key] = bonusLevel
-
+					betterBonusFeatures[key] = bonusLevel
+		newFeatures = _.extend(currentFeatures, betterBonusFeatures)
 		return newFeatures
 
 
diff --git a/services/web/test/UnitTests/coffee/Referal/ReferalAllocatorTests.coffee b/services/web/test/UnitTests/coffee/Referal/ReferalAllocatorTests.coffee
index 1ca8b46d9a..3fe925f91c 100644
--- a/services/web/test/UnitTests/coffee/Referal/ReferalAllocatorTests.coffee
+++ b/services/web/test/UnitTests/coffee/Referal/ReferalAllocatorTests.coffee
@@ -87,7 +87,7 @@ describe 'Referalallocator', ->
 					features:{collaborators:1, dropbox:false, versioning:false} 
 				}
 
-				@User.findOne = sinon.stub().callsArgWith 1, null,stubbedUser
+				@User.findOne = sinon.stub().callsArgWith 1, null, stubbedUser
 				@User.update = sinon.stub().callsArgWith 2, null
 				@ReferalAllocator.assignBonus @user_id, @callback
 
@@ -110,6 +110,25 @@ describe 'Referalallocator', ->
 			it "should call the callback", ->
 				@callback.called.should.equal true
 	
+		describe "when there is nothing to assign", ->
+
+			beforeEach ->
+				@ReferalAllocator._calculateBonuses = sinon.stub().returns({})
+				@stubbedUser =
+					refered_user_count:4
+					features:{collaborators:3, versioning:true, dropbox:false}
+				@Settings.bonus_features =
+					"4":
+						collaborators:3
+						versioning:true
+						dropbox:false
+				@User.findOne = sinon.stub().callsArgWith 1, null, @stubbedUser
+				@User.update = sinon.stub().callsArgWith 2, null
+
+			it "should not call update if there are no bonuses to apply", (done)->
+				@ReferalAllocator.assignBonus @user_id, (err)=>
+					@User.update.called.should.equal false
+					done()
 
 		describe "when the user has better features already", ->
 			
@@ -138,7 +157,7 @@ describe 'Referalallocator', ->
 			it "should not overright if the user has -1 users", (done)->
 				@stubbedUser.features.collaborators = -1
 				@ReferalAllocator.assignBonus @user_id, =>
-					@User.update.calledWith({_id: @user_id }, {$set: features:{dropbox:true, versioning:false} }).should.equal true
+					@User.update.calledWith({_id: @user_id }, {$set: features:{dropbox:true, versioning:false, collaborators:-1} }).should.equal true
 					done()
 
 		describe "when the user is not at a bonus level", ->