Skip to content
This repository has been archived by the owner on Apr 3, 2019. It is now read-only.

fix(metrics): force utm_source=email when signing in from CAD #6342

Merged
merged 1 commit into from Jul 9, 2018

Conversation

philbooth
Copy link
Contributor

Related to: #6258, mozilla/fxa-auth-server#2496

Currently, the vast majority of our fxa_reg - complete events in Amplitude are incorrectly showing up with utm_source set to email. This is because the auth server forces it on all links in email addresses, so when the user clicks the verification link the original utm_source for that flow is overwritten.

A separate PR for the auth server has been opened to prevent this from happening. But in order for that PR not to break legitimate cases where we want utm_source set to email, we must first handle those legitimate cases elsewhere. Hence this PR.

The connect-another-device view can only be reached from a verification email. When a user signs in from that view, we always want the new flow to have utm_source set to email. This change simply hard-codes the utm_source on signin links in that view.

@shane-tomlinson this is what I took out of our meeting with Alex and Leif just now, does it match up with what you're thinking?

Currently, the vast majority of our `fxa_reg - complete` events in
Amplitude are incorrectly showing up with `utm_source` set to `email`.
This is because the auth server forces it on all links in email
addresses, so when the user clicks the verification link the original
`utm_source` for that flow is overwritten.

A separate PR for the auth server [1] has been opened to prevent this
from happening. But in order for that PR not to break legitimate cases
where we want `utm_source` set to `email`, we must first handle those
legitimate cases elsewhere. Hence this PR.

The connect-another-device view can only be reached from a verification
email. When a user signs in from that view, we always want the new flow
to have `utm_source` set to `email`. This change simply hard-codes the
`utm_source` on signin links in that view.

Related issues:

  * #6258
  * mozilla/fxa-auth-server#2496

[1]: mozilla/fxa-auth-server#2505
@philbooth philbooth self-assigned this Jul 6, 2018
@philbooth philbooth added the WIP label Jul 8, 2018
@@ -231,7 +235,19 @@ define(function (require, exports, module) {
* @private
*/
_getEscapedSignInUrl (email) {
return this.getEscapedSyncUrl('signin', ConnectAnotherDeviceView.ENTRYPOINT, { email: email });
return this.getEscapedSyncUrl('signin', ConnectAnotherDeviceView.ENTRYPOINT, {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the exact approach I was thinking @philbooth!

@philbooth
Copy link
Contributor Author

philbooth commented Jul 9, 2018

I've tested this out in fxa-dev now, works well. These are the steps if anyone else wants to try it:

  1. Sign out of Firefox on your mobile device then set identity.fxaccounts.remote.webchannel.uri to https://pbooth.dev.lcip.org.

  2. Sign up for Sync in desktop firefox on pbooth.dev.lcip.org.

  3. Receive verification email. Check that it does not contain utm_source=email in the params.

  4. Verify email in Firefox on your mobile device.

  5. On the connect-another-device view, click the Sign in → button. Check that your signin URL does contain utm_source=email in the params.

Moving to the review column, it would be good to get this on train 116 if nobody objects.

@mozilla/fxa-devs r?

Copy link

@shane-tomlinson shane-tomlinson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r+

@philbooth philbooth merged commit 17ab1fd into master Jul 9, 2018
@philbooth philbooth deleted the pb/6258 branch July 9, 2018 20:56
@shane-tomlinson shane-tomlinson added this to the FxA-0: quality milestone Aug 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants