Refactor KnexJS Multiple Promises

I am running multiple select queries using Knexjs and promises. I need all the queries to return a value before I send the results, which I have been able to achieve. However I don't think the code is very optimised.

knex(table).select('CRMContactId').where('FCIRecognition', '<', -49.00)
            .then(function(results) {

                data.largeclawbacks = results.length;

                knex(table).select('CRMContactId').where('PlanStatus', 'Out of Force').andWhere(function(){this.whereNot('IncomeType', 'Fund Based Commission').andWhereNot('IncomeType', 'Renewal Commission')})
                    .then(function(results) {

                        data.outofforce = results.length;

                        knex(table).select('CRMContactId').where('GroupOneCaption', 'Tier 2').andWhereNot('Payaways Made/Received', 'Payaway Made')
                            .andWhere((builder) => builder.whereIn('Plantype', ['Flexible Benefits','General Insurance','Group Critical Illness','Group Death In Service','Group Dental Insurance','Group Healthcare Cash Plan','Group Income Protection','Group Life','Group Life;Group Income Protection','Group PMI','Group Travel Insurance']))
                            .andWhereNot('Payable', 0)
                            .then(function(results) {

                                data.tier2 = results.length;

                                knex(table).select('CRMContactId').where((builder) => builder.where('GroupOneCaption', 'Tier 3').orWhere('GroupOneCaption', 'Tier 4')).
                                andWhereNot('Payaways Made/Received', 'Payaway Made')
                                    .andWhere((builder) => builder.whereIn('Plantype', ['Accident Sickness & Unemployment Insurance','AVC','Discretionary Managed Service','Endowment','Enhanced Pension Annuity','Executive Pension Plan','FSAVC','General Investment Account','Income Drawdown','Income Protection','Individual Retirement Account', 'Insurance / Investment Bond','Investment Trust','ISA','Long Term Care','Maximum Investment Plan','Money Purchase Contracted','OEIC / Unit Trust','Offshore Bond','Pension Annuity','Pension Term Assurance','Personal Equity Plan','Personal Pension Plan','Regular Savings Plan','Relevant Life Policy','s226 RAC','s32 Buyout Bond','Savings Account','SIPP','SSAS','Stakeholder Individual','Term Protection','Venture Capital Trust','Whole Of Life','Wrap']))
                                    .andWhereNot('Payable', 0)
                                    .then(function(results) {

                                        data.tier3 = results.length;

                                        knex(table).select('CRMContactId').where('FCIRecognition', '>', 500.00).andWhere('IncomeType', 'Renewal Commission')
                                            .then(function(results) {

                                                data.largerenewal = results.length;

                                                knex.raw(`SELECT ContactName AS Adviser, FORMAT(SUM(Payable),2) AS 'Renewal Income' FROM fci_test WHERE IncomeType IN ("Renewal Commission","Fund Based Commission","Ongoing Fee") AND \`Payaways Made/Received\` != 'Payaway Made' GROUP BY ContactName`)
                                                    .then(function(results){

                                                        data.renewalincome = results[0];
                                                        res.send(data)
                                                    })

                                            })
                                    })
                            })
                    })
            })

I am sure there is a better way of coding this and getting the same result.

Solution

I would be concerned first about the readability and then about performance. By first making the code more readable is easier to see which optimization could be applied.

After some refactoring we can get to a code similar to:

