Cleanup

palantir · Dec 9, 2024 · 40ee625 · 40ee625
1 parent efbed3d
commit 40ee625
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 22 deletions.
diff --git a/packages/api/src/derivedProperties/DeriveObjectSet.ts b/packages/api/src/derivedProperties/DeriveObjectSet.ts
@@ -52,22 +52,23 @@ interface FilterableDeriveObjectSet<
   ) => this;
 }
 
-export type StringDeriveAggregateOption =
+type CollectAggregations = "collectSet" | "collectList";
+
+type BaseDeriveAggregations =
   | "approximateDistinct"
   | "exactDistinct"
-  | "approximatePercentile"
-  | "collectToSet"
-  | "collectToList";
+  | "approximatePercentile";
+
+export type StringDeriveAggregateOption =
+  | BaseDeriveAggregations
+  | CollectAggregations;
 export type NumericDeriveAggregateOption =
   | "min"
   | "max"
   | "sum"
   | "avg"
-  | "collectToSet"
-  | "collectToList"
-  | "approximateDistinct"
-  | "exactDistinct"
-  | "approximatePercentile";
+  | BaseDeriveAggregations
+  | CollectAggregations;
 
 interface AggregatableDeriveObjectSet<
   Q extends ObjectOrInterfaceDefinition,
@@ -81,14 +82,13 @@ interface AggregatableDeriveObjectSet<
   >(
     aggregationSpecifier: V,
     opts?: V extends `${any}:${infer P}`
-      ? P extends "collectToSet" | "collectToList" ? { limit: number }
+      ? P extends CollectAggregations ? { limit: number }
       : P extends "approximatePercentile" ? { percentile: number }
       : never
       : never,
   ) => DerivedPropertyDefinition<
-    V extends `${infer N}:${infer P}` ? P extends
-        | "collectToList"
-        | "collectToSet" ? PropertyDef<
+    V extends `${infer N}:${infer P}`
+      ? P extends CollectAggregations ? PropertyDef<
           CompileTimeMetadata<Q>["properties"][N]["type"],
           "nullable",
           "array"

diff --git a/packages/client/src/derivedProperties/createDerivedObjectSet.ts b/packages/client/src/derivedProperties/createDerivedObjectSet.ts
@@ -85,13 +85,10 @@ export function createDeriveObjectSet<Q extends ObjectOrInterfaceDefinition>(
             approximatePercentile: opt?.percentile ?? .5,
           };
           break;
-        case "collectToSet":
-        case "collectToList":
-          const collectionType = aggregationOperation === "collectToSet"
-            ? "collectSet"
-            : "collectList";
+        case "collectSet":
+        case "collectList":
           aggregationOperationDefinition = {
-            type: collectionType,
+            type: aggregationOperation,
             selectedPropertyApiName: aggregationPropertyName,
             limit: opt?.limit ?? 100,
           };

diff --git a/packages/client/src/objectSet/ObjectSet.test.ts b/packages/client/src/objectSet/ObjectSet.test.ts
@@ -479,11 +479,41 @@ describe("ObjectSet", () => {
           // @ts-expect-error
           base.pivotTo("lead").aggregate("employeeId:notAnOp");
 
+          base.pivotTo("lead").aggregate("employeeId:collectList");
+
           return base.pivotTo("lead").aggregate("employeeId:sum");
         },
       });
     });
 
+    // Executed code fails since we're providing bad strings to the function
+    it("correctly narrows types of options for aggregate functions", () => {
+      client(Employee).withProperties({
+        "derivedPropertyName": (base) => {
+          // @ts-expect-error
+          base.pivotTo("lead").aggregate("employeeId:approximateDistinct", {
+            limit: 1,
+          });
+
+          base.pivotTo("lead").aggregate("employeeId:collectList", {
+            limit: 1,
+          });
+
+          base.pivotTo("lead").aggregate("employeeId:collectSet", { limit: 1 });
+
+          base.pivotTo("lead").aggregate("employeeId:collectList", {
+            // @ts-expect-error
+            percentile: 1,
+          });
+
+          return base.pivotTo("lead").aggregate(
+            "employeeId:approximatePercentile",
+            { percentile: 0.5 },
+          );
+        },
+      });
+    });
+
     it("correctly narrows types of selectProperty function", () => {
       client(Employee).withProperties({
         "derivedPropertyName": (base) => {
@@ -503,14 +533,14 @@ describe("ObjectSet", () => {
 
       client(Employee).withProperties({
         "derivedPropertyName": (base) =>
-          base.pivotTo("lead").aggregate("employeeId:collectToList"),
+          base.pivotTo("lead").aggregate("employeeId:collectList"),
       }).where({ "derivedPropertyName": { "$isNull": false } })
         // @ts-expect-error
         .where({ "derivedPropertyName": { "$eq": [1, 2] } });
 
       client(Employee).withProperties({
         "derivedPropertyName": (base) =>
-          base.pivotTo("lead").aggregate("employeeId:collectToSet"),
+          base.pivotTo("lead").aggregate("employeeId:collectSet"),
       }).where({ "derivedPropertyName": { "$isNull": false } })
         // @ts-expect-error
         .where({ "derivedPropertyName": { "$eq": [1, 2] } });
@@ -601,7 +631,7 @@ describe("ObjectSet", () => {
             }),
 
           "secondaryDerivedPropertyName": (base) =>
-            base.pivotTo("lead").aggregate("fullName:collectToSet", {
+            base.pivotTo("lead").aggregate("fullName:collectSet", {
               limit: 10,
             }),
         };