Add Tracks event: calypso_refer_visit_response

[jaswrks]

Changes proposed in this Pull Request

• Listens for a response from the Refer affiliate platform and triggers a new Tracks event (calypso_refer_visit_response) to record each response for further analysis.

---

Testing instructions

Open this Calypso.live URL.

In your browser Console, enable debugging by entering:

localStorage.setItem( 'debug', 'calypso:analytics:*' );

---

Open this Calypso.live URL (or just reload the page).

Filter Console by refer and confirm the following log entries exist.

You can also test using all possible affiliate tracking params:
https://hash-6a33e29e2244a758f9fa26e5b2317e991100a877.calypso.live/start?aff=3343&cid=697355&sid=foo&flags=analytics

---

Now load this Calypso.live URL containing an invalid affiliate ID.

Filter Console by refer and confirm the following log entries exist.

---

Aside from the expected HTTP response failure (due to invalid affiliate ID), please confirm there were no JavaScript errors in your browser console while running any of these tests.

[matticbot]

Test live: https://calypso.live/?branch=update/affiliate-tracking

[lsinger]

AFAIK we don't want to use these methods directly as here, but more like here. The latter is in a React component within a call to connect though, so I'm not sure how that would translate to this data layer module. 🤔 Just thinking out loud here.

[jaswrks]

@jsnajdr or @dmsnell Can you help us answer this question? I want to record a Tracks event on HTTP success or failure. Have I done it properly here in the data-layer? Or is there a better way?

[jsnajdr]

Both handlers (success and error) should return a Redux action to be dispatched as a reaction to the response. And we have a dedicated Redux analytics library:

import { recordTracksEvent } from 'state/analytics/actions';

function onTrackAffiliatePageLoadError( ... ) {
  /* ... */
  return recordTracksEvent( 'calypso_refer_visit_response', eventProps );
}

[jaswrks]

Ah. Thank you so much!

[lsinger]

LGTM. Works as expected. Left a non-blocking code comment.

[dmsnell]

this parsing feels a bit hand-wavey. why is the response coming back as JSON data in the text value?

[jaswrks]

Yeah. This API we're calling responds with JSON even on error, but its treated as a failure and the body is not parsed, instead being passed to onError where we get hand-wavey.

Instead of onSuccess and onError it would be nice to put http() into a mode whereby all responses are handled by an onComplete handler. The onComplete handler could look at the response status and response data in greater detail than is typically necessary -- just for cases like this.

[dmsnell]

this is an appropriate place to use a fromApi parser. it will let us simplify the success handler and the data layer will switch to onError if the parse throws any exception.

const onSuccess = ( action, eventProps ) => recordEvent( 'calypso_refer_visit_response', eventProps );

const fromApi = ( { body: { data: { message, status, success, ...responseData } ) => ( {
	...pick( responseData, whitelistedEventProps ),
	status: status || '',
	success: success || '',
	descriptions: message || 'success',
} );

const onError = ( action, error ) => {
	if ( ! ( error && error.response && 'string' === typeof error.response.text ) ) {
		return;
	}

	const data = JSON.parse( error.response.text );
	if ( 'object' !== typeof data ) {
		return;
	}

	return recordEvent( 'calypso_refer_visit_response', {
		...pick( data, whitelistedEventProps ),
		status: data.status || '',
		success: data.success || '',
		description: data.message || 'success',
	} );
};

…dispatchRequestEx( { fetch, onSuccess, onError, fromApi } )…

[jaswrks]

Really appreciate you explaining fromApi. What you suggest is much more elegant and I see the try/catch block switches back to onError as you noted. Very nice.

[jaswrks]

Suggested changes implemented. This could use a follow-up review now.

[dmsnell]

thanks for the updates. please do share your thoughts if you want to on the changes you made with fromApi; I'd be curious to hear.

I did not test these changes; this approval is based on code auditing of only part of the problem so please rely on others for approval 😄

[lsinger]

🤔 success and status are just empty strings for me:

[jaswrks]

@lsinger Great catch. Thanks! I've updated this PR to resolve that problem.

[lsinger]

Thanks for the update @jaswrks! This seem to work well now. I did notice an extra duplicate field for the first test that I'm not sure is supposed to be there. It's in the "whitelisted" list but I didn't see a mention of it anywhere else:

If this is intended — !

[jaswrks]

@lsinger Thanks! Yep, the duplicate flag is new on the Refer platform side. It works as an indication that you've already been counted by the affiliate platform. That flag is one reason to get this in; i.e., so we can see all of this data in Tracks.