Merge branch 'dw-source-deletion' of github.com:PostHog/posthog into …

…dw-source-deletion
PostHog · Oct 17, 2024 · faa4d31 · faa4d31
2 parents 4422580 + 5e36f50
commit faa4d31
Show file tree

Hide file tree

Showing 4 changed files with 107 additions and 40 deletions.
diff --git a/ee/clickhouse/views/groups.py b/ee/clickhouse/views/groups.py
@@ -177,38 +177,32 @@ def property_definitions(self, request: request.Request, **kw):
 
         return response.Response(group_type_index_to_properties)
 
-    @extend_schema(
-        parameters=[
-            OpenApiParameter(
-                "group_type_index",
-                OpenApiTypes.INT,
-                description="Specify the group type to find property values of",
-                required=True,
-            ),
-            OpenApiParameter(
-                "key",
-                OpenApiTypes.STR,
-                description="Specify the property key to find values for",
-                required=True,
-            ),
-        ]
-    )
     @action(methods=["GET"], detail=False)
     def property_values(self, request: request.Request, **kw):
-        rows = sync_execute(
-            f"""
-            SELECT {trim_quotes_expr("tupleElement(keysAndValues, 2)")} as value
+        value_filter = request.GET.get("value")
+
+        query = f"""
+            SELECT {trim_quotes_expr("tupleElement(keysAndValues, 2)")} as value, count(*) as count
             FROM groups
             ARRAY JOIN JSONExtractKeysAndValuesRaw(group_properties) as keysAndValues
-            WHERE team_id = %(team_id)s AND group_type_index = %(group_type_index)s AND tupleElement(keysAndValues, 1) = %(key)s
-            GROUP BY tupleElement(keysAndValues, 2)
-            ORDER BY value ASC
-        """,
-            {
-                "team_id": self.team.pk,
-                "group_type_index": request.GET["group_type_index"],
-                "key": request.GET["key"],
-            },
-        )
+            WHERE team_id = %(team_id)s
+              AND group_type_index = %(group_type_index)s
+              AND tupleElement(keysAndValues, 1) = %(key)s
+              {f"AND {trim_quotes_expr('tupleElement(keysAndValues, 2)')} ILIKE %(value_filter)s" if value_filter else ""}
+            GROUP BY value
+            ORDER BY count DESC, value ASC
+            LIMIT 20
+        """
+
+        params = {
+            "team_id": self.team.pk,
+            "group_type_index": request.GET["group_type_index"],
+            "key": request.GET["key"],
+        }
+
+        if value_filter:
+            params["value_filter"] = f"%{value_filter}%"
+
+        rows = sync_execute(query, params)
 
-        return response.Response([{"name": name[0]} for name in rows])
+        return response.Response([{"name": name, "count": count} for name, count in rows])
diff --git a/ee/clickhouse/views/test/test_clickhouse_groups.py b/ee/clickhouse/views/test/test_clickhouse_groups.py
@@ -309,17 +309,71 @@ def test_property_values(self):
             group_key="org:6",
             properties={"industry": "technology"},
         )
+        create_group(
+            team_id=self.team.pk,
+            group_type_index=0,
+            group_key="org:7",
+            properties={"industry": "finance-technology"},
+        )
         create_group(
             team_id=self.team.pk,
             group_type_index=1,
             group_key="org:1",
             properties={"industry": "finance"},
         )
+
+        # Test without query parameter
         response_data = self.client.get(
             f"/api/projects/{self.team.id}/groups/property_values/?key=industry&group_type_index=0"
         ).json()
+        self.assertEqual(len(response_data), 3)
+        self.assertEqual(
+            response_data,
+            [
+                {"name": "finance", "count": 1},
+                {"name": "finance-technology", "count": 1},
+                {"name": "technology", "count": 1},
+            ],
+        )
+
+        # Test with query parameter
+        response_data = self.client.get(
+            f"/api/projects/{self.team.id}/groups/property_values/?key=industry&group_type_index=0&value=fin"
+        ).json()
+        self.assertEqual(len(response_data), 2)
+        self.assertEqual(response_data, [{"name": "finance", "count": 1}, {"name": "finance-technology", "count": 1}])
+
+        # Test with query parameter - case insensitive
+        response_data = self.client.get(
+            f"/api/projects/{self.team.id}/groups/property_values/?key=industry&group_type_index=0&value=TECH"
+        ).json()
         self.assertEqual(len(response_data), 2)
