-2
var mongoose = require('mongoose');
import es6Promise from 'es6-promise';
mongoose.Promise = es6Promise.Promise;
const follow = (followerID, toFollowId, cb) => { //REVISE only update
    User.update(
        { _id: toFollowId},
        {$push: {usersFollowing: followerID}},
        function(err){
            if (err){
                cb(true);
            } else {
                User.findByIdAndUpdate(
                    followerID,
                    {$push: {usersBeingFollowed: toFollowId}},
                    {safe: true, new: true},
                    function(err, model){
                        if (err){
                            cb(true);
                        } else {
                            cb(null, model);
                        }
                    }
                )
            }
        }
    )
}

const unfollow = (unfollowerId, toUnfollowId, cb) => { //REVISE only update
    User.update(
        { _id: toUnfollowId},
        {$pull: {usersFollowing: unfollowerId}}).then(
        function(err){
            if (err){
                return  cb(true);
            } else {
                User.findByIdAndUpdate(
                    unfollowerId,
                    {$pull: {usersBeingFollowed: toUnfollowId}},
                    {safe: true, new: true},
                    function(err, model){
                        if (err){
                            cb(true);
                        } else {
                            cb(null, model)
                        }
                    }
                )
            }
        })
}

My follow function, which doesn't use a promise works fine. I tried editing my unfollow function to work as a promise, but it doesn't work. I haven't touched JS since ES5 but my promise understanding is that I just move the callback inside .then() and call it a day. What am I missing here?

n00bie42
  • 163
  • 7
  • 2
    _"it doesn't work"_ <- What does this mean? I think "you just move the callback inside .then() and call it a day is an oversimplification, and your function is a confusing mix of promises and callbacks. If you're only using promises as a mechanism for calling your callbacks (and a non-promise approach is available), then using promises just complicates the matter. – JLRishe Feb 07 '17 at 12:46
  • Does `User.update` return a Promise? Is that promise resolved? – Zac Feb 07 '17 at 15:31

2 Answers2

0

The Mongoose query update method will not execute if you don't pass it the callback function. This is stated in the docs on update:

The operation is only executed when a callback is passed. To force execution without a callback, we must first call update() and then execute it by using the exec() method.

So add .exec() to the chain, which will also return a full-fledged promise.

The promise's then method takes two callback functions, the second one will be called in case of error, so you have to split success and failure code. And to be consistent you should completely switch over to using promises, and abandon the traditional callback pattern. So the unfollow function should also itself return a promise:

const unfollow = (unfollowerId, toUnfollowId) =>
    User.update(
        { _id: toUnfollowId },
        { $pull: { usersFollowing: unfollowerId } }
    ).exec()
    .then( _ =>
        User.findByIdAndUpdate(
            unfollowerId,
            { $pull: { usersBeingFollowed: toUnfollowId } },
            { safe: true, new: true }
        ).exec()
    );

You would call that as:

unfollow(unfollowerId, toUnfollowId).then( model => {
    // success
}, err => {
    // failure
});
trincot
  • 317,000
  • 35
  • 244
  • 286
  • Actually, invoking `.then()` directly does call `.exec()` internally, no problem here – Bergi Feb 07 '17 at 21:55
  • Indeed it would work without it, but the `mongoose.Promise` definition will only be used by `exec()` if I understand the documentation correctly. – trincot Feb 07 '17 at 22:03
  • I read [*"Mongoose queries are not promises. However, they do have a `.then()` function for yield and async/await. If you need a fully-fledged promise, use the `.exec()` function."*](http://mongoosejs.com/docs/promises.html), but the docs are confusing on this point. – trincot Feb 07 '17 at 22:12
  • Ah, right. Queries are *thenables*, which means you can use `.then()` on them - and it does even work like on a real promise, returning another promise for the callback results - but they are no promises, i.e. they have no `.catch()` method, don't inherit from `MPromise.prototype` etc – Bergi Feb 07 '17 at 22:16
0

I just move the callback inside .then() and call it a day. What am I missing here?

That the callback convention changes as well. While node callbacks have (err, result) arguments, promises use two distinct callbacks - one for fulfillment and one for rejection - that get passed only one argument.

While you could in theory do

User.update(…).then(function(_) {
    User.findByIdAndUpdate(…);
}, function(err){
    cb(true);
});

that would be an awful practise. To leverage the true power of promises, you need to return and chain them. In your case, that would be

function unfollow(unfollowerId, toUnfollowId) {
//                                          ^ no more callback
    return User.update(
//  ^^^^^^ return the result of the `then` call
        { _id: toUnfollowId},
        {$pull: {usersFollowing: unfollowerId}}
    ).then(_ => {
        return User.findByIdAndUpdate(
//      ^^^^^^ just return the promise
            unfollowerId,
            {$pull: {usersBeingFollowed: toUnfollowId}},
            {safe: true, new: true}
//          that you get when no longer passing a callback
        );
    });
}

By not passing any error callbacks, rejections will automatically bubble down the chain, and you don't need to care about forwarding all errors explicitly.

Community
  • 1
  • 1
Bergi
  • 630,263
  • 148
  • 957
  • 1,375