feat(OptimizelyConfig): add new fields to OptimizelyConfig #418
Conversation
| case .logicalOp: | ||
| return "" | ||
| case .leaf(.audienceId(let audienceId)): | ||
| return "AUDIENCE(\(audienceId))" |
There was a problem hiding this comment.
can we have comments here to explain .leaf and .array cases? why these are required?
| return "\(result)" | ||
| } | ||
|
|
||
| switch firstItem { |
There was a problem hiding this comment.
can we have comments here?
| var collect = false | ||
|
|
||
| var replaced = "" | ||
| for ch in string { |
There was a problem hiding this comment.
can we have some comments here for explanation?
| let updatedRollouts = projectConfig.project.rollouts.map { rollout -> Rollout in | ||
| let feature = project.featureFlags.filter({ $0.rolloutId == rollout.id }).first | ||
|
|
||
| var rollout = rollout |
There was a problem hiding this comment.
can we name this better? this is a bit confusing?
| /// | ||
| /// Examples: | ||
| /// - "123" => "AUDIENCE("123")" | ||
| /// - ["and", "123", "456"] => "AUDIENCE("123") AND (AUDIENCE("456")" |
There was a problem hiding this comment.
| /// - ["and", "123", "456"] => "AUDIENCE("123") AND (AUDIENCE("456")" | |
| /// - ["and", "123", "456"] => "AUDIENCE("123") AND AUDIENCE("456")" |
| } else { | ||
| return false | ||
| } |
There was a problem hiding this comment.
Not important but i think this would look cleaner.
| } else { | |
| return false | |
| } | |
| } | |
| return false |
| } | ||
|
|
||
| // The first item of the array is supposed to be a logical op (and, or, not) | ||
| // extract it first and joined the rest of the array items with the logical op |
There was a problem hiding this comment.
| // extract it first and joined the rest of the array items with the logical op | |
| // extract it first and join the rest of the array items with the logical op |
| // - copy feature's variable data to variables in all variations | ||
| // - serialize experiment audiences to a string | ||
|
|
||
| // prepare an audience [id: name] mapping for audicens serialization |
There was a problem hiding this comment.
| // prepare an audience [id: name] mapping for audicens serialization | |
| // prepare an audience [id: name] mapping for audience serialization |
| {"id": "20348452263", "conditions": ["and", ["or", ["or", {"value": 18, "type": "custom_attribute", "name": "age", "match": "gt"}]]], "name": "adult"}, | ||
| {"id": "20348352569", "conditions": ["and", ["or", ["or", {"value": 18, "type": "custom_attribute", "name": "age", "match": "lt"}]]], "name": "kid"} | ||
| ], | ||
| "audiences": [ |
There was a problem hiding this comment.
Can we have 1 or 2 unique audiences in both typedAudiences and audiences to verify if they are being merged correctly?
| /// | ||
| /// - Parameter string: before replacement | ||
| /// - Returns: string after replacement | ||
| func replaceAudienceIdsWithNames(string: String, audiencesMap: [String: String]) -> String { |
There was a problem hiding this comment.
can we have unit test for this method.
There was a problem hiding this comment.
We have a unit test case covering this audience mapping -
| @@ -1,7 +1,65 @@ | |||
| { | |||
The following new public properties are added to OptimizelyConfig: