-
Notifications
You must be signed in to change notification settings - Fork 19
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.
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
fix: support audience ids #161
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -56,7 +56,13 @@ func mapVariation(rawVariation datafileEntities.Variation) entities.Variation { | |
|
||
// Maps the raw experiment entity from the datafile into an SDK Experiment entity | ||
func mapExperiment(rawExperiment datafileEntities.Experiment) entities.Experiment { | ||
audienceConditionTree, err := buildAudienceConditionTree(rawExperiment.AudienceConditions) | ||
var audienceConditionTree *entities.TreeNode | ||
var err error | ||
if rawExperiment.AudienceConditions == nil && len(rawExperiment.AudienceIds) > 0 { | ||
audienceConditionTree, err = buildAudienceConditionTree(rawExperiment.AudienceIds) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we have coverage on this path? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added it. |
||
} else if len(rawExperiment.AudienceConditions) > 0 { | ||
audienceConditionTree, err = buildAudienceConditionTree(rawExperiment.AudienceConditions) | ||
} | ||
if err != nil { | ||
// @TODO: handle error | ||
func() {}() // cheat the linters | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,23 +34,23 @@ func (m ExactMatcher) Match(user entities.UserContext) (bool, error) { | |
if stringValue, ok := m.Condition.Value.(string); ok { | ||
attributeValue, err := user.GetStringAttribute(m.Condition.Name) | ||
if err != nil { | ||
return false, err | ||
return false, nil | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we log that we're swallowing these errors? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the error here means that the user does not have that attribute key we are looking for. I can log it as debug but that might be a bit too noisy, wdyt? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Created JIRA ticket to track this. |
||
} | ||
return stringValue == attributeValue, nil | ||
} | ||
|
||
if boolValue, ok := m.Condition.Value.(bool); ok { | ||
attributeValue, err := user.GetBoolAttribute(m.Condition.Name) | ||
if err != nil { | ||
return false, err | ||
return false, nil | ||
} | ||
return boolValue == attributeValue, nil | ||
} | ||
|
||
if floatValue, ok := utils.ToFloat(m.Condition.Value); ok { | ||
attributeValue, err := user.GetFloatAttribute(m.Condition.Name) | ||
if err != nil { | ||
return false, err | ||
return false, nil | ||
} | ||
return floatValue == attributeValue, nil | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mikeng13 this change is causing issues for datafiles with audience conditions defined as:
"audienceConditions": "20413101795"
, i have fixed it in this PR #165