Skip to content

Commit 5db0758

Browse files
authored
Fix GetDetailedFeatureDecisionUnsafe to include holdout experiment/variation info (#429)
This change fixes a bug where GetDetailedFeatureDecisionUnsafe did not return experimentKey and variationKey for holdout decisions, only for feature tests. Holdouts are control groups that need experiment/variation tracking for analytics, similar to feature tests. This aligns the go-sdk with the Swift SDK reference implementation, which includes experiment and variation info for all decision sources except rollouts. Changes: - Updated client.go:878 to include decision.Holdout alongside decision.FeatureTest - Added test case TestGetDetailedFeatureDecisionUnsafeWithHoldoutAndTrackingEnabled - Updated comment to clarify that holdouts should include experiment/variation info
1 parent efec465 commit 5db0758

File tree

2 files changed

+53
-3
lines changed

2 files changed

+53
-3
lines changed

pkg/client/client.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -873,9 +873,9 @@ func (o *OptimizelyClient) GetDetailedFeatureDecisionUnsafe(featureKey string, u
873873
if featureDecision.Variation != nil {
874874
decisionInfo.Enabled = featureDecision.Variation.FeatureEnabled
875875

876-
// This information is only necessary for feature tests.
877-
// For rollouts experiments and variations are an implementation detail only.
878-
if featureDecision.Source == decision.FeatureTest {
876+
// Include experiment and variation info for feature tests and holdouts.
877+
// For rollouts, experiments and variations are an implementation detail only.
878+
if featureDecision.Source == decision.FeatureTest || featureDecision.Source == decision.Holdout {
879879
decisionInfo.VariationKey = featureDecision.Variation.Key
880880
decisionInfo.ExperimentKey = featureDecision.Experiment.Key
881881
}

pkg/client/client_test.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2131,6 +2131,56 @@ func TestGetDetailedFeatureDecisionUnsafeWithFeatureTestAndTrackingEnabled(t *te
21312131
assert.True(t, client.tracer.(*MockTracer).StartSpanCalled)
21322132
}
21332133

2134+
func TestGetDetailedFeatureDecisionUnsafeWithHoldoutAndTrackingEnabled(t *testing.T) {
2135+
mockConfig := new(MockProjectConfig)
2136+
mockConfigManager := new(MockProjectConfigManager)
2137+
mockDecisionService := new(MockDecisionService)
2138+
mockEventProcessor := new(MockEventProcessor)
2139+
testUserContext := entities.UserContext{ID: "test_user_1"}
2140+
2141+
// Test holdout decision path
2142+
testVariation := makeTestVariation("control", true)
2143+
testExperiment := makeTestExperimentWithVariations("holdout_1", []entities.Variation{testVariation})
2144+
testFeature := makeTestFeatureWithExperiment("feature_1", testExperiment)
2145+
mockConfig.On("GetFeatureByKey", testFeature.Key).Return(testFeature, nil)
2146+
mockConfigManager.On("GetConfig").Return(mockConfig, nil)
2147+
mockEventProcessor.On("ProcessEvent", mock.AnythingOfType("event.UserEvent"))
2148+
2149+
// Set up the mock decision service with holdout source
2150+
testDecisionContext := decision.FeatureDecisionContext{
2151+
Feature: &testFeature,
2152+
ProjectConfig: mockConfig,
2153+
}
2154+
2155+
expectedFeatureDecision := decision.FeatureDecision{
2156+
Experiment: testExperiment,
2157+
Variation: &testVariation,
2158+
Source: decision.Holdout,
2159+
}
2160+
2161+
mockDecisionService.On("GetFeatureDecision", testDecisionContext, testUserContext, &decide.Options{}).Return(expectedFeatureDecision, decide.NewDecisionReasons(nil), nil)
2162+
2163+
client := OptimizelyClient{
2164+
ConfigManager: mockConfigManager,
2165+
DecisionService: mockDecisionService,
2166+
EventProcessor: mockEventProcessor,
2167+
logger: logging.GetLogger("", ""),
2168+
tracer: &MockTracer{},
2169+
}
2170+
2171+
decision, err := client.GetDetailedFeatureDecisionUnsafe(testFeature.Key, testUserContext, false)
2172+
assert.NoError(t, err)
2173+
assert.True(t, decision.Enabled)
2174+
assert.Equal(t, decision.ExperimentKey, "holdout_1")
2175+
assert.Equal(t, decision.VariationKey, "control")
2176+
2177+
mockConfig.AssertExpectations(t)
2178+
mockConfigManager.AssertExpectations(t)
2179+
mockDecisionService.AssertExpectations(t)
2180+
mockEventProcessor.AssertExpectations(t)
2181+
assert.True(t, client.tracer.(*MockTracer).StartSpanCalled)
2182+
}
2183+
21342184
func TestGetAllFeatureVariables(t *testing.T) {
21352185
testFeatureKey := "test_feature_key"
21362186
testUserContext := entities.UserContext{ID: "test_user_1"}

0 commit comments

Comments
 (0)