analytics.js-integration-google-analytics icon indicating copy to clipboard operation
analytics.js-integration-google-analytics copied to clipboard

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

Open nickrathwell opened this issue 8 years ago • 5 comments

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;
}

nickrathwell avatar Oct 26 '16 14:10 nickrathwell

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

sperand-io avatar Oct 31 '16 21:10 sperand-io

@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 ?

hankim813 avatar Oct 31 '16 21:10 hankim813

@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

tsholmes avatar Oct 31 '16 22:10 tsholmes

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

hankim813 avatar Nov 01 '16 18:11 hankim813

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 avatar Aug 13 '19 22:08 SegmentDestinationsBot