Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@ interface DateButtonProps {
date: string | null | undefined
type: 'start' | 'end'
onChange: (date: string) => void
saving: boolean
}

const DateButton = ({ date, type, onChange }: DateButtonProps): JSX.Element => {
const DateButton = ({ date, type, onChange, saving }: DateButtonProps): JSX.Element => {
const containerWidth = 'w-44'
const [isOpen, setIsOpen] = useState(false)

Expand All @@ -27,7 +28,7 @@ const DateButton = ({ date, type, onChange }: DateButtonProps): JSX.Element => {
<Popover
actionable
onClickOutside={() => setIsOpen(false)}
visible={isOpen}
visible={isOpen && !saving}
overlay={
<LemonCalendarSelect
value={date ? dayjs(date) : null}
Expand All @@ -46,6 +47,10 @@ const DateButton = ({ date, type, onChange }: DateButtonProps): JSX.Element => {
size="xsmall"
onClick={() => setIsOpen(true)}
fullWidth
// The label keeps showing the saved date until the server confirms the new one.
// Without this the button looks untouched mid-save, so people re-pick a date,
// and every extra pick is another write plus a full metrics recalculation.
loading={saving}
disabledReason={
!date && type === 'start'
? 'No start date'
Expand Down Expand Up @@ -74,7 +79,7 @@ const DateButton = ({ date, type, onChange }: DateButtonProps): JSX.Element => {
}

export const ExperimentDuration = (): JSX.Element => {
const { experiment } = useValues(experimentLogic)
const { experiment, experimentUpdateLoading } = useValues(experimentLogic)
const { changeExperimentStartDate, changeExperimentEndDate } = useActions(experimentLogic)

const { start_date, end_date } = experiment
Expand All @@ -84,9 +89,19 @@ export const ExperimentDuration = (): JSX.Element => {
<Label intent="menu">Duration</Label>
<div className="flex gap-2 items-center">
<div className="flex items-center gap-2">
<DateButton date={start_date} type="start" onChange={changeExperimentStartDate} />
<DateButton
date={start_date}
type="start"
onChange={changeExperimentStartDate}
saving={experimentUpdateLoading}
/>
<IconArrowRight className="text-base" />
<DateButton date={end_date} type="end" onChange={changeExperimentEndDate} />
<DateButton
date={end_date}
type="end"
onChange={changeExperimentEndDate}
saving={experimentUpdateLoading}
/>
</div>
</div>
</div>
Expand Down
75 changes: 75 additions & 0 deletions frontend/src/scenes/experiments/experimentLogic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { api } from 'lib/api.mock'
import { expectLogic } from 'kea-test-utils'

import { lemonToast } from 'lib/lemon-ui/LemonToast/LemonToast'
import { eventUsageLogic } from 'lib/utils/eventUsageLogic'
import { userLogic } from 'scenes/userLogic'

import experimentJson from '~/mocks/fixtures/api/experiments/_experiment_launched_with_funnel_and_trends.json'
Expand Down Expand Up @@ -624,6 +625,32 @@ describe('experimentLogic', () => {
expect(logic.values.experiment.description).toEqual('stale write')
})

it.each([
['start', 'start_date', (date: string): void => logic.actions.changeExperimentStartDate(date)],
['end', 'end_date', (date: string): void => logic.actions.changeExperimentEndDate(date)],
] as const)(
'drops a rejected %s date so every surface still shows what the server stored',
async (_type, field, dispatch) => {
const snapshot = { ...experiment, version: 1 } as Experiment
logic.actions.setUnmodifiedExperiment(snapshot)
logic.actions.setExperiment(snapshot)
api.update.mockRejectedValue({
status: 409,
data: { detail: 'The experiment was changed since you loaded it.', current_version: 5 },
})
const stored = '2026-08-17T09:33:00Z'
getSpy = jest.spyOn(api, 'get').mockResolvedValue({ ...experiment, version: 5, [field]: stored })

await expectLogic(logic, () => {
dispatch('2026-08-12T09:33:00Z')
}).toFinishAllListeners()

// Keeping the rejected pick here is what makes this tab claim a date that the
// list and the metric results never saw.
expect(logic.values.experiment[field]).toEqual(stored)
}
)

it('collapses identical concurrent dispatches into a single request', async () => {
const snapshot = { ...experiment, version: 3 } as Experiment
logic.actions.setUnmodifiedExperiment(snapshot)
Expand Down Expand Up @@ -718,6 +745,54 @@ describe('experimentLogic', () => {
expect(logic.values.unmodifiedExperiment?.version).toEqual(8)
})
})
describe('changing the experiment dates', () => {
let reportSpy: jest.SpyInstance | undefined

beforeEach(() => {
jest.spyOn(api, 'update')
api.update.mockClear()
})

afterEach(() => {
reportSpy?.mockRestore()
reportSpy = undefined
})

it.each([
[
'start',
'start_date',
'reportExperimentStartDateChange',
experimentJson.start_date,
(date: string): void => logic.actions.changeExperimentStartDate(date),
],
[
'end',
'end_date',
'reportExperimentEndDateChange',
null,
(date: string): void => logic.actions.changeExperimentEndDate(date),
],
] as const)(
'reports the %s date the experiment had before the change',
async (_type, field, reportAction, previousDate, dispatch) => {
const newDate = '2026-08-12T09:33:00Z'
const snapshot = { ...experiment, version: 1 } as Experiment
logic.actions.setUnmodifiedExperiment(snapshot)
logic.actions.setExperiment(snapshot)
api.update.mockResolvedValue({ ...snapshot, [field]: newDate, version: 2 })
reportSpy = jest.spyOn(eventUsageLogic.actions, reportAction)

await expectLogic(logic, () => {
dispatch(newDate)
}).toFinishAllListeners()

// Reporting after the update reads the response back and files the new date as
// both sides of the change, which erases the only record of what it used to be.
expect(reportSpy).toHaveBeenCalledWith(expect.objectContaining({ [field]: previousDate }), newDate)
}
)
})
describe('moveMetricsBetweenSections', () => {
const primaryMetric = {
kind: 'ExperimentMetric',
Expand Down
24 changes: 20 additions & 4 deletions frontend/src/scenes/experiments/experimentLogic.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2631,15 +2631,31 @@ export const experimentLogic = kea<experimentLogicType>([
}
},
changeExperimentStartDate: async ({ startDate }) => {
await asyncActions.updateExperiment({ start_date: startDate, update_feature_flag_params: false })
// Read before the update: the loader replaces `experiment` with the server response,
// so reading it afterwards reports the new date as the old one.
const experimentBeforeChange = values.experiment
try {
await asyncActions.updateExperiment({ start_date: startDate, update_feature_flag_params: false })
} catch {
// updateExperiment raises its own toast. The window did not move, so there is
// nothing to report and nothing to recalculate.
return
}
// eslint-disable-next-line no-unused-expressions
values.experiment && eventUsageLogic.actions.reportExperimentStartDateChange(values.experiment, startDate)
experimentBeforeChange &&
eventUsageLogic.actions.reportExperimentStartDateChange(experimentBeforeChange, startDate)
actions.refreshExperimentResults(true, 'config_change')
},
changeExperimentEndDate: async ({ endDate }) => {
await asyncActions.updateExperiment({ end_date: endDate, update_feature_flag_params: false })
const experimentBeforeChange = values.experiment
try {
await asyncActions.updateExperiment({ end_date: endDate, update_feature_flag_params: false })
} catch {
return
}
// eslint-disable-next-line no-unused-expressions
values.experiment && eventUsageLogic.actions.reportExperimentEndDateChange(values.experiment, endDate)
experimentBeforeChange &&
eventUsageLogic.actions.reportExperimentEndDateChange(experimentBeforeChange, endDate)
actions.refreshExperimentResults(true, 'config_change')
},
endExperiment: async ({ openCleanupPr, repository, setRepositoryAsTeamDefault }) => {
Expand Down
11 changes: 9 additions & 2 deletions frontend/src/scenes/experiments/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1339,11 +1339,18 @@ const CONFLICT_UNPRESERVABLE_KEYS = new Set([
'original_experiment',
'update_feature_flag_params',
'feature_flag',
// The dates are a label plus a picker, not a form the user is part-way through, and they set
// the window every metric is computed over. A held rejected date makes this tab disagree with
// the list and the results until the tab reloads, which reads as one experiment with two start
// dates. Re-picking a date costs one click.
'start_date',
'end_date',
])

/** The fields of a 409-rejected update worth keeping in local state: the user's scalar edits.
/** The fields of a 409-rejected update worth keeping in local state: the user's in-progress edits.
* Collection and bookkeeping fields are dropped — re-applying a stale metric array over the
* fresh state would reintroduce exactly the clobbering the conflict prevented. */
* fresh state would reintroduce exactly the clobbering the conflict prevented. So are the dates,
* which must keep matching what the server actually stored. */
export function conflictPreservedFields(payload: ExperimentUpdatePayload): Partial<Experiment> {
return Object.fromEntries(Object.entries(payload).filter(([key]) => !CONFLICT_UNPRESERVABLE_KEYS.has(key)))
}
Expand Down
Loading