From ca73fa20f5bec11c5bfd44b09983c1aaad1e53e8 Mon Sep 17 00:00:00 2001 From: Juraj Majerik Date: Mon, 23 Dec 2024 13:23:46 +0100 Subject: [PATCH] feat(experiments): new secondary metrics view (#27123) --- .../ExperimentView/ExperimentView.tsx | 21 ++- .../experiments/ExperimentView/Goal.tsx | 19 +- .../experiments/ExperimentView/Overview.tsx | 98 ++++++---- .../ExperimentView/SecondaryMetricsTable.tsx | 77 +------- .../ExperimentView/SummaryTable.tsx | 14 +- .../experiments/ExperimentView/components.tsx | 88 +++------ .../Metrics/PrimaryMetricModal.tsx | 4 +- .../Metrics/PrimaryTrendsExposureModal.tsx | 16 +- .../Metrics/SecondaryMetricModal.tsx | 44 ++--- .../experiments/MetricsView/DeltaChart.tsx | 47 ++++- .../experiments/MetricsView/MetricsView.tsx | 102 ++++++---- .../scenes/experiments/MetricsView/const.tsx | 1 + .../scenes/experiments/experimentLogic.tsx | 175 +++++++++--------- 13 files changed, 342 insertions(+), 364 deletions(-) diff --git a/frontend/src/scenes/experiments/ExperimentView/ExperimentView.tsx b/frontend/src/scenes/experiments/ExperimentView/ExperimentView.tsx index c5dc5eb43cb4b..1d66bee399d04 100644 --- a/frontend/src/scenes/experiments/ExperimentView/ExperimentView.tsx +++ b/frontend/src/scenes/experiments/ExperimentView/ExperimentView.tsx @@ -7,6 +7,7 @@ import { WebExperimentImplementationDetails } from 'scenes/experiments/WebExperi import { ExperimentImplementationDetails } from '../ExperimentImplementationDetails' import { experimentLogic } from '../experimentLogic' import { PrimaryMetricModal } from '../Metrics/PrimaryMetricModal' +import { SecondaryMetricModal } from '../Metrics/SecondaryMetricModal' import { MetricsView } from '../MetricsView/MetricsView' import { ExperimentLoadingAnimation, @@ -52,7 +53,11 @@ const ResultsTab = (): JSX.Element => { )} )} - + {featureFlags[FEATURE_FLAGS.EXPERIMENTS_MULTIPLE_METRICS] ? ( + + ) : ( + + )} ) } @@ -70,8 +75,15 @@ const VariantsTab = (): JSX.Element => { } export function ExperimentView(): JSX.Element { - const { experimentLoading, metricResultsLoading, experimentId, metricResults, tabKey, featureFlags } = - useValues(experimentLogic) + const { + experimentLoading, + metricResultsLoading, + secondaryMetricResultsLoading, + experimentId, + metricResults, + tabKey, + featureFlags, + } = useValues(experimentLogic) const { setTabKey } = useActions(experimentLogic) const result = metricResults?.[0] @@ -86,7 +98,7 @@ export function ExperimentView(): JSX.Element { ) : ( <> - {metricResultsLoading ? ( + {metricResultsLoading || secondaryMetricResultsLoading ? ( ) : ( <> @@ -131,6 +143,7 @@ export function ExperimentView(): JSX.Element { )} + diff --git a/frontend/src/scenes/experiments/ExperimentView/Goal.tsx b/frontend/src/scenes/experiments/ExperimentView/Goal.tsx index ac41a2c663b4c..0438835528246 100644 --- a/frontend/src/scenes/experiments/ExperimentView/Goal.tsx +++ b/frontend/src/scenes/experiments/ExperimentView/Goal.tsx @@ -114,7 +114,7 @@ export function MetricDisplayOld({ filters }: { filters?: FilterType }): JSX.Ele export function ExposureMetric({ experimentId }: { experimentId: Experiment['id'] }): JSX.Element { const { experiment, featureFlags } = useValues(experimentLogic({ experimentId })) - const { updateExperimentExposure, loadExperiment, setExperiment } = useActions(experimentLogic({ experimentId })) + const { updateExperimentGoal, loadExperiment, setExperiment } = useActions(experimentLogic({ experimentId })) const [isModalOpen, setIsModalOpen] = useState(false) const metricIdx = 0 @@ -213,16 +213,13 @@ export function ExposureMetric({ experimentId }: { experimentId: Experiment['id' status="danger" size="xsmall" onClick={() => { - // :FLAG: CLEAN UP AFTER MIGRATION - if (featureFlags[FEATURE_FLAGS.EXPERIMENTS_HOGQL]) { - setExperiment({ - ...experiment, - metrics: experiment.metrics.map((metric, idx) => - idx === metricIdx ? { ...metric, exposure_query: undefined } : metric - ), - }) - } - updateExperimentExposure(null) + setExperiment({ + ...experiment, + metrics: experiment.metrics.map((metric, idx) => + idx === metricIdx ? { ...metric, exposure_query: undefined } : metric + ), + }) + updateExperimentGoal() }} > Reset diff --git a/frontend/src/scenes/experiments/ExperimentView/Overview.tsx b/frontend/src/scenes/experiments/ExperimentView/Overview.tsx index c8c44c8ea5c04..351e58e946646 100644 --- a/frontend/src/scenes/experiments/ExperimentView/Overview.tsx +++ b/frontend/src/scenes/experiments/ExperimentView/Overview.tsx @@ -1,62 +1,80 @@ import { useValues } from 'kea' import { CachedExperimentFunnelsQueryResponse, CachedExperimentTrendsQueryResponse } from '~/queries/schema' +import { ExperimentIdType } from '~/types' import { experimentLogic } from '../experimentLogic' import { VariantTag } from './components' -export function Overview(): JSX.Element { - const { experimentId, metricResults, getIndexForVariant, getHighestProbabilityVariant, areResultsSignificant } = - useValues(experimentLogic) +export function WinningVariantText({ + result, + experimentId, +}: { + result: CachedExperimentFunnelsQueryResponse | CachedExperimentTrendsQueryResponse + experimentId: ExperimentIdType +}): JSX.Element { + const { getIndexForVariant, getHighestProbabilityVariant } = useValues(experimentLogic) - const result = metricResults?.[0] - if (!result) { - return <> - } - - function WinningVariantText(): JSX.Element { - const highestProbabilityVariant = getHighestProbabilityVariant( - result as CachedExperimentFunnelsQueryResponse | CachedExperimentTrendsQueryResponse - ) - const index = getIndexForVariant( - result as CachedExperimentFunnelsQueryResponse | CachedExperimentTrendsQueryResponse, - highestProbabilityVariant || '' - ) - if (highestProbabilityVariant && index !== null && result) { - const { probability } = result - - return ( -
- -  is winning with a  - - {`${(probability[highestProbabilityVariant] * 100).toFixed(2)}% probability`}  - - of being best.  -
- ) - } + const highestProbabilityVariant = getHighestProbabilityVariant(result) + const index = getIndexForVariant(result, highestProbabilityVariant || '') + if (highestProbabilityVariant && index !== null && result) { + const { probability } = result - return <> - } - - function SignificanceText(): JSX.Element { return ( -
- Your results are  - - {`${areResultsSignificant(0) ? 'significant' : 'not significant'}`}. +
+ +  is winning with a  + + {`${(probability[highestProbabilityVariant] * 100).toFixed(2)}% probability`}  + of being best. 
) } + return <> +} + +export function SignificanceText({ + metricIndex, + isSecondary = false, +}: { + metricIndex: number + isSecondary?: boolean +}): JSX.Element { + const { isPrimaryMetricSignificant, isSecondaryMetricSignificant } = useValues(experimentLogic) + + return ( +
+ Your results are  + + {`${ + isSecondary + ? isSecondaryMetricSignificant(metricIndex) + : isPrimaryMetricSignificant(metricIndex) + ? 'significant' + : 'not significant' + }`} + . + +
+ ) +} + +export function Overview({ metricIndex = 0 }: { metricIndex?: number }): JSX.Element { + const { experimentId, metricResults } = useValues(experimentLogic) + + const result = metricResults?.[metricIndex] + if (!result) { + return <> + } + return (

Summary

- - + +
) diff --git a/frontend/src/scenes/experiments/ExperimentView/SecondaryMetricsTable.tsx b/frontend/src/scenes/experiments/ExperimentView/SecondaryMetricsTable.tsx index 7f5bcbabfa3a1..e8d5baf4eed0e 100644 --- a/frontend/src/scenes/experiments/ExperimentView/SecondaryMetricsTable.tsx +++ b/frontend/src/scenes/experiments/ExperimentView/SecondaryMetricsTable.tsx @@ -1,4 +1,4 @@ -import { IconInfo, IconPencil, IconPlus } from '@posthog/icons' +import { IconInfo, IconPencil } from '@posthog/icons' import { LemonButton, LemonTable, LemonTableColumns, Tooltip } from '@posthog/lemon-ui' import { useActions, useValues } from 'kea' import { EntityFilterInfo } from 'lib/components/EntityFilterInfo' @@ -8,15 +8,13 @@ import { useState } from 'react' import { Experiment, InsightType } from '~/types' -import { experimentLogic, getDefaultFunnelsMetric, TabularSecondaryMetricResults } from '../experimentLogic' +import { experimentLogic, TabularSecondaryMetricResults } from '../experimentLogic' import { SecondaryMetricChartModal } from '../Metrics/SecondaryMetricChartModal' -import { SecondaryMetricModal } from '../Metrics/SecondaryMetricModal' +import { MAX_SECONDARY_METRICS } from '../MetricsView/const' +import { AddSecondaryMetric } from '../MetricsView/MetricsView' import { VariantTag } from './components' -const MAX_SECONDARY_METRICS = 10 - export function SecondaryMetricsTable({ experimentId }: { experimentId: Experiment['id'] }): JSX.Element { - const [isEditModalOpen, setIsEditModalOpen] = useState(false) const [isChartModalOpen, setIsChartModalOpen] = useState(false) const [modalMetricIdx, setModalMetricIdx] = useState(null) @@ -34,18 +32,7 @@ export function SecondaryMetricsTable({ experimentId }: { experimentId: Experime experimentMathAggregationForTrends, getHighestProbabilityVariant, } = useValues(experimentLogic({ experimentId })) - const { loadExperiment } = useActions(experimentLogic({ experimentId })) - - const openEditModal = (idx: number): void => { - setModalMetricIdx(idx) - setIsEditModalOpen(true) - } - - const closeEditModal = (): void => { - setIsEditModalOpen(false) - setModalMetricIdx(null) - loadExperiment() - } + const { openSecondaryMetricModal } = useActions(experimentLogic({ experimentId })) const openChartModal = (idx: number): void => { setModalMetricIdx(idx) @@ -107,7 +94,7 @@ export function SecondaryMetricsTable({ experimentId }: { experimentId: Experime type="secondary" size="xsmall" icon={} - onClick={() => openEditModal(idx)} + onClick={() => openSecondaryMetricModal(idx)} />
@@ -266,11 +253,7 @@ export function SecondaryMetricsTable({ experimentId }: { experimentId: Experime
{metrics && metrics.length > 0 && (
- +
)}
@@ -292,21 +275,11 @@ export function SecondaryMetricsTable({ experimentId }: { experimentId: Experime Add up to {MAX_SECONDARY_METRICS} secondary metrics to monitor side effects of your experiment. - + )} - ) } - -const AddSecondaryMetricButton = ({ - experimentId, - metrics, - openEditModal, -}: { - experimentId: Experiment['id'] - metrics: any - openEditModal: (metricIdx: number) => void -}): JSX.Element => { - const { experiment } = useValues(experimentLogic({ experimentId })) - const { setExperiment } = useActions(experimentLogic({ experimentId })) - return ( - } - type="secondary" - size="xsmall" - onClick={() => { - const newMetricsSecondary = [...experiment.metrics_secondary, getDefaultFunnelsMetric()] - setExperiment({ - metrics_secondary: newMetricsSecondary, - }) - openEditModal(newMetricsSecondary.length - 1) - }} - disabledReason={ - metrics.length >= MAX_SECONDARY_METRICS - ? `You can only add up to ${MAX_SECONDARY_METRICS} secondary metrics.` - : undefined - } - > - Add metric - - ) -} diff --git a/frontend/src/scenes/experiments/ExperimentView/SummaryTable.tsx b/frontend/src/scenes/experiments/ExperimentView/SummaryTable.tsx index adb1bc9d69616..14ee1d3cbcf7a 100644 --- a/frontend/src/scenes/experiments/ExperimentView/SummaryTable.tsx +++ b/frontend/src/scenes/experiments/ExperimentView/SummaryTable.tsx @@ -22,13 +22,21 @@ import { import { experimentLogic } from '../experimentLogic' import { VariantTag } from './components' -export function SummaryTable(): JSX.Element { +export function SummaryTable({ + metricIndex = 0, + isSecondary = false, +}: { + metricIndex?: number + isSecondary?: boolean +}): JSX.Element { const { experimentId, experiment, metricResults, + secondaryMetricResults, tabularExperimentResults, getMetricType, + getSecondaryMetricType, exposureCountDataForVariant, conversionRateForVariant, experimentMathAggregationForTrends, @@ -36,8 +44,8 @@ export function SummaryTable(): JSX.Element { getHighestProbabilityVariant, credibleIntervalForVariant, } = useValues(experimentLogic) - const metricType = getMetricType(0) - const result = metricResults?.[0] + const metricType = isSecondary ? getSecondaryMetricType(metricIndex) : getMetricType(metricIndex) + const result = isSecondary ? secondaryMetricResults?.[metricIndex] : metricResults?.[metricIndex] if (!result) { return <> } diff --git a/frontend/src/scenes/experiments/ExperimentView/components.tsx b/frontend/src/scenes/experiments/ExperimentView/components.tsx index 87885a3761b79..afa0e77054f99 100644 --- a/frontend/src/scenes/experiments/ExperimentView/components.tsx +++ b/frontend/src/scenes/experiments/ExperimentView/components.tsx @@ -42,7 +42,6 @@ import { Experiment as ExperimentType, ExperimentIdType, ExperimentResults, - FilterType, InsightShortId, InsightType, } from '~/types' @@ -104,8 +103,8 @@ export function VariantTag({ } export function ResultsTag({ metricIndex = 0 }: { metricIndex?: number }): JSX.Element { - const { areResultsSignificant, significanceDetails } = useValues(experimentLogic) - const result: { color: LemonTagType; label: string } = areResultsSignificant(metricIndex) + const { isPrimaryMetricSignificant, significanceDetails } = useValues(experimentLogic) + const result: { color: LemonTagType; label: string } = isPrimaryMetricSignificant(metricIndex) ? { color: 'success', label: 'Significant' } : { color: 'primary', label: 'Not significant' } @@ -210,63 +209,24 @@ export function ResultsQuery({ } export function ExploreButton({ - icon = , metricIndex = 0, + isSecondary = false, }: { - icon?: JSX.Element metricIndex?: number + isSecondary?: boolean }): JSX.Element { - const { metricResults, experiment, featureFlags } = useValues(experimentLogic) - const result = metricResults?.[metricIndex] - - // keep in sync with https://github.com/PostHog/posthog/blob/master/ee/clickhouse/queries/experiments/funnel_experiment_result.py#L71 - // :TRICKY: In the case of no results, we still want users to explore the query, so they can debug further. - // This generates a close enough query that the backend would use to compute results. - const filtersFromExperiment: Partial = { - ...experiment.filters, - date_from: experiment.start_date, - date_to: experiment.end_date, - explicit_date: true, - breakdown: `$feature/${experiment.feature_flag_key ?? experiment.feature_flag?.key}`, - breakdown_type: 'event', - properties: [], - } - - let query: InsightVizNode - if (featureFlags[FEATURE_FLAGS.EXPERIMENTS_HOGQL]) { - const newQueryResults = result as unknown as - | CachedExperimentTrendsQueryResponse - | CachedExperimentFunnelsQueryResponse - - const source = - newQueryResults.kind === NodeKind.ExperimentTrendsQuery - ? newQueryResults.count_query - : newQueryResults.funnels_query + const { metricResults, secondaryMetricResults } = useValues(experimentLogic) + const result = isSecondary ? secondaryMetricResults?.[metricIndex] : metricResults?.[metricIndex] - query = { - kind: NodeKind.InsightVizNode, - source: source as InsightQueryNode, - } - } else { - const oldQueryResults = result as unknown as ExperimentResults['result'] - - if (!oldQueryResults?.filters) { - return <> - } + if (!result) { + return <> + } - query = { - kind: NodeKind.InsightVizNode, - source: filtersToQueryNode( - transformResultFilters( - oldQueryResults?.filters - ? { ...oldQueryResults.filters, explicit_date: true } - : filtersFromExperiment - ) - ), - showTable: true, - showLastComputation: true, - showLastComputationRefresh: false, - } + const query: InsightVizNode = { + kind: NodeKind.InsightVizNode, + source: (result.kind === NodeKind.ExperimentTrendsQuery + ? result.count_query + : result.funnels_query) as InsightQueryNode, } return ( @@ -274,10 +234,10 @@ export function ExploreButton({ className="ml-auto -translate-y-2" size="xsmall" type="primary" - icon={icon} + icon={} to={urls.insightNew(undefined, undefined, query)} > - Explore results + Explore as Insight ) } @@ -462,7 +422,7 @@ export function PageHeaderCustom(): JSX.Element { experiment, isExperimentRunning, isExperimentStopped, - areResultsSignificant, + isPrimaryMetricSignificant, isSingleVariantShipped, featureFlags, hasGoalSet, @@ -516,7 +476,7 @@ export function PageHeaderCustom(): JSX.Element { fullWidth data-attr="refresh-experiment" > - Refresh experiment results + Refresh primary metrics loadSecondaryMetricResults(true)} @@ -595,7 +555,7 @@ export function PageHeaderCustom(): JSX.Element { )} {featureFlags[FEATURE_FLAGS.EXPERIMENT_MAKE_DECISION] && - areResultsSignificant(0) && + isPrimaryMetricSignificant(0) && !isSingleVariantShipped && ( <> @@ -704,7 +664,7 @@ export function ActionBanner(): JSX.Element { experimentLoading, metricResultsLoading, isExperimentRunning, - areResultsSignificant, + isPrimaryMetricSignificant, isExperimentStopped, funnelResultsPersonsTotal, actualRunningTime, @@ -774,7 +734,7 @@ export function ActionBanner(): JSX.Element { } // Running, results present, not significant - if (isExperimentRunning && result && !isExperimentStopped && !areResultsSignificant(0)) { + if (isExperimentRunning && result && !isExperimentStopped && !isPrimaryMetricSignificant(0)) { // Results insignificant, but a large enough sample/running time has been achieved // Further collection unlikely to change the result -> recommmend cutting the losses if ( @@ -808,7 +768,7 @@ export function ActionBanner(): JSX.Element { } // Running, results significant - if (isExperimentRunning && !isExperimentStopped && areResultsSignificant(0) && result) { + if (isExperimentRunning && !isExperimentStopped && isPrimaryMetricSignificant(0) && result) { const { probability } = result const winningVariant = getHighestProbabilityVariant(result) if (!winningVariant) { @@ -856,7 +816,7 @@ export function ActionBanner(): JSX.Element { } // Stopped, results significant - if (isExperimentStopped && areResultsSignificant(0)) { + if (isExperimentStopped && isPrimaryMetricSignificant(0)) { return ( You have stopped this experiment, and it is no longer collecting data. With significant results in hand, @@ -874,7 +834,7 @@ export function ActionBanner(): JSX.Element { } // Stopped, results not significant - if (isExperimentStopped && result && !areResultsSignificant(0)) { + if (isExperimentStopped && result && !isPrimaryMetricSignificant(0)) { return ( You have stopped this experiment, and it is no longer collecting data. Because your results are not diff --git a/frontend/src/scenes/experiments/Metrics/PrimaryMetricModal.tsx b/frontend/src/scenes/experiments/Metrics/PrimaryMetricModal.tsx index 1afce61899d6b..31d4eec71265b 100644 --- a/frontend/src/scenes/experiments/Metrics/PrimaryMetricModal.tsx +++ b/frontend/src/scenes/experiments/Metrics/PrimaryMetricModal.tsx @@ -53,7 +53,7 @@ export function PrimaryMetricModal({ experimentId }: { experimentId: Experiment[ setExperiment({ metrics: newMetrics, }) - updateExperimentGoal(experiment.filters) + updateExperimentGoal() }} > Delete @@ -74,7 +74,7 @@ export function PrimaryMetricModal({ experimentId }: { experimentId: Experiment[ } form="edit-experiment-goal-form" onClick={() => { - updateExperimentGoal(experiment.filters) + updateExperimentGoal() }} type="primary" loading={experimentLoading} diff --git a/frontend/src/scenes/experiments/Metrics/PrimaryTrendsExposureModal.tsx b/frontend/src/scenes/experiments/Metrics/PrimaryTrendsExposureModal.tsx index 7c4f49c114a73..a7d410258d662 100644 --- a/frontend/src/scenes/experiments/Metrics/PrimaryTrendsExposureModal.tsx +++ b/frontend/src/scenes/experiments/Metrics/PrimaryTrendsExposureModal.tsx @@ -1,6 +1,5 @@ import { LemonButton, LemonModal } from '@posthog/lemon-ui' import { useActions, useValues } from 'kea' -import { FEATURE_FLAGS } from 'lib/constants' import { Experiment } from '~/types' @@ -16,8 +15,8 @@ export function PrimaryTrendsExposureModal({ isOpen: boolean onClose: () => void }): JSX.Element { - const { experiment, experimentLoading, featureFlags } = useValues(experimentLogic({ experimentId })) - const { updateExperimentExposure, updateExperiment } = useActions(experimentLogic({ experimentId })) + const { experiment, experimentLoading } = useValues(experimentLogic({ experimentId })) + const { updateExperiment } = useActions(experimentLogic({ experimentId })) return ( { - // :FLAG: CLEAN UP AFTER MIGRATION - if (featureFlags[FEATURE_FLAGS.EXPERIMENTS_HOGQL]) { - updateExperiment({ - metrics: experiment.metrics, - }) - } else { - updateExperimentExposure(experiment.parameters.custom_exposure_filter ?? null) - } + updateExperiment({ + metrics: experiment.metrics, + }) }} type="primary" loading={experimentLoading} diff --git a/frontend/src/scenes/experiments/Metrics/SecondaryMetricModal.tsx b/frontend/src/scenes/experiments/Metrics/SecondaryMetricModal.tsx index 423fb1c48d5c1..b61af3fff9c7e 100644 --- a/frontend/src/scenes/experiments/Metrics/SecondaryMetricModal.tsx +++ b/frontend/src/scenes/experiments/Metrics/SecondaryMetricModal.tsx @@ -7,25 +7,29 @@ import { experimentLogic, getDefaultFunnelsMetric, getDefaultTrendsMetric } from import { SecondaryGoalFunnels } from './SecondaryGoalFunnels' import { SecondaryGoalTrends } from './SecondaryGoalTrends' -export function SecondaryMetricModal({ - experimentId, - metricIdx, - isOpen, - onClose, -}: { - experimentId: Experiment['id'] - metricIdx: number - isOpen: boolean - onClose: () => void -}): JSX.Element { - const { experiment, experimentLoading, getSecondaryMetricType } = useValues(experimentLogic({ experimentId })) - const { setExperiment, updateExperiment } = useActions(experimentLogic({ experimentId })) +export function SecondaryMetricModal({ experimentId }: { experimentId: Experiment['id'] }): JSX.Element { + const { + experiment, + experimentLoading, + getSecondaryMetricType, + isSecondaryMetricModalOpen, + editingSecondaryMetricIndex, + } = useValues(experimentLogic({ experimentId })) + const { setExperiment, updateExperimentGoal, closeSecondaryMetricModal } = useActions( + experimentLogic({ experimentId }) + ) + + if (!editingSecondaryMetricIndex && editingSecondaryMetricIndex !== 0) { + return <> + } + + const metricIdx = editingSecondaryMetricIndex const metricType = getSecondaryMetricType(metricIdx) return ( Delete
- + Cancel { - updateExperiment({ - metrics_secondary: experiment.metrics_secondary, - }) + updateExperimentGoal() }} type="primary" loading={experimentLoading} diff --git a/frontend/src/scenes/experiments/MetricsView/DeltaChart.tsx b/frontend/src/scenes/experiments/MetricsView/DeltaChart.tsx index 08b475148375a..2f7455288b68f 100644 --- a/frontend/src/scenes/experiments/MetricsView/DeltaChart.tsx +++ b/frontend/src/scenes/experiments/MetricsView/DeltaChart.tsx @@ -1,5 +1,5 @@ import { IconActivity, IconGraph, IconMinus, IconPencil, IconTrending } from '@posthog/icons' -import { LemonButton, LemonModal, LemonTag, LemonTagType, Tooltip } from '@posthog/lemon-ui' +import { LemonBanner, LemonButton, LemonModal, LemonTag, LemonTagType, Tooltip } from '@posthog/lemon-ui' import { useActions, useValues } from 'kea' import { humanFriendlyNumber } from 'lib/utils' import { useEffect, useRef, useState } from 'react' @@ -8,7 +8,9 @@ import { themeLogic } from '~/layout/navigation-3000/themeLogic' import { InsightType, TrendExperimentVariant } from '~/types' import { experimentLogic } from '../experimentLogic' -import { ResultsQuery, VariantTag } from '../ExperimentView/components' +import { ExploreButton, ResultsQuery, VariantTag } from '../ExperimentView/components' +import { SignificanceText, WinningVariantText } from '../ExperimentView/Overview' +import { SummaryTable } from '../ExperimentView/SummaryTable' import { NoResultEmptyState } from './NoResultEmptyState' function formatTickValue(value: number): string { @@ -34,6 +36,7 @@ function formatTickValue(value: number): string { } export function DeltaChart({ + isSecondary, result, error, variants, @@ -44,6 +47,7 @@ export function DeltaChart({ tickValues, chartBound, }: { + isSecondary: boolean result: any error: any variants: any[] @@ -64,7 +68,7 @@ export function DeltaChart({ } = useValues(experimentLogic) const { experiment } = useValues(experimentLogic) - const { openPrimaryMetricModal } = useActions(experimentLogic) + const { openPrimaryMetricModal, openSecondaryMetricModal } = useActions(experimentLogic) const [tooltipData, setTooltipData] = useState<{ x: number; y: number; variant: string } | null>(null) const [emptyStateTooltipVisible, setEmptyStateTooltipVisible] = useState(true) const [tooltipPosition, setTooltipPosition] = useState({ x: 0, y: 0 }) @@ -182,7 +186,11 @@ export function DeltaChart({ type="secondary" size="xsmall" icon={} - onClick={() => openPrimaryMetricModal(metricIndex)} + onClick={() => + isSecondary + ? openSecondaryMetricModal(metricIndex) + : openPrimaryMetricModal(metricIndex) + } />
@@ -693,7 +701,7 @@ export function DeltaChart({ flexDirection: 'column', }} > - +
setIsModalOpen(false)} width={1200} - title={`Results for ${metric.name || 'Untitled metric'}`} + title={`Metric results: ${metric.name || 'Untitled metric'}`} footer={ } > +
+ +
+ +
+ + +
+ +
) } -function SignificanceHighlight({ metricIndex = 0 }: { metricIndex?: number }): JSX.Element { - const { areResultsSignificant, significanceDetails } = useValues(experimentLogic) - const result: { color: LemonTagType; label: string } = areResultsSignificant(metricIndex) +function SignificanceHighlight({ + metricIndex = 0, + isSecondary = false, +}: { + metricIndex?: number + isSecondary?: boolean +}): JSX.Element { + const { isPrimaryMetricSignificant, isSecondaryMetricSignificant, significanceDetails } = useValues(experimentLogic) + const isSignificant = isSecondary + ? isSecondaryMetricSignificant(metricIndex) + : isPrimaryMetricSignificant(metricIndex) + const result: { color: LemonTagType; label: string } = isSignificant ? { color: 'success', label: 'Significant' } : { color: 'primary', label: 'Not significant' } - const inner = areResultsSignificant(metricIndex) ? ( + const inner = isSignificant ? (
{result.label} diff --git a/frontend/src/scenes/experiments/MetricsView/MetricsView.tsx b/frontend/src/scenes/experiments/MetricsView/MetricsView.tsx index 1e50c3d5369f3..2628f353942f5 100644 --- a/frontend/src/scenes/experiments/MetricsView/MetricsView.tsx +++ b/frontend/src/scenes/experiments/MetricsView/MetricsView.tsx @@ -4,7 +4,7 @@ import { useActions, useValues } from 'kea' import { IconAreaChart } from 'lib/lemon-ui/icons' import { experimentLogic, getDefaultFunnelsMetric } from '../experimentLogic' -import { MAX_PRIMARY_METRICS } from './const' +import { MAX_PRIMARY_METRICS, MAX_SECONDARY_METRICS } from './const' import { DeltaChart } from './DeltaChart' // Helper function to find nice round numbers for ticks @@ -42,61 +42,92 @@ export function getNiceTickValues(maxAbsValue: number): number[] { return ticks } -function AddMetric({ - metrics, - setExperiment, - openPrimaryMetricModal, -}: { - metrics: any[] - setExperiment: (payload: { metrics: any[] }) => void - openPrimaryMetricModal: (index: number) => void -}): JSX.Element { +function AddPrimaryMetric(): JSX.Element { + const { experiment } = useValues(experimentLogic) + const { setExperiment, openPrimaryMetricModal } = useActions(experimentLogic) + return ( } type="secondary" size="xsmall" onClick={() => { - const newMetrics = [...metrics, getDefaultFunnelsMetric()] + const newMetrics = [...experiment.metrics, getDefaultFunnelsMetric()] setExperiment({ metrics: newMetrics, }) openPrimaryMetricModal(newMetrics.length - 1) }} disabledReason={ - metrics.length >= MAX_PRIMARY_METRICS + experiment.metrics.length >= MAX_PRIMARY_METRICS ? `You can only add up to ${MAX_PRIMARY_METRICS} primary metrics.` : undefined } > - Add metric + Add primary metric ) } -export function MetricsView(): JSX.Element { - const { experiment, getMetricType, metricResults, primaryMetricsResultErrors, credibleIntervalForVariant } = - useValues(experimentLogic) - const { setExperiment, openPrimaryMetricModal } = useActions(experimentLogic) +export function AddSecondaryMetric(): JSX.Element { + const { experiment } = useValues(experimentLogic) + const { setExperiment, openSecondaryMetricModal } = useActions(experimentLogic) + return ( + } + type="secondary" + size="xsmall" + onClick={() => { + const newMetricsSecondary = [...experiment.metrics_secondary, getDefaultFunnelsMetric()] + setExperiment({ + metrics_secondary: newMetricsSecondary, + }) + openSecondaryMetricModal(newMetricsSecondary.length - 1) + }} + disabledReason={ + experiment.metrics_secondary.length >= MAX_SECONDARY_METRICS + ? `You can only add up to ${MAX_SECONDARY_METRICS} secondary metrics.` + : undefined + } + > + Add secondary metric + + ) +} + +export function MetricsView({ isSecondary }: { isSecondary?: boolean }): JSX.Element { + const { + experiment, + getMetricType, + getSecondaryMetricType, + metricResults, + secondaryMetricResults, + primaryMetricsResultErrors, + secondaryMetricsResultErrors, + credibleIntervalForVariant, + } = useValues(experimentLogic) const variants = experiment.parameters.feature_flag_variants - const metrics = experiment.metrics || [] + const metrics = isSecondary ? experiment.metrics_secondary : experiment.metrics + const results = isSecondary ? secondaryMetricResults : metricResults + const errors = isSecondary ? secondaryMetricsResultErrors : primaryMetricsResultErrors // Calculate the maximum absolute value across ALL metrics const maxAbsValue = Math.max( ...metrics.flatMap((_, metricIndex) => { - const result = metricResults?.[metricIndex] + const result = results?.[metricIndex] if (!result) { return [] } return variants.flatMap((variant) => { - const interval = credibleIntervalForVariant(result, variant.key, getMetricType(metricIndex)) + const metricType = isSecondary ? getSecondaryMetricType(metricIndex) : getMetricType(metricIndex) + const interval = credibleIntervalForVariant(result, variant.key, metricType) return interval ? [Math.abs(interval[0] / 100), Math.abs(interval[1] / 100)] : [] }) }) ) - const padding = Math.max(maxAbsValue * 0.05, 0.02) + const padding = Math.max(maxAbsValue * 0.05, 0.1) const chartBound = maxAbsValue + padding const commonTickValues = getNiceTickValues(chartBound) @@ -106,7 +137,9 @@ export function MetricsView(): JSX.Element {
-

Primary metrics

+

+ {isSecondary ? 'Secondary metrics' : 'Primary metrics'} +

@@ -114,11 +147,7 @@ export function MetricsView(): JSX.Element {
{metrics.length > 0 && (
- + {isSecondary ? : }
)}
@@ -128,7 +157,7 @@ export function MetricsView(): JSX.Element {
{metrics.map((metric, metricIndex) => { - const result = metricResults?.[metricIndex] + const result = results?.[metricIndex] const isFirstMetric = metricIndex === 0 return ( @@ -145,10 +174,15 @@ export function MetricsView(): JSX.Element { }`} >
- Add up to {MAX_PRIMARY_METRICS} primary metrics. + Add up to {MAX_PRIMARY_METRICS} {isSecondary ? 'secondary' : 'primary'} metrics.
- + {isSecondary ? : }
)} diff --git a/frontend/src/scenes/experiments/MetricsView/const.tsx b/frontend/src/scenes/experiments/MetricsView/const.tsx index d1f720a7b256a..7df3e2c0aef17 100644 --- a/frontend/src/scenes/experiments/MetricsView/const.tsx +++ b/frontend/src/scenes/experiments/MetricsView/const.tsx @@ -1 +1,2 @@ export const MAX_PRIMARY_METRICS = 10 +export const MAX_SECONDARY_METRICS = 10 diff --git a/frontend/src/scenes/experiments/experimentLogic.tsx b/frontend/src/scenes/experiments/experimentLogic.tsx index c8325ab3b7cf5..44753c32582a8 100644 --- a/frontend/src/scenes/experiments/experimentLogic.tsx +++ b/frontend/src/scenes/experiments/experimentLogic.tsx @@ -57,7 +57,6 @@ import { MultivariateFlagVariant, ProductKey, PropertyMathType, - SecondaryMetricResults, TrendExperimentVariant, TrendResult, TrendsFilterType, @@ -183,9 +182,8 @@ export const experimentLogic = kea([ setEditExperiment: (editing: boolean) => ({ editing }), setFlagImplementationWarning: (warning: boolean) => ({ warning }), setExposureAndSampleSize: (exposure: number, sampleSize: number) => ({ exposure, sampleSize }), - updateExperimentGoal: (filters: Partial) => ({ filters }), + updateExperimentGoal: true, updateExperimentCollectionGoal: true, - updateExperimentExposure: (filters: Partial | null) => ({ filters }), changeExperimentStartDate: (startDate: string) => ({ startDate }), launchExperiment: true, endExperiment: true, @@ -270,6 +268,9 @@ export const experimentLogic = kea([ closePrimaryMetricModal: true, setPrimaryMetricsResultErrors: (errors: any[]) => ({ errors }), updateDistributionModal: (featureFlag: FeatureFlagType) => ({ featureFlag }), + openSecondaryMetricModal: (index: number) => ({ index }), + closeSecondaryMetricModal: true, + setSecondaryMetricsResultErrors: (errors: any[]) => ({ errors }), }), reducers({ experiment: [ @@ -491,9 +492,30 @@ export const experimentLogic = kea([ [] as any[], { setPrimaryMetricsResultErrors: (_, { errors }) => errors, - // Reset errors when loading new results loadMetricResults: () => [], - // Reset errors when loading a new experiment + loadExperiment: () => [], + }, + ], + isSecondaryMetricModalOpen: [ + false, + { + openSecondaryMetricModal: () => true, + closeSecondaryMetricModal: () => false, + }, + ], + editingSecondaryMetricIndex: [ + null as number | null, + { + openSecondaryMetricModal: (_, { index }) => index, + closeSecondaryMetricModal: () => null, + updateExperimentGoal: () => null, + }, + ], + secondaryMetricsResultErrors: [ + [] as any[], + { + setSecondaryMetricsResultErrors: (_, { errors }) => errors, + loadSecondaryMetricResults: () => [], loadExperiment: () => [], }, ], @@ -613,13 +635,18 @@ export const experimentLogic = kea([ actions.updateExperiment({ end_date: endDate.toISOString() }) const duration = endDate.diff(values.experiment?.start_date, 'second') values.experiment && - actions.reportExperimentCompleted(values.experiment, endDate, duration, values.areResultsSignificant(0)) + actions.reportExperimentCompleted( + values.experiment, + endDate, + duration, + values.isPrimaryMetricSignificant(0) + ) }, archiveExperiment: async () => { actions.updateExperiment({ archived: true }) values.experiment && actions.reportExperimentArchived(values.experiment) }, - updateExperimentGoal: async ({ filters }) => { + updateExperimentGoal: async () => { // Reset MDE to the recommended setting actions.setExperiment({ parameters: { @@ -630,12 +657,9 @@ export const experimentLogic = kea([ const { recommendedRunningTime, recommendedSampleSize, minimumDetectableEffect } = values - const filtersToUpdate = { ...filters } - delete filtersToUpdate.properties - actions.updateExperiment({ - filters: filtersToUpdate, metrics: values.experiment.metrics, + metrics_secondary: values.experiment.metrics_secondary, parameters: { ...values.experiment?.parameters, recommended_running_time: recommendedRunningTime, @@ -644,6 +668,7 @@ export const experimentLogic = kea([ }, }) actions.closePrimaryMetricModal() + actions.closeSecondaryMetricModal() }, updateExperimentCollectionGoal: async () => { const { recommendedRunningTime, recommendedSampleSize, minimumDetectableEffect } = values @@ -658,15 +683,6 @@ export const experimentLogic = kea([ }) actions.closeExperimentCollectionGoalModal() }, - updateExperimentExposure: async ({ filters }) => { - actions.updateExperiment({ - metrics: values.experiment.metrics, - parameters: { - custom_exposure_filter: filters ?? undefined, - feature_flag_variants: values.experiment?.parameters?.feature_flag_variants, - }, - }) - }, closeExperimentCollectionGoalModal: () => { if (values.experimentValuesChangedLocally) { actions.loadExperiment() @@ -675,6 +691,9 @@ export const experimentLogic = kea([ closePrimaryMetricModal: () => { actions.loadExperiment() }, + closeSecondaryMetricModal: () => { + actions.loadExperiment() + }, resetRunningExperiment: async () => { actions.updateExperiment({ start_date: null, end_date: null, archived: false }) values.experiment && actions.reportExperimentReset(values.experiment) @@ -871,7 +890,6 @@ export const experimentLogic = kea([ const errorDetailMatch = error.detail.match(/\{.*\}/) const errorDetail = errorDetailMatch ? JSON.parse(errorDetailMatch[0]) : error.detail - // Store the error in primaryMetricsResultErrors const currentErrors = [...(values.primaryMetricsResultErrors || [])] currentErrors[index] = { detail: errorDetail, @@ -887,71 +905,39 @@ export const experimentLogic = kea([ }, ], secondaryMetricResults: [ - null as - | SecondaryMetricResults[] - | (CachedExperimentTrendsQueryResponse | CachedExperimentFunnelsQueryResponse)[] - | null, + null as (CachedExperimentTrendsQueryResponse | CachedExperimentFunnelsQueryResponse | null)[] | null, { loadSecondaryMetricResults: async ( refresh?: boolean - ): Promise< - | SecondaryMetricResults[] - | (CachedExperimentTrendsQueryResponse | CachedExperimentFunnelsQueryResponse)[] - | null - > => { - if (values.featureFlags[FEATURE_FLAGS.EXPERIMENTS_HOGQL]) { - return (await Promise.all( - values.experiment?.metrics_secondary.map(async (metric) => { - try { - // Queries are shareable, so we need to set the experiment_id for the backend to correctly associate the query with the experiment - const queryWithExperimentId = { - ...metric, - experiment_id: values.experimentId, - } - const response: ExperimentResults = await api.create( - `api/projects/${values.currentProjectId}/query`, - { query: queryWithExperimentId, refresh: 'lazy_async' } - ) - - return { - ...response, - fakeInsightId: Math.random().toString(36).substring(2, 15), - last_refresh: response.last_refresh || '', - } - } catch (error) { - return {} - } - }) - )) as unknown as (CachedExperimentTrendsQueryResponse | CachedExperimentFunnelsQueryResponse)[] - } - - const refreshParam = refresh ? '&refresh=true' : '' - - return await Promise.all( - (values.experiment?.secondary_metrics || []).map(async (_, index) => { + ): Promise<(CachedExperimentTrendsQueryResponse | CachedExperimentFunnelsQueryResponse | null)[]> => { + return (await Promise.all( + values.experiment?.metrics_secondary.map(async (metric, index) => { try { - const secResults = await api.get( - `api/projects/${values.currentProjectId}/experiments/${values.experimentId}/secondary_results?id=${index}${refreshParam}` - ) - // :TRICKY: Maintain backwards compatibility for cached responses, remove after cache period has expired - if (secResults && secResults.result && !secResults.result.hasOwnProperty('result')) { - return { - result: { ...secResults.result }, - fakeInsightId: Math.random().toString(36).substring(2, 15), - last_refresh: secResults.last_refresh, - } + const queryWithExperimentId = { + ...metric, + experiment_id: values.experimentId, } + const response = await performQuery(queryWithExperimentId, undefined, refresh) return { - ...secResults.result, + ...response, fakeInsightId: Math.random().toString(36).substring(2, 15), - last_refresh: secResults.last_refresh, } - } catch (error) { - return {} + } catch (error: any) { + const errorDetailMatch = error.detail.match(/\{.*\}/) + const errorDetail = errorDetailMatch ? JSON.parse(errorDetailMatch[0]) : error.detail + + const currentErrors = [...(values.secondaryMetricsResultErrors || [])] + currentErrors[index] = { + detail: errorDetail, + statusCode: error.status, + hasDiagnostics: !!errorDetailMatch, + } + actions.setSecondaryMetricsResultErrors(currentErrors) + return null } }) - ) + )) as (CachedExperimentTrendsQueryResponse | CachedExperimentFunnelsQueryResponse | null)[] }, }, ], @@ -1004,27 +990,19 @@ export const experimentLogic = kea([ (experimentId): Experiment['id'] => experimentId, ], getMetricType: [ - (s) => [s.experiment, s.featureFlags], - (experiment, featureFlags) => + (s) => [s.experiment], + (experiment) => (metricIdx: number = 0) => { - if (featureFlags[FEATURE_FLAGS.EXPERIMENTS_HOGQL]) { - const query = experiment?.metrics?.[metricIdx] - return query?.kind === NodeKind.ExperimentTrendsQuery ? InsightType.TRENDS : InsightType.FUNNELS - } - - return experiment?.filters?.insight || InsightType.FUNNELS + const query = experiment?.metrics?.[metricIdx] + return query?.kind === NodeKind.ExperimentTrendsQuery ? InsightType.TRENDS : InsightType.FUNNELS }, ], getSecondaryMetricType: [ - (s) => [s.experiment, s.featureFlags], - (experiment, featureFlags) => + (s) => [s.experiment], + (experiment) => (metricIdx: number = 0) => { - if (featureFlags[FEATURE_FLAGS.EXPERIMENTS_HOGQL]) { - const query = experiment?.metrics_secondary?.[metricIdx] - return query?.kind === NodeKind.ExperimentTrendsQuery ? InsightType.TRENDS : InsightType.FUNNELS - } - - return experiment?.secondary_metrics?.[metricIdx]?.filters?.insight || InsightType.FUNNELS + const query = experiment?.metrics_secondary?.[metricIdx] + return query?.kind === NodeKind.ExperimentTrendsQuery ? InsightType.TRENDS : InsightType.FUNNELS }, ], isExperimentRunning: [ @@ -1133,13 +1111,26 @@ export const experimentLogic = kea([ return Math.ceil((1600 * conversionRate * (1 - conversionRate / 100)) / (mde * mde)) }, ], - areResultsSignificant: [ + isPrimaryMetricSignificant: [ (s) => [s.metricResults], (metricResults: (CachedExperimentFunnelsQueryResponse | CachedExperimentTrendsQueryResponse | null)[]) => (metricIndex: number = 0): boolean => { return metricResults?.[metricIndex]?.significant || false }, ], + isSecondaryMetricSignificant: [ + (s) => [s.secondaryMetricResults], + ( + secondaryMetricResults: ( + | CachedExperimentFunnelsQueryResponse + | CachedExperimentTrendsQueryResponse + | null + )[] + ) => + (metricIndex: number = 0): boolean => { + return secondaryMetricResults?.[metricIndex]?.significant || false + }, + ], significanceDetails: [ (s) => [s.metricResults], (metricResults: (CachedExperimentFunnelsQueryResponse | CachedExperimentTrendsQueryResponse | null)[]) =>