skip to Main Content

I’m creating a REST API and I want to ensure that every user has unique usernames and email addresses. To do so, I have configured my database models with appropriate properties. But in route logic, I want to check that beforehand to validate if the given username or email address is valid and unique. To accomplish this validation, I use the below code piece:

const alreadyExistingUser = await User.findOne({
    $or: [
        { username: reqData.username },
        { email: reqData.email }
    ]
});

if(alreadyExistingUser) {
    let errorMessage = '';

    if(alreadyExistingUser.username === reqData.username) {
       errorMessage = 'User name is already in use.';
    } else if(alreadyExistingUser.email === reqData.email) {
       errorMessage = 'E-mail address is already in use.'
    } else {
       errorMessage = 'User name or e-mail address already in use.'
    }

    return res
        .status(400)
        .json({
            message: errorMessage
        });
}

Is this code piece valid in your opinion? How would you design this logic?

Side-question: I also wonder if I should add another property such as "result" that tells whether the action is completed successfully or not. Is it a good practice to return a JSON response like this:

return res
    .status(400)
    .json({
        message: errorMessage,
        result: false
    });

2

Answers


  1. try this!

    const isUsername = await User.findOne({ username: reqData.username });
      if (isUsername){
        return res.status(400).send("Username already exist")
      }
    
    const isEmail = await User.findOne({ username: reqData.email});
     if (isEmail ){
        return res.status(400).send("Email already exist")
      }
    
    Login or Signup to reply.
  2. With your code, it will never reach the else case which is errorMessage = 'User name or e-mail address already in use.';. So it is unnecessary.

    And you don’t handle the case if both username and email are same.

    So I would write the if logic something like this:

    if (alreadyExistingUser) {
        let errorMessage = '';
    
        if (alreadyExistingUser.username === reqData.username) {
            if (alreadyExistingUser.email === reqData.email) {
                errorMessage = 'Both User name and E-mail are already in use.';
            } else {
                errorMessage = 'User name is already in use.';
            }
        } else {
            // We know that if username is not in use, than E-mail is in use
            errorMessage = 'E-mail address is already in use.';
        }
    
        return res.status(400).json({
            message: errorMessage,
            result: false,
        });
    }
    

    Also there is no problem giving the client some information the result if it doesn’t contain sensitive information.

    If you don’t want to give information whether the username or email is in use, you can simplify the logic like this:

    if (alreadyExistingUser) {
        return res.status(400).json({
            message: "Username or email is in use",
            result: false,
        });
    }
    
    Login or Signup to reply.
Please signup or login to give your own answer.
Back To Top
Search