feat(experiments): Restart experiments functionality #13363
Conversation
|
I think this is even better than what I had in mind, allowing them to go back to Edit mode makes more sense! We can just call it Only piece I'd add is that I'd want the reset button to be visible while the experiment is running, otherwise it's much harder to discover as I'll be scared to end my experiment early 😅 (will do a proper QA tomorrow) |
|
This is exciting!! It doesn't look like the experiment details are editable after stopping/restarting, correct? This essentially only allows you to change the start date? A couple user stories I'm thinking of here:
This pull satisfies no.1, but not no.2. The only reason I bring up no.2 is because I think we can and should actually solve for both of these with the same system to reduce confusion and UI cruft. @neilkakkar mentioned being able to "edit" the experiment on the fly, so when an experiment is running there is a "Stop" button and and "Edit" button. When something is running and someone goes to edit, we can have a "save" button that has two options: Save, and Save & Restart. Or a date picker to select the start date. If it's easy to send them back to the edit screen and if it's easy to re-calc conversion after experiment metrics are changed (I don't know much about how metrics are currently calculated but I assume it's just like an insight, which can easily tolerate changing filters etc) then I'd say we try to take care of both of these things with one system. |
OK. Updated it again so that it resets the start date AND allows you to go back and edit the experiment 🤗
|
| @@ -177,9 +177,6 @@ def update(self, instance: Experiment, validated_data: dict, *args: Any, **kwarg | |||
| feature_flag.active = True | |||
| feature_flag.save() | |||
| return super().update(instance, validated_data) | |||
|
|
|||
| elif has_start_date: | |||
There was a problem hiding this comment.
LOL, I overestimated the amount of backend work here 🙈
| @@ -1927,8 +1927,8 @@ export interface Experiment { | |||
| recommended_sample_size?: number | |||
| feature_flag_variants: MultivariateFlagVariant[] | |||
| } | |||
| start_date?: string | |||
| end_date?: string | |||
| start_date?: string | null | |||
There was a problem hiding this comment.
Q: why introduce null when undefined already is here?
There was a problem hiding this comment.
I think the similar comparison is like when we set null vs blank in django for models. So in the frontend for this if we set it to undefined we don't have to pass in this key value at all vs if we set it to null we have to pass in this key value even if it's a null setting. Right now when we create an experiment I think we pass in { ....otherExperimentData, start_date: start_date } with no end_date but otherwise we would have to set { ...otherExperimentData, start_date: start_date, end_date: null } because of typing.
I guess technically we could still pass in end_date: undefined instead? But setting it to null is better practice
| @@ -346,6 +348,10 @@ export const experimentLogic = kea<experimentLogicType>([ | |||
| actions.updateExperiment({ archived: true }) | |||
| values.experiment && actions.reportExperimentArchived(values.experiment) | |||
| }, | |||
| restartExperiment: async () => { | |||
| actions.updateExperiment({ start_date: null, end_date: null }) | |||
There was a problem hiding this comment.
Any specific reason it's not this?
| actions.updateExperiment({ start_date: null, end_date: null }) | |
| actions.updateExperiment({ start_date: undefined, end_date: undefined }) |
There was a problem hiding this comment.
Just JavaScript semantics mostly, I use null to indicate intentional setting of an empty value over undefined
| Stop | ||
| </LemonButton> | ||
| <div className="flex flex-row gap-2"> | ||
| <Popconfirm |
There was a problem hiding this comment.
popconfirm added and restart named to reset !
|
@neilkakkar Feel free to merge if you approve this if I'm not around to do so ! |
|
Made popConfirm a bit smaller and show the caret at the right place
|
Problem
Sometimes you start an experiment and want to reset it because of unwanted data that was collected.
Changes
Screen.Recording.2022-12-15.at.3.57.18.PM.mov
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
How did you test this code?