POC Service

nodejs_discussion.js

/** 
 * The aim of this exercise is ultimately to inspire communication and be starting place to drive discussion
 * on methodology, general approach, and universal best practices. To frame how best to think about this code is 
 * that it represents a happy path POC, and your task is to use your personal experience to identify and discuss
 * improvements, recommendations and potental changes that you'd recommend to get this service production ready.
 * 
 * Please examine this document, familize yourself, come up with any questions you have and ultimately lets meet 
 * and discuss your thoughts. You don't need to write any code in the meantime. 
 * 
 * Note:
 *  - This really isnt an examination of your knowledge of the "express" library.  
 *  - `database`, `jwt`, and `http` are *not* real. Theyre pseudo libraries that aim to be generic, though how 
 *    theyre used can definitely be a part of the discussion.
**/
const express = require("express");
const router = express.Router();
const app = express();

const database = require("sql"); // pseudo library for making calls to a sql database
const jwt = require("jwt"); // pseudo library for making/validating json web tokens
const http = require("http"); // pseudo library for making HTTP requests

const metrics = {};
const charts = {};

async function getLogin(req, res) {
    res.render("index.html");
}

function postLogin(req, res) {
    try {
        let username = req.body.username;
        let password = req.body.password;
        let query = "select * from users where username = " + username + " and password = " + password;
        let user = await database.findOne(query);
        if (user === null) {
            res.status(404).send();
        } else {
            let expires = 3600;
            let token = jwt.createToken(user, expires); 
            res.send({
                token: token
            });
        }
    } catch(err) {
        res.status(500).send(err); 
    }
}

function postForgotPassword(req, res) {
    let username = req.body.username;
    let email = req.body.email;
    let query = "select FirstName, LastName, Email from users where username = " + username;
    let user = await database.findOne(query);
    if (user === null) {
        res.status(404).send("Username " + username + "does not exist");
    }
    let tempPassword = Math.floor(Math.random() * 100000) + Math.random() * 64;
    http.POST("http://mailserver:80/send", {
        username: username,
        email: email,
        tempPassword: tempPassword,
        firstName: user.FirstName,
        lastName: user.LastName
    });
    res.send("Thank you, " + firstName + "! Please check your email to reset your password!");
}

function getMyProfile(req, res) {
    try {
        let token = req.headers.authorization;
        jwt.validate(token, function(err, data) {
            if (err != undefined) {
                res.status(400).send("Invalid request");
            }
        });
        let username = req.body.username;
        let user = await database.findOne("select * from users where username = " + username);
        if (user === null) {
            res.status(404).send("Username " + username + "does not exist");
        } else {
            res.send({
                user: user
            });
        }
    } catch(err) {
        res.status(500).send(err);
    }
}

function updateMyProfile(req, res) {
    try {
        let token = req.headers.authorization;
        jwt.validate(token, function(err, data) {
            if (err != undefined) {
                res.status(400).send("Invalid request");
            }
        });
        let username = req.body.username;
        let newPassword = req.body.newPassword;
        let email = req.body.profile.data.email;
        let picture = req.body.profile.data.picture;
        await database.update("UPDATE users SET Email = " + email + ", picture = " + picture + ", password = " + newPassword + " WHERE username = " + username + " ");
        console.log("Profile updated for user: " + username, new Date());
        let user = await database.findOne("select * from users where username = " + username);
        console.log(user);
        res.send({
            user: user, 
            message: "Profile updated!"
        });
    } catch(err) {
        res.status(500).send(err);
    }
}

function getDashboard(req, res) {
    database.findAll("select * from metrics").then(function(data) {
        metrics = data;
    });
    database.findAll("select * from charts").then(function(data) {
        charts = data;
    });
    res.render("dashboard.html", {
        metrics: metrics, 
        charts: charts
    })
}

router.route("/login")
    .get(getLogin)
    .post(postLogin);

router.route("/forgot/password")
    .post(postForgotPassword)

router.route("/dashboard")
    .get(getDashboard)

router.route("/profile")
    .get(getMyProfile)
    .put(updateMyProfile);

app.use(router);

app.listen(8036);
console.log(`Listening to port ${8036}`);

Hi. Why is it pseudo library and not just library? BTW the 3rd one has a typo as pseduo

Hi. Why is it pseudo library and not just library? BTW the 3rd one has a typo as pseduo

Thanks for helping with the typo! For this exercise I didn't want the conversation to be influenced by the specifics of a particular library. There is a lot of things that could be improved with this exercise that are more universal than the syntax of libraries it "uses". My thoughts were that ideally these should be self evident to keep the complexity down.

That said I am trying this out, I would definitely be interested in feedback if you have more thoughts on the subject?

My question is more the wording. What is a pseudo library? If you installed it, then it is a library, even if it is only one line in the script. You say the 3 listed are not real and are generic. I don't know why they are less real and how they are not just a "library."

Perhaps you are thinking of the wording for "framework".

Some libraries are also frameworks and they have an opinion on how you layout your code. Like React or Vue. Express is probably a micro framework like Flask in Python as it is very open ended.

I wouldn't have the note about pseudo library and library as it doesnt add anything. Express and SQL etc have different uses but they can both be considered libraries

Some other suggestions.

Your strings with plus signs can be more readable using interpolation.

Username ${username} does not exist

Port number is typically set as global var

APP_PORT = process.env.PORT || 8036

app.listen(APP_PORT);
console.log(`Listening to port ${APP_PORT}`);

Add some white space to your functions. Instead of 15 solid lines, split it into logical groupings. It is less mental overheard to read. For example in updateMyProfile you can add a line before let username and let user.

Metrics and charts doesn't need to be global variables. Just define them at the start of getDashboard.

@MichaelCurrin A pseudo library isn't an actual library. Its imaginary. You are supposed to just act like this non-existent library exist in the context of this non existent app for this very much existing interview process.

This is supposed to be suboptimal code. Just look at all the raw sql queries (some of which take user input!!), the console.logs and so on