[Reader] Improve analytics tracking for subscribed sites and followed tags #20388

daniloercoli · 2024-03-02T10:15:24Z

Improves the fix for #19836

Moved the logic to the view controller, as suggested in the [Reader] Add subscribed sites and followed tags to analytics #20303
Both Followed Tags and Subscribed Sites analytics are bumped either due to Tags/Sites changed or time elapsed since last bump

To Test:

Install Jetpack and sign-in
Open Reader
🔍 Tags followed are correctly tracked
🔍 Sites subscribed analytics are tracked
Open the reader on Web and add a tag
Go back to the app and PTR
🔍 Tags followed are correctly tracked
Open the reader on Web and add a new site
Go back to the app and PTR
🔍 Sites subscribed analytics are tracked
Navigate the app a bit
Go back to the Reader and PTR (if it doesn't start automatically)
🔍 Sites subscribed and Tags Followed analytics are NOT tracked

To check analytics you can use the Track Live view tool, use logcat and filter by Reader events, or set few breakpoints in the Reader Tracker.

PR Submission Checklist:

I have completed the Regression Notes.
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.

Testing Checklist:

…r due sites list changed or time elapsed since last bump

dangermattic · 2024-03-02T10:16:18Z

	1 Warning
⚠️	Class `FollowedBlogsFetched` is missing tests, but `unit-tests-exemption` label was set to ignore this.

Generated by 🚫 Danger

wpmobilebot · 2024-03-02T10:40:56Z

📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.

	App Name	Jetpack
	Flavor	Jalapeno
	Build Type	Debug
	Version	pr20388-fffbfc1
	Commit	`fffbfc1`
	Direct Download	`jetpack-prototype-build-pr20388-fffbfc1.apk`

Note: Google Login is not supported on these builds.

wpmobilebot · 2024-03-02T10:42:43Z

📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.

	App Name	WordPress
	Flavor	Jalapeno
	Build Type	Debug
	Version	pr20388-fffbfc1
	Commit	`fffbfc1`
	Direct Download	`wordpress-prototype-build-pr20388-fffbfc1.apk`

Note: Google Login is not supported on these builds.

…t and ensure the view model is testable.

RenanLukas · 2024-03-04T19:15:28Z

Thanks for the PR, @daniloercoli !

It's working as expected except for a scenario where the event is tracked every time a pull-to-refresh is performed.
I didn't try to reproduce it again (yet), but here's what I did:

1 - I was already subscribed to blog A and opened the Reader in the app;
2 - I've unsubscribed from blog A using web and did a pull-to-refresh in the app. The expected number was tracked;
3 - I've subscribed to blog A again using web and did a pull-to-refresh in the app again. The expected number was tracked;
4 - After the steps above, whenever I perform a pull-to-refresh in the app it tracks the event, even if the number of blogs didn't change:

com.jetpack.android.prealpha         I  🔵 Tracked: reader_pull_to_refresh
com.jetpack.android.prealpha         I  🔵 Tracked: reader_following_fetched, Properties: {"count":"55","type":"sites"}
com.jetpack.android.prealpha         I  🔵 Tracked: reader_pull_to_refresh
com.jetpack.android.prealpha         I  🔵 Tracked: reader_following_fetched, Properties: {"count":"55","type":"sites"}
com.jetpack.android.prealpha         I  🔵 Tracked: reader_pull_to_refresh
com.jetpack.android.prealpha         I  🔵 Tracked: reader_following_fetched, Properties: {"count":"55","type":"sites"}

codecov · 2024-03-04T19:35:31Z

Codecov Report

Attention: Patch coverage is 11.11111% with 40 lines in your changes are missing coverage. Please review.

Project coverage is 40.15%. Comparing base (4dfebc9) to head (fffbfc1).
Report is 101 commits behind head on trunk.

Files	Patch %	Lines
...ss/android/ui/reader/viewmodels/ReaderViewModel.kt	25.00%	6 Missing and 3 partials ⚠️
.../org/wordpress/android/ui/reader/ReaderEvents.java	25.00%	6 Missing ⚠️
...d/ui/reader/services/update/ReaderUpdateLogic.java	0.00%	6 Missing ⚠️
...rdpress/android/ui/reader/tracker/ReaderTracker.kt	0.00%	6 Missing ⚠️
...ss/android/ui/reader/actions/ReaderTagActions.java	0.00%	5 Missing ⚠️
.../java/org/wordpress/android/ui/prefs/AppPrefs.java	0.00%	3 Missing ⚠️
.../android/ui/reader/subfilter/SubFilterViewModel.kt	0.00%	3 Missing ⚠️
.../org/wordpress/android/ui/prefs/AppPrefsWrapper.kt	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##            trunk   #20388      +/-   ##
==========================================
- Coverage   40.15%   40.15%   -0.01%     
==========================================
  Files        1476     1476              
  Lines       68275    68301      +26     
  Branches    11329    11336       +7     
==========================================
+ Hits        27417    27423       +6     
- Misses      38349    38366      +17     
- Partials     2509     2512       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

daniloercoli · 2024-03-04T21:33:04Z

1 - I was already subscribed to blog A and opened the Reader in the app;
2 - I've unsubscribed from blog A using web and did a pull-to-refresh in the app. The expected number was tracked;
3 - I've subscribed to blog A again using web and did a pull-to-refresh in the app again. The expected number was tracked;
4 - After the steps above, whenever I perform a pull-to-refresh in the app it tracks the event, even if the number of blogs didn't change:

Hey @RenanLukas - I've tried with the steps listed above but i'm not able to reproduce the problem. On my test account I see the correct total number of subscribed sites bumped to analytics, and PTR doesn't trigger additional analytics call.

2024-03-04 22:27:30.741 30717-30717 WordPress-STATS         com.jetpack.android.prealpha         I  🔵 Tracked: reader_pull_to_refresh
2024-03-04 22:28:30.371 30717-30717 WordPress-STATS         com.jetpack.android.prealpha         I  🔵 Tracked: reader_pull_to_refresh
2024-03-04 22:28:34.479 30717-30717 WordPress-STATS         com.jetpack.android.prealpha         I  🔵 Tracked: reader_following_fetched, Properties: {"count":"313","type":"sites"}
2024-03-04 22:28:37.903 30717-30717 WordPress-STATS         com.jetpack.android.prealpha         I  🔵 Tracked: reader_pull_to_refresh
2024-03-04 22:28:42.870 30717-30717 WordPress-STATS         com.jetpack.android.prealpha         I  🔵 Tracked: reader_pull_to_refresh
2024-03-04 22:28:55.208 30717-30717 WordPress-STATS         com.jetpack.android.prealpha         I  🔵 Tracked: reader_pull_to_refresh
2024-03-04 22:29:00.193 30717-30717 WordPress-STATS         com.jetpack.android.prealpha         I  🔵 Tracked: reader_following_fetched, Properties: {"count":"314","type":"sites"}
2024-03-04 22:29:00.807 30717-30717 WordPress-STATS         com.jetpack.android.prealpha         I  🔵 Tracked: reader_pull_to_refresh
2024-03-04 22:29:08.507 30717-30717 WordPress-STATS         com.jetpack.android.prealpha         I  🔵 Tracked: reader_pull_to_refresh

RenanLukas · 2024-03-05T14:51:09Z

@daniloercoli thanks for having a look. I've tried reproducing it again, but this time I couldn't.
I'll review the code so we can merge this PR.

RenanLukas

Thanks for the PR, @daniloercoli ! It's working as expected and code LGTM.

I've left a couple of nitpick comments, but feel free to merge it without addressing them since they're not really important.

RenanLukas · 2024-03-05T15:19:15Z

WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/ReaderViewModel.kt

+
+        if (shouldBumpAnalytics) {
+            readerTracker.trackFollowedTagsCount(event.totalTags)
+            appPrefsWrapper.readerAnalyticsCountTagsTimestamp = DateProvider().getCurrentDate().time


💡 Nitpick/optional: maybe we can use now defined in line 226 instead of getting current date again?

RenanLukas · 2024-03-05T15:21:55Z

WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/ReaderViewModel.kt

+
+        if (shouldBumpAnalytics) {
+            readerTracker.trackSubscribedSitesCount(event.totalSubscriptions)
+            AppPrefs.setReaderAnalyticsCountSitesTimestamp(DateProvider().getCurrentDate().time)


💡 Same nitpick/optional about using now defined in line 240 instead of getting current date again.

…te again with DateProvider.

sonarqubecloud · 2024-03-05T15:31:40Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Move the logic to the ViewController and make sure sites bumped eithe…

3f9aeaa

…r due sites list changed or time elapsed since last bump

daniloercoli added Reader [Type] Task Reader IA labels Mar 2, 2024

daniloercoli added this to the 24.4 milestone Mar 2, 2024

daniloercoli requested a review from RenanLukas March 2, 2024 10:15

daniloercoli added the unit-tests-exemption label Mar 2, 2024

daniloercoli added 3 commits March 3, 2024 19:54

Fix Typo

d3e850b

Add the total number of followed tags to the FollowedTagsFetched even…

2e7a4f2

…t and ensure the view model is testable.

Remove unused import.

b0fe4c6

oguzkocer modified the milestones: 24.4, 24.5 Mar 4, 2024

RenanLukas approved these changes Mar 5, 2024

View reviewed changes

Use the already assigned now variable instead of getting current da…

fffbfc1

…te again with DateProvider.

daniloercoli merged commit 5f540cc into trunk Mar 5, 2024
20 checks passed

daniloercoli deleted the issue-19836-improve-reader-analytics-sites-and-tags-tracking branch March 5, 2024 16:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Reader] Improve analytics tracking for subscribed sites and followed tags #20388

[Reader] Improve analytics tracking for subscribed sites and followed tags #20388

daniloercoli commented Mar 2, 2024

dangermattic commented Mar 2, 2024 •

edited

Loading

wpmobilebot commented Mar 2, 2024 •

edited

Loading

wpmobilebot commented Mar 2, 2024 •

edited

Loading

RenanLukas commented Mar 4, 2024

codecov bot commented Mar 4, 2024 •

edited

Loading

daniloercoli commented Mar 4, 2024

RenanLukas commented Mar 5, 2024

RenanLukas left a comment

RenanLukas Mar 5, 2024

RenanLukas Mar 5, 2024

sonarqubecloud bot commented Mar 5, 2024

[Reader] Improve analytics tracking for subscribed sites and followed tags #20388

[Reader] Improve analytics tracking for subscribed sites and followed tags #20388

Conversation

daniloercoli commented Mar 2, 2024

To Test:

PR Submission Checklist:

Testing Checklist:

dangermattic commented Mar 2, 2024 • edited Loading

wpmobilebot commented Mar 2, 2024 • edited Loading

wpmobilebot commented Mar 2, 2024 • edited Loading

RenanLukas commented Mar 4, 2024

codecov bot commented Mar 4, 2024 • edited Loading

Codecov Report

daniloercoli commented Mar 4, 2024

RenanLukas commented Mar 5, 2024

RenanLukas left a comment

Choose a reason for hiding this comment

RenanLukas Mar 5, 2024

Choose a reason for hiding this comment

RenanLukas Mar 5, 2024

Choose a reason for hiding this comment

sonarqubecloud bot commented Mar 5, 2024

Quality Gate passed

dangermattic commented Mar 2, 2024 •

edited

Loading

wpmobilebot commented Mar 2, 2024 •

edited

Loading

wpmobilebot commented Mar 2, 2024 •

edited

Loading

codecov bot commented Mar 4, 2024 •

edited

Loading