Skip to content

Commit

Permalink
Merge pull request segment-integrations#17 from segment-integrations/fix
Browse files Browse the repository at this point in the history
Dont set people properties from track calls
  • Loading branch information
f2prateek committed Oct 29, 2015
2 parents e2ff6fc + 48c697d commit e38af8a
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 17 deletions.
13 changes: 2 additions & 11 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -181,12 +181,8 @@ Mixpanel.prototype.track = function(track) {
var people = this.options.people;
var props = track.properties();
var revenue = track.revenue();
var superProperties = this.options.superProperties;
var peopleProperties = extendTraits(this.options.peopleProperties);
var superProps = pick(superProperties, props);
var peopleProps = pick(peopleProperties, props);
superProps = alias(superProps, traitAliases);
peopleProps = alias(peopleProps, traitAliases);
// Don't map traits, clients should use identify instead.
var superProps = pick(this.options.superProperties, props);

// delete mixpanel's reserved properties, so they don't conflict
delete props.distinct_id;
Expand All @@ -210,11 +206,6 @@ Mixpanel.prototype.track = function(track) {
window.mixpanel.register(superProps);
}

// set people properties if present in context.mixpanel.peopleProperties
if (!is.empty(peopleProps) && people) {
window.mixpanel.people.set(peopleProps);
}

// track revenue specifically
if (revenue && people) {
window.mixpanel.people.track_charge(revenue);
Expand Down
23 changes: 17 additions & 6 deletions test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ describe('Mixpanel', function() {
describe('#page', function() {
beforeEach(function() {
analytics.stub(window.mixpanel, 'track');
analytics.stub(window.mixpanel.people, 'set');
});

it('should not track anonymous pages by default', function() {
Expand Down Expand Up @@ -125,6 +126,14 @@ describe('Mixpanel', function() {
analytics.page('Category', 'Name');
analytics.didNotCall(window.mixpanel.track);
});

// Exercise a bug where we set people properties in track calls. https://segment.phacility.com/T903
it('will not track page props as a people property', function() {
mixpanel.options.people = true;
analytics.page('Name');
analytics.called(window.mixpanel.track, 'Viewed Name Page');
analytics.didNotCall(window.mixpanel.people.set, { $name: 'Name' });
});
});

describe('#identify', function() {
Expand Down Expand Up @@ -212,7 +221,6 @@ describe('Mixpanel', function() {
analytics.didNotCall(window.mixpanel.register, { subscribed: true });
});


it('shouldn\'t set people properties from the mixpanel.options.peopleProperties object if setAllTraitsByDefault is false', function() {
mixpanel.options.people = false;
mixpanel.options.setAllTraitsByDefault = false;
Expand Down Expand Up @@ -325,23 +333,26 @@ describe('Mixpanel', function() {
it('should set super properties from the mixpanel.options.superProperties object', function() {
mixpanel.options.superProperties = ['accountStatus', 'subscribed', 'email'];
analytics.track('event', { accountStatus: 'Paid', subscribed: true, email: 'androidjones@sky.net' });
analytics.called(window.mixpanel.register, { accountStatus: 'Paid', subscribed: true, $email: 'androidjones@sky.net' });
analytics.called(window.mixpanel.register, { accountStatus: 'Paid', subscribed: true, email: 'androidjones@sky.net' });
});

it('should set people properties from the mixpanl.options.peopleProperties object', function() {
// Exercise a bug where we set people properties in track calls. https://segment.phacility.com/T903
it('should not set people properties from the mixpanl.options.peopleProperties object', function() {
mixpanel.options.people = true;
mixpanel.options.peopleProperties = ['friend'];
analytics.track('event', { friend: 'elmo' });
analytics.called(window.mixpanel.people.set, { friend: 'elmo' });
analytics.didNotCall(window.mixpanel.people.set);
});

it('should set people properties from the Mixpanel\'s special traits', function() {
// Exercise a bug where we set people properties in track calls. https://segment.phacility.com/T903
it('should not set people properties from the Mixpanel\'s special traits', function() {
mixpanel.options.people = true;
mixpanel.options.peopleProperties = ['friend'];
analytics.track('event', { friend: 'elmo', email: 'dog@dog.com' });
analytics.called(window.mixpanel.people.set, { friend: 'elmo', $email: 'dog@dog.com' });
analytics.didNotCall(window.mixpanel.people.set);
});

// Exercise a bug where we set people properties in track calls. https://segment.phacility.com/T903
it('shouldn\'t try to register super properties if not specified', function() {
mixpanel.options.superProperties = [];
analytics.track('event', { friend: 'elmo' });
Expand Down

0 comments on commit e38af8a

Please sign in to comment.