From 5947344dd238c79c5f87b78556b6ee728f3c51be Mon Sep 17 00:00:00 2001 From: MatthewTighe Date: Wed, 9 Nov 2022 09:52:39 -0800 Subject: [PATCH] For 27768: add FirstAppOpenForDay growth data --- .../org/mozilla/fenix/FenixApplication.kt | 2 -- .../java/org/mozilla/fenix/HomeActivity.kt | 6 ++++ .../fenix/components/appstate/AppAction.kt | 4 +-- .../components/appstate/AppStoreReducer.kt | 2 +- .../mozilla/fenix/components/metrics/Event.kt | 5 +++ .../components/metrics/MetricsMiddleware.kt | 3 +- .../components/metrics/MetricsStorage.kt | 13 +++++++- .../java/org/mozilla/fenix/utils/Settings.kt | 5 +++ app/src/main/res/values/preference_keys.xml | 1 + .../metrics/DefaultMetricsStorageTest.kt | 31 +++++++++++++++++++ 10 files changed, 65 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt index b9f48354b..a8e47bd95 100644 --- a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt +++ b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt @@ -377,8 +377,6 @@ open class FenixApplication : LocaleAwareApplication(), Provider { if (settings().isMarketingTelemetryEnabled) { components.analytics.metrics.start(MetricServiceType.Marketing) } - - components.appStore.dispatch(AppAction.MetricsInitializedAction) } protected open fun setupLeakCanary() { diff --git a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt index 6791c3bbd..9f849d6f5 100644 --- a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt +++ b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt @@ -78,6 +78,7 @@ import org.mozilla.fenix.addons.AddonPermissionsDetailsFragmentDirections import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager import org.mozilla.fenix.browser.browsingmode.DefaultBrowsingModeManager +import org.mozilla.fenix.components.appstate.AppAction import org.mozilla.fenix.components.metrics.BreadcrumbsRecorder import org.mozilla.fenix.databinding.ActivityHomeBinding import org.mozilla.fenix.exceptions.trackingprotection.TrackingProtectionExceptionsFragmentDirections @@ -370,6 +371,11 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { Events.defaultBrowserChanged.record(NoExtras()) } + // We attempt to send metrics onResume so that the start of new user sessions is not + // missed. Previously, this was done in FenixApplication::onCreate, but it was decided + // that we should not rely on the application being killed between user sessions. + components.appStore.dispatch(AppAction.ResumedMetricsAction) + DefaultBrowserNotificationWorker.setDefaultBrowserNotificationIfNeeded(applicationContext) } } diff --git a/app/src/main/java/org/mozilla/fenix/components/appstate/AppAction.kt b/app/src/main/java/org/mozilla/fenix/components/appstate/AppAction.kt index aa55e33fa..33ab9a23c 100644 --- a/app/src/main/java/org/mozilla/fenix/components/appstate/AppAction.kt +++ b/app/src/main/java/org/mozilla/fenix/components/appstate/AppAction.kt @@ -193,7 +193,7 @@ sealed class AppAction : Action { } /** - * Indicates that the app's metrics have been initialized and startup data can be sent. + * Indicates that the app has been resumed and metrics that relate to that should be sent. */ - object MetricsInitializedAction : AppAction() + object ResumedMetricsAction : AppAction() } diff --git a/app/src/main/java/org/mozilla/fenix/components/appstate/AppStoreReducer.kt b/app/src/main/java/org/mozilla/fenix/components/appstate/AppStoreReducer.kt index 9383bed3e..5ac962aa6 100644 --- a/app/src/main/java/org/mozilla/fenix/components/appstate/AppStoreReducer.kt +++ b/app/src/main/java/org/mozilla/fenix/components/appstate/AppStoreReducer.kt @@ -220,7 +220,7 @@ internal object AppStoreReducer { val wallpaperState = state.wallpaperState.copy(availableWallpapers = wallpapers) state.copy(wallpaperState = wallpaperState) } - is AppAction.MetricsInitializedAction -> state + is AppAction.ResumedMetricsAction -> state } } diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt index 8df4b18a3..2c9545acb 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt @@ -21,5 +21,10 @@ sealed class Event { * Event recording whether Firefox has been set as the default browser. */ object SetAsDefault : GrowthData("xgpcgt") + + /** + * Event recording the first time Firefox has been resumed in a 24 hour period. + */ + object FirstAppOpenForDay : GrowthData("41hl22") } } diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/MetricsMiddleware.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/MetricsMiddleware.kt index 29b485f0e..5c3374c23 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/MetricsMiddleware.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/MetricsMiddleware.kt @@ -21,8 +21,9 @@ class MetricsMiddleware( } private fun handleAction(action: AppAction) = when (action) { - is AppAction.MetricsInitializedAction -> { + is AppAction.ResumedMetricsAction -> { metrics.track(Event.GrowthData.SetAsDefault) + metrics.track(Event.GrowthData.FirstAppOpenForDay) } else -> Unit } diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/MetricsStorage.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/MetricsStorage.kt index ad7530554..9f8e00b20 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/MetricsStorage.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/MetricsStorage.kt @@ -43,15 +43,26 @@ internal class DefaultMetricsStorage( Event.GrowthData.SetAsDefault -> { !settings.setAsDefaultGrowthSent && checkDefaultBrowser() } + Event.GrowthData.FirstAppOpenForDay -> { + settings.resumeGrowthLastSent.hasBeenMoreThanDaySince() + } } } override suspend fun updateSentState(event: Event) = withContext(dispatcher) { when (event) { - Event.GrowthData.SetAsDefault -> settings.setAsDefaultGrowthSent = true + Event.GrowthData.SetAsDefault -> { + settings.setAsDefaultGrowthSent = true + } + Event.GrowthData.FirstAppOpenForDay -> { + settings.resumeGrowthLastSent = System.currentTimeMillis() + } } } + private fun Long.hasBeenMoreThanDaySince(): Boolean = + System.currentTimeMillis() - this > dayMillis + companion object { private const val dayMillis: Long = 1000 * 60 * 60 * 24 private const val windowStartMillis: Long = dayMillis * 2 diff --git a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt index 25e51dd8f..a40ac535c 100644 --- a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt +++ b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt @@ -1418,4 +1418,9 @@ class Settings(private val appContext: Context) : PreferencesHolder { key = appContext.getPreferenceKey(R.string.pref_key_growth_set_as_default), default = false, ) + + var resumeGrowthLastSent by longPreference( + key = appContext.getPreferenceKey(R.string.pref_key_growth_resume_last_sent), + default = 0, + ) } diff --git a/app/src/main/res/values/preference_keys.xml b/app/src/main/res/values/preference_keys.xml index d68266645..37d6cee8a 100644 --- a/app/src/main/res/values/preference_keys.xml +++ b/app/src/main/res/values/preference_keys.xml @@ -310,4 +310,5 @@ pref_key_growth_set_as_default + pref_key_growth_last_resumed diff --git a/app/src/test/java/org/mozilla/fenix/components/metrics/DefaultMetricsStorageTest.kt b/app/src/test/java/org/mozilla/fenix/components/metrics/DefaultMetricsStorageTest.kt index a4ae00144..82f54adce 100644 --- a/app/src/test/java/org/mozilla/fenix/components/metrics/DefaultMetricsStorageTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/metrics/DefaultMetricsStorageTest.kt @@ -85,4 +85,35 @@ class DefaultMetricsStorageTest { assertTrue(updateSlot.captured) } + + @Test + fun `GIVEN that it has been less than 24 hours since last resumed sent WHEN checked for sending THEN will not be sent`() = runTest(dispatcher) { + val currentTime = System.currentTimeMillis() + every { settings.resumeGrowthLastSent } returns currentTime + + val result = storage.shouldTrack(Event.GrowthData.FirstAppOpenForDay) + + assertFalse(result) + } + + @Test + fun `GIVEN that it has been more than 24 hours since last resumed sent WHEN checked for sending THEN will be sent`() = runTest(dispatcher) { + val currentTime = System.currentTimeMillis() + every { settings.resumeGrowthLastSent } returns currentTime - 1000 * 60 * 60 * 24 * 2 + + val result = storage.shouldTrack(Event.GrowthData.FirstAppOpenForDay) + + assertTrue(result) + } + + @Test + fun `WHEN last resumed state updated THEN settings updated accordingly`() = runTest(dispatcher) { + val updateSlot = slot() + every { settings.resumeGrowthLastSent } returns 0 + every { settings.resumeGrowthLastSent = capture(updateSlot) } returns Unit + + storage.updateSentState(Event.GrowthData.FirstAppOpenForDay) + + assertTrue(updateSlot.captured > 0) + } }