diff --git a/frontend/src/scenes/insights/utils.tsx b/frontend/src/scenes/insights/utils.tsx index 014e98e3078cf..73e8717be04f2 100644 --- a/frontend/src/scenes/insights/utils.tsx +++ b/frontend/src/scenes/insights/utils.tsx @@ -219,7 +219,7 @@ export const BREAKDOWN_OTHER_NUMERIC_LABEL = 9007199254740991 // pow(2, 53) - 1 export const BREAKDOWN_NULL_STRING_LABEL = '$$_posthog_breakdown_null_$$' export const BREAKDOWN_NULL_NUMERIC_LABEL = 9007199254740990 // pow(2, 53) - 2 -export function isOtherBreakdown(breakdown_value: string | number | null | undefined): boolean { +export function isOtherBreakdown(breakdown_value: string | number | null | undefined | ReactNode): boolean { return breakdown_value === BREAKDOWN_OTHER_STRING_LABEL || breakdown_value === BREAKDOWN_OTHER_NUMERIC_LABEL } diff --git a/frontend/src/scenes/insights/views/BoldNumber/BoldNumber.tsx b/frontend/src/scenes/insights/views/BoldNumber/BoldNumber.tsx index b94a691994e6c..3dc6b630c33dd 100644 --- a/frontend/src/scenes/insights/views/BoldNumber/BoldNumber.tsx +++ b/frontend/src/scenes/insights/views/BoldNumber/BoldNumber.tsx @@ -4,7 +4,9 @@ import { LemonRow, Link } from '@posthog/lemon-ui' import clsx from 'clsx' import { useValues } from 'kea' import { PropertyKeyInfo } from 'lib/components/PropertyKeyInfo' +import { FEATURE_FLAGS } from 'lib/constants' import { IconFlare, IconTrendingDown, IconTrendingFlat, IconTrendingUp } from 'lib/lemon-ui/icons' +import { featureFlagLogic } from 'lib/logic/featureFlagLogic' import { percentage } from 'lib/utils' import { useLayoutEffect, useRef, useState } from 'react' import { useEffect } from 'react' @@ -16,6 +18,8 @@ import { insightVizDataLogic } from 'scenes/insights/insightVizDataLogic' import { openPersonsModal } from 'scenes/trends/persons-modal/PersonsModal' import { groupsModel } from '~/models/groupsModel' +import { NodeKind } from '~/queries/schema' +import { isInsightVizNode, isTrendsQuery } from '~/queries/utils' import { ChartParams, TrendResult } from '~/types' import { insightLogic } from '../../insightLogic' @@ -82,7 +86,8 @@ function useBoldNumberTooltip({ export function BoldNumber({ showPersonsModal = true }: ChartParams): JSX.Element { const { insightProps } = useValues(insightLogic) - const { insightData, trendsFilter } = useValues(insightVizDataLogic(insightProps)) + const { insightData, trendsFilter, isTrends, query } = useValues(insightVizDataLogic(insightProps)) + const { featureFlags } = useValues(featureFlagLogic) const [isTooltipShown, setIsTooltipShown] = useState(false) const valueRef = useBoldNumberTooltip({ showPersonsModal, isTooltipShown }) @@ -90,6 +95,13 @@ export function BoldNumber({ showPersonsModal = true }: ChartParams): JSX.Elemen const showComparison = !!trendsFilter?.compare && insightData?.result?.length > 1 const resultSeries = insightData?.result?.[0] as TrendResult | undefined + const isTrendsQueryWithFeatureFlagOn = + featureFlags[FEATURE_FLAGS.HOGQL_INSIGHTS_TRENDS] && + isTrends && + query && + isInsightVizNode(query) && + isTrendsQuery(query.source) + return resultSeries ? (
{ - if (resultSeries.persons?.url) { + if (isTrendsQueryWithFeatureFlagOn) { + openPersonsModal({ + title: resultSeries.label, + query: { + kind: NodeKind.InsightActorsQuery, + source: query.source, + }, + }) + } else if (resultSeries.persons?.url) { openPersonsModal({ url: resultSeries.persons?.url, title: , @@ -124,7 +144,8 @@ export function BoldNumber({ showPersonsModal = true }: ChartParams): JSX.Elemen function BoldNumberComparison({ showPersonsModal }: Pick): JSX.Element | null { const { insightProps } = useValues(insightLogic) - const { insightData } = useValues(insightVizDataLogic(insightProps)) + const { insightData, isTrends, query } = useValues(insightVizDataLogic(insightProps)) + const { featureFlags } = useValues(featureFlagLogic) if (!insightData?.result) { return null @@ -149,6 +170,13 @@ function BoldNumberComparison({ showPersonsModal }: Pick { - if (previousPeriodSeries.persons?.url) { + if (isTrendsQueryWithFeatureFlagOn) { + openPersonsModal({ + title: previousPeriodSeries.label, + query: { + kind: NodeKind.InsightActorsQuery, + source: query.source, + }, + }) + } else if (previousPeriodSeries.persons?.url) { openPersonsModal({ url: previousPeriodSeries.persons?.url, title: , diff --git a/frontend/src/scenes/trends/persons-modal/PersonsModal.tsx b/frontend/src/scenes/trends/persons-modal/PersonsModal.tsx index 4d8410261894e..d194cc4ccdab2 100644 --- a/frontend/src/scenes/trends/persons-modal/PersonsModal.tsx +++ b/frontend/src/scenes/trends/persons-modal/PersonsModal.tsx @@ -15,8 +15,9 @@ import { ProfilePicture } from 'lib/lemon-ui/ProfilePicture' import { Spinner } from 'lib/lemon-ui/Spinner/Spinner' import { Tooltip } from 'lib/lemon-ui/Tooltip' import { capitalizeFirstLetter, isGroupType, midEllipsis, pluralize } from 'lib/utils' -import { useState } from 'react' +import { useCallback, useState } from 'react' import { createRoot } from 'react-dom/client' +import { isOtherBreakdown } from 'scenes/insights/utils' import { GroupActorDisplay, groupDisplayId } from 'scenes/persons/GroupActorDisplay' import { asDisplay } from 'scenes/persons/person-utils' import { PersonDisplay } from 'scenes/persons/PersonDisplay' @@ -85,6 +86,18 @@ export function PersonsModal({ const totalActorsCount = missingActorsCount + actors.length + const getTitle = useCallback(() => { + if (typeof title === 'function') { + return title(capitalizeFirstLetter(actorLabel.plural)) + } + + if (isOtherBreakdown(title)) { + return 'Other' + } + + return title + }, [title, actorLabel.plural]) + return ( <> -

{typeof title === 'function' ? title(capitalizeFirstLetter(actorLabel.plural)) : title}

+

{getTitle()}
{actorsResponse && !!missingActorsCount && ( diff --git a/frontend/src/scenes/trends/viz/ActionsHorizontalBar.tsx b/frontend/src/scenes/trends/viz/ActionsHorizontalBar.tsx index 0937490e4422f..d551e1b4cf1b4 100644 --- a/frontend/src/scenes/trends/viz/ActionsHorizontalBar.tsx +++ b/frontend/src/scenes/trends/viz/ActionsHorizontalBar.tsx @@ -1,12 +1,17 @@ import { useValues } from 'kea' import { getSeriesColor } from 'lib/colors' import { PropertyKeyInfo } from 'lib/components/PropertyKeyInfo' +import { FEATURE_FLAGS } from 'lib/constants' +import { featureFlagLogic } from 'lib/logic/featureFlagLogic' import { useEffect, useState } from 'react' import { insightLogic } from 'scenes/insights/insightLogic' +import { insightVizDataLogic } from 'scenes/insights/insightVizDataLogic' import { formatBreakdownLabel, isNullBreakdown, isOtherBreakdown } from 'scenes/insights/utils' import { cohortsModel } from '~/models/cohortsModel' import { propertyDefinitionsModel } from '~/models/propertyDefinitionsModel' +import { NodeKind } from '~/queries/schema' +import { isInsightVizNode, isTrendsQuery } from '~/queries/utils' import { ChartParams, GraphType } from '~/types' import { InsightEmptyState } from '../../insights/EmptyStates' @@ -25,6 +30,8 @@ export function ActionsHorizontalBar({ showPersonsModal = true }: ChartParams): const { formatPropertyValueForDisplay } = useValues(propertyDefinitionsModel) const { insightProps, hiddenLegendKeys } = useValues(insightLogic) + const { featureFlags } = useValues(featureFlagLogic) + const { isTrends, query } = useValues(insightVizDataLogic(insightProps)) const { indexedResults, labelGroupType, trendsFilter, formula, showValueOnSeries } = useValues( trendsDataLogic(insightProps) ) @@ -69,6 +76,13 @@ export function ActionsHorizontalBar({ showPersonsModal = true }: ChartParams): } }, [indexedResults]) + const isTrendsQueryWithFeatureFlagOn = + featureFlags[FEATURE_FLAGS.HOGQL_INSIGHTS_TRENDS] && + isTrends && + query && + isInsightVizNode(query) && + isTrendsQuery(query.source) + return data && total > 0 ? ( b.aggregated_value - a.aggregated_value) const days = _data.length > 0 ? _data[0].days : [] @@ -95,7 +110,15 @@ export function ActionsPie({ const urls = urlsForDatasets(crossDataset, index, cohorts, formatPropertyValueForDisplay) const selectedUrl = urls[index]?.value - if (selectedUrl) { + if (isTrendsQueryWithFeatureFlagOn) { + openPersonsModal({ + title: label || '', + query: { + kind: NodeKind.InsightActorsQuery, + source: query.source, + }, + }) + } else if (selectedUrl) { openPersonsModal({ urls, urlsIndex: index, diff --git a/mypy-baseline.txt b/mypy-baseline.txt index 3a6d088e53ef0..d7b34f09ced75 100644 --- a/mypy-baseline.txt +++ b/mypy-baseline.txt @@ -408,6 +408,11 @@ posthog/hogql/metadata.py:0: error: Argument "metadata_source" to "translate_hog posthog/hogql/metadata.py:0: error: Incompatible types in assignment (expression has type "Expr", variable has type "SelectQuery | SelectUnionQuery") [assignment] posthog/queries/breakdown_props.py:0: error: Argument 1 to "translate_hogql" has incompatible type "str | int"; expected "str" [arg-type] posthog/hogql_queries/insights/trends/query_builder.py:0: error: Incompatible types in assignment (expression has type "SelectUnionQuery", variable has type "SelectQuery") [assignment] +posthog/hogql_queries/insights/trends/query_builder.py:0: error: Item "None" of "list[Expr] | None" has no attribute "extend" [union-attr] +posthog/hogql_queries/insights/trends/query_builder.py:0: error: Item "None" of "list[Expr] | None" has no attribute "append" [union-attr] +posthog/hogql_queries/insights/trends/query_builder.py:0: error: Item "None" of "list[Expr] | None" has no attribute "append" [union-attr] +posthog/hogql_queries/insights/trends/query_builder.py:0: error: Item "None" of "list[Expr] | None" has no attribute "append" [union-attr] +posthog/hogql_queries/insights/trends/query_builder.py:0: error: Item "None" of "list[Expr] | None" has no attribute "append" [union-attr] posthog/hogql_queries/insights/trends/query_builder.py:0: error: Item "None" of "list[Expr] | None" has no attribute "append" [union-attr] posthog/hogql_queries/insights/trends/query_builder.py:0: error: Item "None" of "list[Expr] | None" has no attribute "append" [union-attr] posthog/hogql_queries/insights/trends/query_builder.py:0: error: Item "None" of "list[Expr] | None" has no attribute "append" [union-attr] diff --git a/posthog/hogql_queries/insights/insight_actors_query_runner.py b/posthog/hogql_queries/insights/insight_actors_query_runner.py index 02dfc829a4902..2de816aa3d30b 100644 --- a/posthog/hogql_queries/insights/insight_actors_query_runner.py +++ b/posthog/hogql_queries/insights/insight_actors_query_runner.py @@ -32,7 +32,7 @@ def to_query(self) -> ast.SelectQuery | ast.SelectUnionQuery: return paths_runner.to_actors_query() elif isinstance(self.source_runner, TrendsQueryRunner): trends_runner = cast(TrendsQueryRunner, self.source_runner) - return trends_runner.to_actors_query() + return trends_runner.to_actors_query(self.query.day) elif isinstance(self.source_runner, RetentionQueryRunner): retention_runner = cast(RetentionQueryRunner, self.source_runner) return retention_runner.to_actors_query(interval=self.query.interval) diff --git a/posthog/hogql_queries/insights/trends/breakdown_values.py b/posthog/hogql_queries/insights/trends/breakdown_values.py index dcbc431f624c0..4d0f62b155f37 100644 --- a/posthog/hogql_queries/insights/trends/breakdown_values.py +++ b/posthog/hogql_queries/insights/trends/breakdown_values.py @@ -83,7 +83,7 @@ def get_breakdown_values(self) -> List[str | int]: if self.chart_display_type == ChartDisplayType.WorldMap: breakdown_limit = BREAKDOWN_VALUES_LIMIT_FOR_COUNTRIES else: - breakdown_limit = self.breakdown_limit or BREAKDOWN_VALUES_LIMIT + breakdown_limit = int(self.breakdown_limit) if self.breakdown_limit is not None else BREAKDOWN_VALUES_LIMIT inner_events_query = parse_select( """ diff --git a/posthog/hogql_queries/insights/trends/query_builder.py b/posthog/hogql_queries/insights/trends/query_builder.py index 90c6d6488046c..b311d1cd89f34 100644 --- a/posthog/hogql_queries/insights/trends/query_builder.py +++ b/posthog/hogql_queries/insights/trends/query_builder.py @@ -23,6 +23,8 @@ class TrendsQueryBuilder: series: EventsNode | ActionsNode timings: HogQLTimings modifiers: HogQLQueryModifiers + person_query_time_frame: Optional[str | int] + is_person_query: bool def __init__( self, @@ -32,6 +34,7 @@ def __init__( series: EventsNode | ActionsNode, timings: HogQLTimings, modifiers: HogQLQueryModifiers, + person_query_time_frame: Optional[str | int] = None, ): self.query = trends_query self.team = team @@ -39,8 +42,13 @@ def __init__( self.series = series self.timings = timings self.modifiers = modifiers + self.person_query_time_frame = person_query_time_frame + self.is_person_query = person_query_time_frame is not None def build_query(self) -> ast.SelectQuery | ast.SelectUnionQuery: + if self.is_person_query: + return self._get_events_subquery(True) + if self._trends_display.should_aggregate_values(): events_query = self._get_events_subquery(False) else: @@ -54,15 +62,6 @@ def build_query(self) -> ast.SelectQuery | ast.SelectUnionQuery: return full_query - def build_persons_query(self) -> ast.SelectQuery: - event_query = self._get_events_subquery(True) - - event_query.select = [ast.Alias(alias="person_id", expr=ast.Field(chain=["e", "person", "id"]))] - event_query.distinct = True - event_query.group_by = None - - return event_query - def _get_date_subqueries(self, ignore_breakdowns: bool = False) -> List[ast.SelectQuery]: if not self._breakdown.enabled or ignore_breakdowns: return [ @@ -166,6 +165,12 @@ def _get_events_subquery(self, no_modifications: Optional[bool]) -> ast.SelectQu default_query.select.append(day_start) default_query.group_by.append(ast.Field(chain=["day_start"])) + # TODO: Move this logic into the below branches when working on adding breakdown support for the person modal + if self.is_person_query: + default_query.select = [ast.Alias(alias="person_id", expr=ast.Field(chain=["e", "person", "id"]))] + default_query.distinct = True + default_query.group_by = None + # No breakdowns and no complex series aggregation if ( not self._breakdown.enabled @@ -356,7 +361,16 @@ def _events_filter(self, ignore_breakdowns: bool = False) -> ast.Expr: filters: List[ast.Expr] = [] # Dates - if not self._aggregation_operation.requires_query_orchestration(): + if self.is_person_query: + to_start_of_time_frame = f"toStartOf{self.query_date_range.interval_name.capitalize()}" + filters.append( + ast.CompareOperation( + left=ast.Call(name=to_start_of_time_frame, args=[ast.Field(chain=["timestamp"])]), + op=ast.CompareOperationOp.Eq, + right=ast.Call(name="toDateTime", args=[ast.Constant(value=self.person_query_time_frame)]), + ) + ) + elif not self._aggregation_operation.requires_query_orchestration(): filters.extend( [ parse_expr( diff --git a/posthog/hogql_queries/insights/trends/trends_query_runner.py b/posthog/hogql_queries/insights/trends/trends_query_runner.py index 899d746953647..cdff1ce337099 100644 --- a/posthog/hogql_queries/insights/trends/trends_query_runner.py +++ b/posthog/hogql_queries/insights/trends/trends_query_runner.py @@ -109,7 +109,7 @@ def to_query(self) -> List[ast.SelectQuery | ast.SelectUnionQuery]: # type: ign return queries - def to_actors_query(self) -> ast.SelectQuery | ast.SelectUnionQuery: + def to_actors_query(self, time_frame: Optional[str | int]) -> ast.SelectQuery | ast.SelectUnionQuery: # type: ignore queries = [] with self.timings.measure("trends_persons_query"): for series in self.series: @@ -125,10 +125,18 @@ def to_actors_query(self) -> ast.SelectQuery | ast.SelectUnionQuery: series=series.series, timings=self.timings, modifiers=self.modifiers, + person_query_time_frame=time_frame, ) - queries.append(query_builder.build_persons_query()) + queries.append(query_builder.build_query()) + + select_queries: List[ast.SelectQuery] = [] + for query in queries: + if isinstance(query, ast.SelectUnionQuery): + select_queries.extend(query.select_queries) + else: + select_queries.append(query) - return ast.SelectUnionQuery(select_queries=queries) + return ast.SelectUnionQuery(select_queries=select_queries) def calculate(self): queries = self.to_query()