Avoiding callbacks, and nest if statements

Well, at first, as @LawrenceCheron said, you should validate first, and not in the else statements.

instead of writing

if (destroyerUser) {
  // Do something
} else {
 // Exit
}

You can write

if (!destroyerUser) {
  // Exit
}

secondly, you can extract the userId to a variable, instead of chaining the objects keys, i.e

instead of writing

if (user.role == "Moderator" || user.role == "Admin") {
  res.json({ success: false, msg: "You don't have sufficient permissions" })
 }

you could write

const userRole = user.role;
if (userRole == "Moderator" || userRole == "Admin") {
  res.json({ success: false, msg: "You don't have sufficient permissions" })
}

and finally, complex statements such as

if (user._id.toString() != destroyerUser._id.toString()) {
 // ...
}

can be extracted to smaller functions

const isSameUserId = (id, sample) => id === sample;
if (!isSameUserId(user._id.toString(), destroyerUser._id.toString()) {
 // ...
}

or combining the above

const isSameUserId = (id, sample) => id === sample;

const userId = user._id.toString();
const destroyerId = destroyerUser._id.toString();

if (!isSameUserId(userId, destroyerId)) {
 // ...
}

There are a few things you can do with nested sideways pyramids. One technique that's pretty universal is to handle errors first and then do your normal code. So, let's say you have:

if (everythingFine) {
  //do normal operation
} else {
  //handle error
}

This is OK but you really run into problems as soon as you get

if (everythingFine) {
  //do operation
  if (operationSuccessful) {
    //do next step
  } else {
    //handle error with operation
  }
} else {
  //handle error
}

You immediately start stacking more and more blocks. What's more, the logic for success is usually a lot big, since you need to do more stuff later. And the error handling logic might be a single line. So, if you want to know what's happening, on error, you need to scroll a lot.

To avoid this, you can exit early. The steps to convert your code are the same:

1. Flip the condition of the if that checks for success to instead check for errors
2. Swap the if block and the else block.
3. Add a return to the if.
4. Remove the else block and flatten it.

And you get the following

if (!everythingFine) {
  //handle error
  return;
}

//do operation
if (!operationSuccessful) {
  //handle error with operation
  return;    
}

//do next step

Now you remove the nesting and drop the else blocks, since you are going to just return and exit the function. If your error handling is a throw statement that already exists the execution, so you don't need return.

The next thing is that you have some duplicate logic here:

if (destroyerUser.role == "Admin") {
    Cyclic.deleteOneById({ _id: user._id }, (err) => {
        if (err) {
            res.json({ success: false, msg: "Error deleting user" });
        } else {
            res.json({ success: true, msg: "Successfully deleted user" });
        }
    })
} else if (destroyerUser.role == "Moderator") {
    if (user.role == "Moderator" || user.role == "Admin") {
        res.json({ success: false, msg: "You don't have sufficient permissions" })
    } else {
        Cyclic.deleteOneById({ _id: user._id }, (err) => {
            if (err) {
                res.json({ success: false, msg: "Error deleting user" });
            } else {
                res.json({ success: true, msg: "Successfully deleted user" });
            }
        })
    }
}

Whether destroyerUser is Admin or Moderator you call the same deleteByOne in either case. And you also return the same success message in both cases. So, you have six different branches in the code whereas you only have three outcomes:

+----------------+-----------+-----------------+-----------------------------------------+
| destroyer role | user role | deletion result |            operation result             |
+----------------+-----------+-----------------+-----------------------------------------+
| Moderator      | Moderator | N/A             | "You don't have sufficient permissions" |
| Moderator      | Admin     | N/A             | "You don't have sufficient permissions" |
| Moderator      | <other>   | success         | "Successfully deleted user"             |
| Moderator      | <other>   | error           | "Error deleting user"                   |
| Admin          | <any>     | success         | "Successfully deleted user"             |
| Admin          | <any>     | error           | "Error deleting user"                   |
+----------------+-----------+-----------------+-----------------------------------------+

Instead you can just do the check if a Moderator is doing the deletion first, and if they do have sufficient permissions, they are an Admin, then just do the deletion once:

if (destroyerUser.role == "Moderator" && (user.role == "Moderator" || user.role == "Admin")) {
    res.json({ success: false, msg: "You don't have sufficient permissions" })
    return;
}

Cyclic.deleteOneById({ _id: user._id }, (err) => {
    if (err) {
        res.json({ success: false, msg: "Error deleting user" });
        return;
    }
    res.json({ success: true, msg: "Successfully deleted user" });
})

So, if we apply these two thing:

• invert the logic and exit early
• remove the duplicate code

Then your code looks like this:

router.delete("/user/:username", passport.authenticate("jwt", { session: false }), (req, res) => {
    var { username = null } = req.params;
    if (username == null) {
        res.json({ success: false, msg: "Username is not valid" });
        return;
    }

    User.getUserById(req.user.id, (err, destroyerUser) => {
        if (err) {
            res.json({ success: false, msg: "User not found" });
            return;
        }
        if (!destroyerUser) {
            res.json({ success: false, msg: "Destroyer user not found" });
            return;
        }
        if (destroyerUser.role !== "Admin" && destroyerUser.role !== "Moderator") {
            res.json({ success: false, msg: "You don't have sufficient permissions" })
            return;
        }

        User.getUserByUsername(username, (err, user) => {
            if (err) {
                res.json({ success: false, msg: "Error finding user" });
                return;
            }
            if (!user) {
                res.json({ success: false, msg: "User not found" });
                return;
            }
            if (user._id.toString() === destroyerUser._id.toString()) {
                res.json({ success: false, msg: "You can't delete yourself" });
                return;
            }
            if (destroyerUser.role == "Moderator" && (user.role == "Moderator" || user.role == "Admin")) {
                res.json({ success: false, msg: "You don't have sufficient permissions" })
                return;
            }

            Cyclic.deleteOneById({ _id: user._id }, (err) => {
                if (err) {
                    res.json({ success: false, msg: "Error deleting user" });
                    return;
                }
                res.json({ success: true, msg: "Successfully deleted user" });
            })
        })
    });
})

One note - I've converted the condition destroyerUser.role == "Admin" || destroyerUser.role == "Moderator" to destroyerUser.role !== "Admin" && destroyerUser.role !== "Moderator" this is because I don't like a boolean NOT in front of long expressions:

if (!(destroyerUser.role == "Admin" || destroyerUser.role == "Moderator"))

It's easy to miss because you see the OR and the two expressions first. Luckily, that's very easily changeable using De Morgan's law for boolean expressions not (A or B) = not A and not B. Hence how I changed it to that.

You can further reduce some of the code duplication by adding a new function to handle all the errors. In all cases you do the same thing, aside from passing a different message, so you could just have:

const error = msg => res.json({ success: false, msg });

So, you can call error("User not found"), for example. It doesn't really reduce the amount of code you have, but it's more consistent this way. Also, if you decide to add more stuff to the error response (e.g., send additional headers, or you want to try and translate the error message), then you have a central location for it.