[#1729] move most chal-user syncing logic to their models rather than

controllers. don't sync task.value or task.history over to user on
challenge.join()! some work-arounds for the task._id/id mismatch (still
not perfect, need to work on some more)

src/controllers/challenges.js

@@ -11,48 +11,6 @@ var Group = require('./../models/group').model;
 var Challenge = require('./../models/challenge').model;
 var api = module.exports;
 
-/**
- * Syncs all new tasks, deleted tasks, etc to the user object
- * @param chal
- * @param user
- * @return nothing, user is modified directly. REMEMBER to save the user!
- */
-var syncChalToUser = function(chal, user) {
-  if (!chal || !user) return;
-  chal.shortName = chal.shortName || chal.name;
-
-  // Sync tags
-  var tags = user.tags || [];
-  var i = _.findIndex(tags, {id: chal._id})
-  if (~i) {
-    if (tags[i].name !== chal.shortName) {
-      // update the name - it's been changed since
-      user.tags[i].name = chal.shortName;
-    }
-  } else {
-    user.tags.push({
-      id: chal._id,
-      name: chal.shortName,
-      challenge: true
-    });
-  }
-
-  // Sync new tasks and updated tasks
-  _.each(chal.tasks, function(task){
-    var list = user[task.type+'s'];
-    var userTask = user.tasks[task.id] || (list.push(task), list[list.length-1]);
-    userTask.challenge = {id:chal._id};
-    userTask.tags = userTask.tags || {};
-    userTask.tags[chal._id] = true;
-    _.merge(userTask, keepAttrs(task));
-  })
-
-  // Flag deleted tasks as "broken"
-  _.each(user.tasks, function(task){
-    if (task.challenge && task.challenge.id==chal._id && !chal.tasks[task.id])
-      task.challenge.broken = 'TASK_DELETED';
-  })
-};
 
 /*
   ------------------------------------------------------------------------
@@ -174,8 +132,7 @@ api.create = function(req, res){
     },
     function(_group, num, cb) {
       // Auto-join creator to challenge (see members.push above)
-      syncChalToUser(chal, user);
+      chal.syncToUser(user, cb);
-      user.save(cb);
     }
   ]);
   async.waterfall(waterfall, function(err){
@@ -184,13 +141,6 @@ api.create = function(req, res){
   });
 }
 
-function keepAttrs(task) {
-  // only sync/compare important attrs
-  var keepAttrs = 'text notes up down priority repeat'.split(' ');
-  if (task.type=='reward') keepAttrs.push('value');
-  return _.pick(task, keepAttrs);
-}
-
 // UPDATE
 api.update = function(req, res){
   var cid = req.params.cid;
@@ -215,22 +165,12 @@ api.update = function(req, res){
       cb(null, saved);
 
       // Compare whether any changes have been made to tasks. If so, we'll want to sync those changes to subscribers
-      function comparableData(obj) {
+      if (before.isOutdated(req.body)) {
-        return (
-          _.chain(obj.habits.concat(obj.dailys).concat(obj.todos).concat(obj.rewards))
-          .sortBy('id') // we don't want to update if they're sort-order is different
-          .transform(function(result, task){
-            result.push(keepAttrs(task));
-          }))
-          .toString(); // for comparing arrays easily
-      }
-      if (comparableData(before) !== comparableData(req.body)) {
         User.find({_id: {$in: saved.members}}, function(err, users){
           console.log('Challenge updated, sync to subscribers');
           if (err) throw err;
           _.each(users, function(user){
-            syncChalToUser(saved, user);
+            saved.syncToUser(user);
-            user.save();
           })
         })
       }
@@ -347,8 +287,7 @@ api.join = function(req, res){
       if (!~user.challenges.indexOf(cid))
         user.challenges.unshift(cid);
       // Add all challenge's tasks to user's tasks
-      syncChalToUser(challenge, user);
+      challenge.syncToUser(user, function(err){
-      user.save(function(err){
         if (err) return cb(err);
         cb(null, challenge); // we want the saved challenge in the return results, due to ng-resource
       });
@@ -360,30 +299,6 @@ api.join = function(req, res){
   });
 }
 
-function unlink(user, cid, keep, tid) {
-  switch (keep) {
-    case 'keep':
-      user.tasks[tid].challenge = {};
-      break;
-    case 'remove':
-      user.deleteTask(tid);
-      break;
-    case 'keep-all':
-      _.each(user.tasks, function(t){
-        if (t.challenge && t.challenge.id == cid) {
-          t.challenge = {};
-        }
-      });
-      break;
-    case 'remove-all':
-      _.each(user.tasks, function(t){
-        if (t.challenge && t.challenge.id == cid) {
-          user.deleteTask(t.id);
-        }
-      })
-      break;
-  }
-}
 
 api.leave = function(req, res){
   var user = res.locals.user;
@@ -398,8 +313,7 @@ api.leave = function(req, res){
     function(chal, cb){
       var i = user.challenges.indexOf(cid)
       if (~i) user.challenges.splice(i,1);
-      unlink(user, chal._id, keep)
+      user.unlink({cid:chal._id, keep:keep}, function(err){
-      user.save(function(err){
         if (err) return cb(err);
         cb(null, chal);
       })
@@ -421,12 +335,7 @@ api.unlink = function(req, res, next) {
   var cid = user.tasks[tid].challenge.id;
   if (!req.query.keep)
     return res.json(400, {err: 'Provide unlink method as ?keep=keep-all (keep, keep-all, remove, remove-all)'});
-  unlink(user, cid, req.query.keep, tid);
+  user.unlink({cid:cid, keep:req.query.keep, tid:tid}, function(err, saved){
-  user.markModified('habits');
-  user.markModified('dailys');
-  user.markModified('todos');
-  user.markModified('rewards');
-  user.save(function(err, saved){
     if (err) return res.json(500,{err:err});
     res.send(200);
   });

src/controllers/user.js

@@ -75,18 +75,6 @@ function addTask(user, task) {
   ---------------
 */
 
-var syncScoreToChallenge = function(task, delta){
-  if (!task.challenge || !task.challenge.id || task.challenge.broken) return;
-  if (task.type == 'reward') return; // we don't want to update the reward GP cost
-  Challenge.findById(task.challenge.id, function(err, chal){
-    if (err) throw err;
-    var t = chal.tasks[task.id]
-    t.value += delta;
-    t.history.push({value: t.value, date: +new Date});
-    chal.save();
-  });
-}
-
 /**
   This is called form deprecated.coffee's score function, and the req.headers are setup properly to handle the login
   Export it also so we can call it from deprecated.coffee
@@ -136,7 +124,7 @@ api.scoreTask = function(req, res, next) {
   });
 
   // if it's a challenge task, sync the score
-  syncScoreToChallenge(task, delta);
+  user.syncScoreToChallenge(task, delta);
 };
 
 /**

src/models/challenge.js

@@ -42,5 +42,81 @@ ChallengeSchema.methods.toJSON = function(){
   return doc;
 }
 
+// --------------
+// Syncing logic
+// --------------
+
+function syncableAttrs(task) {
+  var t = task.toObject(); // lodash doesn't seem to like _.omit on EmbeddedDocument
+  // only sync/compare important attrs
+  var omitAttrs = 'history tags completed streak'.split(' ');
+  if (t.type != 'reward') omitAttrs.push('value');
+  return _.omit(t, omitAttrs);
+}
+
+/**
+ * Compare whether any changes have been made to tasks. If so, we'll want to sync those changes to subscribers
+ */
+function comparableData(obj) {
+  return (
+    _.chain(obj.habits.concat(obj.dailys).concat(obj.todos).concat(obj.rewards))
+      .sortBy('id') // we don't want to update if they're sort-order is different
+      .transform(function(result, task){
+        result.push(syncableAttrs(task));
+      }))
+    .toString(); // for comparing arrays easily
+}
+
+ChallengeSchema.isOutdated = function(newData) {
+  return comparableData(this) !== comparableData(newData);
+}
+
+/**
+ * Syncs all new tasks, deleted tasks, etc to the user object
+ * @param user
+ * @return nothing, user is modified directly. REMEMBER to save the user!
+ */
+ChallengeSchema.methods.syncToUser = function(user, cb) {
+  if (!user) return;
+  var self = this;
+  self.shortName = self.shortName || self.name;
+
+  // Sync tags
+  var tags = user.tags || [];
+  var i = _.findIndex(tags, {id: self._id})
+  if (~i) {
+    if (tags[i].name !== self.shortName) {
+      // update the name - it's been changed since
+      user.tags[i].name = self.shortName;
+    }
+  } else {
+    user.tags.push({
+      id: self._id,
+      name: self.shortName,
+      challenge: true
+    });
+  }
+
+  // Sync new tasks and updated tasks
+  _.each(self.tasks, function(task){
+    var list = user[task.type+'s'];
+    var userTask = user.tasks[task.id] || (list.push(syncableAttrs(task)), list[list.length-1]);
+    userTask.challenge = {id:self._id};
+    userTask.tags = userTask.tags || {};
+    userTask.tags[self._id] = true;
+    _.merge(userTask, syncableAttrs(task));
+  })
+
+  // Flag deleted tasks as "broken"
+  _.each(user.tasks, function(task){
+    if (task.challenge && task.challenge.id==self._id && !self.tasks[task.id]) {
+      task.challenge.broken = 'TASK_DELETED';
+    }
+  })
+
+  user.save(cb);
+};
+
+
 module.exports.schema = ChallengeSchema;
 module.exports.model = mongoose.model("Challenge", ChallengeSchema);

src/models/task.js

@@ -38,4 +38,11 @@ var TaskSchema = new Schema({
   _id: false
 });
 
+/**
+ * Workaround for bug when _id & id were out of sync, we can remove this after challenges has been running for a while
+ */
+TaskSchema.post('init', function(doc){
+  if (!doc.id && doc._id) doc.id = doc._id;
+})
+
 module.exports.schema = TaskSchema;

src/models/user.js

@@ -9,6 +9,7 @@ var Schema = mongoose.Schema;
 var helpers = require('habitrpg-shared/script/helpers');
 var _ = require('lodash');
 var TaskSchema = require('./task').schema;
+var Challenge = require('./challenge').model;
 
 // User Schema
 // -----------
@@ -244,5 +245,50 @@ UserSchema.pre('save', function(next) {
   next();
 });
 
+UserSchema.methods.syncScoreToChallenge = function(task, delta){
+  if (!task.challenge || !task.challenge.id || task.challenge.broken) return;
+  if (task.type == 'reward') return; // we don't want to update the reward GP cost
+  Challenge.findById(task.challenge.id, function(err, chal){
+    if (err) throw err;
+    var t = chal.tasks[task.id];
+    if (!t) return Challenge.syncToUser(this); // this task was removed from the challenge, notify user
+    t.value += delta;
+    t.history.push({value: t.value, date: +new Date});
+    chal.save();
+  });
+}
+
+UserSchema.methods.unlink = function(options, cb) {
+  var cid = options.cid, keep = options.keep, tid = options.tid;
+  var self = this;
+  switch (keep) {
+    case 'keep':
+      self.tasks[tid].challenge = {};
+      break;
+    case 'remove':
+      self.deleteTask(tid);
+      break;
+    case 'keep-all':
+      _.each(self.tasks, function(t){
+        if (t.challenge && t.challenge.id == cid) {
+          t.challenge = {};
+        }
+      });
+      break;
+    case 'remove-all':
+      _.each(self.tasks, function(t){
+        if (t.challenge && t.challenge.id == cid) {
+          self.deleteTask(t.id);
+        }
+      })
+      break;
+  }
+  self.markModified('habits');
+  self.markModified('dailys');
+  self.markModified('todos');
+  self.markModified('rewards');
+  self.save(cb);
+}
+
 module.exports.schema = UserSchema;
 module.exports.model = mongoose.model("User", UserSchema);