-        self.assertEqual(response_data, [{"name": "finance"}, {"name": "technology"}])
+        self.assertEqual(
+            response_data, [{"name": "finance-technology", "count": 1}, {"name": "technology", "count": 1}]
+        )
+
+        # Test with query parameter - no matches
+        response_data = self.client.get(
+            f"/api/projects/{self.team.id}/groups/property_values/?key=industry&group_type_index=0&value=healthcare"
+        ).json()
+        self.assertEqual(len(response_data), 0)
+        self.assertEqual(response_data, [])
+
+        # Test with query parameter - exact match
+        response_data = self.client.get(
+            f"/api/projects/{self.team.id}/groups/property_values/?key=industry&group_type_index=0&value=technology"
+        ).json()
+        self.assertEqual(len(response_data), 2)
+        self.assertEqual(
+            response_data, [{"name": "finance-technology", "count": 1}, {"name": "technology", "count": 1}]
+        )
+
+        # Test with different group_type_index
+        response_data = self.client.get(
+            f"/api/projects/{self.team.id}/groups/property_values/?key=industry&group_type_index=1&value=fin"
+        ).json()
+        self.assertEqual(len(response_data), 1)
+        self.assertEqual(response_data, [{"name": "finance", "count": 1}])
 
     def test_empty_property_values(self):
         create_group(

diff --git a/frontend/src/scenes/feature-flags/FeatureFlag.tsx b/frontend/src/scenes/feature-flags/FeatureFlag.tsx
@@ -638,7 +638,7 @@ function UsageTab({ featureFlag }: { id: string; featureFlag: FeatureFlagType })
         ) {
             enrichUsageDashboard()
         }
-    }, [dashboard])
+    }, [dashboard, hasEnrichedAnalytics, enrichUsageDashboard])
 
     const propertyFilter: AnyPropertyFilter[] = [
         {

diff --git a/frontend/src/scenes/feature-flags/featureFlagLogic.ts b/frontend/src/scenes/feature-flags/featureFlagLogic.ts
@@ -38,6 +38,7 @@ import {
     FilterType,
     InsightModel,
     InsightType,
+    JsonType,
     MultivariateFlagOptions,
     MultivariateFlagVariant,
     NewEarlyAccessFeatureType,
@@ -133,9 +134,11 @@ export const variantKeyToIndexFeatureFlagPayloads = (flag: FeatureFlagType): Fea
         return flag
     }
 
-    const newPayloads = {}
+    const newPayloads: Record<number, JsonType> = {}
     flag.filters.multivariate?.variants.forEach((variant, index) => {
-        newPayloads[index] = flag.filters.payloads?.[variant.key]
+        if (flag.filters.payloads?.[variant.key] !== undefined) {
+            newPayloads[index] = flag.filters.payloads[variant.key]
+        }
     })
     return {
         ...flag,
@@ -148,11 +151,10 @@ export const variantKeyToIndexFeatureFlagPayloads = (flag: FeatureFlagType): Fea
 
 const indexToVariantKeyFeatureFlagPayloads = (flag: Partial<FeatureFlagType>): Partial<FeatureFlagType> => {
     if (flag.filters?.multivariate) {
-        const newPayloads = {}
-        flag.filters?.multivariate?.variants.forEach(({ key }, index) => {
-            const payload = flag.filters?.payloads?.[index]
-            if (payload) {
-                newPayloads[key] = payload
+        const newPayloads: Record<string, JsonType> = {}
+        flag.filters.multivariate.variants.forEach(({ key }, index) => {
+            if (flag.filters?.payloads?.[index] !== undefined) {
+                newPayloads[key] = flag.filters.payloads[index]
             }
         })
         return {
@@ -319,6 +321,22 @@ export const featureFlagLogic = kea<featureFlagLogicType>([
                     }
                     const variants = [...(state.filters.multivariate?.variants || [])]
                     variants.splice(index, 1)
+
+                    const currentPayloads = { ...state.filters.payloads }
+                    const newPayloads: Record<number, any> = {}
+
+                    // TRICKY: In addition to modifying the variant array, we also need to shift the payload indices
+                    // because the variant array is being modified and we need to make sure that the payloads object
+                    // stays in sync with the variant array.
+                    Object.keys(currentPayloads).forEach((key) => {
+                        const payloadIndex = parseInt(key)
+                        if (payloadIndex > index) {
+                            newPayloads[payloadIndex - 1] = currentPayloads[payloadIndex]
+                        } else if (payloadIndex < index) {
+                            newPayloads[payloadIndex] = currentPayloads[payloadIndex]
+                        }
+                    })
+
                     return {
                         ...state,
                         filters: {
@@ -327,6 +345,7 @@ export const featureFlagLogic = kea<featureFlagLogicType>([
                                 ...state.filters.multivariate,
                                 variants,
                             },
+                            payloads: newPayloads,
                         },
                     }
                 },
@@ -642,7 +661,7 @@ export const featureFlagLogic = kea<featureFlagLogicType>([
             createScheduledChange: async () => {
                 const { scheduledChangeOperation, scheduleDateMarker, currentTeamId, schedulePayload } = values
 
-                const fields = {
+                const fields: Record<ScheduledChangeOperationType, keyof ScheduleFlagPayload> = {
                     [ScheduledChangeOperationType.UpdateStatus]: 'active',
                     [ScheduledChangeOperationType.AddReleaseCondition]: 'filters',
                 }