[EP-157] Fix Properties (Phase One) #1371
Conversation
| @@ -115,16 +113,8 @@ public func optimizelyProperties(environment: Environment? = AppEnvironment.curr | |||
| } | |||
|
|
|||
| let allExperiments = optimizelyClient.allExperiments().map { experimentKey -> [String: String] in | |||
| let variation = try? optimizelyClient.getVariationKey( | |||
There was a problem hiding this comment.
Question: Why did this change?
There was a problem hiding this comment.
A couple of relevant pieces of context:
- in phase 8 we are essentially deprecating the entirety of
optimizelyProperties - insights wants the format to be
session_variants_optimizely:[experimentKey1: variation, experimentKey2: variation ...]
I decided to delete some of these properties here because after making the change to the format of session_variants_optimizely i realized those other values were not being used for anything.
| .multiplyingCurrency(100.0) | ||
| .rounded() | ||
| // Two decimal places to represent cent values | ||
| let pledgeTotalUsd = rounded(pledgeTotal.multiplyingCurrency(staticUsdRate), places: 2) |
There was a problem hiding this comment.
This variable is used in a core flow for checkout. Can we add tests for the way we are using it if we don't have them already
There was a problem hiding this comment.
The fix here entails changing the original representation of cents into a dollar + cents value (e.g. 1015 cents -> 10.15). It turns out that we already have a handful of tests written in the PledgeViewModelTests file. A lot of them broke after the fix which made sense because our templates had cent values that all needed to be updated. I want to note, however, that the change here is specifically occurring in the checkoutPropertiesData function which is only used for events/tracking and not rendering components of the UI, from my observation.
After I fixed the templates and tests, it appears our coverage for both this method and the entire PledgeViewModel are good.
📲 What
This PR continues the ongoing work for phase one of our analytics overhaul. The changes, included in here, are primarily renaming and moving properties and their values to their newly defined areas.
🤔 Why
The properties provided context for each of our events and the Insights team has a defined set of criteria for them.