fix(feature flags): disallow groups on early access flags (#19787)

frontend/src/lib/introductions/GroupsIntroductionOption.tsx

@@ -1,35 +1,9 @@
-import { useValues } from 'kea'
-import { groupsAccessLogic, GroupsAccessStatus } from 'lib/introductions/groupsAccessLogic'
 import { IconLock } from 'lib/lemon-ui/icons'
 import { Link } from 'lib/lemon-ui/Link'
-import Select from 'rc-select'
-
-// TODO: Remove, but de-ant FeatureFlagReleaseConditions first
-export function GroupsIntroductionOption({ value }: { value: any }): JSX.Element | null {
-    const { groupsAccessStatus } = useValues(groupsAccessLogic)
-
-    if (
-        ![GroupsAccessStatus.HasAccess, GroupsAccessStatus.HasGroupTypes, GroupsAccessStatus.NoAccess].includes(
-            groupsAccessStatus
-        )
-    ) {
-        return null
-    }
 
+export function GroupsIntroductionOption(): JSX.Element {
     return (
-        <Select.Option
-            key="groups"
-            value={value}
-            disabled
-            style={{
-                height: '100%',
-                width: '100%',
-                overflow: 'hidden',
-                textOverflow: 'ellipsis',
-                backgroundColor: 'var(--side)',
-                color: 'var(--muted)',
-            }}
-        >
+        <div className="cursor-default">
             <IconLock style={{ marginRight: 6, color: 'var(--warning)' }} />
             Unique groups –{' '}
             <Link
@@ -40,6 +14,6 @@ export function GroupsIntroductionOption({ value }: { value: any }): JSX.Element
             >
                 Learn more
             </Link>
-        </Select.Option>
+        </div>
     )
 }

frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.tsx

@@ -1,13 +1,13 @@
 import './FeatureFlag.scss'
 
 import { LemonInput, LemonSelect, Link } from '@posthog/lemon-ui'
-import { Select } from 'antd'
 import { useActions, useValues } from 'kea'
 import { router } from 'kea-router'
 import { allOperatorsToHumanName } from 'lib/components/DefinitionPopover/utils'
 import { PropertyFilters } from 'lib/components/PropertyFilters/PropertyFilters'
 import { isPropertyFilterWithOperator } from 'lib/components/PropertyFilters/utils'
 import { INSTANTLY_AVAILABLE_PROPERTIES } from 'lib/constants'
+import { groupsAccessLogic, GroupsAccessStatus } from 'lib/introductions/groupsAccessLogic'
 import { GroupsIntroductionOption } from 'lib/introductions/GroupsIntroductionOption'
 import { IconCopy, IconDelete, IconErrorOutline, IconOpenInNew, IconPlus, IconSubArrowRight } from 'lib/lemon-ui/icons'
 import { LemonBanner } from 'lib/lemon-ui/LemonBanner'
@@ -59,12 +59,13 @@ export function FeatureFlagReleaseConditions({
         addConditionSet,
     } = useActions(logic)
     const { cohortsById } = useValues(cohortsModel)
+    const { groupsAccessStatus } = useValues(groupsAccessLogic)
 
     const filterGroups: FeatureFlagGroupType[] = isSuper
         ? featureFlag.filters.super_groups || []
         : featureFlag.filters.groups
     // :KLUDGE: Match by select only allows Select.Option as children, so render groups option directly rather than as a child
-    const matchByGroupsIntroductionOption = GroupsIntroductionOption({ value: -2 })
+    const matchByGroupsIntroductionOption = GroupsIntroductionOption()
     const hasNonInstantProperty = (properties: AnyPropertyFilter[]): boolean => {
         return !!properties.find(
             (property) => property.type === 'cohort' || !INSTANTLY_AVAILABLE_PROPERTIES.includes(property.key || '')
@@ -79,6 +80,11 @@ export function FeatureFlagReleaseConditions({
         )
     }
 
+    const includeGroupsIntroductionOption = (): boolean =>
+        [GroupsAccessStatus.HasAccess, GroupsAccessStatus.HasGroupTypes, GroupsAccessStatus.NoAccess].includes(
+            groupsAccessStatus
+        )
+
     const renderReleaseConditionGroup = (group: FeatureFlagGroupType, index: number): JSX.Element => {
         return (
             <div className="w-full" key={`${index}-${filterGroups.length}`}>
@@ -423,34 +429,45 @@ export function FeatureFlagReleaseConditions({
                 {!readOnly && showGroupsOptions && usageContext !== 'schedule' && (
                     <div className="centered">
                         Match by
-                        <Select
+                        <LemonSelect
+                            dropdownMatchSelectWidth={false}
+                            className="ml-2"
+                            data-attr="feature-flag-aggregation-filter"
+                            onChange={(value) => {
+                                // MatchByGroupsIntroductionOption
+                                if (value == -2) {
+                                    return
+                                }
+
+                                const groupTypeIndex = value !== -1 ? value : null
+                                setAggregationGroupTypeIndex(groupTypeIndex)
+                            }}
                             value={
                                 featureFlag.filters.aggregation_group_type_index != null
                                     ? featureFlag.filters.aggregation_group_type_index
                                     : -1
                             }
-                            onChange={(value) => {
-                                const groupTypeIndex = value !== -1 ? value : null
-                                setAggregationGroupTypeIndex(groupTypeIndex)
-                            }}
-                            style={{ marginLeft: 8 }}
-                            data-attr="feature-flag-aggregation-filter"
-                            dropdownMatchSelectWidth={false}
-                            dropdownAlign={{
-                                // Align this dropdown by the right-hand-side of button
-                                points: ['tr', 'br'],
-                            }}
-                        >
-                            <Select.Option key={-1} value={-1}>
-                                Users
-                            </Select.Option>
-                            {Array.from(groupTypes.values()).map((groupType) => (
-                                <Select.Option key={groupType.group_type_index} value={groupType.group_type_index}>
-                                    {capitalizeFirstLetter(aggregationLabel(groupType.group_type_index).plural)}
-                                </Select.Option>
-                            ))}
-                            {matchByGroupsIntroductionOption}
-                        </Select>
+                            options={[
+                                { value: -1, label: 'Users' },
+                                ...Array.from(groupTypes.values()).map((groupType) => ({
+                                    value: groupType.group_type_index,
+                                    label: capitalizeFirstLetter(aggregationLabel(groupType.group_type_index).plural),
+                                    disabledReason:
+                                        !featureFlag?.features || featureFlag.features.length > 0
+                                            ? 'This feature flag cannot be group-based, because it is linked to an early access feature.'
+                                            : null,
+                                })),
+                                ...(includeGroupsIntroductionOption()
+                                    ? [
+                                          {
+                                              value: -2,
+                                              label: 'MatchByGroupsIntroductionOption',
+                                              labelInMenu: matchByGroupsIntroductionOption,
+                                          },
+                                      ]
+                                    : []),
+                            ]}
+                        />
                     </div>
                 )}
             </div>

posthog/api/feature_flag.py

@@ -196,6 +196,11 @@ def properties_all_match(predicate):
             is_valid = properties_all_match(lambda prop: prop.type in ["person", "cohort"])
             if not is_valid:
                 raise serializers.ValidationError("Filters are not valid (can only use person and cohort properties)")
+        elif self.instance is not None and hasattr(self.instance, "features") and self.instance.features.count() > 0:
+            raise serializers.ValidationError(
+                "Cannot change this flag to a group-based when linked to an Early Access Feature."
+            )
+
         else:
             is_valid = properties_all_match(
                 lambda prop: prop.type == "group" and prop.group_type_index == aggregation_group_type_index
@@ -283,6 +288,7 @@ def update(self, instance: FeatureFlag, validated_data: Dict, *args: Any, **kwar
             raise exceptions.ValidationError(
                 "Cannot delete a feature flag that is in use with early access features. Please delete the early access feature before deleting the flag."
             )
+
         request = self.context["request"]
         validated_key = validated_data.get("key", None)
         if validated_key:

posthog/api/test/test_feature_flag.py

@@ -23,6 +23,7 @@
     get_feature_flags_for_team_in_cache,
     FeatureFlagDashboards,
 )
+from posthog.models.early_access_feature import EarlyAccessFeature
 from posthog.models.dashboard import Dashboard
 from posthog.models.feature_flag.feature_flag import FeatureFlagHashKeyOverride
 from posthog.models.group.util import create_group
@@ -3621,6 +3622,47 @@ def test_creating_static_cohort(self):
         response = self.client.get(f"/api/cohort/{cohort.pk}/persons")
         self.assertEqual(len(response.json()["results"]), 1, response)
 
+    def test_cant_update_early_access_flag_with_group(self):
+        feature_flag = FeatureFlag.objects.create(
+            team=self.team,
+            rollout_percentage=100,
+            filters={
+                "aggregation_group_type_index": None,
+                "groups": [{"properties": [], "rollout_percentage": None}],
+            },
+            name="some feature",
+            key="some-feature",
+            created_by=self.user,
+        )
+
+        EarlyAccessFeature.objects.create(
+            team=self.team,
+            name="earlyAccessFeature",
+            description="early access feature",
+            stage="alpha",
+            feature_flag=feature_flag,
+        )
+
+        update_data = {
+            "filters": {
+                "aggregation_group_type_index": 2,
+                "groups": [{"properties": [], "rollout_percentage": 100}],
+            }
+        }
+        response = self.client.patch(
+            f"/api/projects/{self.team.id}/feature_flags/{feature_flag.id}/", update_data, format="json"
+        )
+
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+        self.assertDictContainsSubset(
+            {
+                "type": "validation_error",
+                "code": "invalid_input",
+                "detail": "Cannot change this flag to a group-based when linked to an Early Access Feature.",
+            },
+            response.json(),
+        )
+
 
 class TestCohortGenerationForFeatureFlag(APIBaseTest, ClickhouseTestMixin):
     def test_creating_static_cohort_with_deleted_flag(self):