Skip to content

Commit 8d7011f

Browse files
author
Mike Ng
committed
Revert "Fix impression sent from feature experiment variation toggled off."
This reverts commit c943747.
1 parent c943747 commit 8d7011f

File tree

2 files changed

+18
-47
lines changed

2 files changed

+18
-47
lines changed

OptimizelySDK.Tests/OptimizelyTest.cs

+4-33
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ public class OptimizelyTest
3838
private ProjectConfig Config;
3939
private Mock<EventBuilder> EventBuilderMock;
4040
private Mock<IErrorHandler> ErrorHandlerMock;
41-
private Mock<IEventDispatcher> EventDispatcherMock;
4241
private Optimizely Optimizely;
4342
private IEventDispatcher EventDispatcher;
4443
private const string TestUserId = "testUserId";
@@ -74,19 +73,19 @@ public void Initialize()
7473
logger: LoggerMock.Object,
7574
errorHandler: new NoOpErrorHandler());
7675

77-
EventDispatcherMock = new Mock<IEventDispatcher>();
78-
Optimizely = new Optimizely(TestData.Datafile, EventDispatcherMock.Object, LoggerMock.Object, ErrorHandlerMock.Object);
76+
EventDispatcher = new ValidEventDispatcher();
77+
Optimizely = new Optimizely(TestData.Datafile, EventDispatcher, LoggerMock.Object, ErrorHandlerMock.Object);
7978

8079
Helper = new OptimizelyHelper
8180
{
8281
Datafile = TestData.Datafile,
83-
EventDispatcher = EventDispatcherMock.Object,
82+
EventDispatcher = EventDispatcher,
8483
Logger = LoggerMock.Object,
8584
ErrorHandler = ErrorHandlerMock.Object,
8685
SkipJsonValidation = false,
8786
};
8887

89-
OptimizelyMock = new Mock<Optimizely>(TestData.Datafile, EventDispatcherMock.Object, LoggerMock.Object, ErrorHandlerMock.Object, null, false)
88+
OptimizelyMock = new Mock<Optimizely>(TestData.Datafile, EventDispatcher, LoggerMock.Object, ErrorHandlerMock.Object, null, false)
9089
{
9190
CallBase = true
9291
};
@@ -1540,34 +1539,6 @@ public void TestIsFeatureEnabledGivenFeatureFlagIsEnabledAndUserIsBeingExperimen
15401539
$@"Feature flag ""{featureKey}"" is enabled for user ""{TestUserId}""."));
15411540
}
15421541

1543-
// Should return false and send an impression event when feature is enabled for the user
1544-
// and user is being experimented.
1545-
[Test]
1546-
public void TestIsFeatureEnabledGivenFeatureFlagIsNotEnabledAndUserIsBeingExperimented()
1547-
{
1548-
var featureKey = "double_single_variable_feature";
1549-
var experiment = Config.GetExperimentFromKey("test_experiment_double_feature");
1550-
var variation = Config.GetVariationFromKey("test_experiment_double_feature", "variation");
1551-
var featureFlag = Config.GetFeatureFlagFromKey(featureKey);
1552-
var decision = new FeatureDecision(experiment, variation, FeatureDecision.DECISION_SOURCE_EXPERIMENT);
1553-
1554-
DecisionServiceMock.Setup(ds => ds.GetVariationForFeature(featureFlag, TestUserId, null)).Returns(decision);
1555-
1556-
var optly = Helper.CreatePrivateOptimizely();
1557-
optly.SetFieldOrProperty("DecisionService", DecisionServiceMock.Object);
1558-
1559-
bool result = (bool)optly.Invoke("IsFeatureEnabled", featureKey, TestUserId, null);
1560-
Assert.False(result);
1561-
1562-
// SendImpressionEvent() gets called.
1563-
LoggerMock.Verify(l => l.Log(LogLevel.INFO,
1564-
$@"The user ""{TestUserId}"" is not being experimented on feature ""{featureKey}""."), Times.Never);
1565-
1566-
LoggerMock.Verify(l => l.Log(LogLevel.INFO,
1567-
$@"Feature flag ""{featureKey}"" is not enabled for user ""{TestUserId}""."));
1568-
EventDispatcherMock.Verify(dispatcher => dispatcher.DispatchEvent(It.IsAny<LogEvent>()));
1569-
}
1570-
15711542
// Verify that IsFeatureEnabled returns true if a variation does not get found in the feature
15721543
// flag experiment but found in the rollout rule.
15731544
[Test]

OptimizelySDK/Optimizely.cs

+14-14
Original file line numberDiff line numberDiff line change
@@ -366,12 +366,14 @@ public Variation GetForcedVariation(string experimentKey, string userId)
366366
/// <returns>True if feature is enabled, false or null otherwise</returns>
367367
public virtual bool IsFeatureEnabled(string featureKey, string userId, UserAttributes userAttributes = null)
368368
{
369-
if (string.IsNullOrEmpty(userId)) {
369+
if (string.IsNullOrEmpty(userId))
370+
{
370371
Logger.Log(LogLevel.ERROR, "User ID must not be empty.");
371372
return false;
372373
}
373374

374-
if (string.IsNullOrEmpty(featureKey)) {
375+
if (string.IsNullOrEmpty(featureKey))
376+
{
375377
Logger.Log(LogLevel.ERROR, "Feature flag key must not be empty.");
376378
return false;
377379
}
@@ -384,21 +386,19 @@ public virtual bool IsFeatureEnabled(string featureKey, string userId, UserAttri
384386
return false;
385387

386388
var decision = DecisionService.GetVariationForFeature(featureFlag, userId, userAttributes);
387-
if (decision != null) {
388-
if (decision.Source == FeatureDecision.DECISION_SOURCE_EXPERIMENT) {
389-
SendImpressionEvent(decision.Experiment, decision.Variation, userId, userAttributes);
390-
} else {
391-
Logger.Log(LogLevel.INFO, $@"The user ""{userId}"" is not being experimented on feature ""{featureKey}"".");
392-
}
393-
if (decision.Variation.IsFeatureEnabled) {
394-
Logger.Log(LogLevel.INFO, $@"Feature flag ""{featureKey}"" is enabled for user ""{userId}"".");
395-
return true;
396-
}
389+
if (decision == null || !decision.Variation.IsFeatureEnabled)
390+
{
391+
Logger.Log(LogLevel.INFO, $@"Feature flag ""{featureKey}"" is not enabled for user ""{userId}"".");
392+
return false;
397393
}
398394

395+
if (decision.Source == FeatureDecision.DECISION_SOURCE_EXPERIMENT)
396+
SendImpressionEvent(decision.Experiment, decision.Variation, userId, userAttributes);
397+
else
398+
Logger.Log(LogLevel.INFO, $@"The user ""{userId}"" is not being experimented on feature ""{featureKey}"".");
399399

400-
Logger.Log(LogLevel.INFO, $@"Feature flag ""{featureKey}"" is not enabled for user ""{userId}"".");
401-
return false;
400+
Logger.Log(LogLevel.INFO, $@"Feature flag ""{featureKey}"" is enabled for user ""{userId}"".");
401+
return true;
402402
}
403403

404404
/// <summary>

0 commit comments

Comments
 (0)