Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(groups): Fix groupsMathDefinitions with deleted group types #18221

Merged
merged 2 commits into from
Oct 27, 2023
Merged

fix(groups): Fix groupsMathDefinitions with deleted group types #18221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5170705
fix(groups): Fix `groupsMathDefinitions` with deleted group types
Twixes Oct 26, 2023
d4f6e73
Update UI snapshots for `chromium` (1)
github-actions[bot] Oct 26, 2023
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
Binary file modified frontend/__snapshots__/posthog-3000-sidebar--dashboards.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
8 changes: 4 additions & 4 deletions frontend/src/layout/navigation-3000/sidebars/personsAndGroups.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ export const personsAndGroupsSidebarLogic = kea<personsAndGroupsSidebarLogicType
minimumBatchSize: 100,
},
} as SidebarCategory,
...groupTypes.map(
...Array.from(groupTypes.values()).map(
(groupType) =>
({
key: `groups-${groupType.group_type_index}`,
Expand Down Expand Up @@ -120,7 +120,7 @@ export const personsAndGroupsSidebarLogic = kea<personsAndGroupsSidebarLogicType
Array(5)
.fill(null)
.map((_, groupTypeIndex) => (state) => {
if (s.groupTypes(state).length > groupTypeIndex) {
if (s.groupTypes(state)[groupTypeIndex]) {
groupsListLogic({ groupTypeIndex }).mount()
return groupsListLogic({ groupTypeIndex }).selectors.groups(state)
}
Expand All @@ -140,7 +140,7 @@ export const personsAndGroupsSidebarLogic = kea<personsAndGroupsSidebarLogicType
Array(5)
.fill(null)
.map((_, groupTypeIndex) => (state) => {
if (s.groupTypes(state).length > groupTypeIndex) {
if (s.groupTypes(state)[groupTypeIndex]) {
groupsListLogic({ groupTypeIndex }).mount()
return groupsListLogic({ groupTypeIndex }).selectors.groupsLoading(state)
}
Expand Down Expand Up @@ -184,7 +184,7 @@ export const personsAndGroupsSidebarLogic = kea<personsAndGroupsSidebarLogicType
searchTerm: (searchTerm) => {
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)
}
},
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ export const taxonomicFilterLogic = kea<taxonomicFilterLogicType>({
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,
Expand All @@ -470,7 +470,7 @@ export const taxonomicFilterLogic = kea<taxonomicFilterLogicType>({
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,
Expand Down
40 changes: 21 additions & 19 deletions frontend/src/models/groupsModel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -40,26 +40,22 @@ export const groupsModel = kea<groupsModelType>([
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<GroupTypeIndex, GroupType>(
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
)
Expand All @@ -68,7 +64,7 @@ export const groupsModel = kea<groupsModelType>([
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
)
Expand All @@ -78,12 +74,18 @@ export const groupsModel = kea<groupsModelType>([
(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
Expand Down
10 changes: 6 additions & 4 deletions frontend/src/scenes/cohorts/CohortFilters/cohortFieldLogic.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,12 @@ export const cohortFieldLogic = kea<cohortFieldLogicType>([
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)
),
},
},
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/scenes/data-management/properties/propertyDefinitionsTableLogic.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export const propertyDefinitionsTableLogic = kea<propertyDefinitionsTableLogicTy
propertyTypeOptions: [
(s) => [s.groupTypes, s.aggregationLabel],
(groupTypes, aggregationLabel) => {
const groupChoices: Array<LemonSelectOption<string>> = groupTypes.map((type) => ({
const groupChoices: Array<LemonSelectOption<string>> = Array.from(groupTypes.values()).map((type) => ({
label: `${capitalizeFirstLetter(aggregationLabel(type.group_type_index).singular)} properties`,
value: `group::${type.group_type_index}`,
}))
Expand Down
5 changes: 3 additions & 2 deletions frontend/src/scenes/experiments/Experiment.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ export function Experiment(): JSX.Element {
flagImplementationWarning,
props,
aggregationLabel,
showGroupsOptions,
groupTypes,
experimentMissing,
} = useValues(experimentLogic)
Expand Down Expand Up @@ -338,7 +339,7 @@ export function Experiment(): JSX.Element {
</Link>
)}
</div>
{experimentId === 'new' && groupTypes.length > 0 && (
{experimentId === 'new' && showGroupsOptions && (
<>
<div className="mt-4">
<strong>Default participant type</strong>
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/scenes/experiments/experimentLogic.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export const experimentLogic = kea<experimentLogicType>([
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'],
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/scenes/feature-flags/FeatureFlagInstructions.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ export function FeatureFlagReleaseConditions({
<Select.Option key={-1} value={-1}>
Users
</Select.Option>
{groupTypes.map((groupType) => (
{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>
Expand Down
6 changes: 3 additions & 3 deletions frontend/src/scenes/feature-flags/featureFlagLogic.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -749,9 +749,9 @@ export const featureFlagLogic = kea<featureFlagLogicType>([
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'
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/scenes/groups/Group.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export function Group(): JSX.Element {
const { groupKey, groupTypeIndex } = logicProps
const { setGroupEventsQuery } = useActions(groupLogic)

if (!groupData) {
if (!groupData || !groupType) {
return groupDataLoading ? <SpinnerOverlay sceneLevel /> : <NotFound object="group" />
}

Expand Down
2 changes: 1 addition & 1 deletion frontend/src/scenes/groups/GroupsTabs.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/scenes/groups/groupLogic.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -98,7 +98,7 @@ export const groupLogic = kea<groupLogicType>([
],
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],
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/scenes/insights/filters/AggregationSelect.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export function AggregationSelect({
if (needsUpgradeForGroups || canStartUsingGroups) {
optionSections[0].footer = <GroupIntroductionFooter needsUpgrade={needsUpgradeForGroups} />
} 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}`,
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/scenes/project/Settings/GroupAnalytics.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export function GroupAnalytics(): JSX.Element | null {
</LemonBanner>
)}

<LemonTable columns={columns} dataSource={groupTypes} loading={groupTypesLoading} />
<LemonTable columns={columns} dataSource={Array.from(groupTypes.values())} loading={groupTypesLoading} />

<div className="flex gap-2 mt-4">
<LemonButton type="primary" disabled={!hasChanges} onClick={save}>
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/scenes/project/Settings/groupAnalyticsConfigLogic.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export const groupAnalyticsConfigLogic = kea<groupAnalyticsConfigLogicType>({
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]
Expand Down
44 changes: 23 additions & 21 deletions frontend/src/scenes/trends/mathsLogic.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,27 +324,29 @@ export const mathsLogic = kea<mathsLogicType>({
(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.
<br />
<br />
<i>
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.
</i>
</>
),
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.
<br />
<br />
<i>
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.
</i>
</>
),
category: MathCategory.ActorCount,
} as MathDefinition,
])
.filter(Boolean)
),
],
},
Expand Down
Loading
Loading