diff --git a/ee/clickhouse/views/test/funnel/__snapshots__/test_clickhouse_funnel_person.ambr b/ee/clickhouse/views/test/funnel/__snapshots__/test_clickhouse_funnel_person.ambr index f561791090a1d..7272f10b35d60 100644 --- a/ee/clickhouse/views/test/funnel/__snapshots__/test_clickhouse_funnel_person.ambr +++ b/ee/clickhouse/views/test/funnel/__snapshots__/test_clickhouse_funnel_person.ambr @@ -56,7 +56,6 @@ e."$group_0" as aggregation_target, if(notEmpty(pdi.distinct_id), pdi.person_id, e.person_id) as person_id, person.person_props as person_props, - person.pmat_email as pmat_email, if(event = 'step one', 1, 0) as step_0, if(step_0 = 1, timestamp, null) as latest_0, if(event = 'step two', 1, 0) as step_1, @@ -80,7 +79,6 @@ HAVING argMax(is_deleted, version) = 0) AS pdi ON e.distinct_id = pdi.distinct_id INNER JOIN (SELECT id, - argMax(pmat_email, version) as pmat_email, argMax(properties, version) as person_props FROM person WHERE team_id = 99999 @@ -97,7 +95,7 @@ AND event IN ['step one', 'step three', 'step two'] AND toTimeZone(timestamp, 'UTC') >= toDateTime('2021-05-01 00:00:00', 'UTC') AND toTimeZone(timestamp, 'UTC') <= toDateTime('2021-05-10 23:59:59', 'UTC') - AND (("pmat_email" ILIKE '%g0%' + AND ((replaceRegexpAll(JSONExtractRaw(person_props, 'email'), '^"|"$', '') ILIKE '%g0%' OR replaceRegexpAll(JSONExtractRaw(person_props, 'name'), '^"|"$', '') ILIKE '%g0%' OR replaceRegexpAll(JSONExtractRaw(e.properties, 'distinct_id'), '^"|"$', '') ILIKE '%g0%' OR replaceRegexpAll(JSONExtractRaw(group_properties_0, 'name'), '^"|"$', '') ILIKE '%g0%' diff --git a/frontend/src/layout/navigation-3000/sidebars/cohorts.ts b/frontend/src/layout/navigation-3000/sidebars/cohorts.ts index cb5c16bf4f323..23fac290fe6f8 100644 --- a/frontend/src/layout/navigation-3000/sidebars/cohorts.ts +++ b/frontend/src/layout/navigation-3000/sidebars/cohorts.ts @@ -94,7 +94,7 @@ export const cohortsSidebarLogic = kea([ if (searchTerm) { return fuse.search(searchTerm).map((result) => [result.item, result.matches as FuseSearchMatch[]]) } - return cohorts.map((cohort) => [cohort, null]) + return cohorts.results.map((cohort) => [cohort, null]) }, ], })), diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index 9f973c3ca7882..42e2c65248f14 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -1510,16 +1510,21 @@ const api = { async duplicate(cohortId: CohortType['id']): Promise { return await new ApiRequest().cohortsDuplicate(cohortId).get() }, - async list(): Promise> { - // TODO: Remove hard limit and paginate cohorts - return await new ApiRequest().cohorts().withQueryString('limit=600').get() - }, determineDeleteEndpoint(): string { return new ApiRequest().cohorts().assembleEndpointUrl() }, determineListUrl(cohortId: number | 'new', params: PersonListParams): string { return `/api/cohort/${cohortId}/persons?${toParams(params)}` }, + async listPaginated( + params: { + limit?: number + offset?: number + search?: string + } = {} + ): Promise> { + return await new ApiRequest().cohorts().withQueryString(toParams(params)).get() + }, }, dashboards: { diff --git a/frontend/src/lib/components/InsightLegend/InsightLegendRow.tsx b/frontend/src/lib/components/InsightLegend/InsightLegendRow.tsx index 09ba6a58507e5..cbf1f97fc1454 100644 --- a/frontend/src/lib/components/InsightLegend/InsightLegendRow.tsx +++ b/frontend/src/lib/components/InsightLegend/InsightLegendRow.tsx @@ -48,7 +48,7 @@ export function InsightLegendRow({ rowIndex, item }: InsightLegendRowProps): JSX const formattedBreakdownValue = formatBreakdownLabel( item.breakdown_value, breakdownFilter, - cohorts, + cohorts.results, formatPropertyValueForDisplay ) diff --git a/frontend/src/lib/components/PropertyFilters/components/taxonomicPropertyFilterLogic.ts b/frontend/src/lib/components/PropertyFilters/components/taxonomicPropertyFilterLogic.ts index d2b930c734612..d9daa562377a6 100644 --- a/frontend/src/lib/components/PropertyFilters/components/taxonomicPropertyFilterLogic.ts +++ b/frontend/src/lib/components/PropertyFilters/components/taxonomicPropertyFilterLogic.ts @@ -68,7 +68,8 @@ export const taxonomicPropertyFilterLogic = kea [s.filter, cohortsModel.selectors.cohorts], - (filter, cohorts) => (filter?.type === 'cohort' ? cohorts.find((c) => c.id === filter?.value)?.name : null), + (filter, cohorts) => + filter?.type === 'cohort' ? cohorts.results.find((c) => c.id === filter?.value)?.name : null, ], activeTaxonomicGroup: [ (s) => [s.filter, s.taxonomicGroups], diff --git a/frontend/src/lib/components/TaxonomicFilter/infiniteListLogic.ts b/frontend/src/lib/components/TaxonomicFilter/infiniteListLogic.ts index 47bdd36a16b8e..c4547085fbb21 100644 --- a/frontend/src/lib/components/TaxonomicFilter/infiniteListLogic.ts +++ b/frontend/src/lib/components/TaxonomicFilter/infiniteListLogic.ts @@ -247,7 +247,8 @@ export const infiniteListLogic = kea([ if (group?.logic && group?.value) { let items = group.logic.selectors[group.value]?.(state) - if (group?.value === 'featureFlags' && items.results) { + // Handle paginated responses for cohorts, which return a CountedPaginatedResponse + if (items?.results) { items = items.results } // TRICKY: Feature flags don't support dynamic behavioral cohorts, diff --git a/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx b/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx index 052b3bc459ec4..32c55402c74dc 100644 --- a/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx +++ b/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx @@ -24,7 +24,6 @@ import { ReplayTaxonomicFilters } from 'scenes/session-recordings/filters/Replay import { teamLogic } from 'scenes/teamLogic' import { actionsModel } from '~/models/actionsModel' -import { cohortsModel } from '~/models/cohortsModel' import { dashboardsModel } from '~/models/dashboardsModel' import { groupPropertiesModel } from '~/models/groupPropertiesModel' import { groupsModel } from '~/models/groupsModel' @@ -355,7 +354,7 @@ export const taxonomicFilterLogic = kea([ name: 'Cohorts', searchPlaceholder: 'cohorts', type: TaxonomicFilterGroupType.Cohorts, - logic: cohortsModel, + endpoint: combineUrl(`api/projects/${projectId}/cohorts/`).url, value: 'cohorts', getName: (cohort: CohortType) => cohort.name || `Cohort ${cohort.id}`, getValue: (cohort: CohortType) => cohort.id, @@ -368,7 +367,7 @@ export const taxonomicFilterLogic = kea([ name: 'Cohorts', searchPlaceholder: 'cohorts', type: TaxonomicFilterGroupType.CohortsWithAllUsers, - logic: cohortsModel, + endpoint: combineUrl(`api/projects/${projectId}/cohorts/`).url, value: 'cohortsWithAllUsers', getName: (cohort: CohortType) => cohort.name || `Cohort ${cohort.id}`, getValue: (cohort: CohortType) => cohort.id, diff --git a/frontend/src/models/cohortsModel.test.ts b/frontend/src/models/cohortsModel.test.ts new file mode 100644 index 0000000000000..b34dd64207c20 --- /dev/null +++ b/frontend/src/models/cohortsModel.test.ts @@ -0,0 +1,214 @@ +import { router } from 'kea-router' +import { expectLogic } from 'kea-test-utils' +import api from 'lib/api' +import { urls } from 'scenes/urls' + +import { useMocks } from '~/mocks/jest' +import { initKeaTests } from '~/test/init' +import { CohortType, FilterLogicalOperator } from '~/types' + +import { cohortsModel } from './cohortsModel' + +const MOCK_COHORTS = { + count: 2, + results: [ + { + id: 1, + name: 'Cohort one', + count: 1, + groups: [], + filters: { + properties: { + type: 'AND', + values: [], + }, + }, + is_calculating: false, + is_static: false, + created_at: '2023-08-01T00:00:00Z', + }, + { + id: 2, + name: 'Cohort two', + count: 2, + groups: [], + filters: { + properties: { + type: 'AND', + values: [], + }, + }, + is_calculating: true, + is_static: false, + created_at: '2023-08-02T00:00:00Z', + }, + ], +} + +describe('cohortsModel', () => { + let logic: ReturnType + + beforeEach(() => { + useMocks({ + get: { + '/api/projects/:team/cohorts/': MOCK_COHORTS, + }, + delete: { + '/api/projects/:team/cohorts/:id/': { success: true }, + }, + patch: { + '/api/projects/:team/cohorts/:id/': (req) => { + const data = req.body as Record + return { ...MOCK_COHORTS.results[0], ...data } + }, + }, + }) + initKeaTests() + logic = cohortsModel() + logic.mount() + }) + + describe('core assumptions', () => { + it('loads cohorts on mount', async () => { + await expectLogic(logic).toDispatchActions(['loadCohorts', 'loadCohortsSuccess']) + expect(logic.values.cohorts.results).toHaveLength(2) + expect(logic.values.cohortsWithAllUsers).toHaveLength(3) // includes "All Users" + }) + + it('sets polling timeout for calculating cohorts when on cohorts page', async () => { + // Set the current location to the cohorts page + router.actions.push(urls.cohorts()) + + await expectLogic(logic).toDispatchActions(['loadCohorts', 'loadCohortsSuccess']) + expect(logic.values.pollTimeout).not.toBeNull() + }) + + it('does not set polling timeout when not on cohorts page', async () => { + // Set the current location to a different page + router.actions.push(urls.dashboards()) + + // Mock API to return cohorts with no calculating ones + useMocks({ + get: { + '/api/projects/:team/cohorts/': { + ...MOCK_COHORTS, + results: MOCK_COHORTS.results.map((c) => ({ ...c, is_calculating: false })), + }, + }, + }) + + await expectLogic(logic).toDispatchActions(['loadCohorts', 'loadCohortsSuccess']) + expect(logic.values.pollTimeout).toBeNull() + }) + }) + + describe('cohort filters', () => { + it('can set and update filters', async () => { + // Navigate to cohorts page first + router.actions.push(urls.cohorts()) + + // Wait for initial load + await expectLogic(logic).toDispatchActions(['loadCohortsSuccess']) + + // Test search filter + await expectLogic(logic, () => { + logic.actions.setCohortFilters({ search: 'test' }) + }) + .toDispatchActions(['setCohortFilters', 'loadCohorts', 'loadCohortsSuccess']) + .toMatchValues({ + cohortFilters: expect.objectContaining({ search: 'test' }), + }) + + // Test pagination + await expectLogic(logic, () => { + logic.actions.setCohortFilters({ page: 2 }) + }) + .toDispatchActions(['setCohortFilters', 'loadCohorts', 'loadCohortsSuccess']) + .toMatchValues({ + cohortFilters: expect.objectContaining({ page: 2 }), + }) + }) + }) + + describe('cohort operations', () => { + it('can update a cohort', async () => { + // Wait for initial load + await expectLogic(logic).toDispatchActions(['loadCohortsSuccess']) + + const updatedCohort: CohortType = { + id: 1, + name: 'Updated name', + count: 1, + groups: [], + filters: { + properties: { + type: FilterLogicalOperator.And, + values: [], + }, + }, + is_calculating: false, + is_static: false, + } + + await expectLogic(logic, () => { + logic.actions.updateCohort(updatedCohort) + }).toMatchValues({ + cohorts: expect.objectContaining({ + results: expect.arrayContaining([ + expect.objectContaining({ + id: 1, + name: 'Updated name', + }), + ]), + }), + }) + }) + + it('can delete a cohort', async () => { + // Wait for initial load + await expectLogic(logic).toDispatchActions(['loadCohortsSuccess']) + + jest.spyOn(api.cohorts, 'determineDeleteEndpoint').mockImplementation(() => 'cohorts') + + await expectLogic(logic, () => { + logic.actions.deleteCohort({ id: 1 }) + }) + .toDispatchActions(['deleteCohort']) + .toMatchValues({ + cohorts: expect.objectContaining({ + results: expect.not.arrayContaining([ + expect.objectContaining({ + id: 1, + }), + ]), + }), + }) + }) + }) + + describe('selectors', () => { + it('correctly calculates pagination values', async () => { + // Wait for initial load + await expectLogic(logic).toDispatchActions(['loadCohortsSuccess']) + + await expectLogic(logic).toMatchValues({ + pagination: expect.objectContaining({ + currentPage: 1, + pageSize: 100, + entryCount: 2, + }), + }) + }) + + it('correctly maps cohorts by id', async () => { + await expectLogic(logic) + .toDispatchActions(['loadCohortsSuccess']) + .toMatchValues({ + cohortsById: expect.objectContaining({ + 1: expect.objectContaining({ id: 1, name: 'Cohort one' }), + 2: expect.objectContaining({ id: 2, name: 'Cohort two' }), + }), + }) + }) + }) +}) diff --git a/frontend/src/models/cohortsModel.ts b/frontend/src/models/cohortsModel.ts index d86d5052459f2..947fa022e3017 100644 --- a/frontend/src/models/cohortsModel.ts +++ b/frontend/src/models/cohortsModel.ts @@ -1,13 +1,15 @@ -import Fuse from 'fuse.js' import { actions, afterMount, beforeUnmount, connect, kea, listeners, path, reducers, selectors } from 'kea' import { loaders } from 'kea-loaders' -import api from 'lib/api' +import { actionToUrl, router, urlToAction } from 'kea-router' +import api, { CountedPaginatedResponse } from 'lib/api' import { exportsLogic } from 'lib/components/ExportButton/exportsLogic' +import { PaginationManual } from 'lib/lemon-ui/PaginationControl' import { deleteWithUndo } from 'lib/utils/deleteWithUndo' import { permanentlyMount } from 'lib/utils/kea-logic-builders' import { COHORT_EVENT_TYPES_WITH_EXPLICIT_DATETIME } from 'scenes/cohorts/CohortFilters/constants' import { BehavioralFilterKey } from 'scenes/cohorts/CohortFilters/types' import { personsLogic } from 'scenes/persons/personsLogic' +import { personsManagementSceneLogic } from 'scenes/persons-management/personsManagementSceneLogic' import { isAuthenticatedTeam, teamLogic } from 'scenes/teamLogic' import { urls } from 'scenes/urls' @@ -24,6 +26,18 @@ import type { cohortsModelType } from './cohortsModelType' const POLL_TIMEOUT = 5000 +export const COHORTS_PER_PAGE = 100 + +export interface CohortFilters { + search?: string + page?: number +} + +export const DEFAULT_COHORT_FILTERS: CohortFilters = { + search: undefined, + page: 1, +} + export function processCohort(cohort: CohortType): CohortType { return { ...cohort, @@ -91,7 +105,7 @@ export const cohortsModel = kea([ path(['models', 'cohortsModel']), connect(() => ({ values: [teamLogic, ['currentTeam']], - actions: [exportsLogic, ['startExport']], + actions: [exportsLogic, ['startExport'], personsManagementSceneLogic, ['setTabKey']], })), actions(() => ({ setPollTimeout: (pollTimeout: number | null) => ({ pollTimeout }), @@ -99,15 +113,20 @@ export const cohortsModel = kea([ deleteCohort: (cohort: Partial) => ({ cohort }), cohortCreated: (cohort: CohortType) => ({ cohort }), exportCohortPersons: (id: CohortType['id'], columns?: string[]) => ({ id, columns }), + setCohortFilters: (filters: Partial) => ({ filters }), })), - loaders(() => ({ + loaders(({ values }) => ({ cohorts: { - __default: [] as CohortType[], + __default: { count: 0, results: [] } as CountedPaginatedResponse, loadCohorts: async () => { - // TRICKY in tests this was returning undefined without calling list - const response = await api.cohorts.list() - personsLogic.findMounted({ syncWithUrl: true })?.actions.loadCohorts() // To ensure sync on person page - return response?.results?.map((cohort) => processCohort(cohort)) || [] + const response = await api.cohorts.listPaginated({ + ...values.paramsFromFilters, + }) + personsLogic.findMounted({ syncWithUrl: true })?.actions.loadCohorts() + return { + count: response.count, + results: response.results.map((cohort) => processCohort(cohort)), + } }, }, })), @@ -123,46 +142,73 @@ export const cohortsModel = kea([ if (!cohort) { return state } - return [...state].map((existingCohort) => (existingCohort.id === cohort.id ? cohort : existingCohort)) + return { + ...state, + results: state.results.map((existingCohort) => + existingCohort.id === cohort.id ? cohort : existingCohort + ), + } }, cohortCreated: (state, { cohort }) => { if (!cohort) { return state } - return [cohort, ...state] + return { + ...state, + results: [cohort, ...state.results], + } }, deleteCohort: (state, { cohort }) => { if (!cohort.id) { return state } - return [...state].filter((c) => c.id !== cohort.id) + return { + ...state, + results: state.results.filter((c) => c.id !== cohort.id), + } }, }, + cohortFilters: [ + DEFAULT_COHORT_FILTERS, + { + setCohortFilters: (state, { filters }) => { + return { ...state, ...filters } + }, + }, + ], }), selectors({ - cohortsWithAllUsers: [(s) => [s.cohorts], (cohorts) => [{ id: 'all', name: 'All Users*' }, ...cohorts]], + cohortsWithAllUsers: [(s) => [s.cohorts], (cohorts) => [{ id: 'all', name: 'All Users*' }, ...cohorts.results]], cohortsById: [ (s) => [s.cohorts], (cohorts): Partial> => - Object.fromEntries(cohorts.map((cohort) => [cohort.id, cohort])), + Object.fromEntries(cohorts.results.map((cohort) => [cohort.id, cohort])), ], - - cohortsSearch: [ - (s) => [s.cohorts], - (cohorts): ((term: string) => CohortType[]) => { - const fuse = new Fuse(cohorts ?? [], { - keys: ['name'], - threshold: 0.3, - }) - - return (term) => fuse.search(term).map((result) => result.item) + count: [(selectors) => [selectors.cohorts], (cohorts) => cohorts.count], + paramsFromFilters: [ + (s) => [s.cohortFilters], + (filters: CohortFilters) => ({ + ...filters, + limit: COHORTS_PER_PAGE, + offset: filters.page ? (filters.page - 1) * COHORTS_PER_PAGE : 0, + }), + ], + pagination: [ + (s) => [s.cohortFilters, s.count], + (filters, count): PaginationManual => { + return { + controlled: true, + pageSize: COHORTS_PER_PAGE, + currentPage: filters.page || 1, + entryCount: count, + } }, ], }), listeners(({ actions }) => ({ - loadCohortsSuccess: async ({ cohorts }: { cohorts: CohortType[] }) => { - const is_calculating = cohorts.filter((cohort) => cohort.is_calculating).length > 0 - if (!is_calculating || !window.location.pathname.includes(urls.cohorts())) { + loadCohortsSuccess: async ({ cohorts }: { cohorts: CountedPaginatedResponse }) => { + const is_calculating = cohorts.results.filter((cohort) => cohort.is_calculating).length > 0 + if (!is_calculating || !router.values.location.pathname.includes(urls.cohorts())) { return } actions.setPollTimeout(window.setTimeout(actions.loadCohorts, POLL_TIMEOUT)) @@ -172,7 +218,8 @@ export const cohortsModel = kea([ export_format: ExporterFormat.CSV, export_context: { path: `/api/cohort/${id}/persons`, - }, + columns, + } as { path: string; columns?: string[] }, } if (columns && columns.length > 0) { exportCommand.export_context['columns'] = columns @@ -186,11 +233,47 @@ export const cohortsModel = kea([ callback: actions.loadCohorts, }) }, + setCohortFilters: async () => { + if (!router.values.location.pathname.includes(urls.cohorts())) { + return + } + actions.loadCohorts() + }, + })), + actionToUrl(({ values }) => ({ + setCohortFilters: () => { + const searchParams: Record = { + ...values.cohortFilters, + } + + // Only include non-default values in URL + Object.keys(searchParams).forEach((key) => { + if ( + searchParams[key] === undefined || + searchParams[key] === DEFAULT_COHORT_FILTERS[key as keyof CohortFilters] + ) { + delete searchParams[key] + } + }) + + return [router.values.location.pathname, searchParams, router.values.hashParams, { replace: true }] + }, + })), + urlToAction(({ actions }) => ({ + [urls.cohorts()]: (_, searchParams) => { + const { page, search } = searchParams + const filtersFromUrl: Partial = { + search, + } + + filtersFromUrl.page = page !== undefined ? parseInt(page) : undefined + + actions.setCohortFilters({ ...DEFAULT_COHORT_FILTERS, ...filtersFromUrl }) + }, })), beforeUnmount(({ values }) => { clearTimeout(values.pollTimeout || undefined) }), - afterMount(({ actions, values }) => { if (isAuthenticatedTeam(values.currentTeam)) { // Don't load on shared insights/dashboards diff --git a/frontend/src/queries/nodes/InsightViz/ResultCustomizationsModal.tsx b/frontend/src/queries/nodes/InsightViz/ResultCustomizationsModal.tsx index bcfcc4f6da9c1..13da836db2aaa 100644 --- a/frontend/src/queries/nodes/InsightViz/ResultCustomizationsModal.tsx +++ b/frontend/src/queries/nodes/InsightViz/ResultCustomizationsModal.tsx @@ -108,7 +108,7 @@ function TrendsInfo({ dataset, resultCustomizationBy }: TrendsInfoProps): JSX.El {formatBreakdownLabel( dataset.breakdown_value, breakdownFilter, - cohorts, + cohorts.results, formatPropertyValueForDisplay )} diff --git a/frontend/src/scenes/cohorts/Cohorts.tsx b/frontend/src/scenes/cohorts/Cohorts.tsx index 74a7d156df45d..60ca84d5c4c5a 100644 --- a/frontend/src/scenes/cohorts/Cohorts.tsx +++ b/frontend/src/scenes/cohorts/Cohorts.tsx @@ -21,10 +21,10 @@ import { CohortType, ProductKey } from '~/types' import { cohortsModel } from '../../models/cohortsModel' export function Cohorts(): JSX.Element { - const { cohorts, cohortsSearch, cohortsLoading } = useValues(cohortsModel) - const { deleteCohort, exportCohortPersons } = useActions(cohortsModel) + const { cohorts, cohortsLoading, pagination, cohortFilters } = useValues(cohortsModel) + const { deleteCohort, exportCohortPersons, setCohortFilters } = useActions(cohortsModel) const { searchParams } = useValues(router) - const [searchTerm, setSearchTerm] = useState('') + const [searchTerm, setSearchTerm] = useState(cohortFilters.search || '') const columns: LemonTableColumns = [ { @@ -142,7 +142,7 @@ export function Cohorts(): JSX.Element { productKey={ProductKey.COHORTS} thingName="cohort" description="Use cohorts to group people together, such as users who used your app in the last week, or people who viewed the signup page but didn’t convert." - isEmpty={cohorts?.length == 0 && !cohortsLoading} + isEmpty={cohorts.count == 0 && !cohortsLoading && !searchTerm} docsURL="https://posthog.com/docs/data/cohorts" action={() => router.actions.push(urls.cohort('new'))} customHog={ListHog} @@ -152,7 +152,10 @@ export function Cohorts(): JSX.Element { { + setSearchTerm(search) + setCohortFilters({ search: search || undefined, page: 1 }) + }} value={searchTerm} /> @@ -160,8 +163,8 @@ export function Cohorts(): JSX.Element { columns={columns} loading={cohortsLoading} rowKey="id" - pagination={{ pageSize: 100 }} - dataSource={searchTerm ? cohortsSearch(searchTerm) : cohorts ?? []} + pagination={pagination} + dataSource={cohorts.results} nouns={['cohort', 'cohorts']} data-attr="cohorts-table" /> diff --git a/frontend/src/scenes/feature-flags/FeatureFlagProjects.tsx b/frontend/src/scenes/feature-flags/FeatureFlagProjects.tsx index 731aa051adb39..9caff55937785 100644 --- a/frontend/src/scenes/feature-flags/FeatureFlagProjects.tsx +++ b/frontend/src/scenes/feature-flags/FeatureFlagProjects.tsx @@ -138,7 +138,7 @@ function FeatureFlagCopySection(): JSX.Element { const { currentTeam } = useValues(teamLogic) const { cohorts } = useValues(cohortsModel) - const hasStaticCohort = checkHasStaticCohort(featureFlag, cohorts) + const hasStaticCohort = checkHasStaticCohort(featureFlag, cohorts.results) const hasMultipleProjects = (currentOrganization?.teams?.length ?? 0) > 1 return hasMultipleProjects && featureFlag.can_edit ? ( diff --git a/frontend/src/scenes/funnels/FunnelTooltip.tsx b/frontend/src/scenes/funnels/FunnelTooltip.tsx index db1c8cdbb5d80..e9f69714b8ee7 100644 --- a/frontend/src/scenes/funnels/FunnelTooltip.tsx +++ b/frontend/src/scenes/funnels/FunnelTooltip.tsx @@ -58,7 +58,7 @@ export function FunnelTooltip({ {formatBreakdownLabel( series.breakdown_value, breakdownFilter, - cohorts, + cohorts.results, formatPropertyValueForDisplay )} diff --git a/frontend/src/scenes/insights/InsightTooltip/insightTooltipUtils.tsx b/frontend/src/scenes/insights/InsightTooltip/insightTooltipUtils.tsx index 786a4d9872dc8..383ccd17f2d8e 100644 --- a/frontend/src/scenes/insights/InsightTooltip/insightTooltipUtils.tsx +++ b/frontend/src/scenes/insights/InsightTooltip/insightTooltipUtils.tsx @@ -113,7 +113,12 @@ export function invertDataSource( const pillValues = [] if (s.breakdown_value !== undefined) { pillValues.push( - formatBreakdownLabel(s.breakdown_value, breakdownFilter, cohorts, formatPropertyValueForDisplay) + formatBreakdownLabel( + s.breakdown_value, + breakdownFilter, + cohorts?.results, + formatPropertyValueForDisplay + ) ) } if (s.compare_label) { diff --git a/frontend/src/scenes/insights/views/Funnels/FunnelStepsTable.tsx b/frontend/src/scenes/insights/views/Funnels/FunnelStepsTable.tsx index 2907f1f3cfa0c..45376da9bf15d 100644 --- a/frontend/src/scenes/insights/views/Funnels/FunnelStepsTable.tsx +++ b/frontend/src/scenes/insights/views/Funnels/FunnelStepsTable.tsx @@ -97,7 +97,12 @@ export function FunnelStepsTable(): JSX.Element | null { onMouseEnter={() => setIsHovering(true)} onMouseLeave={() => setIsHovering(false)} > - {formatBreakdownLabel(value, breakdownFilter, cohorts, formatPropertyValueForDisplay)} + {formatBreakdownLabel( + value, + breakdownFilter, + cohorts.results, + formatPropertyValueForDisplay + )} {showCustomizationIcon && } ) diff --git a/frontend/src/scenes/insights/views/InsightsTable/InsightsTable.tsx b/frontend/src/scenes/insights/views/InsightsTable/InsightsTable.tsx index b9ad89d921679..b4c362308bee0 100644 --- a/frontend/src/scenes/insights/views/InsightsTable/InsightsTable.tsx +++ b/frontend/src/scenes/insights/views/InsightsTable/InsightsTable.tsx @@ -146,7 +146,7 @@ export function InsightsTable({ if (breakdownFilter?.breakdown) { const formatItemBreakdownLabel = (item: IndexedTrendResult): string => - formatBreakdownLabel(item.breakdown_value, breakdownFilter, cohorts, formatPropertyValueForDisplay) + formatBreakdownLabel(item.breakdown_value, breakdownFilter, cohorts?.results, formatPropertyValueForDisplay) columns.push({ title: , @@ -182,7 +182,7 @@ export function InsightsTable({ formatBreakdownLabel( Array.isArray(item.breakdown_value) ? item.breakdown_value[index] : item.breakdown_value, breakdownFilter, - cohorts, + cohorts?.results, formatPropertyValueForDisplay, index ) diff --git a/frontend/src/scenes/persons-management/personsManagementSceneLogic.tsx b/frontend/src/scenes/persons-management/personsManagementSceneLogic.tsx index 32534ae82c10a..15467204acc16 100644 --- a/frontend/src/scenes/persons-management/personsManagementSceneLogic.tsx +++ b/frontend/src/scenes/persons-management/personsManagementSceneLogic.tsx @@ -137,7 +137,12 @@ export const personsManagementSceneLogic = kea( }), actionToUrl(({ values }) => ({ setTabKey: ({ tabKey }) => { - return values.tabs.find((x) => x.key === tabKey)?.url || values.tabs[0].url + const tab = values.tabs.find((x) => x.key === tabKey) + if (!tab) { + return values.tabs[0].url + } + // Preserve existing search params when changing tabs + return [tab.url, router.values.searchParams, router.values.hashParams, { replace: true }] }, })), urlToAction(({ actions }) => { diff --git a/frontend/src/scenes/trends/viz/ActionsHorizontalBar.tsx b/frontend/src/scenes/trends/viz/ActionsHorizontalBar.tsx index 3da099c8a3e81..c6fc91c871b80 100644 --- a/frontend/src/scenes/trends/viz/ActionsHorizontalBar.tsx +++ b/frontend/src/scenes/trends/viz/ActionsHorizontalBar.tsx @@ -52,7 +52,7 @@ export function ActionsHorizontalBar({ showPersonsModal = true }: ChartParams): return formatBreakdownLabel( item.breakdown_value, breakdownFilter, - cohorts, + cohorts?.results, formatPropertyValueForDisplay ) }), diff --git a/frontend/src/scenes/trends/viz/ActionsPie.tsx b/frontend/src/scenes/trends/viz/ActionsPie.tsx index b05ccc2eec093..69693facd6aae 100644 --- a/frontend/src/scenes/trends/viz/ActionsPie.tsx +++ b/frontend/src/scenes/trends/viz/ActionsPie.tsx @@ -60,7 +60,7 @@ export function ActionsPie({ inSharedMode, showPersonsModal = true, context }: C return formatBreakdownLabel( item.breakdown_value, breakdownFilter, - cohorts, + cohorts.results, formatPropertyValueForDisplay ) }), diff --git a/posthog/api/cohort.py b/posthog/api/cohort.py index 718ce8689cbc7..ecc3b04b8d2e2 100644 --- a/posthog/api/cohort.py +++ b/posthog/api/cohort.py @@ -305,6 +305,10 @@ def safely_get_queryset(self, queryset) -> QuerySet: if self.action == "list": queryset = queryset.filter(deleted=False) + search_query = self.request.query_params.get("search", None) + if search_query: + queryset = queryset.filter(name__icontains=search_query) + return queryset.prefetch_related("experiment_set", "created_by", "team").order_by("-created_at") @action( diff --git a/posthog/api/test/test_cohort.py b/posthog/api/test/test_cohort.py index 8a32a8b9a5dd0..ea07f5ff91aa1 100644 --- a/posthog/api/test/test_cohort.py +++ b/posthog/api/test/test_cohort.py @@ -375,21 +375,32 @@ def test_static_cohort_to_dynamic_cohort(self, patch_calculate_cohort, patch_cal self.assertEqual(response.status_code, 200) self.assertEqual(patch_calculate_cohort.call_count, 1) - def test_cohort_list(self): + def test_cohort_list_with_search(self): self.team.app_urls = ["http://somewebsite.com"] self.team.save() + Person.objects.create(team=self.team, properties={"prop": 5}) Person.objects.create(team=self.team, properties={"prop": 6}) self.client.post( f"/api/projects/{self.team.id}/cohorts", - data={"name": "whatever", "groups": [{"properties": {"prop": 5}}]}, + data={"name": "cohort1", "groups": [{"properties": {"prop": 5}}]}, + ) + + self.client.post( + f"/api/projects/{self.team.id}/cohorts", + data={"name": "cohort2", "groups": [{"properties": {"prop": 6}}]}, ) response = self.client.get(f"/api/projects/{self.team.id}/cohorts").json() + self.assertEqual(len(response["results"]), 2) + + response = self.client.get(f"/api/projects/{self.team.id}/cohorts?search=cohort1").json() self.assertEqual(len(response["results"]), 1) - self.assertEqual(response["results"][0]["name"], "whatever") - self.assertEqual(response["results"][0]["created_by"]["id"], self.user.id) + self.assertEqual(response["results"][0]["name"], "cohort1") + + response = self.client.get(f"/api/projects/{self.team.id}/cohorts?search=nomatch").json() + self.assertEqual(len(response["results"]), 0) def test_cohort_activity_log(self): self.team.app_urls = ["http://somewebsite.com"]