Skip to content

Commit

Permalink
Refactored showMeanRetention to meanRetentionCalculation
Browse files Browse the repository at this point in the history
  • Loading branch information
Sriram-bk committed Feb 20, 2025
1 parent ce289f3 commit 424b98f
Show file tree
Hide file tree
Showing 17 changed files with 55 additions and 55 deletions.
2 changes: 1 addition & 1 deletion frontend/src/queries/nodes/InsightQuery/defaults.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ const retentionQueryDefault: RetentionQuery = {
type: 'events',
},
retentionType: 'retention_first_time',
showMeanRetention: 'simple',
meanRetentionCalculation: 'simple',
},
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,7 @@ describe('filtersToQueryNode', () => {
returning_entity: { id: '1' },
target_entity: { id: '1' },
period: RetentionPeriod.Day,
show_mean_retention: 'simple',
mean_retention_calculation: 'simple',
}

const result = filtersToQueryNode(filters)
Expand All @@ -683,7 +683,7 @@ describe('filtersToQueryNode', () => {
returningEntity: { id: '1' },
targetEntity: { id: '1' },
period: RetentionPeriod.Day,
showMeanRetention: 'simple',
meanRetentionCalculation: 'simple',
},
}
expect(result).toEqual(query)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -439,8 +439,8 @@ export const retentionFilterToQuery = (filters: Partial<RetentionFilterType>): R
returningEntity: sanitizeRetentionEntity(filters.returning_entity),
targetEntity: sanitizeRetentionEntity(filters.target_entity),
period: filters.period,
showMeanRetention:
filters.show_mean_retention ||
meanRetentionCalculation:
filters.mean_retention_calculation ||
(typeof filters.show_mean === 'boolean' ? (filters.show_mean ? 'simple' : 'none') : 'simple'),
cumulative: filters.cumulative,
})
Expand Down
26 changes: 13 additions & 13 deletions frontend/src/queries/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1244,6 +1244,11 @@
"description": "Whether retention should be rolling (aka unbounded, cumulative). Rolling retention means that a user coming back in period 5 makes them count towards all the previous periods.",
"type": "boolean"
},
"meanRetentionCalculation": {
"description": "Whether an additional series should be shown, showing the mean conversion for each period across cohorts.",
"enum": ["simple", "weighted", "none"],
"type": "string"
},
"period": {
"$ref": "#/definitions/RetentionPeriod",
"default": "Day",
Expand All @@ -1266,11 +1271,6 @@
"description": "DEPRECATED: Whether an additional series should be shown, showing the mean conversion for each period across cohorts.",
"type": "boolean"
},
"showMeanRetention": {
"description": "Whether an additional series should be shown, showing the mean conversion for each period across cohorts.",
"enum": ["simple", "weighted", "none"],
"type": "string"
},
"targetEntity": {
"$ref": "#/definitions/RetentionEntity",
"description": "Activation event (event putting the actor into the initial cohort)."
Expand Down Expand Up @@ -13237,6 +13237,10 @@
"cumulative": {
"type": "boolean"
},
"meanRetentionCalculation": {
"enum": ["simple", "weighted", "none"],
"type": "string"
},
"period": {
"$ref": "#/definitions/RetentionPeriod",
"default": "Day"
Expand All @@ -13255,10 +13259,6 @@
"showMean": {
"type": "boolean"
},
"showMeanRetention": {
"enum": ["simple", "weighted", "none"],
"type": "string"
},
"targetEntity": {
"$ref": "#/definitions/RetentionEntity"
},
Expand All @@ -13276,6 +13276,10 @@
"cumulative": {
"type": "boolean"
},
"mean_retention_calculation": {
"enum": ["simple", "weighted", "none"],
"type": "string"
},
"period": {
"$ref": "#/definitions/RetentionPeriod"
},
Expand All @@ -13293,10 +13297,6 @@
"show_mean": {
"type": "boolean"
},
"show_mean_retention": {
"enum": ["simple", "weighted", "none"],
"type": "string"
},
"target_entity": {
"$ref": "#/definitions/RetentionEntity"
},
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/queries/schema/schema-assistant-queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ export interface AssistantRetentionFilter {
/** DEPRECATED: Whether an additional series should be shown, showing the mean conversion for each period across cohorts. */
showMean?: RetentionFilterLegacy['show_mean']
/** Whether an additional series should be shown, showing the mean conversion for each period across cohorts. */
showMeanRetention?: RetentionFilterLegacy['show_mean_retention']
meanRetentionCalculation?: RetentionFilterLegacy['mean_retention_calculation']
/**
* Whether retention should be rolling (aka unbounded, cumulative).
* Rolling retention means that a user coming back in period 5 makes them count towards all the previous periods.
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/queries/schema/schema-general.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1095,7 +1095,7 @@ export type RetentionFilter = {
/** @default Day */
period?: RetentionFilterLegacy['period']
showMean?: RetentionFilterLegacy['show_mean']
showMeanRetention?: RetentionFilterLegacy['show_mean_retention']
meanRetentionCalculation?: RetentionFilterLegacy['mean_retention_calculation']
cumulative?: RetentionFilterLegacy['cumulative']
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export function RetentionMeanDropdown(): JSX.Element | null {
const { retentionFilter } = useValues(insightVizDataLogic(insightProps))
const { updateInsightFilter } = useActions(insightVizDataLogic(insightProps))

const showMeanRetention = retentionFilter?.showMeanRetention ?? RETENTION_MEAN_NONE
const meanRetentionCalculation = retentionFilter?.meanRetentionCalculation ?? RETENTION_MEAN_NONE

if (!canEditInsight) {
return null
Expand All @@ -21,9 +21,9 @@ export function RetentionMeanDropdown(): JSX.Element | null {
<LemonSelect
className="w-44"
size="small"
value={showMeanRetention}
onChange={(showMeanRetention) => {
updateInsightFilter({ showMeanRetention })
value={meanRetentionCalculation}
onChange={(meanRetentionCalculation) => {
updateInsightFilter({ meanRetentionCalculation })
}}
options={[
{
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/scenes/insights/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -574,11 +574,11 @@ function parseAndMigrateQuery<T>(query: string): T | null {
try {
const parsedQuery = JSON.parse(query)
// We made a database migration to support weighted and simple mean in retention tables.
// To do this we created a new column showMeanRetention and deprecated showMean.
// To do this we created a new column meanRetentionCalculation and deprecated showMean.
// This ensures older URLs are parsed correctly.
const retentionFilter = parsedQuery?.source?.retentionFilter
if (retentionFilter && 'showMean' in retentionFilter && typeof retentionFilter.showMean === 'boolean') {
retentionFilter.showMeanRetention = retentionFilter.showMean ? 'simple' : 'none'
retentionFilter.meanRetentionCalculation = retentionFilter.showMean ? 'simple' : 'none'
delete retentionFilter.showMean
}
return parsedQuery
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/scenes/insights/utils/cleanFilters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -309,8 +309,8 @@ export function cleanFilters(
breakdown_type: filters.breakdown_type,
retention_reference: filters.retention_reference,
show_mean: filters.show_mean,
...(filters.show_mean_retention && filters.show_mean_retention !== RETENTION_MEAN_NONE
? { show_mean_retention: filters.show_mean_retention }
...(filters.mean_retention_calculation && filters.mean_retention_calculation !== RETENTION_MEAN_NONE
? { mean_retention_calculation: filters.mean_retention_calculation }
: {}),
cumulative: filters.cumulative,
total_intervals: Math.min(Math.max(filters.total_intervals ?? 11, 0), 100),
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/scenes/insights/utils/queryUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ const cleanInsightQuery = (query: InsightQueryNode, opts?: CompareQueryOpts): In
toggledLifecycles: undefined,
showLabelsOnSeries: undefined,
showMean: undefined,
showMeanRetention: undefined,
meanRetentionCalculation: undefined,
cumulative: undefined,
yAxisScaleType: undefined,
hiddenLegendIndexes: undefined,
Expand Down
8 changes: 4 additions & 4 deletions frontend/src/scenes/retention/RetentionTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export function RetentionTable({ inSharedMode = false }: { inSharedMode?: boolea
const { openModal } = useActions(retentionModalLogic(insightProps))
const backgroundColor = theme?.['preset-1'] || '#000000' // Default to black if no color found
const backgroundColorMean = theme?.['preset-2'] || '#000000' // Default to black if no color found
const showMeanRetention = retentionFilter?.showMeanRetention ?? RETENTION_MEAN_NONE
const meanRetentionCalculation = retentionFilter?.meanRetentionCalculation ?? RETENTION_MEAN_NONE

return (
<table
Expand All @@ -38,7 +38,7 @@ export function RetentionTable({ inSharedMode = false }: { inSharedMode?: boolea
))}
</tr>

{showMeanRetention === 'weighted' && tableRows.length > 0 ? (
{meanRetentionCalculation === 'weighted' && tableRows.length > 0 ? (
<tr className="border-b" key={-2}>
{range(0, tableRows[0].length).map((columnIndex) => (
<td key={columnIndex} className="pb-2">
Expand Down Expand Up @@ -82,7 +82,7 @@ export function RetentionTable({ inSharedMode = false }: { inSharedMode?: boolea
</tr>
) : undefined}

{showMeanRetention === 'simple' && tableRows.length > 0 ? (
{meanRetentionCalculation === 'simple' && tableRows.length > 0 ? (
<tr className="border-b" key={-1}>
{range(0, tableRows[0].length).map((columnIndex) => (
<td key={columnIndex} className="pb-2">
Expand Down Expand Up @@ -130,7 +130,7 @@ export function RetentionTable({ inSharedMode = false }: { inSharedMode?: boolea
{row.map((column, columnIndex) => (
<td
key={columnIndex}
className={clsx({ 'pt-2': rowIndex === 0 && showMeanRetention !== 'none' })}
className={clsx({ 'pt-2': rowIndex === 0 && meanRetentionCalculation !== 'none' })}
>
{columnIndex <= (hideSizeColumn ? 0 : 1) ? (
<span className="RetentionTable__TextTab">{column}</span>
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2450,7 +2450,7 @@ export interface RetentionFilterType extends FilterType {

//frontend only
show_mean?: boolean // deprecated
show_mean_retention?: 'simple' | 'weighted' | typeof RETENTION_MEAN_NONE
mean_retention_calculation?: 'simple' | 'weighted' | typeof RETENTION_MEAN_NONE
}
export interface LifecycleFilterType extends FilterType {
/** @deprecated */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,7 @@ def _insight_filter(filter: dict, allow_variables: bool = False):
),
period=filter.get("period"),
showMean=filter.get("show_mean"),
showMeanRetention=filter.get("show_mean_retention"),
meanRetentionCalculation=filter.get("mean_retention_calculation"),
cumulative=filter.get("cumulative"),
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1505,7 +1505,7 @@ def test_retention_filter(self):
"target_entity": {"id": "$pageview", "name": "$pageview", "type": "events"},
"period": "Week",
"show_mean": True,
"show_mean_retention": "simple",
"mean_retention_calculation": "simple",
"cumulative": True,
}

Expand Down Expand Up @@ -1533,7 +1533,7 @@ def test_retention_filter(self):
"order": None,
},
showMean=True,
showMeanRetention="simple",
meanRetentionCalculation="simple",
cumulative=True,
),
)
Expand Down
6 changes: 3 additions & 3 deletions posthog/management/commands/migrate_retention_show_mean.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

def migrate_show_mean_from_boolean_to_string(batch_size: int, live_run: bool = False) -> None:
"""
Migrate the showMean boolean field to showMeanRetention string field in retention insights.
Migrate the showMean boolean field to meanRetentionCalculation string field in retention insights.
"""
retention_insights = Insight.objects.filter(
deleted=False,
Expand All @@ -25,7 +25,7 @@ def migrate_show_mean_from_boolean_to_string(batch_size: int, live_run: bool = F
if live_run:
with transaction.atomic():
# Convert boolean to string - if True, use 'simple' else 'none'
insight.query["source"]["retentionFilter"]["showMeanRetention"] = (
insight.query["source"]["retentionFilter"]["meanRetentionCalculation"] = (
"simple" if show_mean_value else "none"
)
insight.save()
Expand All @@ -42,7 +42,7 @@ def migrate_show_mean_from_boolean_to_string(batch_size: int, live_run: bool = F


class Command(BaseCommand):
help = "Migrate retention insights showMean boolean field to showMeanRetention string field"
help = "Migrate retention insights showMean boolean field to meanRetentionCalculation string field"

def add_arguments(self, parser):
parser.add_argument(
Expand Down
28 changes: 14 additions & 14 deletions posthog/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,17 +144,17 @@ class AssistantMessageType(StrEnum):
AI_FAILURE = "ai/failure"


class RetentionReference(StrEnum):
TOTAL = "total"
PREVIOUS = "previous"


class ShowMeanRetention(StrEnum):
class MeanRetentionCalculation(StrEnum):
SIMPLE = "simple"
WEIGHTED = "weighted"
NONE = "none"


class RetentionReference(StrEnum):
TOTAL = "total"
PREVIOUS = "previous"


class AssistantSetPropertyFilterOperator(StrEnum):
IS_SET = "is_set"
IS_NOT_SET = "is_not_set"
Expand Down Expand Up @@ -5837,6 +5837,7 @@ class RetentionFilter(BaseModel):
extra="forbid",
)
cumulative: Optional[bool] = None
meanRetentionCalculation: Optional[MeanRetentionCalculation] = None
period: Optional[RetentionPeriod] = RetentionPeriod.DAY
retentionReference: Optional[RetentionReference] = Field(
default=None,
Expand All @@ -5845,7 +5846,6 @@ class RetentionFilter(BaseModel):
retentionType: Optional[RetentionType] = None
returningEntity: Optional[RetentionEntity] = None
showMean: Optional[bool] = None
showMeanRetention: Optional[ShowMeanRetention] = None
targetEntity: Optional[RetentionEntity] = None
totalIntervals: Optional[int] = 11

Expand All @@ -5855,6 +5855,7 @@ class RetentionFilterLegacy(BaseModel):
extra="forbid",
)
cumulative: Optional[bool] = None
mean_retention_calculation: Optional[MeanRetentionCalculation] = None
period: Optional[RetentionPeriod] = None
retention_reference: Optional[RetentionReference] = Field(
default=None,
Expand All @@ -5863,7 +5864,6 @@ class RetentionFilterLegacy(BaseModel):
retention_type: Optional[RetentionType] = None
returning_entity: Optional[RetentionEntity] = None
show_mean: Optional[bool] = None
show_mean_retention: Optional[ShowMeanRetention] = None
target_entity: Optional[RetentionEntity] = None
total_intervals: Optional[int] = None

Expand Down Expand Up @@ -6225,6 +6225,12 @@ class AssistantRetentionFilter(BaseModel):
" coming back in period 5 makes them count towards all the previous periods."
),
)
meanRetentionCalculation: Optional[MeanRetentionCalculation] = Field(
default=None,
description=(
"Whether an additional series should be shown, showing the mean conversion for each period across cohorts."
),
)
period: Optional[RetentionPeriod] = Field(
default=RetentionPeriod.DAY, description="Retention period, the interval to track cohorts by."
)
Expand All @@ -6251,12 +6257,6 @@ class AssistantRetentionFilter(BaseModel):
" across cohorts."
),
)
showMeanRetention: Optional[ShowMeanRetention] = Field(
default=None,
description=(
"Whether an additional series should be shown, showing the mean conversion for each period across cohorts."
),
)
targetEntity: Optional[RetentionEntity] = Field(
default=None, description="Activation event (event putting the actor into the initial cohort)."
)
Expand Down
2 changes: 1 addition & 1 deletion posthog/schema_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ def serialize_query(self, next_serializer):
"showLabelsOnSeries",
"showMean",
"cumulative",
"showMeanRetention",
"meanRetentionCalculation",
"yAxisScaleType",
"hiddenLegendIndexes",
"hiddenLegendBreakdowns",
Expand Down

0 comments on commit 424b98f

Please sign in to comment.