Skip to content

Commit 62d13fd

Browse files
committed
Fix for edge case when importing an experiment from a feature experiment that has no trackingKey
1 parent 6b55620 commit 62d13fd

File tree

6 files changed

+49
-59
lines changed

6 files changed

+49
-59
lines changed

packages/back-end/src/controllers/experiments.ts

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -310,18 +310,20 @@ export async function getNewFeatures(req: AuthRequest, res: Response) {
310310
projectFeatures.forEach((f) => {
311311
if (f?.rules && f?.rules.length > 0) {
312312
f.rules.forEach((r) => {
313-
if (
314-
r.type === "experiment" &&
315-
r?.trackingKey &&
316-
!expMap.get(r?.trackingKey)
317-
) {
318-
// this feature experiment has no report:
319-
newFeatures.set(r.trackingKey, {
320-
feature: f,
321-
rule: r,
322-
trackingKey: r.trackingKey,
323-
partialExperiment: getExperimentDefinitionFromFeatureAndRule(f, r),
324-
});
313+
if (r.type === "experiment") {
314+
const tKey = r.trackingKey || f.id;
315+
if (!expMap.get(tKey)) {
316+
// this feature experiment has no report:
317+
newFeatures.set(tKey, {
318+
feature: f,
319+
rule: r,
320+
trackingKey: tKey,
321+
partialExperiment: getExperimentDefinitionFromFeatureAndRule(
322+
f,
323+
r
324+
),
325+
});
326+
}
325327
}
326328
});
327329
}
@@ -343,8 +345,8 @@ const getExperimentDefinitionFromFeatureAndRule = (
343345
const totalPercent = expRule.values.reduce((sum, w) => sum + w.weight, 0);
344346

345347
const expDefinition: Partial<ExperimentInterfaceStringDates> = {
346-
trackingKey: expRule.trackingKey,
347-
name: expRule.trackingKey + " experiment",
348+
trackingKey: expRule.trackingKey || feature.id,
349+
name: (expRule.trackingKey || feature.id) + " experiment",
348350
hypothesis: expRule.description || "",
349351
description: `Experiment analysis for the feature [**${feature.id}**](/features/${feature.id})`,
350352
variations: expRule.values.map((v, i) => {

packages/front-end/components/Experiment/NewExperimentForm.tsx

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -88,27 +88,28 @@ const NewExperimentForm: FC<NewExperimentFormProps> = ({
8888
project,
8989
} = useDefinitions();
9090
const { refreshWatching } = useWatching();
91-
const initialPhases: ExperimentPhaseStringDates[] = isImport
92-
? [
93-
{
94-
coverage: initialValue.phases?.[0].coverage || 1,
95-
dateStarted: getValidDate(initialValue.phases?.[0]?.dateStarted)
96-
.toISOString()
97-
.substr(0, 16),
98-
dateEnded: getValidDate(initialValue.phases?.[0]?.dateEnded)
99-
.toISOString()
100-
.substr(0, 16),
101-
phase: initialValue.phases?.[0].phase || "main",
102-
reason: "",
103-
groups: [],
104-
variationWeights:
105-
initialValue.phases?.[0].variationWeights ||
106-
getEvenSplit(
107-
initialValue.variations ? initialValue.variations.length : 2
108-
),
109-
},
110-
]
111-
: [];
91+
const initialPhases: ExperimentPhaseStringDates[] =
92+
isImport && initialValue
93+
? [
94+
{
95+
coverage: initialValue.phases?.[0].coverage || 1,
96+
dateStarted: getValidDate(initialValue.phases?.[0]?.dateStarted)
97+
.toISOString()
98+
.substr(0, 16),
99+
dateEnded: getValidDate(initialValue.phases?.[0]?.dateEnded)
100+
.toISOString()
101+
.substr(0, 16),
102+
phase: initialValue.phases?.[0].phase || "main",
103+
reason: "",
104+
groups: [],
105+
variationWeights:
106+
initialValue.phases?.[0].variationWeights ||
107+
getEvenSplit(
108+
initialValue.variations ? initialValue.variations.length : 2
109+
),
110+
},
111+
]
112+
: [];
112113

113114
useEffect(() => {
114115
track("New Experiment Form", {

packages/front-end/components/Experiment/NewFeatureExperiments.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ export default function NewFeatureExperiments() {
2525
}
2626

2727
return (
28-
<div className="mb-3" style={{ maxHeight: "120px", overflowY: "auto" }}>
28+
<div className="mb-3" style={{ maxHeight: "107px", overflowY: "auto" }}>
2929
{data.features.map((o, i) => {
3030
const f = o.feature;
3131
const expDefinition = o.partialExperiment;

packages/front-end/components/Features/ExperimentSummary.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import {
2+
ExperimentRule,
23
ExperimentValue,
34
FeatureInterface,
45
FeatureValueType,
@@ -24,23 +25,22 @@ export default function ExperimentSummary({
2425
trackingKey,
2526
experiment,
2627
feature,
28+
expRule,
2729
}: {
2830
values: ExperimentValue[];
2931
type: FeatureValueType;
3032
hashAttribute: string;
3133
trackingKey: string;
3234
feature: FeatureInterface;
3335
experiment?: ExperimentInterfaceStringDates;
36+
expRule: ExperimentRule;
3437
}) {
3538
const totalPercent = values.reduce((sum, w) => sum + w.weight, 0);
3639
const { datasources, metrics } = useDefinitions();
3740
const [newExpModal, setNewExpModal] = useState(false);
3841
const [experimentInstructions, setExperimentInstructions] = useState(false);
3942

40-
const expDefinition = getExperimentDefinitionFromFeature(
41-
feature,
42-
trackingKey
43-
);
43+
const expDefinition = getExperimentDefinitionFromFeature(feature, expRule);
4444

4545
return (
4646
<div>

packages/front-end/components/Features/Rule.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,7 @@ export const Rule = forwardRef<HTMLDivElement, RuleProps>(
191191
experiment={experiments[rule.trackingKey || feature.id]}
192192
hashAttribute={rule.hashAttribute || ""}
193193
trackingKey={rule.trackingKey || feature.id}
194+
expRule={rule}
194195
/>
195196
)}
196197
</div>

packages/front-end/services/features.ts

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -374,32 +374,18 @@ export function useAttributeMap(): Map<string, AttributeData> {
374374

375375
export function getExperimentDefinitionFromFeature(
376376
feature: FeatureInterface,
377-
trackingKey: string = ""
377+
expRule: ExperimentRule
378378
) {
379-
let expRule: ExperimentRule;
380-
if (feature?.rules && feature?.rules?.length > 0) {
381-
feature.rules.forEach((r) => {
382-
if (r.type === "experiment") {
383-
// to handle the case where there are multiple experiment rules per feature, make sure it matches the requested one:
384-
if (trackingKey) {
385-
if (r.trackingKey === trackingKey) {
386-
expRule = r;
387-
}
388-
} else {
389-
expRule = r;
390-
}
391-
}
392-
});
393-
}
394-
if (!expRule || !expRule.trackingKey) {
379+
const trackingKey = expRule?.trackingKey || feature.id;
380+
if (!trackingKey) {
395381
return null;
396382
}
397383

398384
const totalPercent = expRule.values.reduce((sum, w) => sum + w.weight, 0);
399385

400386
const expDefinition: Partial<ExperimentInterfaceStringDates> = {
401-
trackingKey: expRule.trackingKey,
402-
name: expRule.trackingKey + " experiment",
387+
trackingKey: trackingKey,
388+
name: trackingKey + " experiment",
403389
hypothesis: expRule.description || "",
404390
description: `Experiment analysis for the feature [**${feature.id}**](/features/${feature.id})`,
405391
variations: expRule.values.map((v, i) => {

0 commit comments

Comments
 (0)