Automattic / pocket-casts-android

Pocket Casts Android 🎧
Mozilla Public License 2.0
2.53k stars 209 forks source link

Simplify Analytics configuration #2410

Closed MiSikora closed 1 week ago

MiSikora commented 1 week ago

Description

I'm doing this work to make my work with clip analytics easier.

We had a strange configuration of object AnalyticsTracker and class AnalyticsTrackerWrapper. The first one was used to register all trackers and the latter one was used for DI but it was a bit clunky to use because it called object AnalyticsTracker inside.

This PR removes object AnalyticsTracker and uses proper DI for class AnalyticsTracker. Previously DI was impossible because there was a cyclic dependency, which I had to break in this PR by introducing AccountManagerStatusInfo. The PR also decouples it from Settings making testing a bit easier without a need for mocks.

You can ignore 6605bbae3fc67a613482d80d048ba8c0af216d7c as it is just a renaming PR. The meat of the PR is in f399dc90e5531fbaf6b3d2e33c4fd5807a81bf89.

Testing Instructions

  1. Open the app as the unsigned user and verify that tracking works.
  2. Analytic events should have "user_is_logged_in":false property.
  3. Sign in.
  4. Analytic events should have "user_is_logged_in":true property. Signing out at this point will fail to reflect the property change (#2409).
  5. You can verify in the network inspector that user ID (_ui) is still correctly tracked. However user type will be incorrect due to this bug - https://github.com/Automattic/Automattic-Tracks-Android/pull/223. Screenshot 2024-06-28 at 10 43 33
  6. Go to privacy settings and disable Analytics.
  7. Notice that events are no longer tracked.

Checklist

I have tested any UI changes...

dangermattic commented 1 week ago
2 Warnings
:warning: This PR is larger than 500 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
:warning: Class AccountManagerStatusInfo is missing tests, but unit-tests-exemption label was set to ignore this.

Generated by :no_entry_sign: Danger