search

mozilla-mobile / fenix

⚠️ Fenix (Firefox for Android) moved to a new repository. It is now developed and maintained as part of: https://github.com/mozilla-mobile/firefox-android
https://github.com/mozilla-mobile/firefox-android
Mozilla Public License 2.0
6.47k stars 1.27k forks source link

Bug 1812204 - Add usage growth data event #28641

Closed MatthewTighe closed 1 year ago

MatthewTighe commented 1 year ago

Note: this cannot land without an event token added to the new Event, UsageThreshold. Otherwise, the event will not be correctly sent.

bugzilla ticket

GitHub Automation

Fixes #1812204

MatthewTighe commented 1 year ago

I am marking this as ready to review so that we can align on any fundamental changes to logic. This still should not be landed until after an event token is generated and added to UsageThreshold, but I am waiting on DS to get that to me.

jonalmeida commented 1 year ago

Please update to use the appropriate issue format for title and commit messages as well. e.g. Bug 1812204 - Add usage growth data event

MatthewTighe commented 1 year ago

Updated based on suggestions above. It turned into a slight refactor as I got the sense that there wasn't a real need for UsageRecorder to exist as a standalone class. Still hopeful we will soon have an opportunity to engage in more real planning around the design of this feature.

I left it as a separate commit in case we want to revert to a standalone class, but will squash before I land. Thanks for the suggestions!

MatthewTighe commented 1 year ago

FWIW, I think it's fine to have UsageRecorder as a separate class since it's doing one specific thing, observing the activity callbacks and then reporting them to the storage. What you've done here works well too. 🙂

I played with a few different things, including defining UsageRecorder as an interface that could be referenced as a type in the MetricsStorage interface, but this felt like the happiest medium to me for now.

MatthewTighe commented 1 year ago

Updated naming and kdocs per suggestions and responded to your comment @jonalmeida. I can confirm the event is reported correctly on the Adjust backend so I am not sure if your concern is still applicable, please let me know what you think 🙂

MatthewTighe commented 1 year ago

@mergifyio backport releases_v110.0.0

mergify[bot] commented 1 year ago

backport releases_v110.0.0

✅ Backports have been created

* [#28751 Bug 1812204 - Add usage growth data event (backport #28641)](https://github.com/mozilla-mobile/fenix/pull/28751) has been created for branch `releases_v110.0.0`