From b0633898a39e09b24a6e87e20705123cb9bf39b1 Mon Sep 17 00:00:00 2001
From: Stephanie Yang <stephanie@pilosa.com>
Date: Fri, 30 Jul 2021 16:11:06 -0500
Subject: [PATCH 1/3] add ability to set an aggregate sort on int fields

---
 .../GroupByBuilder/GroupByBuilder.tsx         | 26 +++---
 .../QueryBuilder/GroupBySort/GroupBySort.tsx  | 83 ++++++++++---------
 lattice/src/App/QueryBuilder/utils.ts         |  9 +-
 3 files changed, 62 insertions(+), 56 deletions(-)

diff --git a/lattice/src/App/QueryBuilder/GroupByBuilder/GroupByBuilder.tsx b/lattice/src/App/QueryBuilder/GroupByBuilder/GroupByBuilder.tsx
index c0486352f..c2129c755 100644
--- a/lattice/src/App/QueryBuilder/GroupByBuilder/GroupByBuilder.tsx
+++ b/lattice/src/App/QueryBuilder/GroupByBuilder/GroupByBuilder.tsx
@@ -157,22 +157,18 @@ export const GroupByBuilder: FC<GroupByBuilderProps> = ({
         </div>
         <GroupBySort
           sort={sort ? sort : []}
-          onUpdate={(value) => {
+          onUpdate={(updatedSort) => {
             let isInvalid = false;
-            if (
-              value.length > 0 &&
-              value[0].sortValue.includes('sum') &&
-              !value[0].field
-            ) {
-              isInvalid = true;
-            } else if (
-              value.length > 1 &&
-              value[1].sortValue.includes('sum') &&
-              !value[1].field
-            ) {
-              isInvalid = true;
-            }
-            onChange({ ...query, sort: value, isInvalid });
+            updatedSort.forEach((s) => {
+              if (
+                ['sum', 'aggregate'].includes(s.sortValue) &&
+                !s.sum &&
+                !s.aggregate
+              ) {
+                isInvalid = true;
+              }
+            });
+            onChange({ ...query, sort: updatedSort, isInvalid });
           }}
           fields={table.fields
             .filter((field) => field.options.type === 'int')
diff --git a/lattice/src/App/QueryBuilder/GroupBySort/GroupBySort.tsx b/lattice/src/App/QueryBuilder/GroupBySort/GroupBySort.tsx
index c75f9320b..ce66494d6 100644
--- a/lattice/src/App/QueryBuilder/GroupBySort/GroupBySort.tsx
+++ b/lattice/src/App/QueryBuilder/GroupBySort/GroupBySort.tsx
@@ -4,7 +4,8 @@ import css from './GroupBySort.module.scss';
 
 export type SortOption = {
   sortValue: string;
-  field?: string;
+  sum?: string;
+  aggregate?: string;
 };
 
 type GroupBySortProps = {
@@ -28,25 +29,23 @@ export const GroupBySort: FC<GroupBySortProps> = ({
     { label: 'Count (desc)', value: 'count desc' },
     { label: 'Count (asc)', value: 'count asc' },
     { label: 'Sum (desc)', value: 'sum desc' },
-    { label: 'Sum (asc)', value: 'sum asc' }
+    { label: 'Sum (asc)', value: 'sum asc' },
+    { label: 'Aggregate (desc)', value: 'aggregate desc' },
+    { label: 'Aggregate (asc)', value: 'aggregate asc' }
   ];
 
   const onPrimaryChange = (value: string) => {
     const split = value.split(' ');
-    const isSum = split[0] === 'sum';
-    const fieldValue = hasPrimary ? sort[0].field : undefined;
 
     if (!hasSecondary) {
-      onUpdate([{ sortValue: value, field: isSum ? fieldValue : undefined }]);
-    } else if (hasSecondary && sort[1].sortValue.includes(split[0])) {
-      onUpdate([{ sortValue: value, field: isSum ? fieldValue : undefined }]);
+      onUpdate([{ sortValue: value }]);
+    } else if (sort[1].sortValue.includes(split[0])) {
+      onUpdate([{ sortValue: value }]);
     } else {
-      onUpdate([
-        { sortValue: value, field: isSum ? fieldValue : undefined },
-        sort[1]
-      ]);
+      onUpdate([{ sortValue: value }, sort[1]]);
     }
   };
+
   const onSecondaryChange = (value: string) => {
     let sortOp = { sortValue: value };
 
@@ -56,15 +55,31 @@ export const GroupBySort: FC<GroupBySortProps> = ({
     onUpdate([sort[0], sortOp]);
   };
 
-  const onUpdateSumField = (isPrimary: boolean, value: string) => {
+  const renderFieldSelect = (
+    sortFor: 'primary' | 'secondary',
+    sortType: 'sum' | 'aggregate'
+  ) => {
+    const sortIdx = sortFor === 'primary' ? 0 : 1;
+
+    return (
+      <Select
+        className={css.fieldSelect}
+        label="Field"
+        value={sort[sortIdx][sortType] ? sort[sortIdx][sortType] : ''}
+        options={fields}
+        onChange={(value) => onUpdateField(sortType, sortIdx, value)}
+        error={showErrors ? !sort[sortIdx][sortType] : false}
+      />
+    );
+  };
+
+  const onUpdateField = (
+    sortType: 'sum' | 'aggregate',
+    sortIdx: number,
+    value: string
+  ) => {
     let clone = [...sort];
-
-    if (isPrimary) {
-      clone[0].field = value;
-    } else {
-      clone[1].field = value;
-    }
-
+    clone[sortIdx][sortType] = value;
     onUpdate(clone);
   };
 
@@ -79,16 +94,11 @@ export const GroupBySort: FC<GroupBySortProps> = ({
           allowEmpty={true}
         />
 
-        {primary.includes('sum') ? (
-          <Select
-            className={css.fieldSelect}
-            label="Field"
-            value={sort[0].field ? sort[0].field : ''}
-            options={fields}
-            onChange={(value) => onUpdateSumField(true, value)}
-            error={showErrors ? !sort[0].field : false}
-          />
-        ) : null}
+        {primary.includes('sum')
+          ? renderFieldSelect('primary', 'sum')
+          : primary.includes('aggregate')
+          ? renderFieldSelect('primary', 'aggregate')
+          : null}
       </div>
 
       {hasPrimary ? (
@@ -103,16 +113,11 @@ export const GroupBySort: FC<GroupBySortProps> = ({
             allowEmpty={true}
           />
 
-          {secondary.includes('sum') ? (
-            <Select
-              className={css.fieldSelect}
-              label="Field"
-              value={sort[1].field ? sort[1].field : ''}
-              options={fields}
-              onChange={(value) => onUpdateSumField(false, value)}
-              error={showErrors ? !sort[1].field : false}
-            />
-          ) : null}
+          {secondary.includes('sum')
+            ? renderFieldSelect('secondary', 'sum')
+            : secondary.includes('aggregate')
+            ? renderFieldSelect('secondary', 'aggregate')
+            : null}
         </div>
       ) : null}
     </div>
diff --git a/lattice/src/App/QueryBuilder/utils.ts b/lattice/src/App/QueryBuilder/utils.ts
index 441b0ab8d..570d32845 100644
--- a/lattice/src/App/QueryBuilder/utils.ts
+++ b/lattice/src/App/QueryBuilder/utils.ts
@@ -121,12 +121,17 @@ export const stringifyGroupBy = (query: any) => {
   if (sort?.length > 0) {
     sortString = sort[0].sortValue;
     if (sort[0].sortValue.includes('sum')) {
-      aggregateString = `, aggregate=Sum(field=${sort[0].field})`;
+      aggregateString = `, aggregate=Sum(field=${sort[0].sum})`;
+    } else if(sort[0].sortValue.includes('aggregate')) {
+      aggregateString = `, aggregate=Count(Distinct(field=${sort[0].aggregate}))`;
     }
+
     if (sort.length > 1) {
       sortString = `${sortString}, ${sort[1].sortValue}`;
       if (sort[1].sortValue.includes('sum')) {
-        aggregateString = `, aggregate=Sum(field=${sort[1].field})`;
+        aggregateString = `, aggregate=Sum(field=${sort[1].sum})`;
+      } else if(sort[1].sortValue.includes('aggregate')) {
+        aggregateString = `, aggregate=Count(Distinct(field=${sort[1].aggregate}))`;
       }
     }
     sortString = `, sort="${sortString}"`;

From 97df326ac6305e0621070a9305ff092de1489462 Mon Sep 17 00:00:00 2001
From: Stephanie Yang <stephanie@pilosa.com>
Date: Fri, 30 Jul 2021 16:11:18 -0500
Subject: [PATCH 2/3] update test and add additional test for aggregate sort

---
 lattice/src/App/QueryBuilder/QueryBuilder.tsx |  5 +-
 lattice/src/App/QueryBuilder/utils.test.ts    | 49 ++++++++++++++++---
 lattice/src/App/QueryBuilder/utils.ts         | 31 +++++++++---
 3 files changed, 69 insertions(+), 16 deletions(-)

diff --git a/lattice/src/App/QueryBuilder/QueryBuilder.tsx b/lattice/src/App/QueryBuilder/QueryBuilder.tsx
index d4c890324..93d92a3c4 100644
--- a/lattice/src/App/QueryBuilder/QueryBuilder.tsx
+++ b/lattice/src/App/QueryBuilder/QueryBuilder.tsx
@@ -100,7 +100,10 @@ export const QueryBuilder: FC<QueryBuilderProps> = ({
         }
         break;
       case 'GroupBy':
-        queryString = stringifyGroupBy(query);
+        const groupBy = stringifyGroupBy(query);
+        if(!groupBy.error) {
+          queryString = groupBy.queryString;
+        }
         break;
       case 'Count':
         const cleanCountQuery = cleanRows();
diff --git a/lattice/src/App/QueryBuilder/utils.test.ts b/lattice/src/App/QueryBuilder/utils.test.ts
index d34e15ef9..daae5964d 100644
--- a/lattice/src/App/QueryBuilder/utils.test.ts
+++ b/lattice/src/App/QueryBuilder/utils.test.ts
@@ -746,7 +746,7 @@ describe('Query types', () => {
       groupByCall: {
         primary: 'primary'
       }
-    })).toEqual('GroupBy(Rows(primary))');
+    })).toEqual({ error: false, queryString: 'GroupBy(Rows(primary))' });
   });
 
   it('stringifies GroupBy queries with secondary grouping', () => {
@@ -755,7 +755,7 @@ describe('Query types', () => {
         primary: 'primary',
         secondary: 'secondary'
       }
-    })).toEqual('GroupBy(Rows(primary), Rows(secondary))');
+    })).toEqual({ error: false, queryString: 'GroupBy(Rows(primary), Rows(secondary))' });
   });
 
   it('stringifies GroupBy queries with a filter', () => {
@@ -764,7 +764,7 @@ describe('Query types', () => {
         primary: 'primary'
       },
       filter: 'filterString'
-    })).toEqual('GroupBy(Rows(primary), filter=filterString)');
+    })).toEqual({ error: false, queryString: 'GroupBy(Rows(primary), filter=filterString)' });
   });
 
   it('stringifies GroupBy queries with a sort', () => {
@@ -773,7 +773,7 @@ describe('Query types', () => {
         primary: 'primary'
       },
       sort: [{ sortValue: 'count desc' }]
-    })).toEqual('GroupBy(Rows(primary), sort=\"count desc\")');
+    })).toEqual({ error: false, queryString: 'GroupBy(Rows(primary), sort=\"count desc\")' });
   });
 
   it('stringifies GroupBy queries with a secondary sum sort', () => {
@@ -783,9 +783,20 @@ describe('Query types', () => {
       },
       sort: [
         { sortValue: 'count desc' },
-        { sortValue: 'sum asc', field: 'field' }
+        { sortValue: 'sum asc', sum: 'field' }
       ]
-    })).toEqual('GroupBy(Rows(primary), sort=\"count desc, sum asc\", aggregate=Sum(field=field))');
+    })).toEqual({ error: false, queryString: 'GroupBy(Rows(primary), sort=\"count desc, sum asc\", aggregate=Sum(field=field))' });
+  });
+
+  it('stringifies GroupBy queries with an aggregate sort', () => {
+    expect(stringifyGroupBy({
+      groupByCall: {
+        primary: 'primary'
+      },
+      sort: [
+        { sortValue: 'aggregate asc', aggregate: 'field' }
+      ]
+    })).toEqual({ error: false, queryString: 'GroupBy(Rows(primary), sort=\"aggregate asc\", aggregate=Count(Distinct(field=field)))' });
   });
 
   it('stringifies GroupBy queries with a filter and sort', () => {
@@ -795,8 +806,30 @@ describe('Query types', () => {
       },
       filter: 'filterString',
       sort: [
-        { sortValue: 'sum asc', field: 'field' }
+        { sortValue: 'sum asc', sum: 'field' }
       ]
-    })).toEqual('GroupBy(Rows(primary), filter=filterString, sort=\"sum asc\", aggregate=Sum(field=field))');
+    })).toEqual({ error: false, queryString: 'GroupBy(Rows(primary), filter=filterString, sort=\"sum asc\", aggregate=Sum(field=field))' });
+  });
+
+  it('fails when stringifying a GroupBy query with sum sort when field is not set', () => {
+    expect(stringifyGroupBy({
+      groupByCall: {
+        primary: 'primary'
+      },
+      sort: [
+        { sortValue: 'sum desc' }
+      ]
+    })).toEqual({ error: true, queryString: 'GroupBy(Rows(primary), sort=\"sum desc\")' });
+  });
+
+  it('fails when stringifying a GroupBy query with aggregate sort when field is not set', () => {
+    expect(stringifyGroupBy({
+      groupByCall: {
+        primary: 'primary'
+      },
+      sort: [
+        { sortValue: 'aggregate asc' }
+      ]
+    })).toEqual({ error: true, queryString: 'GroupBy(Rows(primary), sort=\"aggregate asc\")' });
   });
 });
