3

I'm currently working on a project in express and I'm using knex.js to handle migrations and queries.

I'm still trying to grasp the concept of promises and how I can run multiple queries with knex.

I have the following code which inserts a new record into my database, this is located in my Unit model file.

this.addUnit = function(unit_prefixV, unit_nameV, unit_descriptionV, profile_id) {
         return new Promise(function(resolve, reject) {
             knex.insert({ unit_prefix: unit_prefixV, unit_name: unit_nameV, unit_description: unit_descriptionV })
                .into('units').then(function(unit) {
                    resolve(unit)
                }).catch(function(error) {
                    reject(error)
                })
         })
    }

In my routes.js file I then call this on a post request, like so:

app.post('/dashboard/unit/add', ensureAuthenticated, function(req, res) {
        let postErrors = []
        if (req.body.unit_name.trim() == "") {
            postErrors.push('Unit name cannot be empty.')
        }

        if (req.body.unit_prefix.trim() == "") {
            postErrors.push('Unit prefix cannot be empty.')
        }

        if (req.body.unit_description.trim() == "") {
            postErrors.push('Unit description cannot be empty.')
        }

        if (postErrors.length > 0) {
            res.render('addUnit', { errors: postErrors, user: req.user })
        } else {
            unitModel.addUnit(req.body.unit_prefix.trim(), req.body.unit_name.trim(), req.body.unit_description.trim(), req.session.passport.user.id).then(function(unit) {
                res.redirect('/dashboard')
            })
        }
    })

This successfully inserts a new record into my units table, however, I would like to select the user id from the users table with the matching profile_id and then insert another record into my users_units table. All within the this.addUnit function.

For reference my users table consists of:

  • id
  • google_id

my users_units table consists of:

  • user_id
  • unit_id

I've made an attempt to chain the queries but it only executed the initial insert query and not the others. Here is that rather ugly attempt:

this.addUnit = function(unit_prefixV, unit_nameV, unit_descriptionV, profile_id) {
         return new Promise(function(resolve, reject) {
             knex.insert({ unit_prefix: unit_prefixV, unit_name: unit_nameV, unit_description: unit_descriptionV })
                .into('units').then(function(unit) {
                    knex('users').where({ "google_id": profile_id }).select('id').then(function(uid) {
                        knex.insert({ user_id: uid, unit_id: unit }).into('users_units').then(function(user_units) {
                            resolve(user_unit)
                        }).catch(function(error) {
                            reject(error)
                        })
                        resolve(uid)
                    })
                    console.log(unit)
                    resolve(unit)
                }).catch(function(error) {
                    reject(error)
                })
         })
    }

Any help will be greatly appreciated!

Matt Kent
  • 1,145
  • 1
  • 11
  • 26
  • 1
    Avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! – Bergi Mar 24 '18 at 19:25
  • Here is some page explaining how to chain promises, knex promises is no different https://developers.google.com/web/fundamentals/primers/promises#chaining – Mikael Lepistö Mar 24 '18 at 21:06

1 Answers1

4

You're nearly there. There are just a few simple point to grasp :

  • A Promise can be reolved only once
  • An explicit Promise is not needed anyway because a naturally occurring promise can be returned
  • return a Promise at each stage ...
  • ... until the innermost stage, from which the returned value is the finally delivered result.
  • Errors needn't be eplicitly handled unless you want to inject your own custom error messages or take remedial action.

Having taken all that on board, you might write :

this.addUnit = function(unit_prefixV, unit_nameV, unit_descriptionV, profile_id) {
    return knex.insert({ 'unit_prefix':unit_prefixV, 'unit_name':unit_nameV, 'unit_description':unit_descriptionV }).into('units')
 // ^^^^^^
    .then(function(unit) {
        return knex('users').where({ 'google_id':profile_id }).select('id')
     // ^^^^^^
        .then(function(uid) {
            return knex.insert({ 'unit_id':unit, 'user_id':uid }).into('users_units')
         // ^^^^^^
            .then(function(user_units) {
                return { 'unit_id':unit, 'user_id':uid, 'user_units':user_units };
             // ^^^^^^
            });
        });
    });
}

If the caller is interested only in success/failure of the process and not the full { unit, uid, user_units } object, then the innermost .then() can be omitted :

this.addUnit = function(unit_prefixV, unit_nameV, unit_descriptionV, profile_id) {
    return knex.insert({ 'unit_prefix':unit_prefixV, 'unit_name':unit_nameV, 'unit_description':unit_descriptionV }).into('units')
    .then(function(unit) {
        return knex('users').where({ 'google_id':profile_id }).select('id')
        .then(function(uid) {
            return knex.insert({ 'unit_id':unit, 'user_id':uid }).into('users_units');
        });
    });
}

The promise returned by .addUnit() will still deliver user_units, which the caller can use or ignore.

There's a major proviso to these solutions (and others); a multi-stage update query like this should really be wrapped in a transaction - ie something that allows earlier stages to be rolled back. Otherwise a failure part way through is likely to leave the database in some indeterminate state. This answer is as good a starting point as any.

Roamer-1888
  • 19,138
  • 5
  • 33
  • 44
  • Thank you so much for your answer. One thing is I get the error `Unhandled rejection Error: ER_TRUNCATED_WRONG_VALUE_FOR_FIELD: Incorrect integer value: '[object Object]' for column 'user_id' at row 1` – Matt Kent Mar 24 '18 at 21:56
  • Looks like the inner `knex.insert()` is wrongly constructed. `uid` appears to be Object but the query expects Integer. Could be a consequence of a mistake at the previous stage - `knex('users').where(...)`? Sorry I can't be more definitive. – Roamer-1888 Mar 24 '18 at 23:39
  • Nesting promises is a terrible idea and a well known [antipattern](https://stackoverflow.com/a/23803744/10094651). They should always be chained. – tlt Apr 09 '20 at 20:48
  • 1
    @denkquer, a flat chain is indeed preferred, *when achievable*. For that reason, nesting is *not always* a terrible idea; in fact quite the opposite. If you look carefully at my code, you will see that I chose *not* to flatten for [a very good reason](https://stackoverflow.com/a/28250687/3478010). The innermost `knex.insert()` needs `unit` to be in scope. This would not happen if the chain was flattened. BTW, you have linked to the "explicit-construction anti-pattern", which is a very different scenario. – Roamer-1888 Apr 10 '20 at 01:05