Jetpack Section: Backup Screen Tracking Events by ParaskP7 · Pull Request #13956 · wordpress-mobile/WordPress-Android

ParaskP7 · 2021-02-03T16:26:00Z

This PR adds five new track events specific to the new Backup screen and enhances an existing Activity Log Detail event with a source property so that it differentiates between the activity_log or backup entry points.

To test:

Launch app and go to My Site tab.
Got to: My Site -> Toolbar Avatar -> App Settings -> Privacy settings
Switch on Collect information to enable tracking on your device.
Go to My Site -> Toolbar Avatar -> App Settings -> Test feature configuration.
Enable BackupsFeatureConfig, scroll down and click the RESTART THE APP button. DO NOT confuse this feature flag with the BackupFeatureConfig (singular). Make sure to enable the right feature flag.
Open Android Logcat, filter the logcat by tracked and start noticing the 🔵 Tracked events.
Go back to My Site and click on the Backup menu item. Notice the jetpack_backup_list_opened track event is triggered.
Find a rewindable item and click on it. Notice the activity_log_detail_opened track event is triggered with backup as its source.
Click the RESTORE button and then the RESTORE SITE dialog button. Notice that you will be redirected back to the Backup screen and that the jetpack_backup_rewind_started track event is triggered.
Click the Date Range chip. Notice the jetpack_backup_filterbar_range_button_tapped track event is triggered.
Select a date range and click SAVE. Notice that you will be redirected back to the Backup screen and that the jetpack_backup_filterbar_select_range track event is triggered.
Click the x inner button within the chip to clear the filter. Notice the jetpack_backup_filterbar_reset_range track event is triggered.

PR submission checklist:

I have considered adding unit tests where possible.
I have considered adding accessibility improvements for my changes.
I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

This 'source' property will help identify whether the 'ActivityLogDetail' screen is being launched from within the 'activity_log' or 'backup' screen.

Also this commit adds back the 'activity_log_rewind_started' track event, which existed within the 'RewindStatusService.rewind(...)' function that got replaced by the restore use cases.

peril-wordpress-mobile · 2021-02-03T16:26:10Z

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

ParaskP7 · 2021-02-03T16:28:24Z

👋 @leandroalonso !

I added you as a reviewer as well just as an FYI to you so that you are aware of this since you were working on the Backup screen on iOS.

peril-wordpress-mobile · 2021-02-03T16:34:08Z

You can test the changes on this Pull Request by downloading the APK here.

malinajirka

Thanks @ParaskP7! I'm having some doubts about some of the events we came up with. Let's discuss this during the Android Sync meeting today.

I'm not approving the PR yet, I'll approve it when we make a final decision on how to proceed.

Click the RESTORE button and then the RESTORE SITE dialog button. Notice that you will be redirected back to the Backup screen and that the jetpack_backup_rewind_started track event is triggered.

Should this trigger also jetpack_restore_opened and when you confirm the dialog -> jetpack_restore_confirmed?

Related question: should jetpack_restore_opened event be fired when I click on "MySite -> Backup -> more button on one of the items -> "Restore to this point" ?

jetpack_backup_rewind_started

I'm not sure about this event. I tried to keep the separation between backup/activityLog only on the level of the entry points - -> so we don't need to pass backup/activityLog flag into all the flows and keep having ifs all over the place. I understand that activity_log_rewind_started is an existing event, but I'm wondering if we should probably make an exception here and stop recording it (or record the same event even for backup).

malinajirka · 2021-02-04T07:52:19Z

WordPress/src/main/java/org/wordpress/android/ui/ActivityLauncher.java

    }

