search

segment-integrations / analytics.js-integration-intercom

The Intercom analytics.js integration.
https://segment.com/docs/integrations/intercom/
MIT License
7 stars 12 forks source link

correctly map monthlySpend and revenue EPD-219 #19

Closed anoonan closed 7 years ago

anoonan commented 7 years ago

This PR properly monthlySpend to monthly_spend as well as maps revenue to price according to intercom's docs: https://developers.intercom.com/reference#event-metadata-types

Currently our server-side integration does so this PR is creating parity between the two.

@ladanazita

codecov-io commented 7 years ago

Codecov Report

Merging #19 into master will increase coverage by 0.11%.

@@            Coverage Diff             @@
##           master      #19      +/-   ##
==========================================
+ Coverage   98.64%   98.76%   +0.11%     
==========================================
  Files           1        1              
  Lines          74       81       +7     
==========================================
+ Hits           73       80       +7     
  Misses          1        1
Impacted Files Coverage Δ
lib/index.js 98.76% <100%> (+0.11%) :white_check_mark:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a9ee2d2...28cdf1c. Read the comment docs.

f2prateek commented 7 years ago

Closes https://github.com/segment-integrations/analytics.js-integration-intercom/issues/1.

Weird we're doing this in the server side integration. monthlySpend is not spec'd by Segment https://segment.com/docs/spec/

hankim813 commented 7 years ago

@f2prateek yeah agreed -- but I'd rather side with upkeeping parity rather than doing all the retroactive work to add this to the spec, convert server side to use integration specific options (without breaking changes)