knex(table).select('CRMContactId')
  .where('FCIRecognition', '<', -49.00)
  .then(function(results) {

    data.largeclawbacks = results.length;

    knex(table).select('CRMContactId')
      .where('PlanStatus', 'Out of Force')
      .andWhere((builder) => {
        builder.whereNot('IncomeType', 'Fund Based Commission')
          .andWhereNot('IncomeType', 'Renewal Commission');
      })
      .then(function(results) {

        data.outofforce = results.length;

        knex(table).select('CRMContactId')
          .where('GroupOneCaption', 'Tier 2')
          .andWhereNot('Payaways Made/Received', 'Payaway Made')
          .whereIn('Plantype', tier2PlanTypes)
          .andWhereNot('Payable', 0)
          .then(function(results) {

            data.tier2 = results.length;

            knex(table).select('CRMContactId')
              .whereIn('GroupOneCaption', ['Tier 3', 'Tier 4'])
              .andWhereNot('Payaways Made/Received', 'Payaway Made')
              .whereIn('Plantype', tier3PlanTypes)
              .andWhereNot('Payable', 0)
              .then(function(results) {

                data.tier3 = results.length;

                knex(table).select('CRMContactId')
                  .where('FCIRecognition', '>', 500.00)
                  .andWhere('IncomeType', 'Renewal Commission')
                  .then(function(results) {

                    data.largerenewal = results.length;

                    knex.raw(`SELECT ContactName AS Adviser, FORMAT(SUM(Payable),2) AS 'Renewal Income' FROM fci_test 
                              WHERE IncomeType IN ("Renewal Commission","Fund Based Commission","Ongoing Fee") 
                                AND \`Payaways Made/Received\` != 'Payaway Made' GROUP BY ContactName`)
                      .then(function(results){
                          data.renewalincome = results[0];
                          res.send(data)
                      });
                  })
              })
          })
      })
  });

It does not look like much, but I can see more clear that all queries are independent of each other (I will use this to optimize)

After, further refactoring I save every query in a constant and then use Promise.all to issue all of them at once and the way for them to finish in order to send the response.

const largeclawbacksQuery = knex(table).select('CRMContactId')
  .where('FCIRecognition', '<', -49.00);

const outofforceQuery = knex(table).select('CRMContactId')
  .where('PlanStatus', 'Out of Force')
  .andWhere((builder) => {
    builder.whereNot('IncomeType', 'Fund Based Commission')
      .andWhereNot('IncomeType', 'Renewal Commission')
  });

const tier2Query = knex(table).select('CRMContactId')
    .where('GroupOneCaption', 'Tier 2')
    .andWhereNot('Payaways Made/Received', 'Payaway Made')
    .whereIn('Plantype', tier2PlanTypes)
    .andWhereNot('Payable', 0);

const tier3Query = knex(table).select('CRMContactId')
  .whereIn('GroupOneCaption', ['Tier 3', 'Tier 4'])
  .andWhereNot('Payaways Made/Received', 'Payaway Made')
  .whereIn('Plantype', tier3PlanTypes)
  .andWhereNot('Payable', 0);

const largerenewalQuery = knex(table).select('CRMContactId')
  .where('FCIRecognition', '>', 500.00)
  .andWhere('IncomeType', 'Renewal Commission');

const renewalincomeQuery = knex.raw(
  `SELECT ContactName AS Adviser, FORMAT(SUM(Payable),2) AS 'Renewal Income' FROM fci_test 
    WHERE IncomeType IN ("Renewal Commission","Fund Based Commission","Ongoing Fee") 
      AND \`Payaways Made/Received\` != 'Payaway Made' GROUP BY ContactName`
);

Promise.all([largeclawbacksQuery, outofforceQuery, tier2Query, tier3Query, largerenewalQuery, renewalincomeQuery])
  .then((result) => {
    res.send({
      largeclawbacks: result[0].length,
      outofforce: result[1].length,
      tier2: results[2].length,
      tier3: results[3].length,
      largerenewal: results[4].length,
      renewalincome: results[4][0],
    });
  });

Important points:

whereIn can be chained and they will translate to sql as WHERE afield IN avalues AND bfield IN bvalues
Line length can improve readability and in turn make code easier to read
We can wait for a query builder to finish its query if we treat it as a promise

Further improvement:

Every method (whereIn, where, orWhere, etc) returns a query builder partial query can be reused by cloning a query builder instance as explained here. This could help you define a base query for tier2Query and tier3Query.
We can wait for promises to resolve using a better API, using something like promise-all-properties
Instead of querying all records to get the value of length you could ask for the COUNT directly, this will improve performance.