From 5170705c790997b65f6464f783c38ebbff1941ec Mon Sep 17 00:00:00 2001 From: Michael Matloka Date: Thu, 26 Oct 2023 16:34:20 +0200 Subject: [PATCH] fix(groups): Fix `groupsMathDefinitions` with deleted group types --- .../sidebars/personsAndGroups.ts | 8 ++-- .../TaxonomicFilter/taxonomicFilterLogic.tsx | 4 +- frontend/src/models/groupsModel.ts | 40 +++++++++-------- .../cohorts/CohortFilters/cohortFieldLogic.ts | 10 +++-- .../propertyDefinitionsTableLogic.ts | 2 +- .../src/scenes/experiments/Experiment.tsx | 5 ++- .../scenes/experiments/experimentLogic.tsx | 2 +- .../feature-flags/FeatureFlagInstructions.tsx | 4 +- .../FeatureFlagReleaseConditions.tsx | 2 +- .../scenes/feature-flags/featureFlagLogic.ts | 6 +-- frontend/src/scenes/groups/Group.tsx | 2 +- frontend/src/scenes/groups/GroupsTabs.tsx | 2 +- frontend/src/scenes/groups/groupLogic.ts | 4 +- .../insights/filters/AggregationSelect.tsx | 2 +- .../project/Settings/GroupAnalytics.tsx | 2 +- .../Settings/groupAnalyticsConfigLogic.ts | 2 +- frontend/src/scenes/trends/mathsLogic.tsx | 44 ++++++++++--------- frontend/src/types.ts | 6 ++- 18 files changed, 78 insertions(+), 69 deletions(-) diff --git a/frontend/src/layout/navigation-3000/sidebars/personsAndGroups.ts b/frontend/src/layout/navigation-3000/sidebars/personsAndGroups.ts index 7dcd526374a3c..2cefac31cf5ba 100644 --- a/frontend/src/layout/navigation-3000/sidebars/personsAndGroups.ts +++ b/frontend/src/layout/navigation-3000/sidebars/personsAndGroups.ts @@ -90,7 +90,7 @@ export const personsAndGroupsSidebarLogic = kea ({ key: `groups-${groupType.group_type_index}`, @@ -120,7 +120,7 @@ export const personsAndGroupsSidebarLogic = kea (state) => { - if (s.groupTypes(state).length > groupTypeIndex) { + if (s.groupTypes(state)[groupTypeIndex]) { groupsListLogic({ groupTypeIndex }).mount() return groupsListLogic({ groupTypeIndex }).selectors.groups(state) } @@ -140,7 +140,7 @@ export const personsAndGroupsSidebarLogic = kea (state) => { - if (s.groupTypes(state).length > groupTypeIndex) { + if (s.groupTypes(state)[groupTypeIndex]) { groupsListLogic({ groupTypeIndex }).mount() return groupsListLogic({ groupTypeIndex }).selectors.groupsLoading(state) } @@ -184,7 +184,7 @@ export const personsAndGroupsSidebarLogic = kea { actions.setPersonsListFilters({ search: searchTerm }) actions.loadPersons() - for (const { group_type_index: groupTypeIndex } of values.groupTypes) { + for (const { group_type_index: groupTypeIndex } of Object.values(values.groupTypes)) { groupsListLogic({ groupTypeIndex }).actions.setSearch(searchTerm, false) } }, diff --git a/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx b/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx index f15d9b8778d3c..f1c1a7630f65f 100644 --- a/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx +++ b/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx @@ -453,7 +453,7 @@ export const taxonomicFilterLogic = kea({ groupAnalyticsTaxonomicGroupNames: [ (s) => [s.groupTypes, s.currentTeamId, s.aggregationLabel], (groupTypes, teamId, aggregationLabel): TaxonomicFilterGroup[] => - groupTypes.map((type) => ({ + Array.from(groupTypes.values()).map((type) => ({ name: `${capitalizeFirstLetter(aggregationLabel(type.group_type_index).plural)}`, searchPlaceholder: `${aggregationLabel(type.group_type_index).plural}`, type: `${TaxonomicFilterGroupType.GroupNamesPrefix}_${type.group_type_index}` as unknown as TaxonomicFilterGroupType, @@ -470,7 +470,7 @@ export const taxonomicFilterLogic = kea({ groupAnalyticsTaxonomicGroups: [ (s) => [s.groupTypes, s.currentTeamId, s.aggregationLabel], (groupTypes, teamId, aggregationLabel): TaxonomicFilterGroup[] => - groupTypes.map((type) => ({ + Array.from(groupTypes.values()).map((type) => ({ name: `${capitalizeFirstLetter(aggregationLabel(type.group_type_index).singular)} properties`, searchPlaceholder: `${aggregationLabel(type.group_type_index).singular} properties`, type: `${TaxonomicFilterGroupType.GroupsPrefix}_${type.group_type_index}` as unknown as TaxonomicFilterGroupType, diff --git a/frontend/src/models/groupsModel.ts b/frontend/src/models/groupsModel.ts index 458f71e70e7c5..9f7cd247a88de 100644 --- a/frontend/src/models/groupsModel.ts +++ b/frontend/src/models/groupsModel.ts @@ -1,6 +1,6 @@ import { kea, path, connect, selectors, events } from 'kea' import api from 'lib/api' -import { GroupType } from '~/types' +import { GroupType, GroupTypeIndex } from '~/types' import { teamLogic } from 'scenes/teamLogic' import type { groupsModelType } from './groupsModelType' import { TaxonomicFilterGroupType } from 'lib/components/TaxonomicFilter/types' @@ -40,26 +40,22 @@ export const groupsModel = kea([ selectors({ groupTypes: [ (s) => [s.groupTypesRaw], - (groupTypesRaw) => { - const groupTypes: GroupType[] = new Array(groupTypesRaw.length) - - for (const groupType of groupTypesRaw) { - groupTypes[groupType.group_type_index] = groupType - } - - return groupTypes - }, + (groupTypesRaw) => + new Map( + groupTypesRaw.map((groupType) => [groupType.group_type_index, groupType]) + ), ], groupTypesLoading: [(s) => [s.groupTypesRawLoading], (groupTypesRawLoading) => groupTypesRawLoading], showGroupsOptions: [ (s) => [s.groupsAccessStatus, s.groupsEnabled, s.groupTypes], - (status, enabled, groupTypes) => status !== GroupsAccessStatus.Hidden || (enabled && groupTypes.length > 0), + (status, enabled, groupTypes) => + status !== GroupsAccessStatus.Hidden || (enabled && Array.from(groupTypes.values()).length > 0), ], groupsTaxonomicTypes: [ (s) => [s.groupTypes], (groupTypes): TaxonomicFilterGroupType[] => { - return groupTypes.map( + return Array.from(groupTypes.values()).map( (groupType: GroupType) => `${TaxonomicFilterGroupType.GroupsPrefix}_${groupType.group_type_index}` as unknown as TaxonomicFilterGroupType ) @@ -68,7 +64,7 @@ export const groupsModel = kea([ groupNamesTaxonomicTypes: [ (s) => [s.groupTypes], (groupTypes): TaxonomicFilterGroupType[] => { - return groupTypes.map( + return Array.from(groupTypes.values()).map( (groupType: GroupType) => `${TaxonomicFilterGroupType.GroupNamesPrefix}_${groupType.group_type_index}` as unknown as TaxonomicFilterGroupType ) @@ -78,12 +74,18 @@ export const groupsModel = kea([ (s) => [s.groupTypes], (groupTypes) => (groupTypeIndex: number | null | undefined, deferToUserWording: boolean = false): Noun => { - if (groupTypeIndex != undefined && groupTypes.length > 0 && groupTypes[groupTypeIndex]) { - const groupType = groupTypes[groupTypeIndex] - - return { - singular: groupType.name_plural || groupType.group_type, - plural: groupType.name_plural || `${groupType.group_type}(s)`, + if (groupTypeIndex != undefined) { + const groupType = groupTypes.get(groupTypeIndex as GroupTypeIndex) + if (groupType) { + return { + singular: groupType.name_plural || groupType.group_type, + plural: groupType.name_plural || `${groupType.group_type}(s)`, + } + } else { + return { + singular: 'unknown group', + plural: 'unknown groups', + } } } return deferToUserWording diff --git a/frontend/src/scenes/cohorts/CohortFilters/cohortFieldLogic.ts b/frontend/src/scenes/cohorts/CohortFilters/cohortFieldLogic.ts index f425e299aca12..7ab38207877e3 100644 --- a/frontend/src/scenes/cohorts/CohortFilters/cohortFieldLogic.ts +++ b/frontend/src/scenes/cohorts/CohortFilters/cohortFieldLogic.ts @@ -71,10 +71,12 @@ export const cohortFieldLogic = kea([ label: 'Persons', }, ...Object.fromEntries( - groupTypes.map((type) => [ - `${ActorGroupType.GroupPrefix}_${type.group_type_index}`, - { label: aggregationLabel(type.group_type_index).plural }, - ]) + Array.from(groupTypes.values()) + .map((type) => [ + `${ActorGroupType.GroupPrefix}_${type.group_type_index}`, + { label: aggregationLabel(type.group_type_index).plural }, + ]) + .filter(Boolean) ), }, }, diff --git a/frontend/src/scenes/data-management/properties/propertyDefinitionsTableLogic.ts b/frontend/src/scenes/data-management/properties/propertyDefinitionsTableLogic.ts index 7e475bc95a1f9..933c08d6442fc 100644 --- a/frontend/src/scenes/data-management/properties/propertyDefinitionsTableLogic.ts +++ b/frontend/src/scenes/data-management/properties/propertyDefinitionsTableLogic.ts @@ -100,7 +100,7 @@ export const propertyDefinitionsTableLogic = kea [s.groupTypes, s.aggregationLabel], (groupTypes, aggregationLabel) => { - const groupChoices: Array> = groupTypes.map((type) => ({ + const groupChoices: Array> = Array.from(groupTypes.values()).map((type) => ({ label: `${capitalizeFirstLetter(aggregationLabel(type.group_type_index).singular)} properties`, value: `group::${type.group_type_index}`, })) diff --git a/frontend/src/scenes/experiments/Experiment.tsx b/frontend/src/scenes/experiments/Experiment.tsx index 2717a1f523750..97a6a198f5363 100644 --- a/frontend/src/scenes/experiments/Experiment.tsx +++ b/frontend/src/scenes/experiments/Experiment.tsx @@ -63,6 +63,7 @@ export function Experiment(): JSX.Element { flagImplementationWarning, props, aggregationLabel, + showGroupsOptions, groupTypes, experimentMissing, } = useValues(experimentLogic) @@ -338,7 +339,7 @@ export function Experiment(): JSX.Element { )} - {experimentId === 'new' && groupTypes.length > 0 && ( + {experimentId === 'new' && showGroupsOptions && ( <>
Default participant type @@ -371,7 +372,7 @@ export function Experiment(): JSX.Element { }} options={[ { value: -1, label: 'Persons' }, - ...groupTypes.map((groupType) => ({ + ...Array.from(groupTypes.values()).map((groupType) => ({ value: groupType.group_type_index, label: capitalizeFirstLetter( aggregationLabel(groupType.group_type_index).plural diff --git a/frontend/src/scenes/experiments/experimentLogic.tsx b/frontend/src/scenes/experiments/experimentLogic.tsx index b75e1c68251a0..fdc993be666cd 100644 --- a/frontend/src/scenes/experiments/experimentLogic.tsx +++ b/frontend/src/scenes/experiments/experimentLogic.tsx @@ -80,7 +80,7 @@ export const experimentLogic = kea([ key((props) => props.experimentId || 'new'), path((key) => ['scenes', 'experiment', 'experimentLogic', key]), connect(() => ({ - values: [teamLogic, ['currentTeamId'], groupsModel, ['aggregationLabel', 'groupTypes']], + values: [teamLogic, ['currentTeamId'], groupsModel, ['aggregationLabel', 'groupTypes', 'showGroupsOptions']], actions: [ experimentsLogic, ['updateExperiments', 'addToExperiments'], diff --git a/frontend/src/scenes/feature-flags/FeatureFlagInstructions.tsx b/frontend/src/scenes/feature-flags/FeatureFlagInstructions.tsx index 2c32d7f615bf3..eadbc456bf678 100644 --- a/frontend/src/scenes/feature-flags/FeatureFlagInstructions.tsx +++ b/frontend/src/scenes/feature-flags/FeatureFlagInstructions.tsx @@ -3,7 +3,7 @@ import { useActions, useValues } from 'kea' import { IconInfo, IconOpenInNew } from 'lib/lemon-ui/icons' import './FeatureFlagInstructions.scss' import { LemonCheckbox, LemonSelect } from '@posthog/lemon-ui' -import { FeatureFlagType } from '~/types' +import { FeatureFlagType, GroupTypeIndex } from '~/types' import { BOOTSTRAPPING_OPTIONS, FF_ANCHOR, @@ -66,7 +66,7 @@ export function CodeInstructions({ const { groupTypes } = useValues(groupsModel) const groupType = featureFlag?.filters?.aggregation_group_type_index != null - ? groupTypes[featureFlag?.filters?.aggregation_group_type_index] + ? groupTypes.get(featureFlag.filters.aggregation_group_type_index as GroupTypeIndex) : undefined const { reportFlagsCodeExampleInteraction, reportFlagsCodeExampleLanguage } = useActions(eventUsageLogic) diff --git a/frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.tsx b/frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.tsx index 88b7ccf9e728e..d16ffe3c2d065 100644 --- a/frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.tsx +++ b/frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.tsx @@ -440,7 +440,7 @@ export function FeatureFlagReleaseConditions({ Users - {groupTypes.map((groupType) => ( + {Array.from(groupTypes.values()).map((groupType) => ( {capitalizeFirstLetter(aggregationLabel(groupType.group_type_index).plural)} diff --git a/frontend/src/scenes/feature-flags/featureFlagLogic.ts b/frontend/src/scenes/feature-flags/featureFlagLogic.ts index 172ca75db8329..e145ddc23056f 100644 --- a/frontend/src/scenes/feature-flags/featureFlagLogic.ts +++ b/frontend/src/scenes/feature-flags/featureFlagLogic.ts @@ -749,9 +749,9 @@ export const featureFlagLogic = kea([ variantRolloutSum === 100, ], aggregationTargetName: [ - (s) => [s.featureFlag, s.groupTypes, s.aggregationLabel], - (featureFlag, groupTypes, aggregationLabel): string => { - if (featureFlag && featureFlag.filters.aggregation_group_type_index != null && groupTypes.length > 0) { + (s) => [s.featureFlag, s.aggregationLabel], + (featureFlag, aggregationLabel): string => { + if (featureFlag && featureFlag.filters.aggregation_group_type_index != null) { return aggregationLabel(featureFlag.filters.aggregation_group_type_index).plural } return 'users' diff --git a/frontend/src/scenes/groups/Group.tsx b/frontend/src/scenes/groups/Group.tsx index f3ed2116c2aed..21c53e74133f4 100644 --- a/frontend/src/scenes/groups/Group.tsx +++ b/frontend/src/scenes/groups/Group.tsx @@ -69,7 +69,7 @@ export function Group(): JSX.Element { const { groupKey, groupTypeIndex } = logicProps const { setGroupEventsQuery } = useActions(groupLogic) - if (!groupData) { + if (!groupData || !groupType) { return groupDataLoading ? : } diff --git a/frontend/src/scenes/groups/GroupsTabs.tsx b/frontend/src/scenes/groups/GroupsTabs.tsx index 82b3d7d297cf0..89900d8054480 100644 --- a/frontend/src/scenes/groups/GroupsTabs.tsx +++ b/frontend/src/scenes/groups/GroupsTabs.tsx @@ -32,7 +32,7 @@ export function GroupsTabs({ activeGroupTypeIndex }: { activeGroupTypeIndex: num link: urls.groups(0), }, ] - : groupTypes.map( + : Array.from(groupTypes.values()).map( (groupType) => ({ label: capitalizeFirstLetter(aggregationLabel(groupType.group_type_index).plural), diff --git a/frontend/src/scenes/groups/groupLogic.ts b/frontend/src/scenes/groups/groupLogic.ts index 0800a07890f14..dce2cfdc6e04b 100644 --- a/frontend/src/scenes/groups/groupLogic.ts +++ b/frontend/src/scenes/groups/groupLogic.ts @@ -3,7 +3,7 @@ import api from 'lib/api' import { toParams } from 'lib/utils' import { teamLogic } from 'scenes/teamLogic' import { groupsModel } from '~/models/groupsModel' -import { Breadcrumb, Group, PropertyFilterType, PropertyOperator } from '~/types' +import { Breadcrumb, Group, GroupTypeIndex, PropertyFilterType, PropertyOperator } from '~/types' import type { groupLogicType } from './groupLogicType' import { urls } from 'scenes/urls' import { capitalizeFirstLetter } from 'lib/utils' @@ -98,7 +98,7 @@ export const groupLogic = kea([ ], groupType: [ (s, p) => [s.groupTypes, p.groupTypeIndex], - (groupTypes, index): string => groupTypes[index]?.group_type, + (groupTypes, index): string | null => groupTypes.get(index as GroupTypeIndex)?.group_type ?? null, ], breadcrumbs: [ (s, p) => [s.groupTypeName, p.groupTypeIndex, p.groupKey, s.groupData], diff --git a/frontend/src/scenes/insights/filters/AggregationSelect.tsx b/frontend/src/scenes/insights/filters/AggregationSelect.tsx index a4ef9cc6cfb52..766860da847a8 100644 --- a/frontend/src/scenes/insights/filters/AggregationSelect.tsx +++ b/frontend/src/scenes/insights/filters/AggregationSelect.tsx @@ -84,7 +84,7 @@ export function AggregationSelect({ if (needsUpgradeForGroups || canStartUsingGroups) { optionSections[0].footer = } else { - groupTypes.forEach((groupType) => { + Array.from(groupTypes.values()).forEach((groupType) => { baseValues.push(`$group_${groupType.group_type_index}`) optionSections[0].options.push({ value: `$group_${groupType.group_type_index}`, diff --git a/frontend/src/scenes/project/Settings/GroupAnalytics.tsx b/frontend/src/scenes/project/Settings/GroupAnalytics.tsx index ae5c1ef2c8f98..4d563af695bdb 100644 --- a/frontend/src/scenes/project/Settings/GroupAnalytics.tsx +++ b/frontend/src/scenes/project/Settings/GroupAnalytics.tsx @@ -81,7 +81,7 @@ export function GroupAnalytics(): JSX.Element | null { )} - +
diff --git a/frontend/src/scenes/project/Settings/groupAnalyticsConfigLogic.ts b/frontend/src/scenes/project/Settings/groupAnalyticsConfigLogic.ts index 22c9aed1ffafc..f5ac88fb37b6d 100644 --- a/frontend/src/scenes/project/Settings/groupAnalyticsConfigLogic.ts +++ b/frontend/src/scenes/project/Settings/groupAnalyticsConfigLogic.ts @@ -42,7 +42,7 @@ export const groupAnalyticsConfigLogic = kea({ listeners: ({ values, actions }) => ({ save: async () => { const { groupTypes, singularChanges, pluralChanges } = values - const payload = groupTypes.map((groupType) => { + const payload = Array.from(groupTypes.values()).map((groupType) => { const result = { ...groupType } if (singularChanges[groupType.group_type_index]) { result.name_singular = singularChanges[groupType.group_type_index] diff --git a/frontend/src/scenes/trends/mathsLogic.tsx b/frontend/src/scenes/trends/mathsLogic.tsx index e62d0a64b308f..bbf2374a00977 100644 --- a/frontend/src/scenes/trends/mathsLogic.tsx +++ b/frontend/src/scenes/trends/mathsLogic.tsx @@ -324,27 +324,29 @@ export const mathsLogic = kea({ (s) => [s.groupTypes, s.aggregationLabel], (groupTypes, aggregationLabel) => Object.fromEntries( - groupTypes.map((groupType) => [ - apiValueToMathType('unique_group', groupType.group_type_index), - { - name: `Unique ${aggregationLabel(groupType.group_type_index).plural}`, - shortName: `unique ${aggregationLabel(groupType.group_type_index).plural}`, - description: ( - <> - Number of unique {aggregationLabel(groupType.group_type_index).plural} who performed - the event in the specified period. -
-
- - Example: If 7 users in a single $ - {aggregationLabel(groupType.group_type_index).singular} perform an event 9 times - in the given period, it counts only as 1. - - - ), - category: MathCategory.ActorCount, - } as MathDefinition, - ]) + Array.from(groupTypes.values()) + .map((groupType) => [ + apiValueToMathType('unique_group', groupType.group_type_index), + { + name: `Unique ${aggregationLabel(groupType.group_type_index).plural}`, + shortName: `unique ${aggregationLabel(groupType.group_type_index).plural}`, + description: ( + <> + Number of unique {aggregationLabel(groupType.group_type_index).plural} who + performed the event in the specified period. +
+
+ + Example: If 7 users in a single $ + {aggregationLabel(groupType.group_type_index).singular} perform an event 9 + times in the given period, it counts only as 1. + + + ), + category: MathCategory.ActorCount, + } as MathDefinition, + ]) + .filter(Boolean) ), ], }, diff --git a/frontend/src/types.ts b/frontend/src/types.ts index c59ab2d0a9f50..c29dc07b9f987 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -2473,9 +2473,11 @@ export interface PersonProperty { count: number } +export type GroupTypeIndex = 0 | 1 | 2 | 3 | 4 + export interface GroupType { group_type: string - group_type_index: number + group_type_index: GroupTypeIndex name_singular?: string | null name_plural?: string | null } @@ -2483,7 +2485,7 @@ export interface GroupType { export type GroupTypeProperties = Record> export interface Group { - group_type_index: number + group_type_index: GroupTypeIndex group_key: string created_at: string group_properties: Record