sending 2 flash messages when 2 queries to the databse show that fields exist in passport configuration

Question

On signup I check the database to see if the username and email exists if they exist it should return an error. all usernames and emails should be unique. It seems that my problem is that I could only get one error. If the email exists already I get the flash message but if the username also exists it doesn't say anything about the username already existing. I want to display both errors If both exist already on post in a flash message

I'm assuming the problem is returning done() for the email. since I have 2 queries It only looks at the first one that was hit. I don't really know.

I was trying to do research on this and I know that you could pass in an array for req.flash but I just could get that working because it's returning the first one it sees that is existing I think.

I also read that the third parameter for done is an info object that I think gives info about the user and I was thinking of loading the existing information there but I just couldn't put it together. I tried other stuff too. but I'll just show you the stuff that works with the querying the database because I failed horribly

passport.use("local-signup", new LocalStrategy({
    usernameField : "email",
    passwordField : "password",
    passReqToCallback : true
},
    function(req, email, password, done){
        var arr = [];
        User.findOne({"email" : email}, function(err, user){
            if(err) return done(err);
            if(user){
                // arr.push("That email is already taken")
                return done(null, false, req.flash("signupMessage", "That username is taken"))
            }else{
                User.findOne({"username" : req.body.username}, function(err,user){
                    if(err) return done(err);
                    if(user){
                        // arr.push()
                        return done(null, false, req.flash("signupMessage", "That username is taken"))
                    }else{
                        var newUser = new User();
                        newUser.username = req.body.username;
                        newUser.password = password;
                        newUser.email = req.body.email;

                        newUser.save(function(err, doc){
                            if(err) throw err;
                            console.log("doc", " " , doc)
                            return done(null, newUser);
                        })                          
                    }
                })

            }
        })
    }
))

EJS

app.get("/signup", function(req, res){
    console.log(req.session)
    console.log(req.flash("signupMessage"))
    res.render("signup", {authed : authed, user : user, message: req.flash("signupMessage")})
})
app.post("/signup",passport.authenticate("local-signup", {
    successRedirect : "/",
    failureRedirect : "/signup",
    failureFlash : true
}))

Answer 1

This is a basic control flow problem. Your returning. Don't. Will have to repeat some of the code as its an extra functionality, or use a flag and hit the db before. Also not sure how you will show both messages, will have to work that out separately. Merge them into one ("User name and email are already registered. Please enter a different user name and email id.")

User.findOne({"email" : email}, function(err, user){
        if(err) return done(err);
        if(user){
            // arr.push("That email is already taken")
        // do not know what all done does maybe a ligther versio that just sends the error
            done(null, false, req.flash("signupMessage", "That Email is taken"))

        User.findOne({"username" : req.body.username}, function(err,user){
                if(err) return done(err);
                if(user){
                    // arr.push()
                    return done(null, false, req.flash("signupMessage", "That username is taken"))
                }//no else here
                         return;                         

        }else{
            User.findOne({"username" : req.body.username}, function(err,user){
                if(err) return done(err);
                if(user){
                    // arr.push()
                    return done(null, false, req.flash("signupMessage", "That username is taken"))
                }else{
                    var newUser = new User();
                    newUser.username = req.body.username;
                    newUser.password = password;
                    newUser.email = req.body.email;

                    newUser.save(function(err, doc){
                        if(err) throw err;
                        console.log("doc", " " , doc)
                        return done(null, newUser);
                    })                          
                }
            })

        }
    })
    }
))

Answer 2

Tweaking the control flow is 1 way to solve this but if you are looking for a more elegant solution then you can try the promise.all() way.

What the promise.all() does is it actually fires all of the async calls all at once and return when;

1. All of the async calls succeeds (resolves)
2. Any 1 of the async call returns with reject (fails)

Obviously point 2 is not something you are looking for as you are wanting to report both email and username together for situation when they are not unique.

To workaround this, you'll have to use reflect to make promise.all() return only when all promises have replied (suceeds or fails).

Consider the code below:

this.reflect = function reflect(promise){
  return promise.then(function(/*resolve return value=*/v){ return { v:v, status: "resolved" }},
      function(/*rejection error=*/e){ return { e:e, status: "rejected" }});
};

var promises = [
  new Promise(function(resolve, reject) {
    // User.findOne({"username" : req.body.username}, function(err,user){
    // if (err) { return reject("error!"); }...
    // resolve("Name is unique!!");
    // });
    resolve("name is unique");
  }),
  new Promise(function(resolve, reject) {
    reject("email is NOT unique");
  })
];

Promise.all(promises.map(this.reflect)).then(function(results) {
  if (results[0].status === "rejected") {
    console.log("Error reason: " + results[0].e);
  }
  else {
    console.log("Succeeds, reason: " + results[0].v);
  }
  if (results[1].status === "rejected") {
    console.log("Error reason: " + results[1].e);
  }
  else {
    console.log("Succeeds, reason: " + results[1].v);
  }

  // You can call whatever callback you want here.
  // e.g. return done(null, "Test");
});

Output:

Succeeds, reason: name is unique
Error reason: email is NOT unique

Note: You should be able to make a few alteration to the code above to suit your needs. Give me a shout if you are still having trouble.

Reference:
https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Promise

Answer 3

Try it:

    var signupMessages = [];
    User.findOne({"email" : email}, function(err, user){
        if(err) return done(err);
        if(user) signupMessages.push("That email is taken");
        User.findOne({"username" : req.body.username}, function(err,user){
                if(err) return done(err);
                if(user) signupMessages.push("That username is taken");
                if(signupMessages.length > 0){
                    return done(null, false, req.flash("signupMessage", signupMessages));
                } else { ...

In this case req.flash("signupMessage") will return array of messages