-    public static void viewActivityLogDetailForResult(Activity activity, SiteModel site, String activityId) {
+    public static void viewActivityLogDetailForResult(


🔍 Nitpick: We usually don't use "parameter per line" formatting in Java classes. We might want to update this to

public static void viewActivityLogDetailForResult(Activity activity, SiteModel site, String activityId, boolean rewindableOnly) {

Wdyt?

Oh cool, thanks for letting me know. I saw that this was not being done on the other methods as well and I was wondering why. Will update to your suggestion. 🙏

Ohh, you are right - there is a lot of methods which use the formatting you used. In that case, don't worry about it ;).

ParaskP7 · 2021-02-04T09:16:37Z

Many thanks for the thorough review @malinajirka ! 🌟

Thanks @ParaskP7! I'm having some doubts about some of the events we came up with. Let's discuss this during the Android Sync meeting today.

Definitely! 👌

I'm not approving the PR yet, I'll approve it when we make a final decision on how to proceed.

Makes sense. 👍

Should this trigger also jetpack_restore_opened and when you confirm the dialog -> jetpack_restore_confirmed?

No, this actual event is a bit tricky and it is not a "new" one per se. This was located within the RewindStatusService.rewind(...) function and was being triggered then. If you traverse back and check the history on the ActivityLogViewModel class you will notice that this was triggered during onRewindConfirmed(...), which is now renamed to onRestoreConfirmed(...). So, the new Backup screen related event is actually replicating the existing event for the Activity Log screen, thus me keeping the rewind naming convention, like jetpack_backup_rewind_started, since the other one is named as activity_log_rewind_started. Does this make sense?

Related question: should jetpack_restore_opened event be fired when I click on MySite -> Backup -> more button on one of the items -> Restore to this point?

This track events are related to the Restore Flow Screen and thus @zwarm will handle that (or I'll take this over from Annmarie, to be discussed). But, definitely I didn't want them as part of this PR, trying to keep them separate.

I'm not sure about this event. I tried to keep the separation between backup/activityLog only on the level of the entry points - -> so we don't need to pass backup/activityLog flag into all the flows and keep having ifs all over the place. I understand that activity_log_rewind_started is an existing event, but I'm wondering if we should probably make an exception here and stop recording it (or record the same event even for backup).

I touched on that a bit above, now realising that you kind of have the picture. This is up to us to decide, I am comfortable in us stopping recording it, but at the same time why not just keep it for a while to get some results, figure out whether it is valuable and then drop it afterwards, especially since we already have data for the activity_log_rewind_started and can start comparing that to the new jetpack_backup_rewind_started track event. Wdyt?

PS: Currently, we cannot record the same event for backup since within the ActivityLogDetail screen we only have the RESTORE flow. This event is purely connecting the ActivityLogDetail with that restore trigger.

malinajirka

Thanks for the clarification @ParaskP7!

As discussed activity_log_rewind_started is currently tracked only in the legacy flows. We need to make decision if it's find to stop tracking this event in the new flows or if we need to retain it.

All looks good to me ;)!

ParaskP7 · 2021-02-04T11:01:07Z

Thanks for the clarification @ParaskP7!

🙏

As discussed activity_log_rewind_started is currently tracked only in the legacy flows. We need to make decision if it's find to stop tracking this event in the new flows or if we need to retain it.

👌

All looks good to me ;)!

🙏

ParaskP7 added 8 commits February 3, 2021 17:52

Refactor: Make nosara client field final

ee823d2

Feature: Add jetpack backup list opened track event

235d0c6

Feature: Add source property to activity log detail opened track event

ce21ca7

This 'source' property will help identify whether the 'ActivityLogDetail' screen is being launched from within the 'activity_log' or 'backup' screen.

Feature: Add jetpack backup rewind started track event

dab7fc5

Also this commit adds back the 'activity_log_rewind_started' track event, which existed within the 'RewindStatusService.rewind(...)' function that got replaced by the restore use cases.

Feature: Add jetpack backup filterbar range button tapped track event

48bc3c8

Feature: Add jetpack backup filterbar select range track event

a19b140

Feature: Add jetpack backup filterbar reset range track event

5416bae

Fix: Replace immutable with mutable map to avoid analytics utils crash

252b941

ParaskP7 added Analytics Jetpack Jetpack Mobile labels Feb 3, 2021

ParaskP7 added this to the 16.7 milestone Feb 3, 2021

ParaskP7 requested a review from malinajirka February 3, 2021 16:26

ParaskP7 self-assigned this Feb 3, 2021

ParaskP7 mentioned this pull request Feb 3, 2021

Jetpack Section: Backup Screen #13629

Closed

40 tasks

ParaskP7 requested a review from leandroalonso February 3, 2021 16:27

malinajirka self-assigned this Feb 4, 2021

malinajirka reviewed Feb 4, 2021

View reviewed changes

malinajirka approved these changes Feb 4, 2021

View reviewed changes

ParaskP7 merged commit 3dc50f2 into develop Feb 4, 2021

ParaskP7 deleted the issue/13629-backup-screen-tracking-events branch February 4, 2021 11:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Jetpack Section: Backup Screen Tracking Events#13956

Jetpack Section: Backup Screen Tracking Events#13956
ParaskP7 merged 8 commits intodevelopfrom
issue/13629-backup-screen-tracking-events

ParaskP7 commented Feb 3, 2021 •

edited

Loading

Uh oh!

peril-wordpress-mobile bot commented Feb 3, 2021

Uh oh!

ParaskP7 commented Feb 3, 2021

Uh oh!

peril-wordpress-mobile bot commented Feb 3, 2021

Uh oh!

malinajirka left a comment

Uh oh!

malinajirka Feb 4, 2021

Uh oh!

ParaskP7 Feb 4, 2021

Uh oh!

malinajirka Feb 4, 2021

Uh oh!

ParaskP7 commented Feb 4, 2021 •

edited

Loading

Uh oh!

malinajirka left a comment

Uh oh!

ParaskP7 commented Feb 4, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

ParaskP7 commented Feb 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peril-wordpress-mobile bot commented Feb 3, 2021

Uh oh!

ParaskP7 commented Feb 3, 2021

Uh oh!

peril-wordpress-mobile bot commented Feb 3, 2021

Uh oh!

malinajirka left a comment

Choose a reason for hiding this comment

Uh oh!

malinajirka Feb 4, 2021

Choose a reason for hiding this comment

Uh oh!

ParaskP7 Feb 4, 2021

Choose a reason for hiding this comment

Uh oh!

malinajirka Feb 4, 2021

Choose a reason for hiding this comment

Uh oh!

ParaskP7 commented Feb 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

malinajirka left a comment

Choose a reason for hiding this comment

Uh oh!

ParaskP7 commented Feb 4, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ParaskP7 commented Feb 3, 2021 •

edited

Loading

ParaskP7 commented Feb 4, 2021 •

edited

Loading