Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import {
FeatureFlagReleaseConditionsLogicProps,
featureFlagReleaseConditionsLogic,
isDistinctIdFilter,
withResolvedFlagLabels,
} from './featureFlagReleaseConditionsLogic'

function PropertyValueComponent({
Expand Down Expand Up @@ -365,7 +366,7 @@ export function FeatureFlagReleaseConditions({
pageKey={`feature-flag-${id}-${group.sort_key}-${filterGroups.length}-${
filters.aggregation_group_type_index ?? ''
}`}
propertyFilters={group?.properties}
propertyFilters={withResolvedFlagLabels(group?.properties, getFlagKey)}
logicalRowDivider
addText="Add condition"
onChange={(properties) => updateConditionSet(index, undefined, properties)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ import {
FeatureFlagGroupTypeWithSortKey,
featureFlagReleaseConditionsLogic,
isDistinctIdFilter,
withResolvedFlagLabels,
} from './featureFlagReleaseConditionsLogic'

interface FeatureFlagReleaseConditionsCollapsibleProps extends FeatureFlagReleaseConditionsLogicProps {
Expand Down Expand Up @@ -115,7 +116,8 @@ type MatchByOption = {
function summarizeProperties(
properties: AnyPropertyFilter[],
aggregationTargetName: string,
getDistinctIdName: (distinctId: string) => string
getDistinctIdName: (distinctId: string) => string,
getFlagKey: (flagId: string) => string
): string {
if (!properties || properties.length === 0) {
// Capitalize first letter of aggregation target name
Expand All @@ -128,7 +130,7 @@ function summarizeProperties(
if (property.type === PropertyFilterType.Cohort) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 summarizeProperties still shows raw ID during loading window

getFlagKey falls back to the raw flag ID when the cache hasn't resolved yet (flagKeyCache[flagId] || flagId), so getFlagKey(property.key) here will return the numeric string while loading. This means the condition-group header summary can still display the bare ID transiently. By contrast, withResolvedFlagLabels guards against this with resolved !== property.key, so the pill views don't have this issue. The PR description acknowledges transient ID display, but if you want the same "wait until resolved, then show name" behaviour in the summary, this line could apply the same guard and only use the resolved value when it differs from property.key.

key = 'Cohort'
} else if (property.type === PropertyFilterType.Flag) {
key = property.label || property.key || 'flag'
key = property.label || (property.key ? getFlagKey(property.key) : '') || 'flag'
} else {
key = property.key || 'property'
}
Expand Down Expand Up @@ -177,6 +179,7 @@ interface ConditionHeaderProps {
affectedCount: number | undefined
aggregationTargetName: string
getDistinctIdName: (distinctId: string) => string
getFlagKey: (flagId: string) => string
onDuplicate: () => void
onRemove: () => void
}
Expand All @@ -188,12 +191,14 @@ function ConditionHeader({
affectedCount,
aggregationTargetName,
getDistinctIdName,
getFlagKey,
onDuplicate,
onRemove,
}: ConditionHeaderProps): JSX.Element {
// Use description if available, otherwise summarize the filters
const summary =
group.description || summarizeProperties(group.properties || [], aggregationTargetName, getDistinctIdName)
group.description ||
summarizeProperties(group.properties || [], aggregationTargetName, getDistinctIdName, getFlagKey)
const rollout = group.rollout_percentage ?? 100

const actualCount =
Expand Down Expand Up @@ -331,6 +336,7 @@ interface ConditionProps {
totalCounts: Record<string, number | undefined>
aggregationTargetName: (conditionGroupTypeIndex?: number | null) => string
getDistinctIdName: (distinctId: string) => string
getFlagKey: (flagId: string) => string
taxonomicGroupTypesForCondition: (conditionGroupTypeIndex: number | null | undefined) => TaxonomicFilterGroupType[]
groupTypes: Map<GroupTypeIndex, GroupType>
setConditionAggregation: (index: number, groupTypeIndex: number | null) => void
Expand Down Expand Up @@ -400,6 +406,7 @@ const ConditionContent = ({
totalCounts,
aggregationTargetName,
getDistinctIdName,
getFlagKey,
taxonomicGroupTypesForCondition,
groupTypes,
setConditionAggregation,
Expand Down Expand Up @@ -529,6 +536,7 @@ const ConditionContent = ({
affectedCount={group.sort_key ? affectedCounts[group.sort_key] : undefined}
aggregationTargetName={aggregationTargetName(group.aggregation_group_type_index)}
getDistinctIdName={getDistinctIdName}
getFlagKey={getFlagKey}
onDuplicate={onDuplicate}
onRemove={onRemove}
/>
Expand Down Expand Up @@ -604,7 +612,7 @@ const ConditionContent = ({
<PropertyFilters
orFiltering={true}
pageKey={`feature-flag-workflow-${id}-${group.sort_key!}`}
propertyFilters={group?.properties}
propertyFilters={withResolvedFlagLabels(group?.properties, getFlagKey)}
logicalRowDivider
addText="Add filter"
onChange={(properties) => {
Expand Down Expand Up @@ -831,6 +839,7 @@ export function FeatureFlagReleaseConditionsCollapsible({
totalCounts,
aggregationTargetName,
getDistinctIdName,
getFlagKey,
taxonomicGroupTypesForCondition,
filters: releaseFilters,
groupTypes,
Expand Down Expand Up @@ -929,7 +938,8 @@ export function FeatureFlagReleaseConditionsCollapsible({
summarizeProperties(
group.properties || [],
aggregationTargetName(group.aggregation_group_type_index),
getDistinctIdName
getDistinctIdName,
getFlagKey
)
const rollout = group.rollout_percentage ?? 100
return (
Expand Down Expand Up @@ -1227,6 +1237,7 @@ export function FeatureFlagReleaseConditionsCollapsible({
totalCounts={totalCounts}
aggregationTargetName={aggregationTargetName}
getDistinctIdName={getDistinctIdName}
getFlagKey={getFlagKey}
taxonomicGroupTypesForCondition={taxonomicGroupTypesForCondition}
groupTypes={groupTypes}
setConditionAggregation={setConditionAggregation}
Expand Down Expand Up @@ -1269,7 +1280,8 @@ export function FeatureFlagReleaseConditionsCollapsible({
aggregationTargetName(
draggedGroup.aggregation_group_type_index
),
getDistinctIdName
getDistinctIdName,
getFlagKey
)}
</span>
</div>
Expand Down Expand Up @@ -1302,6 +1314,7 @@ export function FeatureFlagReleaseConditionsCollapsible({
totalCounts={totalCounts}
aggregationTargetName={aggregationTargetName}
getDistinctIdName={getDistinctIdName}
getFlagKey={getFlagKey}
taxonomicGroupTypesForCondition={taxonomicGroupTypesForCondition}
groupTypes={groupTypes}
setConditionAggregation={setConditionAggregation}
Expand Down Expand Up @@ -1352,6 +1365,7 @@ export function FeatureFlagReleaseConditionsCollapsible({
totalCounts={totalCounts}
aggregationTargetName={aggregationTargetName}
getDistinctIdName={getDistinctIdName}
getFlagKey={getFlagKey}
taxonomicGroupTypesForCondition={taxonomicGroupTypesForCondition}
groupTypes={groupTypes}
setConditionAggregation={setConditionAggregation}
Expand Down Expand Up @@ -1384,6 +1398,7 @@ export function FeatureFlagReleaseConditionsCollapsible({
totalCounts={totalCounts}
aggregationTargetName={aggregationTargetName}
getDistinctIdName={getDistinctIdName}
getFlagKey={getFlagKey}
taxonomicGroupTypesForCondition={taxonomicGroupTypesForCondition}
groupTypes={groupTypes}
setConditionAggregation={setConditionAggregation}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,11 @@ import {

import { EarlyExitIndicator } from './EarlyExitIndicator'
import { FeatureFlagConditionWarning } from './FeatureFlagConditionWarning'
import { featureFlagReleaseConditionsLogic, isDistinctIdFilter } from './featureFlagReleaseConditionsLogic'
import {
featureFlagReleaseConditionsLogic,
isDistinctIdFilter,
withResolvedFlagLabels,
} from './featureFlagReleaseConditionsLogic'

interface FeatureFlagReleaseConditionsReadonlyProps {
id: string
Expand Down Expand Up @@ -124,7 +128,8 @@ export function FeatureFlagReleaseConditionsReadonly({
filters,
})

const { filterGroups, aggregationTargetName, properties, getDistinctIdName } = useValues(releaseConditionsLogic)
const { filterGroups, aggregationTargetName, properties, getDistinctIdName, getFlagKey } =
useValues(releaseConditionsLogic)

return (
<div className="flex flex-col gap-2">
Expand Down Expand Up @@ -158,6 +163,7 @@ export function FeatureFlagReleaseConditionsReadonly({
index={index}
aggregationTargetName={aggregationTargetName(group.aggregation_group_type_index)}
getDistinctIdName={getDistinctIdName}
getFlagKey={getFlagKey}
/>
</div>
))}
Expand All @@ -175,15 +181,17 @@ interface ConditionSetCardProps {
index: number
aggregationTargetName: string
getDistinctIdName: (distinctId: string) => string
getFlagKey: (flagId: string) => string
}

function ConditionSetCard({
group,
index,
aggregationTargetName,
getDistinctIdName,
getFlagKey,
}: ConditionSetCardProps): JSX.Element {
const properties = group.properties || []
const properties = withResolvedFlagLabels(group.properties, getFlagKey)
const rollout = group.rollout_percentage ?? 100

const getSummary = (): JSX.Element => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,22 @@ export function isDistinctIdFilter(property: AnyPropertyFilter): boolean {
return property.type === PropertyFilterType.Person && property.key === 'distinct_id'
}

// Flag-dependency filters store the numeric flag ID in `key`. Backfill the human-readable flag
// key onto `label` (which every display surface prefers) so the UI never shows a bare ID once
// `getFlagKey` has resolved it. Leaves filters untouched when the key can't be resolved yet.
export function withResolvedFlagLabels(
properties: AnyPropertyFilter[] | undefined,
getFlagKey: (flagId: string) => string
): AnyPropertyFilter[] {
return (properties ?? []).map((property) => {
if (property.type !== PropertyFilterType.Flag || !property.key || property.label) {
Comment thread
posthog-bot-comment-resolver[bot] marked this conversation as resolved.
Outdated
return property
}
const resolved = getFlagKey(property.key)
return resolved && resolved !== property.key ? { ...property, label: resolved } : property
})
}

// Server caps batch_by_distinct_ids per request; chunk client-side so every id resolves.
const DISTINCT_ID_BATCH_SIZE = 200

Expand Down
Loading