diff --git a/frontend/web/components/experiments/CreateExperimentWizard.tsx b/frontend/web/components/experiments/CreateExperimentWizard.tsx index a04ffc6d3127..01df5c5d663a 100644 --- a/frontend/web/components/experiments/CreateExperimentWizard.tsx +++ b/frontend/web/components/experiments/CreateExperimentWizard.tsx @@ -4,8 +4,6 @@ import { useCreateExperimentMutation, useStartExperimentMutation, } from 'common/services/useExperiment' -import { useGetFeatureStatesQuery } from 'common/services/useFeatureState' -import { useProjectEnvironments } from 'common/hooks/useProjectEnvironments' import { ENABLE_EXPERIMENT_LIFECYCLE, METRIC_DIRECTION_TO_EXPECTED_DIRECTION, @@ -18,7 +16,7 @@ import RolloutStep from './steps/RolloutStep' import { VariationSplitEntry, getControlPercentage, - getVariationSplitDefaults, + getEvenSplit, toRolloutFeatureValue, } from './rollout' import isValidPercentage from 'common/utils/isValidPercentage' @@ -55,32 +53,11 @@ const CreateExperimentWizard: FC = ({ ) const [completedSteps, setCompletedSteps] = useState>(new Set()) - const { getEnvironmentIdFromKey } = useProjectEnvironments(projectId) - const numericEnvId = getEnvironmentIdFromKey(environmentId) - - const { data: featureStatesData } = useGetFeatureStatesQuery( - { environment: numericEnvId, feature: selectedFeature?.id }, - { skip: !selectedFeature || !numericEnvId }, - ) - - const environmentFeatureState = useMemo( - () => - featureStatesData?.results?.find( - (state) => !state.feature_segment && !state.identity, - ), - [featureStatesData], - ) - useEffect(() => { setVariationSplit( - selectedFeature - ? getVariationSplitDefaults( - selectedFeature.multivariate_options, - environmentFeatureState?.multivariate_feature_state_values, - ) - : [], + selectedFeature ? getEvenSplit(selectedFeature.multivariate_options) : [], ) - }, [selectedFeature, environmentFeatureState]) + }, [selectedFeature]) const [createExperiment, { isLoading: isCreating }] = useCreateExperimentMutation() diff --git a/frontend/web/components/experiments/results/ExperimentExposuresPanel.tsx b/frontend/web/components/experiments/results/ExperimentExposuresPanel.tsx index af5d84d69aab..2525702d4c3a 100644 --- a/frontend/web/components/experiments/results/ExperimentExposuresPanel.tsx +++ b/frontend/web/components/experiments/results/ExperimentExposuresPanel.tsx @@ -1,10 +1,9 @@ -import { FC, useCallback, useEffect, useMemo, useState } from 'react' +import { FC, useCallback, useMemo } from 'react' import moment from 'moment' import { LineChart } from 'components/charts' import ContentCard from 'components/base/grid/ContentCard' import Button from 'components/base/forms/Button' import Icon from 'components/icons/Icon' -import useCountdown from 'common/hooks/useCountdown' import { colorIconDanger } from 'common/theme/tokens' import { useGetExperimentExposuresQuery, @@ -19,14 +18,9 @@ import { } from './derive' import type { VariantTotal } from './derive' import { - DEFAULT_RETRY_AFTER_S, - POLL_TIMEOUT_MS, - REFRESH_POLL_INTERVAL_MS, canRefreshExposures, deriveExposuresViewState, - getExposuresRefreshLabel, } from './exposuresViewState' -import RefreshControl from './RefreshControl' import './results.scss' const AsOfLabel: FC<{ asOf: string | null }> = ({ asOf }) => ( @@ -45,75 +39,31 @@ const buildLegendLabels = (totals: VariantTotal[]): Record => { return labels } -const parseRetryAfter = (err: unknown): number | null => { - const fetchErr = err as { - status?: number - retryAfter?: number | null - } - if (fetchErr.status !== 429) return null - if (fetchErr.retryAfter) return fetchErr.retryAfter - return DEFAULT_RETRY_AFTER_S -} - type ExperimentExposuresPanelProps = { experiment: Experiment environmentId: string exposuresOverride?: ExperimentExposures } -const REFRESH_DISABLED_COPY: Record = { - final: 'Refresh is disabled because the experiment is complete.', - not_started: 'Start the experiment to compute exposures.', -} - const ExperimentExposuresPanel: FC = ({ environmentId, experiment, exposuresOverride, }) => { - const [pollInterval, setPollInterval] = useState(0) - const [refreshRequested, setRefreshRequested] = useState(false) - const [pollStartedAt, setPollStartedAt] = useState(null) - const [retryAfter, startRetryCountdown] = useCountdown() const { data: fetched } = useGetExperimentExposuresQuery( { environmentId, experimentId: experiment.id }, { - pollingInterval: pollInterval, refetchOnMountOrArgChange: true, skip: !!exposuresOverride, }, ) const exposures = exposuresOverride ?? fetched - const [refresh, { isLoading: isSubmitting }] = - useRefreshExperimentExposuresMutation() + const [refreshExposures] = useRefreshExperimentExposuresMutation() const viewState = deriveExposuresViewState(exposures) const availability = canRefreshExposures(experiment.status, exposures) const payload = exposures?.payload ?? null - const pollTimedOut = - pollStartedAt !== null && Date.now() - pollStartedAt > POLL_TIMEOUT_MS - const shouldPoll = - !pollTimedOut && (viewState.kind === 'refreshing' || refreshRequested) - const nextPollInterval = shouldPoll ? REFRESH_POLL_INTERVAL_MS : 0 - useEffect(() => { - setPollInterval(nextPollInterval) - }, [nextPollInterval]) - - useEffect(() => { - if (viewState.kind === 'loaded' || viewState.kind === 'error') { - setRefreshRequested(false) - setPollStartedAt(null) - } - }, [viewState.kind]) - - useEffect(() => { - if (pollTimedOut) { - setRefreshRequested(false) - setPollStartedAt(null) - } - }, [pollTimedOut]) - const identities = useMemo( () => getVariantIdentities(experiment.feature), [experiment.feature], @@ -127,61 +77,24 @@ const ExperimentExposuresPanel: FC = ({ [payload, identities], ) - const isRefreshing = - refreshRequested || viewState.kind === 'refreshing' || isSubmitting + const isRefreshing = viewState.kind === 'refreshing' const headline = payload ? getHeadlineTotal(payload) : 0 const hasData = !!payload && headline > 0 - const handleRefresh = useCallback(async () => { - setRefreshRequested(true) - setPollStartedAt(Date.now()) - const result = await refresh({ + const handleComputeNow = useCallback(async () => { + const result = await refreshExposures({ environmentId, experimentId: experiment.id, }) if ('error' in result && result.error) { - setRefreshRequested(false) - setPollStartedAt(null) - const seconds = parseRetryAfter(result.error) - if (seconds !== null) { - startRetryCountdown(seconds) - } else { - toast('Failed to refresh exposures', 'danger') - } + toast('Failed to compute exposures', 'danger') } - }, [refresh, environmentId, experiment.id, startRetryCountdown]) - - const refreshLabel = getExposuresRefreshLabel(retryAfter, isRefreshing) - - const action = ( - - {refreshLabel.message} - - ) - } - onRefresh={handleRefresh} - /> - ) + }, [refreshExposures, environmentId, experiment.id]) const asOf = exposures?.as_of ?? null return ( @@ -262,7 +175,7 @@ const ExperimentExposuresPanel: FC = ({ : 'No exposure data computed yet.'} {!isRefreshing && availability.canRefresh && (
-
diff --git a/frontend/web/components/experiments/results/ExperimentResultsRefreshControl.tsx b/frontend/web/components/experiments/results/ExperimentResultsRefreshControl.tsx index 70382fd88259..fc42c3bd33bb 100644 --- a/frontend/web/components/experiments/results/ExperimentResultsRefreshControl.tsx +++ b/frontend/web/components/experiments/results/ExperimentResultsRefreshControl.tsx @@ -2,7 +2,9 @@ import { FC, useCallback, useEffect, useState } from 'react' import useCountdown from 'common/hooks/useCountdown' import { useGetExperimentBayesianResultsQuery, + useGetExperimentExposuresQuery, useRefreshExperimentBayesianResultsMutation, + useRefreshExperimentExposuresMutation, } from 'common/services/useExperiment' import { ExperimentStatus } from 'common/types/responses' import RefreshControl from './RefreshControl' @@ -14,6 +16,7 @@ import { deriveResultsViewState, getResultsRefreshLabel, } from './resultsViewState' +import { deriveExposuresViewState } from './exposuresViewState' const parseRetryAfter = (err: unknown): number | null => { const fetchErr = err as { @@ -25,6 +28,17 @@ const parseRetryAfter = (err: unknown): number | null => { return DEFAULT_RETRY_AFTER_S } +const getMaxRetryAfter = (errors: unknown[]): number | null => { + let max: number | null = null + for (const err of errors) { + const seconds = parseRetryAfter(err) + if (seconds !== null && (max === null || seconds > max)) { + max = seconds + } + } + return max +} + type ExperimentResultsRefreshControlProps = { environmentId: string experimentId: number @@ -48,27 +62,40 @@ const ExperimentResultsRefreshControl: FC< { environmentId, experimentId }, { pollingInterval: pollInterval }, ) - const [refresh, { isLoading: isSubmitting }] = + const { data: exposures } = useGetExperimentExposuresQuery( + { environmentId, experimentId }, + { pollingInterval: pollInterval }, + ) + const [refreshResults, { isLoading: isSubmittingResults }] = useRefreshExperimentBayesianResultsMutation() + const [refreshExposures, { isLoading: isSubmittingExposures }] = + useRefreshExperimentExposuresMutation() - const viewState = deriveResultsViewState(results) + const resultsViewState = deriveResultsViewState(results) + const exposuresViewState = deriveExposuresViewState(exposures) const availability = canRefreshResults(status, results) + const eitherRefreshing = + resultsViewState.kind === 'refreshing' || + exposuresViewState.kind === 'refreshing' + const bothSettled = + resultsViewState.kind !== 'refreshing' && + exposuresViewState.kind !== 'refreshing' + const pollTimedOut = pollStartedAt !== null && Date.now() - pollStartedAt > POLL_TIMEOUT_MS - const shouldPoll = - !pollTimedOut && (viewState.kind === 'refreshing' || refreshRequested) + const shouldPoll = !pollTimedOut && (eitherRefreshing || refreshRequested) const nextPollInterval = shouldPoll ? REFRESH_POLL_INTERVAL_MS : 0 useEffect(() => { setPollInterval(nextPollInterval) }, [nextPollInterval]) useEffect(() => { - if (viewState.kind === 'loaded' || viewState.kind === 'error') { + if (refreshRequested && bothSettled) { setRefreshRequested(false) setPollStartedAt(null) } - }, [viewState.kind]) + }, [refreshRequested, bothSettled]) useEffect(() => { if (pollTimedOut) { @@ -78,25 +105,51 @@ const ExperimentResultsRefreshControl: FC< }, [pollTimedOut]) const isRefreshing = - refreshRequested || viewState.kind === 'refreshing' || isSubmitting + refreshRequested || + eitherRefreshing || + isSubmittingResults || + isSubmittingExposures const handleRefresh = useCallback(async () => { setRefreshRequested(true) setPollStartedAt(Date.now()) - const result = await refresh({ environmentId, experimentId }) - if ('error' in result && result.error) { - setRefreshRequested(false) - setPollStartedAt(null) - const seconds = parseRetryAfter(result.error) + const [resultsResult, exposuresResult] = await Promise.all([ + refreshResults({ environmentId, experimentId }), + refreshExposures({ environmentId, experimentId }), + ]) + const errors: unknown[] = [] + if ('error' in resultsResult && resultsResult.error) { + errors.push(resultsResult.error) + } + if ('error' in exposuresResult && exposuresResult.error) { + errors.push(exposuresResult.error) + } + if (errors.length > 0) { + const seconds = getMaxRetryAfter(errors) + const hasNonRetryable = errors.some((e) => parseRetryAfter(e) === null) + if (hasNonRetryable) { + toast('Failed to refresh experiment data', 'danger') + } if (seconds !== null) { startRetryCountdown(seconds) } else { - toast('Failed to refresh results', 'danger') + setRefreshRequested(false) + setPollStartedAt(null) } } - }, [refresh, environmentId, experimentId, startRetryCountdown]) + }, [ + refreshResults, + refreshExposures, + environmentId, + experimentId, + startRetryCountdown, + ]) - const label = getResultsRefreshLabel(retryAfter, isRefreshing, viewState) + const label = getResultsRefreshLabel( + retryAfter, + isRefreshing, + resultsViewState, + ) return ( - Refresh results + Refresh ) } diff --git a/frontend/web/components/experiments/results/__tests__/exposuresViewState.test.ts b/frontend/web/components/experiments/results/__tests__/exposuresViewState.test.ts index 7142512a4237..3f4387dde81d 100644 --- a/frontend/web/components/experiments/results/__tests__/exposuresViewState.test.ts +++ b/frontend/web/components/experiments/results/__tests__/exposuresViewState.test.ts @@ -80,7 +80,7 @@ describe('canRefreshExposures', () => { describe('getExposuresRefreshLabel', () => { it('prefers a retry countdown over the in-progress message', () => { expect(getExposuresRefreshLabel(90, true)).toEqual({ - message: 'Computing… retry in 1m 30s', + message: 'Refresh available in 1m 30s', tone: 'muted', }) }) diff --git a/frontend/web/components/experiments/results/__tests__/resultsViewState.test.ts b/frontend/web/components/experiments/results/__tests__/resultsViewState.test.ts index 42e404d69946..69d97d6c0768 100644 --- a/frontend/web/components/experiments/results/__tests__/resultsViewState.test.ts +++ b/frontend/web/components/experiments/results/__tests__/resultsViewState.test.ts @@ -86,7 +86,7 @@ describe('getResultsRefreshLabel', () => { it('prefers a retry countdown over everything else', () => { expect( getResultsRefreshLabel(90, true, { kind: 'error', staleAvailable: true }), - ).toEqual({ message: 'Computing… retry in 1m 30s', tone: 'muted' }) + ).toEqual({ message: 'Refresh available in 1m 30s', tone: 'muted' }) }) it('shows an in-progress message while refreshing', () => { diff --git a/frontend/web/components/experiments/results/derive.ts b/frontend/web/components/experiments/results/derive.ts index 94f4f583ad7e..dc001f98b914 100644 --- a/frontend/web/components/experiments/results/derive.ts +++ b/frontend/web/components/experiments/results/derive.ts @@ -110,6 +110,14 @@ export const getHeadlineTotal = (summary: ExposuresSummary): number => 0, ) +export const getResultsTotalUsers = ( + results: BayesianResultsSummary, +): number => { + const firstMetric = results.metrics[0] + if (!firstMetric) return 0 + return Object.values(firstMetric.variants).reduce((sum, v) => sum + v.n, 0) +} + export const isLiftFavourable = ( lift: number, direction: ExpectedDirection, diff --git a/frontend/web/components/experiments/results/exposuresViewState.ts b/frontend/web/components/experiments/results/exposuresViewState.ts index 311d6763af8f..4f216cc6a8b0 100644 --- a/frontend/web/components/experiments/results/exposuresViewState.ts +++ b/frontend/web/components/experiments/results/exposuresViewState.ts @@ -58,7 +58,7 @@ export const getExposuresRefreshLabel = ( ): RefreshLabel | null => { if (retryAfter !== null) { return { - message: `Computing… retry in ${formatCountdown(retryAfter)}`, + message: `Refresh available in ${formatCountdown(retryAfter)}`, tone: 'muted', } } diff --git a/frontend/web/components/experiments/results/resultsViewState.ts b/frontend/web/components/experiments/results/resultsViewState.ts index 876c4c00d626..97e03805d453 100644 --- a/frontend/web/components/experiments/results/resultsViewState.ts +++ b/frontend/web/components/experiments/results/resultsViewState.ts @@ -60,7 +60,7 @@ export const getResultsRefreshLabel = ( ): RefreshLabel | null => { if (retryAfter !== null) { return { - message: `Computing… retry in ${formatCountdown(retryAfter)}`, + message: `Refresh available in ${formatCountdown(retryAfter)}`, tone: 'muted', } } diff --git a/frontend/web/components/pages/ExperimentDetailPage.tsx b/frontend/web/components/pages/ExperimentDetailPage.tsx index 73f8fc84ab08..b04450197eac 100644 --- a/frontend/web/components/pages/ExperimentDetailPage.tsx +++ b/frontend/web/components/pages/ExperimentDetailPage.tsx @@ -6,7 +6,10 @@ import { useGetExperimentExposuresQuery, useGetExperimentQuery, } from 'common/services/useExperiment' -import { getHeadlineTotal } from 'components/experiments/results/derive' +import { + getHeadlineTotal, + getResultsTotalUsers, +} from 'components/experiments/results/derive' import ExperimentDetailHeader from 'components/experiments/results/ExperimentDetailHeader' import ExperimentConfiguration from 'components/experiments/results/ExperimentConfiguration' import ExperimentRecommendation from 'components/experiments/results/ExperimentRecommendation' @@ -76,9 +79,12 @@ const ExperimentDetailPage: FC = () => { ) } - const usersEnrolled = exposures?.payload + const resultsTotalUsers = + results && results.metrics.length > 0 ? getResultsTotalUsers(results) : null + const exposuresTotalUsers = exposures?.payload ? getHeadlineTotal(exposures.payload) : null + const usersEnrolled = resultsTotalUsers ?? exposuresTotalUsers return (