Issues/7856 refresh meta data

[aerych]

Fixes #7856

This PR ensures that analytics metadata is refreshed before bumping the account_created stat so that its not being sent as an anonymous user.

Since a username or email address is all that's needed to de-anonymize events, we either pass the username/email address (as in the Google sign up flow) or attempt to retrieve a saved email address from SharedPreferences (as in the email sign up flow).

To test:
The easiest way to test this is to sign up with each flow, then check the live view for the wpandroid_account_created event in the Tracks dashboard and wait to see your event appear.

Alternatively set breakpoints in AnalyticsTrackerNosara.refreshMetaData and ensure that mNosaraClient.trackAliasUser is called when signing up.

@hypest could I trouble you to take a peek at this one? Oguz has picked up the review after some early feedback from Stefanos. :)

[oguzkocer]

@aerych I don't think I fully understand the decisions in this PR.

• Is there a reason we are not doing a similar thing to trackAnalyticsSignIn in trackCreatedAccount, meaning why are we not passing the AccountStore and SiteStore to AnalyticsUtils.trackAnalyticsAccountCreated? I looked at the current code and I think that should work.
• Although this is not in the current change set, I am a bit confused why we have a call to LoginAnalyticsTracker.trackCreatedAccount from both the WPMainActivity and SignupGoogleFragment. Shouldn't we be able to handle both type of signups from WPMainActivity and track the same way? That'll also resolve my first question.
• If for some reason we do need it in preferences, we should move the related code to AppPrefs for consistency. The key should probably be moved to DeletablePrefKey, but again, only if we still have the preference code.

My suggestion would be:

• Check the WPMainActivity.onAuthenticationChanged for google signup. Unless I am missing something, it should be triggered when SignupGoogleFragment.onAuthenticationChanged is triggered. If that's the case, remove the SignupGoogleFragment.onAuthenticationChanged and move the logic under WPMainActivity.onAuthenticationChanged.
• Change the parameters for LoginAnalyticsTracker.trackCreatedAccount to accept an AccountStore and SiteStore and make that change in WPMainActivity.onAuthenticationChanged.
• In AnalyticsUtils.trackAnalyticsAccountCreated remove all that custom logic and follow a similar pattern to AnalyticsUtils.trackAnalyticsSignIn. Better yet, extract the common parts of them.
• Remove the preferences code completely.

@aerych I hope this helps! Let me know if you need help with any of this. I am happy to look into it further for you! Also feel free to ping someone from the login project in case I am missing something.

[aerych]

We caught up on Slack and did some pair programming to improve the patch, but just for the sake of looping back on Github...

Is there a reason we are not doing a similar thing to trackAnalyticsSignIn in trackCreatedAccount, meaning why are we not passing the AccountStore and SiteStore to AnalyticsUtils.trackAnalyticsAccountCreated?

It was because the AccountStore isn't updated with a valid username and email address when the code in WPMainActivity runs. You suggested a nice alternative approach that we'll try. :)

Although this is not in the current change set, I am a bit confused why we have a call to LoginAnalyticsTracker.trackCreatedAccount from both the WPMainActivity and SignupGoogleFragment. Shouldn't we be able to handle both type of signups from WPMainActivity and track the same way?

I'm not sure, but my guess is it was tricky to tell the difference between account creation and logging in from outside of the flow. As it stands the sign up stat is being bumped in both cases which I have mixed feelings about but felt it was out of scope to address here.

If for some reason we do need it in preferences, we should move the related code to AppPrefs for consistency. The key should probably be moved to DeletablePrefKey, but again, only if we still have the preference code.

Just to document the backstory, after some initial tinkering I had a chat with Nick and another with Stefanos about what I thought our options were. The trick is we need updated account info before refreshing metadata, and we need to refresh metadata before bumping account_created because we don't want that and subsequent stats to be anonymous. Where Stefanos and I ended up was thinking we could have a new service or a headless fragement listen for the onchange event after sign up, and then bump the stat that way. But when I started, there was something about it that was nagging at me. We should already have all the info we need to refreshmeta data without the service call. That's when I started thinking about using StoredPreferences.

The approach you suggested on our call is a bit of a hybrid between the stored prefs approach here, and the service/fragement approach I chatted with Stefanos about (in the sense that it refreshes the AccountStore), but with fewer moving parts, and less code required. :)

Update forthcoming!

[aerych]

Changes implemented. I confirmed that events are correctly showing up in the tracks admin for both email and google flows.
Ready for another peek @oguzkocer !

[oguzkocer]

Looks good @aerych! I have a few minor suggestions, but this is good to merge as is!

[oguzkocer]

Setting this flag doesn't actually lets us dispatch the event, it makes sure it's always handled in the OnAccountChanged even if the user quits the app after signup is completed and before OnAccountChanged could be triggered. Feel free to move this comment under onAccountChanged (or the helper method I mentioned in the other comment) if you think it'll be easier to understand where it's used.

[aerych]

I'll move it to the helper

[oguzkocer]

As we discussed on the call, I think it'd be a good improvement to have this logic by itself in a helper method.

private void trackMagicLinkSignupIfNecessary() {
    if (AppPrefs.getShouldTrackMagicLinkSignup()) {
        AccountModel account = mAccountStore.getAccount();
        if (!TextUtils.isEmpty(account.getUserName()) && !TextUtils.isEmpty(account.getEmail())) {
            mLoginAnalyticsListener.trackCreatedAccount(account.getUserName(), account.getEmail());
            mLoginAnalyticsListener.trackSignupMagicLinkSucceeded();
            AppPrefs.removeShouldTrackMagicLinkSignup();
        }
    }
}

[aerych]

Arg! Yes, this change slipped my mind.

[aerych]

Thanks Oguz! One last peek for paranoia sake?

[oguzkocer]

Looks great

Will merge once Travis is done!