From 68f71c6f1d65394c1bf8843f597fa43efd7392f4 Mon Sep 17 00:00:00 2001 From: mcarare Date: Thu, 14 Apr 2022 12:23:52 +0300 Subject: [PATCH] For #24761: Remove wrapper from autoplay settings metrics. --- app/metrics.yaml | 3 +- .../mozilla/fenix/components/metrics/Event.kt | 13 -------- .../components/metrics/GleanMetricsService.kt | 8 ----- .../SitePermissionsFragment.kt | 6 ++-- ...tePermissionsManagePhoneFeatureFragment.kt | 33 ++++++++++++++----- detekt-baseline.xml | 3 -- 6 files changed, 30 insertions(+), 36 deletions(-) diff --git a/app/metrics.yaml b/app/metrics.yaml index 79f687a49..e5529ef49 100644 --- a/app/metrics.yaml +++ b/app/metrics.yaml @@ -6185,7 +6185,8 @@ autoplay: autoplay_setting: description: | The new setting for autoplay: block_cellular, - block_audio, or block_all. + block_audio, allow_all or block_all. + type: string bugs: - https://github.com/mozilla-mobile/fenix/issues/11579 data_reviews: 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 a8e65e518..b08cdeaf8 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 @@ -4,9 +4,7 @@ package org.mozilla.fenix.components.metrics -import org.mozilla.fenix.GleanMetrics.Autoplay import org.mozilla.fenix.GleanMetrics.SearchTerms -import java.util.Locale sealed class Event { @@ -63,17 +61,6 @@ sealed class Event { get() = keyName } - object AutoPlaySettingVisited : Event() - - data class AutoPlaySettingChanged(val setting: AutoplaySetting) : Event() { - enum class AutoplaySetting { - BLOCK_CELLULAR, BLOCK_AUDIO, BLOCK_ALL, ALLOW_ALL - } - - override val extras: Map? - get() = mapOf(Autoplay.settingChangedKeys.autoplaySetting to setting.toString().lowercase(Locale.ROOT)) - } - data class SearchTermGroupCount(val count: Int) : Event() { override val extras: Map get() = hashMapOf(SearchTerms.numberOfSearchTermGroupKeys.count to count.toString()) diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt index f8f1a4f20..074f81ea3 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt @@ -8,7 +8,6 @@ import android.content.Context import mozilla.components.service.glean.Glean import mozilla.components.service.glean.private.NoExtraKeys import mozilla.components.support.base.log.logger.Logger -import org.mozilla.fenix.GleanMetrics.Autoplay import org.mozilla.fenix.GleanMetrics.Awesomebar import org.mozilla.fenix.GleanMetrics.BrowserSearch import org.mozilla.fenix.GleanMetrics.HomeMenu @@ -83,13 +82,6 @@ private val Event.wrapper: EventWrapper<*>? } ) - is Event.AutoPlaySettingVisited -> EventWrapper( - { Autoplay.visitedSetting.record(it) } - ) - is Event.AutoPlaySettingChanged -> EventWrapper( - { Autoplay.settingChanged.record(it) }, - { Autoplay.settingChangedKeys.valueOf(it) } - ) is Event.ProgressiveWebAppOpenFromHomescreenTap -> EventWrapper( { ProgressiveWebApp.homescreenTap.record(it) } ) diff --git a/app/src/main/java/org/mozilla/fenix/settings/sitepermissions/SitePermissionsFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/sitepermissions/SitePermissionsFragment.kt index 76a705603..770b36e24 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/sitepermissions/SitePermissionsFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/sitepermissions/SitePermissionsFragment.kt @@ -9,10 +9,10 @@ import androidx.navigation.Navigation import androidx.preference.Preference import androidx.preference.Preference.OnPreferenceClickListener import androidx.preference.PreferenceFragmentCompat +import mozilla.components.service.glean.private.NoExtras +import org.mozilla.fenix.GleanMetrics.Autoplay import org.mozilla.fenix.R -import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.getPreferenceKey -import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.settings import org.mozilla.fenix.ext.showToolbar import org.mozilla.fenix.settings.PhoneFeature @@ -73,7 +73,7 @@ class SitePermissionsFragment : PreferenceFragmentCompat() { .actionSitePermissionsToManagePhoneFeatures(phoneFeature) if (phoneFeature == PhoneFeature.AUTOPLAY_AUDIBLE) { - requireComponents.analytics.metrics.track(Event.AutoPlaySettingVisited) + Autoplay.visitedSetting.record(NoExtras()) } Navigation.findNavController(requireView()).navigate(directions) diff --git a/app/src/main/java/org/mozilla/fenix/settings/sitepermissions/SitePermissionsManagePhoneFeatureFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/sitepermissions/SitePermissionsManagePhoneFeatureFragment.kt index acbff187b..ed1202b6a 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/sitepermissions/SitePermissionsManagePhoneFeatureFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/sitepermissions/SitePermissionsManagePhoneFeatureFragment.kt @@ -24,11 +24,10 @@ import androidx.navigation.fragment.navArgs import mozilla.components.feature.sitepermissions.SitePermissionsRules import mozilla.components.feature.sitepermissions.SitePermissionsRules.Action.ALLOWED import mozilla.components.feature.sitepermissions.SitePermissionsRules.Action.BLOCKED +import org.mozilla.fenix.GleanMetrics.Autoplay import org.mozilla.fenix.R -import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.databinding.FragmentManageSitePermissionsFeaturePhoneBinding import org.mozilla.fenix.ext.components -import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.settings import org.mozilla.fenix.ext.showToolbar import org.mozilla.fenix.settings.PhoneFeature @@ -43,6 +42,13 @@ const val AUTOPLAY_BLOCK_AUDIBLE = 1 const val AUTOPLAY_ALLOW_ON_WIFI = 2 const val AUTOPLAY_ALLOW_ALL = 3 +/** + * Possible values for autoplay setting changed extra key. + */ +enum class AutoplaySettingMetricsExtraKey { + BLOCK_CELLULAR, BLOCK_AUDIO, BLOCK_ALL, ALLOW_ALL +} + @SuppressWarnings("TooManyFunctions") class SitePermissionsManagePhoneFeatureFragment : Fragment() { @@ -190,29 +196,27 @@ class SitePermissionsManagePhoneFeatureFragment : Fragment() { */ private fun saveActionInSettings(autoplaySetting: Int) { settings.setAutoplayUserSetting(autoplaySetting) - val setting: Event.AutoPlaySettingChanged.AutoplaySetting val (audible, inaudible) = when (autoplaySetting) { AUTOPLAY_ALLOW_ALL -> { - setting = Event.AutoPlaySettingChanged.AutoplaySetting.ALLOW_ALL ALLOWED to ALLOWED } AUTOPLAY_ALLOW_ON_WIFI -> { - setting = Event.AutoPlaySettingChanged.AutoplaySetting.BLOCK_CELLULAR BLOCKED to BLOCKED } AUTOPLAY_BLOCK_AUDIBLE -> { - setting = Event.AutoPlaySettingChanged.AutoplaySetting.BLOCK_AUDIO BLOCKED to ALLOWED } AUTOPLAY_BLOCK_ALL -> { - setting = Event.AutoPlaySettingChanged.AutoplaySetting.BLOCK_ALL BLOCKED to BLOCKED } else -> return } - requireComponents.analytics.metrics.track(Event.AutoPlaySettingChanged(setting)) + autoplaySetting.toAutoplayMetricsExtraKey()?.let { extraKey -> + Autoplay.settingChanged.record(Autoplay.SettingChangedExtra(extraKey)) + } + settings.setSitePermissionsPhoneFeatureAction(AUTOPLAY_AUDIBLE, audible) settings.setSitePermissionsPhoneFeatureAction(AUTOPLAY_INAUDIBLE, inaudible) context?.components?.useCases?.sessionUseCases?.reload?.invoke() @@ -269,4 +273,17 @@ class SitePermissionsManagePhoneFeatureFragment : Fragment() { this } } + + /** + * Returns a [AutoplaySettingMetricsExtraKey] from an AUTOPLAY setting value. + */ + private fun Int.toAutoplayMetricsExtraKey(): String? { + return when (this) { + AUTOPLAY_BLOCK_ALL -> AutoplaySettingMetricsExtraKey.BLOCK_ALL.name.lowercase() + AUTOPLAY_BLOCK_AUDIBLE -> AutoplaySettingMetricsExtraKey.BLOCK_AUDIO.name.lowercase() + AUTOPLAY_ALLOW_ON_WIFI -> AutoplaySettingMetricsExtraKey.BLOCK_CELLULAR.name.lowercase() + AUTOPLAY_ALLOW_ALL -> AutoplaySettingMetricsExtraKey.ALLOW_ALL.name.lowercase() + else -> null + } + } } diff --git a/detekt-baseline.xml b/detekt-baseline.xml index 143a24a8b..718343327 100644 --- a/detekt-baseline.xml +++ b/detekt-baseline.xml @@ -199,8 +199,6 @@ UndocumentedPublicClass:Event.kt$Event$AndroidAutofillSearchItemSelected : Event UndocumentedPublicClass:Event.kt$Event$AndroidAutofillUnlockCanceled : Event UndocumentedPublicClass:Event.kt$Event$AndroidAutofillUnlockSuccessful : Event - UndocumentedPublicClass:Event.kt$Event$AutoPlaySettingChanged : Event - UndocumentedPublicClass:Event.kt$Event$AutoPlaySettingVisited : Event UndocumentedPublicClass:Event.kt$Event$AverageTabsPerSearchTermGroup : Event UndocumentedPublicClass:Event.kt$Event$BaiduTopSiteRemoved : Event UndocumentedPublicClass:Event.kt$Event$BookmarkClicked : Event @@ -420,7 +418,6 @@ UndocumentedPublicClass:Event.kt$Event$WallpaperSettingsOpened : Event UndocumentedPublicClass:Event.kt$Event$WallpaperSwitched : Event UndocumentedPublicClass:Event.kt$Event$WhatsNewTapped : Event - UndocumentedPublicClass:Event.kt$Event.AutoPlaySettingChanged$AutoplaySetting UndocumentedPublicClass:Event.kt$Event.BrowserMenuItemTapped$Item UndocumentedPublicClass:Event.kt$Event.DarkThemeSelected$Source UndocumentedPublicClass:Event.kt$Event.Messaging$MessageClicked : Messaging