search

segment-integrations / analytics.js-integration-google-analytics

The Google Analytics analytics.js integration.
https://segment.com/docs/integrations/google-analytics/
MIT License
20 stars 23 forks source link

update ecommerce spec syntax to v2 #28

Closed wcjohnson11 closed 8 years ago

wcjohnson11 commented 8 years ago

This PR changes the ecommerce V1 syntax Action + Object to the new V2 syntax, Object + Action.

This PR will be waiting on a few blockers, a refactor of the Analytics-Events repo to be more concise and readable as we add more events to our spec and for similar changes to be made to all other integrations currently using V1 syntax.

Tests are failing because it isn't properly aliasing against the new spec'd events. These tests will need to pass, dependent on a new version of Analytics Events and Analytics.js-Private before being merged.

hankim813 commented 8 years ago

@ndhoule this original PR removed some tests from having to use toArray() which I remember we had to use because of this

However, not sure what had changed but some tests that used to be comparing via deepEqual passed without having toArray(). For the purpose of isolating this PR to strictly converting the ecommerce syntax, I've re-added the toArray() where they used to be. I can't see how this PR would change the way GA saves its arguments but either way, might be worth revisiting later and figure out why some deepEqual comparisons requires this helper function to pass while others don't. (I checked this by checking out master and removing all the toArrays and indeed some tests failed -- while some tests passed either way, with or without toArray)

wcjohnson11 commented 8 years ago

lgtm