feat: Added a new getFeatureVariableJson Api #467
Conversation
| if (variable.type === 'string' && variable.subType === 'json') { | ||
| variable.type = 'json'; | ||
| } |
There was a problem hiding this comment.
Can we add a comment explaining why this is overwritten? Also I think it's a bad idea to mutate datafile; it is assigned once at the top of the file and could be used in other tests.
Instead could we do something like this?
var expectedVariableType =
variable.type === "string" && variable.subType === "json"
? "json"
: variable.type;
assert.include(variablesMap[variable.key], {
id: variable.id,
key: variable.key,
type: expectedVariableType,
value: variable.defaultValue,
});| @@ -119,6 +119,13 @@ export var createProjectConfig = function(datafile) { | |||
|
|
|||
| projectConfig.featureKeyMap = fns.keyBy(projectConfig.featureFlags || [], 'key'); | |||
| objectValues(projectConfig.featureKeyMap || {}).forEach(function(feature) { | |||
| // Convert type:string and subType:json to type:json to generalize implementation | |||
There was a problem hiding this comment.
I think this comment needs a little more information. We can mention that json variables in the datafile have a type of "string" for backwards compatibility, but we want to erase the concept of subType, and represent them as a first-class type within the project config.
| @@ -3000,6 +3000,36 @@ describe('lib/optimizely', function() { | |||
| }); | |||
| }); | |||
|
|
|||
| it('returns the right value from getFeatureVariable and send notification with featureEnabled true', function() { | |||
There was a problem hiding this comment.
These strings passed to it() should mention the type of variable being tested (I know you are following what was already here). It's weird that they all have the same description string.
Summary
Implemented getFeatureVariableJson API
Test plan
Added unit tests for getFeatureVariableJson.