Reusing code in Node.js module

Question

I just want to know if you'd do a refactoring to reuse code the same way I did here. Sorry for the following long context. :)

I'm learning Express.js with a simple web app and working on the login page with a reset password form. This form asks for an email, which is checked against the DB and then a token and an expiration time of 1 hour is set in the user profile and a URL is sent to the user. The URL is something like http://mywebsite.com/account/reset/43aea78ba678fd8ed746b2b0b79c34da9380a5a6 so when the user accesses this URL I have a couple of routers to deal with the password reset from here:

One to check the token: router.get( '/account/reset/:token', authController.reset ) which redirect to a new page with a form to reset the password.
And the other to update the new password: router.post( '/account/reset/:token', authController.confirmedPasswords, authController.update )

Here is the module (controller) which has the logic to deal with these tasks:

const mongoose = require( 'mongoose' )
const User = mongoose.model( 'User' )
const promisify = require( 'es6-promisify' )

const findUserByTokenAndDate = ( token, date ) => {
    return User.findOne( {
        resetPasswordToken: token,
        resetPasswordExpires: { $gt: date },
    } )
}

exports.reset = async ( req, res ) => {
    // const user = await User.findOne( {
    //     resetPasswordToken: req.params.token,
    //     resetPasswordExpires: { $gt: Date.now() },
    // } )
    const user = await findUserByTokenAndDate( req.params.token, Date.now() )

    if ( ! user )
    {
        req.flash( 'error', 'Password reset token is invalid or has expired' )
        return res.redirect( '/login' )
    }

    res.render( 'reset', { title: 'Reset your Password' } )
}

exports.confirmedPasswords = ( req, res, next ) => {
    if ( req.body.password === req.body['password-confirm'] )
    {
        return next()
    }

    req.flash( 'error', 'Passwords do not match!' )
    res.redirect( 'back' )
}

exports.update = async ( req, res ) => {
    // const user = await User.findOne( {
    //     resetPasswordToken: req.params.token,
    //     resetPasswordExpires: { $gt: Date.now() },
    // } )
    const user = await findUserByTokenAndDate( req.params.token, Date.now() )

    if ( ! user )
    {
        req.flash( 'error', 'Password reset is invalid or has expired' )
        return res.redirect( '/login' )
    }

    const setPassword = promisify( user.setPassword, user )
    await setPassword( req.body.password )

    user.resetPasswordToken = undefined
    user.resetPasswordExpires = undefined
    const updateUser = await user.save()
    await req.login( updateUser ) // This is to tell password.js which user to log in

    req.flash( 'success', 'Your password has been reset! You are now logged in' )
    res.redirect( '/' )
}

The commented code is the one I reused with the function findUserByTokenAndDate.

Is this more testable than other solutions?
Would you have created a new module, just for keeping code like the one in the function findUserByTokenAndDate?
Is this good practice?

Please, note that this is a very simple piece of code that might not even worth to reuse, but I'm looking for good practices in the case of a more complex or larger piece of code.

Thanks!

Cisco · Accepted Answer · 2017-07-17 13:18:32Z

1

Is this more testable than other solutions?

Depends on who you ask. Yes for me since you're reusing the same logic in other places, it makes sense to abstract it out to it's own function. However, if it's only being used in two places then you don't necessarily have/need to extract it to avoid duplicates. It saves time being able to see the logic of the code right there rather than having to track down the module it's in.

Would you have created a new module, just for keeping code like the one in the function findUserByTokenAndDate?

I would have created a separate module with any/all utility functions such as findUserByTokenAndDate. Then I can just test just the utility functions and nothing else.

Is this good practice?

Some would say that it's over-engineering.

answered Jul 17, 2017 at 13:18

Cisco

23.5k6 gold badges44 silver badges67 bronze badges

Sign up to request clarification or add additional context in comments.

Collectives™ on Stack Overflow

Reusing code in Node.js module

1 Answer 1

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Your Answer

Sign up or log in

Post as a guest

Related