From c6eb28e0f338599d3e48e5ecee61acabe7ddd17c Mon Sep 17 00:00:00 2001 From: Michael Matloka Date: Thu, 28 Mar 2024 22:54:31 +0100 Subject: [PATCH] Unify display strings --- frontend/src/scenes/insights/utils.tsx | 12 +++++++----- .../src/scenes/trends/viz/ActionsHorizontalBar.tsx | 12 +++++++++--- .../insights/trends/breakdown_values.py | 2 ++ .../insights/trends/trends_query_runner.py | 6 ++++-- posthog/queries/trends/breakdown.py | 7 ++++--- 5 files changed, 26 insertions(+), 13 deletions(-) diff --git a/frontend/src/scenes/insights/utils.tsx b/frontend/src/scenes/insights/utils.tsx index 8ea2a9faf3de7..212eff05f6c75 100644 --- a/frontend/src/scenes/insights/utils.tsx +++ b/frontend/src/scenes/insights/utils.tsx @@ -216,11 +216,13 @@ export function formatAggregationValue( return Array.isArray(formattedValue) ? formattedValue[0] : formattedValue } -// NB! Sync this with breakdown.py +// NB! Sync this with breakdown_values.py export const BREAKDOWN_OTHER_STRING_LABEL = '$$_posthog_breakdown_other_$$' export const BREAKDOWN_OTHER_NUMERIC_LABEL = 9007199254740991 // pow(2, 53) - 1 +export const BREAKDOWN_OTHER_DISPLAY = 'Other (i.e. all remaining values)' export const BREAKDOWN_NULL_STRING_LABEL = '$$_posthog_breakdown_null_$$' export const BREAKDOWN_NULL_NUMERIC_LABEL = 9007199254740990 // pow(2, 53) - 2 +export const BREAKDOWN_NULL_DISPLAY = 'None (i.e. no value)' export function isOtherBreakdown(breakdown_value: string | number | null | undefined | ReactNode): boolean { return ( @@ -277,17 +279,17 @@ export function formatBreakdownLabel( return cohorts?.filter((c) => c.id == breakdown_value)[0]?.name ?? (breakdown_value || '').toString() } else if (typeof breakdown_value == 'number') { return isOtherBreakdown(breakdown_value) - ? 'Other (i.e. all remaining values)' + ? BREAKDOWN_OTHER_DISPLAY : isNullBreakdown(breakdown_value) - ? 'None (i.e. no value)' + ? BREAKDOWN_NULL_DISPLAY : formatPropertyValueForDisplay ? formatPropertyValueForDisplay(breakdown, breakdown_value)?.toString() ?? 'None' : String(breakdown_value) } else if (typeof breakdown_value == 'string') { return isOtherBreakdown(breakdown_value) || breakdown_value === 'nan' - ? 'Other (i.e. all remaining values)' + ? BREAKDOWN_OTHER_DISPLAY : isNullBreakdown(breakdown_value) || breakdown_value === '' - ? 'None (i.e. no value)' + ? BREAKDOWN_NULL_DISPLAY : breakdown_value } else if (Array.isArray(breakdown_value)) { return breakdown_value diff --git a/frontend/src/scenes/trends/viz/ActionsHorizontalBar.tsx b/frontend/src/scenes/trends/viz/ActionsHorizontalBar.tsx index 858a319e26263..281a3726984ad 100644 --- a/frontend/src/scenes/trends/viz/ActionsHorizontalBar.tsx +++ b/frontend/src/scenes/trends/viz/ActionsHorizontalBar.tsx @@ -6,7 +6,13 @@ 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 { + BREAKDOWN_NULL_DISPLAY, + BREAKDOWN_OTHER_DISPLAY, + formatBreakdownLabel, + isNullBreakdown, + isOtherBreakdown, +} from 'scenes/insights/utils' import { cohortsModel } from '~/models/cohortsModel' import { propertyDefinitionsModel } from '~/models/propertyDefinitionsModel' @@ -43,9 +49,9 @@ export function ActionsHorizontalBar({ showPersonsModal = true }: ChartParams): { labels: _data.map((item) => isOtherBreakdown(item.label) - ? 'Other (i.e. all remaining values)' + ? BREAKDOWN_OTHER_DISPLAY : isNullBreakdown(item.label) - ? 'None (i.e. no value)' + ? BREAKDOWN_NULL_DISPLAY : item.label ), data: _data.map((item) => item.aggregated_value), diff --git a/posthog/hogql_queries/insights/trends/breakdown_values.py b/posthog/hogql_queries/insights/trends/breakdown_values.py index 4462bf6bff5c9..4214febed5c52 100644 --- a/posthog/hogql_queries/insights/trends/breakdown_values.py +++ b/posthog/hogql_queries/insights/trends/breakdown_values.py @@ -21,8 +21,10 @@ BREAKDOWN_OTHER_STRING_LABEL = "$$_posthog_breakdown_other_$$" BREAKDOWN_OTHER_NUMERIC_LABEL = 9007199254740991 # pow(2, 53) - 1, for JS compatibility +BREAKDOWN_OTHER_DISPLAY = "Other (i.e. all remaining values)" BREAKDOWN_NULL_STRING_LABEL = "$$_posthog_breakdown_null_$$" BREAKDOWN_NULL_NUMERIC_LABEL = 9007199254740990 # pow(2, 53) - 2, for JS compatibility +BREAKDOWN_NULL_DISPLAY = "None (i.e. no value)" class BreakdownValues: diff --git a/posthog/hogql_queries/insights/trends/trends_query_runner.py b/posthog/hogql_queries/insights/trends/trends_query_runner.py index cc0cfc7087509..6e68205038990 100644 --- a/posthog/hogql_queries/insights/trends/trends_query_runner.py +++ b/posthog/hogql_queries/insights/trends/trends_query_runner.py @@ -23,7 +23,9 @@ from posthog.hogql.query import execute_hogql_query from posthog.hogql.timings import HogQLTimings from posthog.hogql_queries.insights.trends.breakdown_values import ( + BREAKDOWN_NULL_DISPLAY, BREAKDOWN_NULL_STRING_LABEL, + BREAKDOWN_OTHER_DISPLAY, BREAKDOWN_OTHER_STRING_LABEL, ) from posthog.hogql_queries.insights.trends.display import TrendsDisplay @@ -245,9 +247,9 @@ def to_actors_query_options(self) -> InsightActorsQueryOptionsResponse: label = cohort_name value = value elif value == BREAKDOWN_OTHER_STRING_LABEL: - label = "Other (i.e. all remaining values)" + label = BREAKDOWN_OTHER_DISPLAY elif value == BREAKDOWN_NULL_STRING_LABEL: - label = "None (i.e. no value)" + label = BREAKDOWN_NULL_DISPLAY elif is_boolean_breakdown: label = self._convert_boolean(value) else: diff --git a/posthog/queries/trends/breakdown.py b/posthog/queries/trends/breakdown.py index be5ce8c5abd41..84b304ff99b28 100644 --- a/posthog/queries/trends/breakdown.py +++ b/posthog/queries/trends/breakdown.py @@ -17,6 +17,7 @@ PropertyOperatorType, TREND_FILTER_TYPE_EVENTS, ) +from posthog.hogql_queries.insights.trends.breakdown_values import BREAKDOWN_NULL_DISPLAY, BREAKDOWN_OTHER_DISPLAY from posthog.models.action.util import format_action_filter from posthog.models.entity import Entity from posthog.models.event.sql import EVENT_JOIN_PERSON_SQL @@ -740,11 +741,11 @@ def _determine_breakdown_label( if breakdown_type == "cohort": return get_breakdown_cohort_name(breakdown_value) elif str(value) == BREAKDOWN_OTHER_STRING_LABEL or value == BREAKDOWN_OTHER_NUMERIC_LABEL: - return "Other (i.e. all remaining values)" + return BREAKDOWN_OTHER_DISPLAY elif str(value) == BREAKDOWN_NULL_STRING_LABEL or value == BREAKDOWN_NULL_NUMERIC_LABEL: - return "None (i.e. no value)" + return BREAKDOWN_NULL_DISPLAY else: - return str(value) or "None (i.e. no value)" + return str(value) or BREAKDOWN_NULL_DISPLAY def _person_join_condition(self) -> Tuple[str, Dict]: if self.person_on_events_mode == PersonOnEventsMode.V1_ENABLED: