Help: Update Tracks props on contact form

[jeremeylduvall]

Every Tracks event currently has site_plan_id passed along as a super prop. The /help/contact page isn't specific to a site though meaning no blog_id is passed along and the site plan isn't registered for events on the contact form. This adds in a prop of site_plan so we can track live chats and Kayako tickets created per plan.

To test

1. Open the console and run localStorage.setItem('debug', 'calypso:analytics'); to turn on analytics tracking in the console.
2. Load up this branch. Visit /help/contact with a site that has a plan of some sort (Jetpack or WP.com).
3. Enter some test content into the form and click either A) start a chat or B) send an email.
4. There might be a better way to test this particularly with a sandbox, but I prevented starting an actual live chat or Kayako ticket by disabling pieces of the functions here and here.

In the console, the site plan should be tracked alongside the associated actions of calypso_help_live_chat_begin and calypso_help_contact_submit.

Related discussion: p1475167065000286-slack-dotcom-strategery

(I also fixed a line length warning from ESLint)

cc @kriskarkoski

[matticbot]

Test live: https://calypso.live/?branch=add/update-contact-form-tracks-data

[jeremeylduvall]

cc @omarjackman or @jordwest if you have a second to take a look ^

[timmyc]

Since this is a connected component, it might be nice to utilize the getSitePlan selector and pass in product_id as a prop to this component

[timmyc]

same as above, could re-use the productId prop here.

[jeremeylduvall]

@timmyc Thanks for the review! I tried the approach using connect, but getSitePlan expects two arguments, state and siteId. In this case, siteId is hard to come by. I tried using getSelectedSiteId(), but for reasons I'm not totally sure of, state is in the wrong form and the function returns null when used like this:

export default connect(
    ( state ) => {
        return {
            olarkTimedOut: isOlarkTimedOut( state ),
            isEmailVerified: isCurrentUserEmailVerified( state ),
            productId: getSitePlan( state, getSelectedSiteId( state ) ),
        };
    }
)( HelpContact );

We discussed this a it in Slack, but is there a better way to get siteId here?

[omarjackman]

@jeremeylduvall

Upon looking at this further it seems like you kinda had it right the first time :(. The site object that you were using is not the currently selected site in the calypso state tree. Its the selected site on the contact form via the sites dropdown list. I think your fine using site.plan.product_id

I think to support using the state selectors in the way that @timmyc suggested you'd need to make this component not use import siteList from 'lib/sites-list'; anymore and re-architect it so that its fully connected.

[omarjackman]

The code looks pretty good to me and if @timmyc has no issue with my comment above you've got a 🚢 from me.

[jeremeylduvall]

@omarjackman Took a last look at this with Timmy, and it looks like there's a small snag.

If a user purchases an upgrade and then is somehow removed from a site, olark.isUserEligible still returns true. A user with an old upgrade but no site could visit the contact form and see either the chat or ticket form. In that case, site doesn't exist and site.plan.product_id breaks.

Seems like we could either:

1. Setup a conditional that makes sure site exists before recording this Tracks data
2. Fix olark.isUserEligible so that it returns false if a user doesn't have a site. That doesn't feel right though since they own an upgrade.

Thoughts? This is probably a small population of users, but I have seen it before.

[jeremeylduvall]

Alright-this variation could work. I added a ternary to check if site exists:

site_plan: ( site ? site.plan.product_id : 1 )

In this context, 1 equates to a site without a plan. Four cases to consider:

1. A user with a site and a plan visits the contact form. Should record the correct plan.
2. A user with a site and an individual upgrade visits the contact form. It should record 1 since the site doesn't have a plan although it might have a legacy individual upgrade.
3. A user with a site without any upgrades visits the contact form. They should only see the forum submission choice. No site plan recorded.
4. A user without a site but a legacy owner of an upgrade on WordPress.com visits the contact form. They should see either chat or ticket submission form. Since site doesn't exist for this user, we record 1.

The only downside is that in case #4, it's possible that the user without a site actually owns a plan on WordPress.com, but we'll record them as 1 (a non-plan user) anyway. This is such a small group though, I think it's alright. The other option would be to record it as null, but that seems very non-obvious if you didn't know why it was happening.

[omarjackman]

@jeremeylduvall

Is 1 used as a real product_id and will using it in this way cause any ambiguity?

I suggest that for this tracks ping you assume that any user can start a chat even if they have no upgrades. That should narrow down your cases to consider to two:

1. A user has a site. In this case I would set the product_id to what ever site.plan.product_id is.
2. A user doesn't have any sites. In this case I would set the product_id to null to indicate that there is no plan

If using null adds more ambiguity here between legacy .com upgrades and a site upgrade then maybe we should just change the name of the tracks ping property to something more specific like site_plan_product_id and use another property like has_legacy_upgrade to represent the legacy upgrade users. I don't immediately know how you would determine if a user has legacy upgrades but assuming you could then I would go that route.

[jeremeylduvall]

Thanks Omar! I think using null is probably a safer bet here to reduce confusion. I renamed the Tracks property to site_plan_product_id to make it more descriptive. Users with a site will have the proper site.plan.product_id recorded. Users without a site will register as null. For our purposes, I think this is alright. The population of users without a site that submit a ticket or start a chat are relatively rare, and we primarily want to start collecting data around those with a site so I think this is a good start.

I'm not quite sure how to grab the upgrades for a user without a site offhand either. I can look at addressing that in a separate PR, but for now, I think this gets us the data we need.