Conversation
jaeopt
left a comment
jaeopt
left a comment
There was a problem hiding this comment.
Looks good! A few suggestions to consider.
| /** | ||
| * Get variation id for the user | ||
| * @param {OptimizelyUserContext} userContext | ||
| * @param {string} experimentId |
There was a problem hiding this comment.
| * @param {string} experimentId | |
| * @param {string} ruleId |
| getDecision( | ||
| projectConfig: ProjectConfig, | ||
| userContext: OptimizelyUserContext, | ||
| experimentId: string, |
There was a problem hiding this comment.
| experimentId: string, | |
| ruleId: string, |
| } | ||
| } | ||
|
|
||
| const variation = await this.fetchVariation(ruleId, userContext.getUserId(), filteredAttributes); |
There was a problem hiding this comment.
| const variation = await this.fetchVariation(ruleId, userContext.getUserId(), filteredAttributes); | |
| const variation = await this.fetchDecision(ruleId, userContext.getUserId(), filteredAttributes); |
| const cmabAttributeIds = experiment.cmab.attributeIds; | ||
|
|
||
| Object.keys(attributes).forEach((key) => { | ||
| const attributeId = projectConfig.attributeKeyMap[key].id; |
There was a problem hiding this comment.
What about user attributes not in projectConfig.attributes?
There was a problem hiding this comment.
We can start with cmab.attributes? We can avoid walk-through all the user attributes.
In this way, we can also create the fixed order of filtered attributes -
for aId in cmab.attributes {
aKey = project.atrributes[aId]
if aKey in UserContext {
filteredAttribute.add()
There was a problem hiding this comment.
Updated.
also added test for attribute order insensitivity.
| } | ||
|
|
||
| const cachedValue = await this.cmabCache.get(cacheKey); | ||
| const attributesHash = String(murmurhash.v3(JSON.stringify(filteredAttributes))); |
There was a problem hiding this comment.
Can we sort by keys? Same set of attributes can be passed in different orders in UserContext?
3 participants
Summary
Test plan
Issues