Skip to content

Commit 4510b72

Browse files
Zaimwa9Zaimwa9
authored
feat(experiments): wizard UX improvements (#7978)
1 parent 1042aad commit 4510b72

9 files changed

Lines changed: 100 additions & 143 deletions

‎frontend/web/components/experiments/CreateExperimentWizard.tsx‎

Lines changed: 3 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@ import {
44
useCreateExperimentMutation,
55
useStartExperimentMutation,
66
} from 'common/services/useExperiment'
7-
import { useGetFeatureStatesQuery } from 'common/services/useFeatureState'
8-
import { useProjectEnvironments } from 'common/hooks/useProjectEnvironments'
97
import {
108
ENABLE_EXPERIMENT_LIFECYCLE,
119
METRIC_DIRECTION_TO_EXPECTED_DIRECTION,
@@ -18,7 +16,7 @@ import RolloutStep from './steps/RolloutStep'
1816
import {
1917
VariationSplitEntry,
2018
getControlPercentage,
21-
getVariationSplitDefaults,
19+
getEvenSplit,
2220
toRolloutFeatureValue,
2321
} from './rollout'
2422
import isValidPercentage from 'common/utils/isValidPercentage'
@@ -55,32 +53,11 @@ const CreateExperimentWizard: FC<CreateExperimentWizardProps> = ({
5553
)
5654
const [completedSteps, setCompletedSteps] = useState<Set<number>>(new Set())
5755

58-
const { getEnvironmentIdFromKey } = useProjectEnvironments(projectId)
59-
const numericEnvId = getEnvironmentIdFromKey(environmentId)
60-
61-
const { data: featureStatesData } = useGetFeatureStatesQuery(
62-
{ environment: numericEnvId, feature: selectedFeature?.id },
63-
{ skip: !selectedFeature || !numericEnvId },
64-
)
65-
66-
const environmentFeatureState = useMemo(
67-
() =>
68-
featureStatesData?.results?.find(
69-
(state) => !state.feature_segment && !state.identity,
70-
),
71-
[featureStatesData],
72-
)
73-
7456
useEffect(() => {
7557
setVariationSplit(
76-
selectedFeature
77-
? getVariationSplitDefaults(
78-
selectedFeature.multivariate_options,
79-
environmentFeatureState?.multivariate_feature_state_values,
80-
)
81-
: [],
58+
selectedFeature ? getEvenSplit(selectedFeature.multivariate_options) : [],
8259
)
83-
}, [selectedFeature, environmentFeatureState])
60+
}, [selectedFeature])
8461

8562
const [createExperiment, { isLoading: isCreating }] =
8663
useCreateExperimentMutation()

‎frontend/web/components/experiments/results/ExperimentExposuresPanel.tsx‎

Lines changed: 8 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
1-
import { FC, useCallback, useEffect, useMemo, useState } from 'react'
1+
import { FC, useCallback, useMemo } from 'react'
22
import moment from 'moment'
33
import { LineChart } from 'components/charts'
44
import ContentCard from 'components/base/grid/ContentCard'
55
import Button from 'components/base/forms/Button'
66
import Icon from 'components/icons/Icon'
7-
import useCountdown from 'common/hooks/useCountdown'
87
import { colorIconDanger } from 'common/theme/tokens'
98
import {
109
useGetExperimentExposuresQuery,
@@ -19,14 +18,9 @@ import {
1918
} from './derive'
2019
import type { VariantTotal } from './derive'
2120
import {
22-
DEFAULT_RETRY_AFTER_S,
23-
POLL_TIMEOUT_MS,
24-
REFRESH_POLL_INTERVAL_MS,
2521
canRefreshExposures,
2622
deriveExposuresViewState,
27-
getExposuresRefreshLabel,
2823
} from './exposuresViewState'
29-
import RefreshControl from './RefreshControl'
3024
import './results.scss'
3125

3226
const AsOfLabel: FC<{ asOf: string | null }> = ({ asOf }) => (
@@ -45,75 +39,31 @@ const buildLegendLabels = (totals: VariantTotal[]): Record<string, string> => {
4539
return labels
4640
}
4741

48-
const parseRetryAfter = (err: unknown): number | null => {
49-
const fetchErr = err as {
50-
status?: number
51-
retryAfter?: number | null
52-
}
53-
if (fetchErr.status !== 429) return null
54-
if (fetchErr.retryAfter) return fetchErr.retryAfter
55-
return DEFAULT_RETRY_AFTER_S
56-
}
57-
5842
type ExperimentExposuresPanelProps = {
5943
experiment: Experiment
6044
environmentId: string
6145
exposuresOverride?: ExperimentExposures
6246
}
6347

64-
const REFRESH_DISABLED_COPY: Record<string, string> = {
65-
final: 'Refresh is disabled because the experiment is complete.',
66-
not_started: 'Start the experiment to compute exposures.',
67-
}
68-
6948
const ExperimentExposuresPanel: FC<ExperimentExposuresPanelProps> = ({
7049
environmentId,
7150
experiment,
7251
exposuresOverride,
7352
}) => {
74-
const [pollInterval, setPollInterval] = useState(0)
75-
const [refreshRequested, setRefreshRequested] = useState(false)
76-
const [pollStartedAt, setPollStartedAt] = useState<number | null>(null)
77-
const [retryAfter, startRetryCountdown] = useCountdown()
7853
const { data: fetched } = useGetExperimentExposuresQuery(
7954
{ environmentId, experimentId: experiment.id },
8055
{
81-
pollingInterval: pollInterval,
8256
refetchOnMountOrArgChange: true,
8357
skip: !!exposuresOverride,
8458
},
8559
)
8660
const exposures = exposuresOverride ?? fetched
87-
const [refresh, { isLoading: isSubmitting }] =
88-
useRefreshExperimentExposuresMutation()
61+
const [refreshExposures] = useRefreshExperimentExposuresMutation()
8962

9063
const viewState = deriveExposuresViewState(exposures)
9164
const availability = canRefreshExposures(experiment.status, exposures)
9265
const payload = exposures?.payload ?? null
9366

94-
const pollTimedOut =
95-
pollStartedAt !== null && Date.now() - pollStartedAt > POLL_TIMEOUT_MS
96-
const shouldPoll =
97-
!pollTimedOut && (viewState.kind === 'refreshing' || refreshRequested)
98-
const nextPollInterval = shouldPoll ? REFRESH_POLL_INTERVAL_MS : 0
99-
useEffect(() => {
100-
setPollInterval(nextPollInterval)
101-
}, [nextPollInterval])
102-
103-
useEffect(() => {
104-
if (viewState.kind === 'loaded' || viewState.kind === 'error') {
105-
setRefreshRequested(false)
106-
setPollStartedAt(null)
107-
}
108-
}, [viewState.kind])
109-
110-
useEffect(() => {
111-
if (pollTimedOut) {
112-
setRefreshRequested(false)
113-
setPollStartedAt(null)
114-
}
115-
}, [pollTimedOut])
116-
11767
const identities = useMemo(
11868
() => getVariantIdentities(experiment.feature),
11969
[experiment.feature],
@@ -127,61 +77,24 @@ const ExperimentExposuresPanel: FC<ExperimentExposuresPanelProps> = ({
12777
[payload, identities],
12878
)
12979

130-
const isRefreshing =
131-
refreshRequested || viewState.kind === 'refreshing' || isSubmitting
80+
const isRefreshing = viewState.kind === 'refreshing'
13281
const headline = payload ? getHeadlineTotal(payload) : 0
13382
const hasData = !!payload && headline > 0
13483

135-
const handleRefresh = useCallback(async () => {
136-
setRefreshRequested(true)
137-
setPollStartedAt(Date.now())
138-
const result = await refresh({
84+
const handleComputeNow = useCallback(async () => {
85+
const result = await refreshExposures({
13986
environmentId,
14087
experimentId: experiment.id,
14188
})
14289
if ('error' in result && result.error) {
143-
setRefreshRequested(false)
144-
setPollStartedAt(null)
145-
const seconds = parseRetryAfter(result.error)
146-
if (seconds !== null) {
147-
startRetryCountdown(seconds)
148-
} else {
149-
toast('Failed to refresh exposures', 'danger')
150-
}
90+
toast('Failed to compute exposures', 'danger')
15191
}
152-
}, [refresh, environmentId, experiment.id, startRetryCountdown])
153-
154-
const refreshLabel = getExposuresRefreshLabel(retryAfter, isRefreshing)
155-
156-
const action = (
157-
<RefreshControl
158-
disabled={!availability.canRefresh || retryAfter !== null}
159-
disabledReason={
160-
availability.reason
161-
? REFRESH_DISABLED_COPY[availability.reason]
162-
: undefined
163-
}
164-
isRefreshing={isRefreshing}
165-
label={
166-
refreshLabel && (
167-
<span
168-
className={
169-
refreshLabel.tone === 'danger' ? 'text-danger' : undefined
170-
}
171-
>
172-
{refreshLabel.message}
173-
</span>
174-
)
175-
}
176-
onRefresh={handleRefresh}
177-
/>
178-
)
92+
}, [refreshExposures, environmentId, experiment.id])
17993

18094
const asOf = exposures?.as_of ?? null
18195

18296
return (
18397
<ContentCard
184-
action={action}
18598
className='experiment-results__exposures-card'
18699
title='Enrollment over time'
187100
>
@@ -262,7 +175,7 @@ const ExperimentExposuresPanel: FC<ExperimentExposuresPanelProps> = ({
262175
: 'No exposure data computed yet.'}
263176
{!isRefreshing && availability.canRefresh && (
264177
<div className='mt-2'>
265-
<Button onClick={handleRefresh} size='small' theme='secondary'>
178+
<Button onClick={handleComputeNow} size='small' theme='secondary'>
266179
Compute now
267180
</Button>
268181
</div>

‎frontend/web/components/experiments/results/ExperimentResultsRefreshControl.tsx‎

Lines changed: 69 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@ import { FC, useCallback, useEffect, useState } from 'react'
22
import useCountdown from 'common/hooks/useCountdown'
33
import {
44
useGetExperimentBayesianResultsQuery,
5+
useGetExperimentExposuresQuery,
56
useRefreshExperimentBayesianResultsMutation,
7+
useRefreshExperimentExposuresMutation,
68
} from 'common/services/useExperiment'
79
import { ExperimentStatus } from 'common/types/responses'
810
import RefreshControl from './RefreshControl'
@@ -14,6 +16,7 @@ import {
1416
deriveResultsViewState,
1517
getResultsRefreshLabel,
1618
} from './resultsViewState'
19+
import { deriveExposuresViewState } from './exposuresViewState'
1720

1821
const parseRetryAfter = (err: unknown): number | null => {
1922
const fetchErr = err as {
@@ -25,6 +28,17 @@ const parseRetryAfter = (err: unknown): number | null => {
2528
return DEFAULT_RETRY_AFTER_S
2629
}
2730

31+
const getMaxRetryAfter = (errors: unknown[]): number | null => {
32+
let max: number | null = null
33+
for (const err of errors) {
34+
const seconds = parseRetryAfter(err)
35+
if (seconds !== null && (max === null || seconds > max)) {
36+
max = seconds
37+
}
38+
}
39+
return max
40+
}
41+
2842
type ExperimentResultsRefreshControlProps = {
2943
environmentId: string
3044
experimentId: number
@@ -48,27 +62,40 @@ const ExperimentResultsRefreshControl: FC<
4862
{ environmentId, experimentId },
4963
{ pollingInterval: pollInterval },
5064
)
51-
const [refresh, { isLoading: isSubmitting }] =
65+
const { data: exposures } = useGetExperimentExposuresQuery(
66+
{ environmentId, experimentId },
67+
{ pollingInterval: pollInterval },
68+
)
69+
const [refreshResults, { isLoading: isSubmittingResults }] =
5270
useRefreshExperimentBayesianResultsMutation()
71+
const [refreshExposures, { isLoading: isSubmittingExposures }] =
72+
useRefreshExperimentExposuresMutation()
5373

54-
const viewState = deriveResultsViewState(results)
74+
const resultsViewState = deriveResultsViewState(results)
75+
const exposuresViewState = deriveExposuresViewState(exposures)
5576
const availability = canRefreshResults(status, results)
5677

78+
const eitherRefreshing =
79+
resultsViewState.kind === 'refreshing' ||
80+
exposuresViewState.kind === 'refreshing'
81+
const bothSettled =
82+
resultsViewState.kind !== 'refreshing' &&
83+
exposuresViewState.kind !== 'refreshing'
84+
5785
const pollTimedOut =
5886
pollStartedAt !== null && Date.now() - pollStartedAt > POLL_TIMEOUT_MS
59-
const shouldPoll =
60-
!pollTimedOut && (viewState.kind === 'refreshing' || refreshRequested)
87+
const shouldPoll = !pollTimedOut && (eitherRefreshing || refreshRequested)
6188
const nextPollInterval = shouldPoll ? REFRESH_POLL_INTERVAL_MS : 0
6289
useEffect(() => {
6390
setPollInterval(nextPollInterval)
6491
}, [nextPollInterval])
6592

6693
useEffect(() => {
67-
if (viewState.kind === 'loaded' || viewState.kind === 'error') {
94+
if (refreshRequested && bothSettled) {
6895
setRefreshRequested(false)
6996
setPollStartedAt(null)
7097
}
71-
}, [viewState.kind])
98+
}, [refreshRequested, bothSettled])
7299

73100
useEffect(() => {
74101
if (pollTimedOut) {
@@ -78,25 +105,51 @@ const ExperimentResultsRefreshControl: FC<
78105
}, [pollTimedOut])
79106

80107
const isRefreshing =
81-
refreshRequested || viewState.kind === 'refreshing' || isSubmitting
108+
refreshRequested ||
109+
eitherRefreshing ||
110+
isSubmittingResults ||
111+
isSubmittingExposures
82112

83113
const handleRefresh = useCallback(async () => {
84114
setRefreshRequested(true)
85115
setPollStartedAt(Date.now())
86-
const result = await refresh({ environmentId, experimentId })
87-
if ('error' in result && result.error) {
88-
setRefreshRequested(false)
89-
setPollStartedAt(null)
90-
const seconds = parseRetryAfter(result.error)
116+
const [resultsResult, exposuresResult] = await Promise.all([
117+
refreshResults({ environmentId, experimentId }),
118+
refreshExposures({ environmentId, experimentId }),
119+
])
120+
const errors: unknown[] = []
121+
if ('error' in resultsResult && resultsResult.error) {
122+
errors.push(resultsResult.error)
123+
}
124+
if ('error' in exposuresResult && exposuresResult.error) {
125+
errors.push(exposuresResult.error)
126+
}
127+
if (errors.length > 0) {
128+
const seconds = getMaxRetryAfter(errors)
129+
const hasNonRetryable = errors.some((e) => parseRetryAfter(e) === null)
130+
if (hasNonRetryable) {
131+
toast('Failed to refresh experiment data', 'danger')
132+
}
91133
if (seconds !== null) {
92134
startRetryCountdown(seconds)
93135
} else {
94-
toast('Failed to refresh results', 'danger')
136+
setRefreshRequested(false)
137+
setPollStartedAt(null)
95138
}
96139
}
97-
}, [refresh, environmentId, experimentId, startRetryCountdown])
140+
}, [
141+
refreshResults,
142+
refreshExposures,
143+
environmentId,
144+
experimentId,
145+
startRetryCountdown,
146+
])
98147

99-
const label = getResultsRefreshLabel(retryAfter, isRefreshing, viewState)
148+
const label = getResultsRefreshLabel(
149+
retryAfter,
150+
isRefreshing,
151+
resultsViewState,
152+
)
100153

101154
return (
102155
<RefreshControl
@@ -116,7 +169,7 @@ const ExperimentResultsRefreshControl: FC<
116169
}
117170
onRefresh={handleRefresh}
118171
>
119-
Refresh results
172+
Refresh
120173
</RefreshControl>
121174
)
122175
}

‎frontend/web/components/experiments/results/__tests__/exposuresViewState.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ describe('canRefreshExposures', () => {
8080
describe('getExposuresRefreshLabel', () => {
8181
it('prefers a retry countdown over the in-progress message', () => {
8282
expect(getExposuresRefreshLabel(90, true)).toEqual({
83-
message: 'Computing… retry in 1m 30s',
83+
message: 'Refresh available in 1m 30s',
8484
tone: 'muted',
8585
})
8686
})

0 commit comments

Comments
 (0)