Refactor and better test EarnedAchievement.createForAchievement

server/handlers/earned_achievement_handler.coffee

@@ -49,7 +49,7 @@ class EarnedAchievementHandler extends Handler
     achievementID = req.body.achievement
     triggeredBy = req.body.triggeredBy
     collection = req.body.collection
-    if collection isnt 'level.sessions'
+    if collection isnt 'level.sessions' and not testing # TODO: remove this restriction
       return @sendBadInputError(res, 'Only doing level session achievements for now.')
 
     model = mongoose.modelNameByCollection(collection)
@@ -71,9 +71,8 @@ class EarnedAchievementHandler extends Handler
       else if not trigger
         return @sendNotFoundError(res, 'Could not find trigger.')
       else if achievement.get('proportionalTo') and earned
-        EarnedAchievement.createForAchievement(achievement, trigger, null, earned, (earnedAchievementDoc) =>
+        EarnedAchievement.createForAchievement(achievement, trigger, {previouslyEarnedAchievement: earned}).then (earnedAchievementDoc) =>
           @sendCreated(res, (earnedAchievementDoc or earned)?.toObject())
-        )
       else if earned
         achievementEarned = achievement.get('rewards')
         actuallyEarned = earned.get('earnedRewards')
@@ -92,13 +91,12 @@ class EarnedAchievementHandler extends Handler
             return @sendSuccess(res, earned.toObject())
           )
       else
-        EarnedAchievement.createForAchievement(achievement, trigger, null, null, (earnedAchievementDoc) =>
+        EarnedAchievement.createForAchievement(achievement, trigger).then (earnedAchievementDoc) =>
           if earnedAchievementDoc
             @sendCreated(res, earnedAchievementDoc.toObject())
           else
             console.error "Couldn't create achievement", achievement, trigger
             @sendNotFoundError res, "Couldn't create achievement"
-        )
     )
 
   upsertNonNumericRewards: (user, achievement, done) ->

server/models/EarnedAchievement.coffee

@@ -2,6 +2,7 @@ mongoose = require 'mongoose'
 jsonschema = require '../../app/schemas/models/earned_achievement'
 util = require '../../app/core/utils'
 log = require 'winston'
