Add additional properties to the Block Editor activation events #10300

daniloercoli · 2019-07-30T09:54:59Z

Add additional properties to gutenberg_enabled , and gutenberg_disabled analytics events:
The property source can now have the following values:

via-site-settings
on-block-post-open
on-site-creation

Test 1:

Flip the block editor setting in Site settings and make sure the property is added correctly

Test 2:

Create a new site, and make sure the property is added with the correct value
Tap on new post, and make sure nothing is tracked

Test 3:

Clear your editor settings on the web
Re-install the app
open a block based post
the popup should appear, and the correct property value bumped with analytics

cc @etoledom Since we need the iOS counterpart of this PR

Update release notes:

[ x ] If there are user facing changes, I have added an item to RELEASE-NOTES.txt.

…ed analytics events: The property `source` can now have the following values: - via-site-settings - on-block-post-open - on-site-creation

…e the block editor

hypest · 2019-07-30T10:21:57Z

Let's target the 13.0 release branch for this one and the WPiOS one when it gets opened? cc @etoledom and headsup @loremattei .

daniloercoli · 2019-07-30T10:34:08Z

Let's target the 13.0 release branch for this one and the WPiOS one when it gets opened? cc @etoledom and headsup @loremattei .

I wanted to target 13.0 branch, but it's still not there :)

WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostActivity.java

…e code.

…ss-Android into add/analitycs-to-GB-editor-flip * 'develop' of https://github.com/wordpress-mobile/WordPress-Android: Don't revert firebase test-targets change Don't revert EditorPage change Fix a crash on the help screen (issue reported at login phase) where we were accessing the selected site, but there isn't any site in the app yet. Revert "Run connected tests on CI for every PR(round 2)"

@hypest

…s) info are not still there. We should not track this to false, when the user has instead it on `gutenberg`. The analytics are then bumped later at next startup, or when switching site. cc @hypest

hypest · 2019-07-30T14:20:42Z

Hey Danilo, saw 6c2f05a but I wonder if it will make it hard for us to understand the tracks if we don't set a value there. Like, when that case happens that we don't still have the setting at hand, how will we interpret the tracks we see? WDYT? 🤔

daniloercoli · 2019-07-30T14:35:38Z

Hey Danilo, saw 6c2f05a but I wonder if it will make it hard for us to understand the tracks if we don't set a value there. Like, when that case happens that we don't still have the setting at hand, how will we interpret the tracks we see? WDYT? 🤔

Well, right after the login we don't have the info to bump the property IS_GUTENBERG_ENABLED correctly, since sites info are still not there. I think its's better to not add a wrong value (false) and do not pass it altogether. The correct value will be bump later. I think we can refresh metadata when the editor info are returned from the server, even though it could increase the calls to refreshMetadata.

It should not be a big difference, backend query side, since we can read the most recent event from the Track and check if the user has GB on or off.

On a side note, there is no user related GB setting anymore, wondering if we need to keep this property at user level...can't we move it to the general properties added when tracking events with site details trackWithSiteDetails?

jkmassel · 2019-07-30T18:22:48Z

I'm bumping this to 13.1 as part of the 13.0 code freeze.

If you'd like it to be released as part of 13.0, please merge it against release/13.0 then DM me, and I'll be happy to issue a new beta release! :)

hypest · 2019-07-31T10:22:42Z

WordPress/src/main/java/org/wordpress/android/util/analytics/AnalyticsUtils.java

