Skip to content

Commit f05273e

Browse files
authored
fix(ExperimentWhitelistService): Fix finding variation by key (#155)
Previously, GetDecision of ExperimentWhitelistService was incorrectly using variation key to look up variations in the Variations map of an Experiment struct, in which keys are variation IDs, not variation keys. With this fix, we iterate over variations to find the one with the desired key. To avoid needing to search all variations, we can add a map keyed by variation key as a follow up.
1 parent 68abf6c commit f05273e

File tree

2 files changed

+13
-10
lines changed

2 files changed

+13
-10
lines changed

pkg/decision/experiment_whitelist_service.go

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,13 +47,16 @@ func (s ExperimentWhitelistService) GetDecision(decisionContext ExperimentDecisi
4747
return decision, nil
4848
}
4949

50-
variation, ok := decisionContext.Experiment.Variations[variationKey]
51-
if !ok {
52-
decision.Reason = reasons.InvalidWhitelistVariationAssignment
53-
return decision, nil
50+
// TODO(Matt): Add a VariationsByKey map to the Experiment struct, and use it to look up Variation by key
51+
for _, variation := range decisionContext.Experiment.Variations {
52+
variation := variation
53+
if variation.Key == variationKey {
54+
decision.Reason = reasons.WhitelistVariationAssignmentFound
55+
decision.Variation = &variation
56+
return decision, nil
57+
}
5458
}
5559

56-
decision.Reason = reasons.WhitelistVariationAssignmentFound
57-
decision.Variation = &variation
60+
decision.Reason = reasons.InvalidWhitelistVariationAssignment
5861
return decision, nil
5962
}

pkg/decision/helpers_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ var testTargetedExp1116 = entities.Experiment{
226226
// Experiment with a whitelist
227227
const testExpWhitelistKey = "test_experiment_whitelist"
228228

229-
var testExpWhitelistVar2229 = entities.Variation{ID: "2229", Key: "2229"}
229+
var testExpWhitelistVar2229 = entities.Variation{ID: "2229", Key: "var_2229"}
230230
var testExpWhitelist = entities.Experiment{
231231
ID: "1117",
232232
Key: testExpWhitelistKey,
@@ -237,8 +237,8 @@ var testExpWhitelist = entities.Experiment{
237237
entities.Range{EntityID: "2229", EndOfRange: 10000},
238238
},
239239
Whitelist: map[string]string{
240-
"test_user_1": "2229",
241-
// Note: this is an invalid entry, there is no variation 2230 in this experiment
242-
"test_user_2": "2230",
240+
"test_user_1": "var_2229",
241+
// Note: this is an invalid entry, there is no variation with key "var_2230" in this experiment
242+
"test_user_2": "var_2230",
243243
},
244244
}

0 commit comments

Comments
 (0)