From 31b27b56ca9ef54523524000b1c0d35dc3c5ba7d Mon Sep 17 00:00:00 2001
From: Jeremy Nagel <jeremy@gofar.co>
Date: Sat, 30 May 2020 15:12:21 +1000
Subject: [PATCH] Fix test

---
 lib/xsql.js    | 43 ++++++++++++++++++++++++++++++++-----------
 tests/tests.js | 17 +++++++----------
 2 files changed, 39 insertions(+), 21 deletions(-)

diff --git a/lib/xsql.js b/lib/xsql.js
index f0ca5e1a26..2fce28455b 100644
--- a/lib/xsql.js
+++ b/lib/xsql.js
@@ -348,13 +348,16 @@ class Xsql {
         if (i) {
           queryParamsObj.query += ", ";
         }
-        if (orderByCols[i][0] === "-") {
-          let len = orderByCols[i].length;
-          queryParamsObj.query += " ?? DESC";
-          queryParamsObj.params.push(orderByCols[i].substring(1, len));
+        const aggregationFunction = this.getAggregationFunction(orderByCols[i]);
+        const columnName = this.getColumnNameWithoutAggregationFunctions(orderByCols[i]);
+        const orderByDirection = orderByCols[i][0] === "-" ? 'DESC' : 'ASC';
+
+        if (aggregationFunction) {
+          queryParamsObj.query += `${aggregationFunction}(??) ${orderByDirection}`;
+          queryParamsObj.params.push(columnName);
         } else {
-          queryParamsObj.query += " ?? ASC";
-          queryParamsObj.params.push(orderByCols[i]);
+          queryParamsObj.query += `?? ${orderByDirection}`;
+          queryParamsObj.params.push(columnName);
         }
       }
     }
@@ -376,7 +379,7 @@ class Xsql {
 
   getColumnsForGroupBy(tableName, reqQueryParams, queryParamsObj) {
     const updatedQueryParams = Object.assign({}, reqQueryParams);
-    if ("_groupbyfields" in updatedQueryParams) {
+    if ('_groupbyfields' in updatedQueryParams) {
       // allows you to group by different fields than you have in the select
       updatedQueryParams['_fields'] = updatedQueryParams['_groupbyfields'];
     }
@@ -424,21 +427,39 @@ class Xsql {
       if (i) {
         queryParamsObj.query += ",";
       }
-      queryParamsObj.query += "??";
-      queryParamsObj.params.push(cols[i]);
+      const aggregationFunction = this.getAggregationFunction(cols[i]);
+
+      if (aggregationFunction) {
+        queryParamsObj.query += `${aggregationFunction}(??)`;
+        const columnName = this.getColumnNameWithoutAggregationFunctions(cols[i]);
+        queryParamsObj.params.push(columnName);
+      } else {
+        queryParamsObj.query += "??";
+        queryParamsObj.params.push(cols[i]);
+      }
     }
 
     return cols.join(",");
   }
 
+  getAggregationFunction(rawColumnName) {
+    const AGGREGATION_FUNCTION_REGEX = /^[-]?(AVG|BIT_AND|BIT_OR|BIT_XOR|COUNT|COUNTDISTINCT|GROUP_CONCAT|JSON_ARRAYAGG|JSON_OBJECTAGG|MAX|MIN|STD|STDDEV|STDDEV_POP|STDDEV_SAMP|SUM|VAR_POP|VAR_SAMP|VARIANCE)\((.*)\)$/i;
+    const aggFuncMatch = rawColumnName.match(AGGREGATION_FUNCTION_REGEX);
+    if (aggFuncMatch && aggFuncMatch.length === 3) {
+      // match will look like (3) ["AVG(timestamp)", "AVG", "timestamp", index: 0, input: "AVG(timestamp)", groups: undefined]
+      return aggFuncMatch[1];
+    }
+    return null;
+  }
+
   getColumnNameWithoutAggregationFunctions(rawColumnName) {
-    const AGGREGATION_FUNCTION_REGEX = /^(AVG|BIT_AND|BIT_OR|BIT_XOR|COUNT|COUNTDISTINCT|GROUP_CONCAT|JSON_ARRAYAGG|JSON_OBJECTAGG|MAX|MIN|STD|STDDEV|STDDEV_POP|STDDEV_SAMP|SUM|VAR_POP|VAR_SAMP|VARIANCE)\((.*)\)$/;
+    const AGGREGATION_FUNCTION_REGEX = /^[-]?(AVG|BIT_AND|BIT_OR|BIT_XOR|COUNT|COUNTDISTINCT|GROUP_CONCAT|JSON_ARRAYAGG|JSON_OBJECTAGG|MAX|MIN|STD|STDDEV|STDDEV_POP|STDDEV_SAMP|SUM|VAR_POP|VAR_SAMP|VARIANCE)\((.*)\)$/i;
     const aggFuncMatch = rawColumnName.match(AGGREGATION_FUNCTION_REGEX);
     if (aggFuncMatch && aggFuncMatch.length === 3) {
       // match will look like (3) ["AVG(timestamp)", "AVG", "timestamp", index: 0, input: "AVG(timestamp)", groups: undefined]
       return aggFuncMatch[2];
     }
-    return rawColumnName;
+    return rawColumnName.replace(/-/, '');
   }
 
   removeUnknownColumns(inputColumns, tableName) {
diff --git a/tests/tests.js b/tests/tests.js
index b5e9f87913..8eeda2039f 100644
--- a/tests/tests.js
+++ b/tests/tests.js
@@ -1178,23 +1178,20 @@ describe("xmysql : tests", function() {
     }
   );
 
-  it(
-    "GET " +
-      apiPrefix +
-      "customers/groupby?_fields=city&_sort=city&_groupbyfields=country should PASS",
+  it(`GET ${apiPrefix}customers/groupby?_fields=city&_sort=city&_groupbyfields=country should PASS`,
     function(done) {
-      //post to an url with data
       agent
-        .get(apiPrefix + "customers/groupby?_fields=avg(creditLimit),country,city&_sort=city&_groupbyfields=country,city") //enter url
-        .expect(200) //200 for success 4xx for failure
+        .get(apiPrefix + "customers/groupby?_fields=avg(creditLimit),country,city&_sort=-avg(creditLimit),city&_groupbyfields=country,city")
+        .expect(200)
         .end(function(err, res) {
-          // Handle /api/v error
           if (err) {
             return done(err);
           }
 
-          //validate response
-          res.body[0]["city"].should.be.equals("Aachen");
+          res.body[0]['avg(`creditLimit`)'].should.be.equals(210500);
+          res.body[0]["country"].should.be.equals("USA");
+          res.body[0]["city"].should.be.equals("San Rafael");
+          res.body[0]["_count"].should.be.equals(1);
           res.body.length.should.be.equals(95);
 
           return done();