+co = require 'co'
 
 EarnedAchievementSchema = new mongoose.Schema({
   notified:
@@ -16,81 +17,74 @@ EarnedAchievementSchema.pre 'save', (next) ->
 EarnedAchievementSchema.index({user: 1, achievement: 1}, {unique: true, name: 'earned achievement index'})
 EarnedAchievementSchema.index({user: 1, changed: -1}, {name: 'latest '})
 
-EarnedAchievementSchema.statics.createForAchievement = (achievement, doc, originalDocObj=null, previouslyEarnedAchievement=null, done) ->
-  User = require './User'
+EarnedAchievementSchema.statics.createForAchievement = co.wrap (achievement, doc, options={}) ->
+  { previouslyEarnedAchievement, originalDocObj } = options
+  
+  User = require('./User')
   userObjectID = doc.get(achievement.get('userField'))
-  userID = if _.isObject userObjectID then userObjectID.toHexString() else userObjectID # Standardize! Use strings, not ObjectId's
+  userID = if _.isObject userObjectID then userObjectID.toHexString() else userObjectID # Standardize! Use ObjectIds
 
-  earned =
+  earnedAttrs = {
     user: userID
     achievement: achievement._id.toHexString()
     achievementName: achievement.get 'name'
     earnedRewards: achievement.get 'rewards'
+  }
 
   pointWorth = achievement.get('worth') ? 10
   gemWorth = achievement.get('rewards')?.gems ? 0
   earnedPoints = 0
   earnedGems = 0
+  earnedDoc = null
 
-  wrapUp = (earnedAchievementDoc) ->
-    # Update user's experience points
+  isRepeatable = achievement.get('proportionalTo')?
+
+  if isRepeatable
+    proportionalTo = achievement.get('proportionalTo')
+    docObj = doc.toObject()
+    newAmount = util.getByPath(docObj, proportionalTo) or 0
+
+    if proportionalTo is 'simulatedBy' and newAmount > 0 and not previouslyEarnedAchievement and Math.random() < 0.1
+      # Because things like simulatedBy get updated with $inc and not the post-save plugin hook,
+      # we (infrequently) fetch the previously earned achievement so we can really update.
+      previouslyEarnedAchievement = yield EarnedAchievement.findOne({user: earnedAttrs.user, achievement: earnedAttrs.achievement})
+
+    if previouslyEarnedAchievement
+      originalAmount = previouslyEarnedAchievement.get('achievedAmount') or 0
+    else if originalDocObj  # This branch could get buggy if unchangedCopy tracking isn't working.
+      originalAmount = util.getByPath(originalDocObj, proportionalTo) or 0
+    else
+      originalAmount = 0
+
+    if originalAmount isnt newAmount
+      expFunction = achievement.getExpFunction()
+      earnedAttrs.notified = false
+      earnedAttrs.achievedAmount = newAmount
+      earnedPoints = earnedAttrs.earnedPoints = (expFunction(newAmount) - expFunction(originalAmount)) * pointWorth
+      earnedGems = earnedAttrs.earnedGems = (expFunction(newAmount) - expFunction(originalAmount)) * gemWorth ? 0
+      earnedAttrs.previouslyAchievedAmount = originalAmount
+      yield EarnedAchievement.update({achievement: earnedAttrs.achievement, user: earnedAttrs.user}, earnedAttrs, {upsert: true})
+      earnedDoc = new EarnedAchievement(earnedAttrs)
+
+  else # not alreadyAchieved
+    earnedAttrs.earnedPoints = pointWorth
+    earnedAttrs.earnedGems = gemWorth
+    earnedDoc = new EarnedAchievement(earnedAttrs)
+    yield earnedDoc.save()
+    earnedPoints = pointWorth
+    earnedGems = gemWorth
+
+  User.saveActiveUser(userID, "achievement")
+
+  if earnedDoc
     update = {$inc: {points: earnedPoints, 'earned.gems': earnedGems}}
     for rewardType, rewards of achievement.get('rewards') ? {}
       continue if rewardType is 'gems'
       if rewards.length
         update.$addToSet ?= {}
         update.$addToSet["earned.#{rewardType}"] = $each: rewards
-    User.update {_id: mongoose.Types.ObjectId(userID)}, update, {}, (err, result) ->
-      log.error err if err?
-      done?(earnedAchievementDoc)
+    yield User.update({_id: mongoose.Types.ObjectId(userID)}, update, {})
 
-  isRepeatable = achievement.get('proportionalTo')?
-  if isRepeatable
-    #log.debug 'Upserting repeatable achievement called \'' + (achievement.get 'name') + '\' for ' + userID
-    proportionalTo = achievement.get 'proportionalTo'
-    docObj = doc.toObject()
-    newAmount = util.getByPath(docObj, proportionalTo) or 0
-    updateEarnedAchievement = (originalAmount) ->
-      #console.log 'original amount is', originalAmount, 'and new amount is', newAmount, 'for', proportionalTo, 'with doc', docObj, 'and previously earned achievement amount', previouslyEarnedAchievement?.get('achievedAmount'), 'because we had originalDocObj', originalDocObj
-
-      if originalAmount isnt newAmount
-        expFunction = achievement.getExpFunction()
-        earned.notified = false
-        earned.achievedAmount = newAmount
-        #console.log 'earnedPoints is', (expFunction(newAmount) - expFunction(originalAmount)) * pointWorth, 'was', earned.earnedPoints, earned.previouslyAchievedAmount, 'got exp function for new amount', newAmount, expFunction(newAmount), 'for original amount', originalAmount, expFunction(originalAmount), 'with point worth', pointWorth
-        earnedPoints = earned.earnedPoints = (expFunction(newAmount) - expFunction(originalAmount)) * pointWorth
-        earnedGems = earned.earnedGems = (expFunction(newAmount) - expFunction(originalAmount)) * gemWorth ? 0
-        earned.previouslyAchievedAmount = originalAmount
-        EarnedAchievement.update {achievement: earned.achievement, user: earned.user}, earned, {upsert: true}, (err) ->
-          return log.error err if err?
-
-        wrapUp(new EarnedAchievement(earned))
-      else
-        done?()
-
-    if proportionalTo is 'simulatedBy' and newAmount > 0 and not previouslyEarnedAchievement and Math.random() < 0.1
-      # Because things like simulatedBy get updated with $inc and not the post-save plugin hook,
-      # we (infrequently) fetch the previously earned achievement so we can really update.
-      EarnedAchievement.findOne {user: earned.user, achievement: earned.achievement}, (err, previouslyEarnedAchievement) ->
-        log.error err if err?
-        updateEarnedAchievement previouslyEarnedAchievement?.get('achievedAmount') or 0
-    else if previouslyEarnedAchievement
-      updateEarnedAchievement previouslyEarnedAchievement.get('achievedAmount') or 0
-    else if originalDocObj  # This branch could get buggy if unchangedCopy tracking isn't working.
-      updateEarnedAchievement util.getByPath(originalDocObj, proportionalTo) or 0
-    else
-      updateEarnedAchievement 0
-
-  else # not alreadyAchieved
-    #log.debug 'Creating a new earned achievement called \'' + (achievement.get 'name') + '\' for ' + userID
-    earned.earnedPoints = pointWorth
-    earned.earnedGems = gemWorth
-    (new EarnedAchievement(earned)).save (err, doc) ->
-      return log.error err if err?
-      earnedPoints = pointWorth
-      earnedGems = gemWorth
-      wrapUp(doc)
-
-  User.saveActiveUser userID, "achievement"
+  return earnedDoc
 
 module.exports = EarnedAchievement = mongoose.model('EarnedAchievement', EarnedAchievementSchema)

server/plugins/achievements.coffee

@@ -42,7 +42,6 @@ AchievablePlugin = (schema, options) ->
           alreadyAchieved = if isNew then false else LocalMongo.matchesQuery unchangedCopy, query
           newlyAchieved = LocalMongo.matchesQuery(docObj, query)
           return unless newlyAchieved and (not alreadyAchieved or isRepeatable)
-          #log.info "Making an achievement: #{achievement.get('name')} #{achievement.get('_id')} for doc: #{doc.get('name')} #{doc.get('_id')}"
-          EarnedAchievement.createForAchievement(achievement, doc, unchangedCopy)
+          EarnedAchievement.createForAchievement(achievement, doc, {originalDocObj: unchangedCopy})
 
 module.exports = AchievablePlugin

spec/server/functional/achievement.spec.coffee

@@ -164,7 +164,7 @@ describe 'DELETE /db/achievement/:handle', ->
 describe 'POST /db/earned_achievement', ->
   beforeEach addAllAchievements
   
-  it 'can be used to manually create them for level achievements, which do not happen automatically', utils.wrap (done) ->
+  it 'manually creates earned achievements for level achievements, which do not happen automatically', utils.wrap (done) ->
     session = new LevelSession({
       permissions: simplePermissions
       creator: @admin._id
@@ -185,7 +185,20 @@ describe 'POST /db/earned_achievement', ->
     earnedAchievements = yield EarnedAchievement.find()
     expect(earnedAchievements.length).toBe(1)
     done()
-
+    
+  it 'works for proportional achievements', utils.wrap (done) ->
+    user = yield utils.initUser()
+    yield utils.loginUser(user)
+    yield user.update({simulatedBy: 10})
+    json = {achievement: @repeatable.id, triggeredBy: user.id, collection: 'users'}
+    [res, body] = yield request.postAsync {uri: getURL('/db/earned_achievement'), json: json}
+    expect(res.statusCode).toBe(201)
+    expect(body.earnedPoints).toBe(10)
+    yield user.update({simulatedBy: 30})
+    [res, body] = yield request.postAsync {uri: getURL('/db/earned_achievement'), json: json}
+    expect(res.statusCode).toBe(201)
+    expect(body.earnedPoints).toBe(20) # this is kinda weird, TODO: just return total amounts
+    done()
 
 describe 'automatically achieving achievements', ->
   beforeEach addAllAchievements