Merge pull request #79 from palantir/ssanjay/aggFixes

Minor aggregation syntax changes
palantir · Feb 20, 2024 · 1ff9eb5 · 1ff9eb5
2 parents 90e196c + e97d480
commit 1ff9eb5
Show file tree

Hide file tree

Showing 7 changed files with 50 additions and 48 deletions.
diff --git a/examples/basic/cli/src/examples/fetchAggregationForEmployeesGrouped.ts b/examples/basic/cli/src/examples/fetchAggregationForEmployeesGrouped.ts
@@ -82,21 +82,20 @@ fetchAggregationForEmployeesGrouped()
   expectType<
     TypeOf<
       Array<{
-        group: {
+        $group: {
           locationType: string | undefined;
         };
-        values: {
-          "employeeNumber": {
-            max: number | undefined;
-            avg: number | undefined;
-            min: number | undefined;
-          };
-          locationCity: {
-            approximateDistinct: number;
-          };
-          locationName: {
-            approximateDistinct: number;
-          };
+
+        "employeeNumber": {
+          max: number | undefined;
+          avg: number | undefined;
+          min: number | undefined;
+        };
+        locationCity: {
+          approximateDistinct: number;
+        };
+        locationName: {
+          approximateDistinct: number;
         };
       }>,
       typeof result
@@ -107,10 +106,10 @@ fetchAggregationForEmployeesGrouped()
   invariant(Array.isArray(result), "groups means we should get an array");
   invariant(Object.keys(result).length >= 1, "there should be one group");
   invariant(
-    "employeeNumber" in result[0].values
-      && "locationName" in result[0].values
-      && "locationCity" in result[0].values,
+    "employeeNumber" in result[0]
+      && "locationName" in result[0]
+      && "locationCity" in result[0],
     "The keys should be the expected ones",
   );
-  invariant(Object.keys(result[0].values.employeeNumber).length === 3);
+  invariant(Object.keys(result[0].employeeNumber).length === 3);
 }
diff --git a/examples/basic/cli/src/examples/fetchAggregationForEmployeesGroupedThin.ts b/examples/basic/cli/src/examples/fetchAggregationForEmployeesGroupedThin.ts
@@ -43,8 +43,8 @@ export async function fetchAggregationForEmployeesGroupedThin(
     },
   );
 
-  result[0].values.employeeNumber.avg;
-  result[0].group.locationType;
+  result[0].employeeNumber.avg;
+  result[0].$group.locationType;
 
   // const {action, authorized, lastResultIsValid, loading} = useOsdkAction(client.actions.doFoo);
   // isOk(result) && result.value.validation.result;
@@ -68,35 +68,34 @@ export async function fetchAggregationForEmployeesGroupedThin(
   expectType<
     TypeOf<
       Array<{
-        group: {
+        $group: {
           locationType: string | undefined;
         };
-        values: {
-          "employeeNumber": {
-            max: number | undefined;
-            avg: number | undefined;
-            min: number | undefined;
-          };
-          locationCity: {
-            approximateDistinct: number;
-          };
-          locationName: {
-            approximateDistinct: number;
-          };
+
+        "employeeNumber": {
+          max: number | undefined;
+          avg: number | undefined;
+          min: number | undefined;
+        };
+        locationCity: {
+          approximateDistinct: number;
+        };
+        locationName: {
+          approximateDistinct: number;
         };
       }>,
       typeof result
     >
   >(true);
 
-  result[0].values.employeeNumber;
+  result[0].employeeNumber;
   invariant(Array.isArray(result), "groups means we should get an array");
   invariant(Object.keys(result).length >= 1, "there should be one group");
   invariant(
-    "employeeNumber" in result[0].values
-      && "locationName" in result[0].values
-      && "locationCity" in result[0].values,
+    "employeeNumber" in result[0]
+      && "locationName" in result[0]
+      && "locationCity" in result[0],
     "The keys should be the expected ones",
   );
-  invariant(Object.keys(result[0].values.employeeNumber).length === 3);
+  invariant(Object.keys(result[0].employeeNumber).length === 3);
 }
diff --git a/examples/basic/sdk/src/syntax.test.ts b/examples/basic/sdk/src/syntax.test.ts
@@ -174,8 +174,8 @@ async function aggregateThingsWithGroupsAndHandleErrors() {
   });
 
   if (isOk(result)) {
-    for (const { group, values } of result) {
-      console.log(`${group.text}: ${values.priority.max}`);
+    for (const aggEntry of result) {
+      console.log(`${aggEntry.$group.text}: ${aggEntry.priority.max}`);
     }
   }
 }

diff --git a/packages/client/changelog/@unreleased/pr-79.v2.yml b/packages/client/changelog/@unreleased/pr-79.v2.yml
@@ -0,0 +1,5 @@
+type: improvement
+improvement:
+  description: Minor aggregation syntax changes
+  links:
+  - https://github.com/palantir/osdk-ts/pull/79
diff --git a/packages/client/src/object/aggregateOrThrow.test.ts b/packages/client/src/object/aggregateOrThrow.test.ts
@@ -165,16 +165,16 @@ describe("aggregateOrThrow", () => {
       },
       {
         select: {
-          text: "approximateDistinct",
+          id: "approximateDistinct",
         },
         groupBy: {
           text: "exact",
         },
       },
     );
     expectType<Array<any>>(grouped);
-    expectType<string | undefined>(grouped[0].group.text);
-    expectType<number>(grouped[0].values.text.approximateDistinct);
+    expectType<string | undefined>(grouped[0].$group.text);
+    expectType<number>(grouped[0].id.approximateDistinct);
   });
 
   it("works with where: todo", async () => {

diff --git a/packages/client/src/object/aggregateOrThrow.ts b/packages/client/src/object/aggregateOrThrow.ts
@@ -85,8 +85,8 @@ export async function aggregateOrThrow<
   const ret: AggregationResultsWithGroups<Q, AO["select"], any> = result.data
     .map((entry) => {
       return {
-        group: entry.group as any,
-        values: legacyToModernSingleAggregationResult(entry),
+        $group: entry.group as any,
+        ...legacyToModernSingleAggregationResult(entry),
       };
     }) as any; // fixme
 

diff --git a/packages/client/src/query/aggregations/AggregationResultsWithGroups.ts b/packages/client/src/query/aggregations/AggregationResultsWithGroups.ts
@@ -24,11 +24,10 @@ export type AggregationResultsWithGroups<
   Q extends ObjectOrInterfaceDefinition<any, any>,
   A extends AggregationClause<Q>,
   G extends GroupByClause<Q> | undefined,
-> = {
-  group: {
+> = ({
+  $group: {
     [P in keyof G & keyof Q["properties"]]: OsdkObjectPropertyType<
       Q["properties"][P]
     >;
   };
-  values: AggregationResultsWithoutGroups<Q, A>;
-}[];
+} & AggregationResultsWithoutGroups<Q, A>)[];