diff --git a/migrations/20140220_challenge_memberCount.js b/migrations/20140220_challenge_memberCount.js new file mode 100644 index 0000000000..1b83408943 --- /dev/null +++ b/migrations/20140220_challenge_memberCount.js @@ -0,0 +1,3 @@ +db.challenges.find({},{members:1}).forEach(function(chal){ + db.challenges.update({_id:chal._id}, {$set:{memberCount:chal.members.length}}); +}); diff --git a/src/controllers/challenges.js b/src/controllers/challenges.js index 81f4e4e2c7..a876e09d45 100644 --- a/src/controllers/challenges.js +++ b/src/controllers/challenges.js @@ -18,7 +18,7 @@ var api = module.exports; ------------------------------------------------------------------------ */ -api.list = function(req, res) { +api.list = function(req, res, next) { var user = res.locals.user; async.waterfall([ function(cb){ @@ -35,18 +35,17 @@ api.list = function(req, res) { {group: 'habitrpg'} // public group ] }) - .select('name leader description group members prize official') + .select('name leader description group memberCount prize official') + .select({members:{$elemMatch:{$in:[user._id]}}}) .populate('group', '_id name') .populate('leader', 'profile.name') .sort('-official -timestamp') .exec(cb); } ], function(err, challenges){ - if (err) return res.json(500,{err:err}); + if (err) return next(err); _.each(challenges, function(c){ - c._isMember = !!~c.members.indexOf(user._id); - c.memberCount = _.size(c.members); - c.members = undefined; + c._isMember = c.members.length > 0; }) res.json(challenges); }); @@ -323,15 +322,20 @@ api.selectWinner = function(req, res) { }) } -api.join = function(req, res){ +api.join = function(req, res, next){ var user = res.locals.user; var cid = req.params.cid; async.waterfall([ - function(cb){ + function(cb) { Challenge.findByIdAndUpdate(cid, {$addToSet:{members:user._id}}, cb); }, - function(challenge, cb){ + function(challenge, cb) { + + // Trigger updating challenge member count in the background. We can't do it above because we don't have + // _.size(challenge.members). We can't do it in pre(save) because we're calling findByIdAndUpdate above. + Challenge.update({_id:cid}, {$set:{memberCount:_.size(challenge.members)}}).exec(); + if (!~user.challenges.indexOf(cid)) user.challenges.unshift(cid); // Add all challenge's tasks to user's tasks @@ -341,14 +345,14 @@ api.join = function(req, res){ }); } ], function(err, result){ - if(err) return res.json(500,{err:err}); + if(err) return next(err); result._isMember = true; res.json(result); }); } -api.leave = function(req, res){ +api.leave = function(req, res, next){ var user = res.locals.user; var cid = req.params.cid; // whether or not to keep challenge's tasks. strictly default to true if "keep-all" isn't provided @@ -359,6 +363,11 @@ api.leave = function(req, res){ Challenge.findByIdAndUpdate(cid, {$pull:{members:user._id}}, cb); }, function(chal, cb){ + + // Trigger updating challenge member count in the background. We can't do it above because we don't have + // _.size(challenge.members). We can't do it in pre(save) because we're calling findByIdAndUpdate above. + Challenge.update({_id:cid}, {$set:{memberCount:_.size(chal.members)}}).exec(); + var i = user.challenges.indexOf(cid) if (~i) user.challenges.splice(i,1); user.unlink({cid:chal._id, keep:keep}, function(err){ @@ -367,7 +376,7 @@ api.leave = function(req, res){ }) } ], function(err, result){ - if(err) return res.json(500,{err:err}); + if(err) return next(err); result._isMember = false; res.json(result); }); diff --git a/src/models/challenge.js b/src/models/challenge.js index a01cea9575..d4362aeee2 100644 --- a/src/models/challenge.js +++ b/src/models/challenge.js @@ -28,16 +28,8 @@ ChallengeSchema.virtual('tasks').get(function () { return tasks; }); -// FIXME this isn't always triggered, since we sometimes use update() or findByIdAndUpdate() -// @see https://github.com/LearnBoost/mongoose/issues/964 -ChallengeSchema.pre('save', function(next){ - this.memberCount = _.size(this.members); - next() -}) - ChallengeSchema.methods.toJSON = function(){ var doc = this.toObject(); - doc.memberCount = doc.members ? _.size(doc.members) : doc.memberCount; // @see pre('save') comment above doc._isMember = this._isMember; return doc; }