segment-integrations / analytics-ios-integration-adobe-analytics

Segment's Adobe Analytics Integration for Analytics-iOS
MIT License
1 stars 2 forks source link

Refactor lVarsV2 #14

Closed ladanazita closed 6 years ago

ladanazita commented 6 years ago
Moves logic to check settings.lVarsV2 into
method to mapContextValues, renaming this to
mapContextValuesAndlVars.

This simplifies realTrack to simply set the
event and contextData, and checks the
lVarsV2 and contextValue settings when mapping
`screen` and ecommerce events.
ladanazita commented 6 years ago

I tried to clean this up as much as possible. There are still many checks, but I believe they are necessary by nature of the logic required to build contextData when both lVars and contextValues are configured.

@brennan parity wise — I've also mapped lVarsV2 and contextValues on ecommerce events as well as screen events.

brennan commented 6 years ago

Gotcha @ladanazita - I would agree with mapping lVars on screen events, but not on ecommerce events. The latter would suggest that we're accepting unspecced properties from the customer's ecommerce events, which we shouldn't.

ladanazita commented 6 years ago

Hmm, in general are we saying that people shouldn't pass in custom properties on ecommerce events? I could see a case for lVars on ecommerce events, for instance if they send in wish list items as an array.

Also, our client-side integration maps lVars on ecommerce events

brennan commented 6 years ago

Yeah - I don't think people should pass in custom properties on ecommerce events. I don't see a specced property called "Wishlist" of data type array in our ecommerce spec, either.

ladanazita commented 6 years ago

@brennan yeah that was an example of a custom property I just made up.

What about for parity?

ladanazita commented 6 years ago

@f2prateek ping!

ladanazita commented 6 years ago

Reverting lVars based on our discussion documented here