2
\$\begingroup\$

Currently I have the following callback system:

var saveTask = function(err, result) {
    if (err) return callback(err, result);

    var newid = mongoose.Types.ObjectId();
    var task = new Task({
        _id: newid,
        taskname: req.body.name,
        teamid: req.body.team,
        content: req.body.content,
        creator: req.user.userId
    });

    task.save(function (err) {
        if (!err) {
            log.info("New task created with id: %s", task._id);
            return callback(null, task);
        } else {
            if(err.name === 'ValidationError') {
                return callback('400', 'Validation error');
            } else {
                return callback('500', 'Server error');
            }
            log.error('Internal error(%d): %s', res.statusCode, err.message);
        }
    });
};

if (req.body.team) {
    valTeam.isMember(req.body.team, req.user._id, function (err, done) {
        if (err) {
            saveTask('403', 'Not the owner or member of this team');
        } else {
            saveTask(null, true);
        }
    });
} else {
    saveTask(null, true);
}

valTeam.isMember

exports.isMember = function(teamid, userid, callback) {
    Team.find({'_id':teamid, $or:[{'creator': userid }, {'userlist': { $in : [userid]}}]}, function(err, result) {
            if (err) return err;
            console.log(result);
            if (!result.length)
                return callback('404', false);
            else
                return callback(null, true);
    });
}

In short, if team is sent by POST, I'm checking if the user is member of that ID in valTeam.isMember. Am I using the correct syntax and best method to call back my saveTask function to save the task if the user is part of the team?

This code currently works, but I feel like there should be an easier way to do it? How could I use a promise to achieve the same thing?

Thanks in advance.

\$\endgroup\$
1
\$\begingroup\$

Assuming you're using Node.JS v6.x, or lower but with babel (this is ES6 syntax):

function saveTask(req) {
  // Always return a promise.
  return new Promise((resolve, reject) => {
    // Build a task.
    const _id = mongoos.Types.ObjectId();
    const task = new Task({
      _id, // Not a mistake. it's valid ES6 code.
      taskname: req.body.name,
      teamid: req.body.team,
      content: req.body.content,
      createror: req.user.userId
    });

    // Save it.
    task.save((err) => {
      if (!err) {
        log.info('New task created with id: %s', _id);
        // Resolve promise with task.
        // If you don't actually need task, you could 
        // resolve(req); // to pass req along.
        resolve(task);
      } else {
        log.error('Internal error(%d): %s', err.code, err.message);
        // Reject promise with err.
        reject(err);
      }
    });
  });
}

// Utility function to run a specific code sequence.
const postTask = (req) => valTeam.isMember(req).then(saveTask);
// If `valTeam` is not a module library, then this might be better:
// const postTask = (req, valTeam) => valTeam.isMember(req).then(saveTask);
// and call with `postTask(req, valTeam)`

And here's the isMember() function

/**
 * Utility function to replace:
 *
 * if (req.body.team) { ... }
 *
 * with a promise.
 */
const hasTeamId = (req) => new Promise((resolve, reject) => {
  if (req.body.team) {
    // Has team id. Pass `req`
    resolve(req);
  } else {
    reject(new Error('No team id provided'));
  }
});

// Function isMember() always returns a promise
module.exports.isMember = function (req) {
  const teamid = req.body.team;
  const userid = req.user._id;

  // Internal utility function to check if a given team it exists.
  const checkTeamExists = (req) => {
    // Execute query
    Team.findOne({'_id': teamid, $or:[{'creator': userid }, {'userlist': { $in : [userid]}}]}, function(err, result) {
      // On callback, solve the promise.
      if (!err && result) {
        // No error, and at least 1 document in results.
        // Resolve promise.
        resolve(req);
      } else {
        // 1) Either error, or
        // 2) No error, but empty results => Create a new error.
        // Reject promise.
        reject(err || new Error('404 - Not found'));
      }
    });
  };

  return hasTeamId(req).then(checkTeamExists);
};

Also, if you have Mongoose 4.x+ which supports Promises, better use this:

// ALTERNATIVE FOR MONGOOSE v4.x+
// Function isMember() always returns a promise
module.exports.isMember = function (req) {
  const teamid = req.body.team;
  const userid = req.user._id;

  // Internal utility function to check if a given team it exists.
  const checkTeamExists = (req) => {
    // Execute query ( In mongoose 4.x+, queries return Promises )
    return Team.findOne({'_id': teamid, $or:[{'creator': userid }, {'userlist': { $in : [userid]}}]})
      .then((result) => {
        // On callback, solve the promise.
        if (result) {
          //at least 1 document in results.
          return req;
        } else {
          // not found.
          throw new Error('404 - Not found');
        }
      });
  };

  return hasTeamId(req).then(checkTeamExists);
}

Finally, use the above like so:

postTask(req)
  .then((task) => {
    // Successfully saved task.
    // Handle task, if needed, otherwise just remove the `then()` branch.
  })
  .catch((err) => {
    // Some error occured, either in `isMember` or in `saveTask`
    // Handle error, if needed, otherwise just remove the `catch()` branch.
  });
\$\endgroup\$
  • \$\begingroup\$ Worked absolute wonders mate, thanks for your time and all the comments you wrote along the way. Just had one question, the teamid isn't a requirement, so I just need to bypass checkTeamExists and save the task without it. How would I be able to do so? \$\endgroup\$ – Sanction May 7 '16 at 14:03
  • \$\begingroup\$ Glad I could help. If you want to bypass checking the database for teamid but keep the variable check inside req, just remove .then(checkTeamExists). If you want to bypass teamid checks altogether (the isMember() function), just use saveTask instead of postTask. Promises give you this advantage of chaining various tasks, as needed. \$\endgroup\$ – Mihai Potra May 8 '16 at 20:31

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.