diff --git a/lattice/src/App/QueryBuilder/utils.ts b/lattice/src/App/QueryBuilder/utils.ts
index 570d32845..c90cafaed 100644
--- a/lattice/src/App/QueryBuilder/utils.ts
+++ b/lattice/src/App/QueryBuilder/utils.ts
@@ -116,22 +116,39 @@ export const stringifyRowData = (rowCalls: RowGrouping[], operator?: Operator) =
 export const stringifyGroupBy = (query: any) => {
   const { groupByCall, filter, sort } = query;
 
+  let isInvalid = false;
   let sortString = '';
   let aggregateString = '';
   if (sort?.length > 0) {
     sortString = sort[0].sortValue;
     if (sort[0].sortValue.includes('sum')) {
-      aggregateString = `, aggregate=Sum(field=${sort[0].sum})`;
-    } else if(sort[0].sortValue.includes('aggregate')) {
-      aggregateString = `, aggregate=Count(Distinct(field=${sort[0].aggregate}))`;
+      if (sort[0].sum) {
+        aggregateString = `, aggregate=Sum(field=${sort[0].sum})`;
+      } else {
+        isInvalid = true;
+      }
+    } else if (sort[0].sortValue.includes('aggregate')) {
+      if (sort[0].aggregate) {
+        aggregateString = `, aggregate=Count(Distinct(field=${sort[0].aggregate}))`;
+      } else {
+        isInvalid = true;
+      }
     }
 
     if (sort.length > 1) {
       sortString = `${sortString}, ${sort[1].sortValue}`;
       if (sort[1].sortValue.includes('sum')) {
-        aggregateString = `, aggregate=Sum(field=${sort[1].sum})`;
-      } else if(sort[1].sortValue.includes('aggregate')) {
-        aggregateString = `, aggregate=Count(Distinct(field=${sort[1].aggregate}))`;
+        if (sort[1].sum) {
+          aggregateString = `, aggregate=Sum(field=${sort[1].sum})`;
+        } else {
+          isInvalid = true;
+        }
+      } else if (sort[1].sortValue.includes('aggregate')) {
+        if (sort[1].aggregate) {
+          aggregateString = `, aggregate=Count(Distinct(field=${sort[1].aggregate}))`;
+        } else {
+          isInvalid = true;
+        }
       }
     }
     sortString = `, sort="${sortString}"`;
@@ -141,7 +158,7 @@ export const stringifyGroupBy = (query: any) => {
     ? `GroupBy(Rows(${groupByCall.primary}), Rows(${groupByCall.secondary})${filterString}${sortString}${aggregateString})`
     : `GroupBy(Rows(${groupByCall.primary})${filterString}${sortString}${aggregateString})`;
 
-  return queryString;
+  return { error: isInvalid, queryString };
 }
 
 export const cleanupRows = (rowCalls: RowGrouping[]) => {

From 1609582ba1ee2ecd668b94d2b25526fcca146bc8 Mon Sep 17 00:00:00 2001
From: Stephanie Yang <stephanie@pilosa.com>
Date: Mon, 2 Aug 2021 19:01:26 -0500
Subject: [PATCH 3/3] Update aggregate label to cardinality

---
 lattice/src/App/QueryBuilder/GroupBySort/GroupBySort.tsx | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lattice/src/App/QueryBuilder/GroupBySort/GroupBySort.tsx b/lattice/src/App/QueryBuilder/GroupBySort/GroupBySort.tsx
index ce66494d6..c395a8ea6 100644
--- a/lattice/src/App/QueryBuilder/GroupBySort/GroupBySort.tsx
+++ b/lattice/src/App/QueryBuilder/GroupBySort/GroupBySort.tsx
@@ -30,8 +30,8 @@ export const GroupBySort: FC<GroupBySortProps> = ({
     { label: 'Count (asc)', value: 'count asc' },
     { label: 'Sum (desc)', value: 'sum desc' },
     { label: 'Sum (asc)', value: 'sum asc' },
-    { label: 'Aggregate (desc)', value: 'aggregate desc' },
-    { label: 'Aggregate (asc)', value: 'aggregate asc' }
+    { label: 'Cardinality (desc)', value: 'aggregate desc' },
+    { label: 'Cardinality (asc)', value: 'aggregate asc' }
   ];
 
   const onPrimaryChange = (value: string) => {