-            // If saved site exist, then add info
-            if (selectedSite != null) {
+            // If saved site exist, then add editor info
+            if (selectedSite != null && !TextUtils.isEmpty(selectedSite.getMobileEditor())) {


Apparently (see here that the flag is set in the user section) we'll write false by default even if we skip setting the flag here. Perhaps leaving empty is a better approach so, wdyt about introducing an additional flag in AnalyticsMetadata to denote if we have set the flag at AnalyticsMetadata.mIsGutenbergEnabled at all or not?

Addressed in b59bd91

@etoledom

…rties, when there is at least one site with the Block Editor enabled on it. cc @etoledom

hypest · 2019-08-05T09:39:23Z

WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostActivity.java

+
+            if (!newSitePopupNeeded) {
+                // Don't track the activation of the block editor on new sites.
+                // The flip to GB happened at site creation time, and the flip is tracked there with proper details


Hmm, should the SiteUtils.enableBlockEditor(mDispatcher, mSite); (line number 1557 above) be inside this conditional too then? 🤔

Well, I think it's better to keep it out, since the user can change the option back to Aztec from settings right after the site creation. Even before creating/opening a new post. It's an edge case I know, but better to show both the dialog and repeat the network call to the server. It's just a call, that doesn't change anything on new sites 99% of the cases.

As per chat convo - cleaned up the code and made more easy to follow in 4db8228

hypest · 2019-08-05T11:46:50Z

Test 1:

Flip the block editor setting in Site settings and make sure the property is added correctly

Test 2:

Create a new site, and make sure the property is added with the correct value

Tap on new post, and make sure nothing is tracked

Test 3:

Clear your editor settings on the web

Re-install the app

open a block based post

the popup should appear, and the correct property value bumped with analytics

Verified that all test cases worked fine for me 👍

@props

…ollow. @props @hypest

hypest

LGTM!

daniloercoli added 2 commits July 30, 2019 11:48

Add additional properties to gutenberg_enabled , and gutenberg_disabl…

66d6e54

…ed analytics events: The property `source` can now have the following values: - via-site-settings - on-block-post-open - on-site-creation

Bump analytics on new site creation

5da815e

daniloercoli requested review from etoledom and hypest July 30, 2019 09:54

daniloercoli added this to the 13.0 milestone Jul 30, 2019

daniloercoli added Analytics Posting/Editing labels Jul 30, 2019

Introduce enum to track source property of the events enable/disabl…

e3375bd

…e the block editor

hypest reviewed Jul 30, 2019

View reviewed changes

WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostActivity.java Outdated Show resolved Hide resolved

daniloercoli added 2 commits July 30, 2019 13:58

Add utils method in Enum declaration that helped a bit on cleaning th…

5809aa7

…e code.

daniloercoli requested a review from hypest July 30, 2019 13:43

Do not bump editor analytics just after login phase, when the editor(…

6c2f05a

…s) info are not still there. We should not track this to false, when the user has instead it on `gutenberg`. The analytics are then bumped later at next startup, or when switching site. cc @hypest

Fix Lint

e07d3a7

jkmassel modified the milestones: 13.0, 13.1 Jul 30, 2019

daniloercoli changed the base branch from develop to release/13.0 July 31, 2019 07:43

hypest reviewed Jul 31, 2019

View reviewed changes

Make sure to add the gutenberg_enabled properties to the user_prope…

b59bd91

…rties, when there is at least one site with the Block Editor enabled on it. cc @etoledom

etoledom mentioned this pull request Aug 1, 2019

[Gutenberg] Adding Tracks properties to gutenberg enable/disable event wordpress-mobile/WordPress-iOS#12258

Merged

1 task

daniloercoli modified the milestones: 13.1, 13.0 ❄️ Aug 5, 2019

hypest reviewed Aug 5, 2019

View reviewed changes

Clean up the code in setGutenbergEnabledIfNeeded and make easy to f…

4db8228

…ollow. @props @hypest

hypest approved these changes Aug 5, 2019

View reviewed changes

hypest merged commit 32c9374 into release/13.0 Aug 5, 2019

oguzkocer deleted the add/analitycs-to-GB-editor-flip branch April 15, 2020 12:29

Add additional properties to the Block Editor activation events #10300

Add additional properties to the Block Editor activation events #10300

Uh oh!

Conversation

daniloercoli commented Jul 30, 2019

Uh oh!

hypest commented Jul 30, 2019

Uh oh!

daniloercoli commented Jul 30, 2019

Uh oh!

Uh oh!

hypest commented Jul 30, 2019

Uh oh!

daniloercoli commented Jul 30, 2019

Uh oh!

jkmassel commented Jul 30, 2019

Uh oh!

hypest Jul 31, 2019

Choose a reason for hiding this comment

Uh oh!

daniloercoli Jul 31, 2019

Choose a reason for hiding this comment

Uh oh!

hypest Aug 5, 2019

Choose a reason for hiding this comment

Uh oh!

daniloercoli Aug 5, 2019

Choose a reason for hiding this comment

Uh oh!

daniloercoli Aug 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hypest commented Aug 5, 2019

Uh oh!

hypest left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

daniloercoli Aug 5, 2019 •

edited

Loading