0

I have a sample piece of Node.js code which pulls a user from the database based on an email, does some checks, does a findOne by ID and updates, like this:

 User.findOne({ email }, (err, user) => {
if (err) { return next(err); }

if (!user) {
  return res.status(422).send({ error: { message: "User doesnt exists", resend: false } });
}

if (user.auth.used) {
  return res.status(422).send({ error: { message: "link already used", resend: false } });
}

if (new Date() > user.auth.expires) {
  return res.status(422).send({ error: { message: "link already expired", resend: true } });
}

if (token !== user.auth.token) {
  return res.status(422).send({ error: { message: "something has gone wrong, please sign up again", resend: false } });
}

User.findByIdAndUpdate(user.id, { role: 1, auth: { used: true } }, (err) => {
  if (err) { return next(err); }

  const { email, firstname, lastname } = user;

  res.json({ token: tokenForUser(user), email, firstname, lastname });
});

});

Could I not just update and save the user I already have, like this?:

Token.findOne({ token: req.body.token }, function (err, token)
{
    if (!token || token !== user.auth.token) return res.status(422).send({ error: { message: "Link doesn't exist or has expired", resend: true } });

    // If we found a token, find a matching user
    User.findOne({ _id: token._userId }, function (err, user)
    {
        if (!user) return res.status(422).send({ error: { message: "We couldn't find a user for this token", resend: false } });
        if (user.isVerified) return res.status(422).send({ error: { message: "link already used", resend: true } });

        // Verify and save the user
        user.isVerified = true;
        user.save(function (err)
        {
            if (err) { return res.status(500).send({ msg: err.message }); }
            res.json({ token: tokenForUser(user), req.user.email, req.user.firstName, req.user.lastName, req.user.company })
        });
    });
});
Boris K
  • 3,442
  • 9
  • 48
  • 87
  • Have you accidentally posted the same code in both the sections?, if not please highlight the change between the first approach and the second approach – Clement Amarnath Oct 23 '17 at 13:20
  • It's not entirely clear what you're trying to achieve. Could you clarify? – mickdekkers Oct 23 '17 at 14:15
  • @ClementAmarnath sorry, good catch, edited! – Boris K Oct 24 '17 at 06:53
  • @mickdekkers I'd prefer to avoid the additional User.findByIDAndUpdate(), if I still have access to the original user from User.findOne() – Boris K Oct 24 '17 at 06:54
  • 1
    @BorisK I see what you mean. According to the docs and [other StackOverflow answers](https://stackoverflow.com/a/14199685/1233003), it should be possible, but I'm also having trouble getting it to work. I'm trying it with the [example code](http://mongoosejs.com/) on the mongoose website. [Code here.](https://gist.github.com/mickdekkers/57a85b43e727700d43075e2bbb8a2a52) The odd thing is that the second console.log doesn't show the `isVerified` property either. – mickdekkers Oct 24 '17 at 09:49
  • 1
    I suspect they overrode the [`toString` method](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/toString) to return the contents of the `_doc` property on the document object because this would explain why the second console.log doesn't show `isVerified`. It's still strange that the call to save doesn't seem to do anything though. – mickdekkers Oct 24 '17 at 10:01
  • 1
    @BorisK I can't find any more information on this, but it might be a good idea to [open an issue](https://github.com/Automattic/mongoose/issues/new) on the mongoose repo. It looks like this might be a regression, given that the examples on StackOverflow seemed to work for others. – mickdekkers Oct 25 '17 at 08:32

0 Answers0