Skip to content
This repository has been archived by the owner on Aug 14, 2019. It is now read-only.

Google Analytics Integration extractCheckoutOptions bug, Impact Checkout Step Viewed and Completed #36

Open
nickrathwell opened this issue Oct 26, 2016 · 5 comments
Labels
migrated The issue has been migrated

Comments

@nickrathwell
Copy link

nickrathwell commented Oct 26, 2016

Base on the documentation:

https://segment.com/docs/spec/ecommerce/v2/#checkout-step-viewed
https://segment.com/docs/spec/ecommerce/v2/#checkout-step-completed

The checkout options are to be returned in this format:

analytics.track('Checkout Step Completed', {
  checkout_id: '50314b8e9bcf000000000000',
  step: 2,
  shipping_method: 'Fedex',
  payment_method: 'Visa'
});

However the Google Analytics Integration is coded to look for these value in that format

analytics.track('Checkout Step Completed', {
  checkout_id: '50314b8e9bcf000000000000',
  step: 2,
  shipping_method: 'Fedex',
  payment_method: 'Visa'
});

See code https://github.com/segment-integrations/analytics.js-integration-google-analytics/blob/master/lib/index.js#L903

function extractCheckoutOptions(props) {
  var options = [
    props.paymentMethod,
    props.shippingMethod
  ];
  // Remove all nulls, and join with commas.
  var valid = reject(options);
  return valid.length > 0 ? valid.join(', ') : null;
}
@sperand-io
Copy link
Contributor

good catch! @hankim813 can we switch to case-insensitive lookups there via regex or direct fallbacks?

@hankim813
Copy link
Contributor

hankim813 commented Oct 31, 2016

@sperand-io if these are spec'd properties of an ecommerce event, we should update the facade dependency to add shippingMethod() and paymentMethod() to .track() messages that covers all the cases

thoughts @tsholmes ?

@tsholmes
Copy link

@hankim813 yeah if they are spec'd ecommerce props, we should have a facade method for them that runs it through proxy to avoid casing issues

@hankim813
Copy link
Contributor

@sperand-io EPD-510 <-- created ticket internally to track this issue. Should be a simple fix.

@SegmentDestinationsBot
Copy link
Contributor

Hi @nickrathwell, as part of the monorepo migration, this issue has been moved to new issue. Our engineers have been notified and will prioritize and work on it ASAP. Thank you!

For more information, see README.md.

@SegmentDestinationsBot SegmentDestinationsBot added the migrated The issue has been migrated label Aug 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
migrated The issue has been migrated
Projects
None yet
Development

No branches or pull requests

5 participants