Skip to content

Fix impression sent from feature experiment variation toggled off. #84

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 19, 2018
Merged

Fix impression sent from feature experiment variation toggled off. #84

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 33 additions & 4 deletions OptimizelySDK.Tests/OptimizelyTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public class OptimizelyTest
private ProjectConfig Config;
private Mock<EventBuilder> EventBuilderMock;
private Mock<IErrorHandler> ErrorHandlerMock;
private Mock<IEventDispatcher> EventDispatcherMock;
private Optimizely Optimizely;
private IEventDispatcher EventDispatcher;
private const string TestUserId = "testUserId";
Expand Down Expand Up @@ -73,19 +74,19 @@ public void Initialize()
logger: LoggerMock.Object,
errorHandler: new NoOpErrorHandler());

EventDispatcher = new ValidEventDispatcher();
Optimizely = new Optimizely(TestData.Datafile, EventDispatcher, LoggerMock.Object, ErrorHandlerMock.Object);
EventDispatcherMock = new Mock<IEventDispatcher>();
Optimizely = new Optimizely(TestData.Datafile, EventDispatcherMock.Object, LoggerMock.Object, ErrorHandlerMock.Object);

Helper = new OptimizelyHelper
{
Datafile = TestData.Datafile,
EventDispatcher = EventDispatcher,
EventDispatcher = EventDispatcherMock.Object,
Logger = LoggerMock.Object,
ErrorHandler = ErrorHandlerMock.Object,
SkipJsonValidation = false,
};

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

// Should return false and send an impression event when feature is enabled for the user
// and user is being experimented.
[Test]
public void TestIsFeatureEnabledGivenFeatureFlagIsNotEnabledAndUserIsBeingExperimented()
{
var featureKey = "double_single_variable_feature";
var experiment = Config.GetExperimentFromKey("test_experiment_double_feature");
var variation = Config.GetVariationFromKey("test_experiment_double_feature", "variation");
var featureFlag = Config.GetFeatureFlagFromKey(featureKey);
var decision = new FeatureDecision(experiment, variation, FeatureDecision.DECISION_SOURCE_EXPERIMENT);

DecisionServiceMock.Setup(ds => ds.GetVariationForFeature(featureFlag, TestUserId, null)).Returns(decision);

var optly = Helper.CreatePrivateOptimizely();
optly.SetFieldOrProperty("DecisionService", DecisionServiceMock.Object);

bool result = (bool)optly.Invoke("IsFeatureEnabled", featureKey, TestUserId, null);
Assert.False(result);

// SendImpressionEvent() gets called.
LoggerMock.Verify(l => l.Log(LogLevel.INFO,
$@"The user ""{TestUserId}"" is not being experimented on feature ""{featureKey}""."), Times.Never);

LoggerMock.Verify(l => l.Log(LogLevel.INFO,
$@"Feature flag ""{featureKey}"" is not enabled for user ""{TestUserId}""."));
EventDispatcherMock.Verify(dispatcher => dispatcher.DispatchEvent(It.IsAny<LogEvent>()));
}

// Verify that IsFeatureEnabled returns true if a variation does not get found in the feature
// flag experiment but found in the rollout rule.
[Test]
Expand Down
28 changes: 14 additions & 14 deletions OptimizelySDK/Optimizely.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,14 +366,12 @@ public Variation GetForcedVariation(string experimentKey, string userId)
/// <returns>True if feature is enabled, false or null otherwise</returns>
public virtual bool IsFeatureEnabled(string featureKey, string userId, UserAttributes userAttributes = null)
{
if (string.IsNullOrEmpty(userId))
{
if (string.IsNullOrEmpty(userId)) {
Logger.Log(LogLevel.ERROR, "User ID must not be empty.");
return false;
}

if (string.IsNullOrEmpty(featureKey))
{
if (string.IsNullOrEmpty(featureKey)) {
Logger.Log(LogLevel.ERROR, "Feature flag key must not be empty.");
return false;
}
Expand All @@ -386,19 +384,21 @@ public virtual bool IsFeatureEnabled(string featureKey, string userId, UserAttri
return false;

var decision = DecisionService.GetVariationForFeature(featureFlag, userId, userAttributes);
if (decision == null || !decision.Variation.IsFeatureEnabled)
{
Logger.Log(LogLevel.INFO, $@"Feature flag ""{featureKey}"" is not enabled for user ""{userId}"".");
return false;
if (decision != null) {
if (decision.Source == FeatureDecision.DECISION_SOURCE_EXPERIMENT) {
SendImpressionEvent(decision.Experiment, decision.Variation, userId, userAttributes);
} else {
Logger.Log(LogLevel.INFO, $@"The user ""{userId}"" is not being experimented on feature ""{featureKey}"".");
}
if (decision.Variation.IsFeatureEnabled) {
Logger.Log(LogLevel.INFO, $@"Feature flag ""{featureKey}"" is enabled for user ""{userId}"".");
return true;
}
}

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

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

/// <summary>
Expand Down