From 381fdc94e10842aea738ff3090bdecb6e0a8ca0a Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Fri, 20 Dec 2024 11:08:30 +0100 Subject: [PATCH] move out of main cta viewmodel checks (#5414) Task/Issue URL: https://app.asana.com/0/1202552961248957/1208737359937198/f ### Description Moves to io main threads db access detected in CtaViewmodel ### Steps to test this PR _Feature 1_ - [x] Smoke tests onboarding - [ ] ### UI changes | Before | After | | ------ | ----- | !(Upload before screenshot)|(Upload after screenshot)| --- .../com/duckduckgo/app/cta/ui/CtaViewModel.kt | 69 ++++++++++--------- .../app/cta/ui/OnboardingDaxDialogTests.kt | 10 ++- 2 files changed, 43 insertions(+), 36 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/cta/ui/CtaViewModel.kt b/app/src/main/java/com/duckduckgo/app/cta/ui/CtaViewModel.kt index 429c4902c5c2..54c6c7c5e661 100644 --- a/app/src/main/java/com/duckduckgo/app/cta/ui/CtaViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/cta/ui/CtaViewModel.kt @@ -139,40 +139,41 @@ class CtaViewModel @Inject constructor( } suspend fun onCtaShown(cta: Cta) { - cta.shownPixel?.let { - val canSendPixel = when (cta) { - is DaxCta -> cta.canSendShownPixel() - else -> true + withContext(dispatchers.io()) { + cta.shownPixel?.let { + val canSendPixel = when (cta) { + is DaxCta -> cta.canSendShownPixel() + else -> true + } + if (canSendPixel) { + pixel.fire(it, cta.pixelShownParameters()) + } } - if (canSendPixel) { - pixel.fire(it, cta.pixelShownParameters()) + if (cta is OnboardingDaxDialogCta && cta.markAsReadOnShow) { + dismissedCtaDao.insert(DismissedCta(cta.ctaId)) } - } - if (cta is OnboardingDaxDialogCta && cta.markAsReadOnShow) { - dismissedCtaDao.insert(DismissedCta(cta.ctaId)) - } - if (cta is BrokenSitePromptDialogCta) { - brokenSitePrompt.ctaShown() - } - withContext(dispatchers.io()) { + if (cta is BrokenSitePromptDialogCta) { + brokenSitePrompt.ctaShown() + } + if (cta is DaxBubbleCta.DaxPrivacyProCta || cta is DaxBubbleCta.DaxExperimentPrivacyProCta) { extendedOnboardingPixelsPlugin.testPrivacyProOnboardingShownMetricPixel()?.getPixelDefinitions()?.forEach { pixel.fire(it.pixelName, it.params) } } - } - // Temporary pixel - val isVisitSiteSuggestionsCta = - cta is DaxBubbleCta.DaxIntroVisitSiteOptionsCta || cta is DaxBubbleCta.DaxExperimentIntroVisitSiteOptionsCta || - cta is OnboardingDaxDialogCta.DaxSiteSuggestionsCta || cta is OnboardingDaxDialogCta.DaxExperimentSiteSuggestionsCta - if (isVisitSiteSuggestionsCta) { - if (userBrowserProperties.daysSinceInstalled() <= MIN_DAYS_TO_COUNT_ONBOARDING_CTA_SHOWN) { - val count = onboardingStore.visitSiteCtaDisplayCount ?: 0 - pixel.fire(AppPixelName.ONBOARDING_VISIT_SITE_CTA_SHOWN, mapOf("count" to count.toString())) - onboardingStore.visitSiteCtaDisplayCount = count + 1 - } else { - onboardingStore.clearVisitSiteCtaDisplayCount() + // Temporary pixel + val isVisitSiteSuggestionsCta = + cta is DaxBubbleCta.DaxIntroVisitSiteOptionsCta || cta is DaxBubbleCta.DaxExperimentIntroVisitSiteOptionsCta || + cta is OnboardingDaxDialogCta.DaxSiteSuggestionsCta || cta is OnboardingDaxDialogCta.DaxExperimentSiteSuggestionsCta + if (isVisitSiteSuggestionsCta) { + if (userBrowserProperties.daysSinceInstalled() <= MIN_DAYS_TO_COUNT_ONBOARDING_CTA_SHOWN) { + val count = onboardingStore.visitSiteCtaDisplayCount ?: 0 + pixel.fire(AppPixelName.ONBOARDING_VISIT_SITE_CTA_SHOWN, mapOf("count" to count.toString())) + onboardingStore.visitSiteCtaDisplayCount = count + 1 + } else { + onboardingStore.clearVisitSiteCtaDisplayCount() + } } } } @@ -250,8 +251,8 @@ class CtaViewModel @Inject constructor( } suspend fun getFireDialogCta(): OnboardingDaxDialogCta? { - if (!daxOnboardingActive() || daxDialogFireEducationShown()) return null return withContext(dispatchers.io()) { + if (!daxOnboardingActive() || daxDialogFireEducationShown()) return@withContext null if (highlightsOnboardingExperimentManager.isHighlightsEnabled()) { return@withContext OnboardingDaxDialogCta.DaxExperimentFireButtonCta(onboardingStore, appInstallStore) } else { @@ -261,8 +262,8 @@ class CtaViewModel @Inject constructor( } suspend fun getSiteSuggestionsDialogCta(): OnboardingDaxDialogCta? { - if (!daxOnboardingActive() || !canShowDaxIntroVisitSiteCta()) return null return withContext(dispatchers.io()) { + if (!daxOnboardingActive() || !canShowDaxIntroVisitSiteCta()) return@withContext null if (highlightsOnboardingExperimentManager.isHighlightsEnabled()) { return@withContext OnboardingDaxDialogCta.DaxExperimentSiteSuggestionsCta(onboardingStore, appInstallStore) } else { @@ -272,8 +273,8 @@ class CtaViewModel @Inject constructor( } suspend fun getEndStaticDialogCta(): OnboardingDaxDialogCta.DaxExperimentEndStaticCta? { - if (!daxOnboardingActive() && daxDialogEndShown()) return null return withContext(dispatchers.io()) { + if (!daxOnboardingActive() && daxDialogEndShown()) return@withContext null return@withContext OnboardingDaxDialogCta.DaxExperimentEndStaticCta(onboardingStore, appInstallStore) } } @@ -473,7 +474,7 @@ class CtaViewModel @Inject constructor( } } - private suspend fun isSiteNotAllowedForOnboarding(site: Site): Boolean { + private fun isSiteNotAllowedForOnboarding(site: Site): Boolean { val uri = site.url.toUri() if (subscriptions.isPrivacyProUrl(uri)) return true @@ -502,9 +503,11 @@ class CtaViewModel @Inject constructor( // We only want to show New Tab when the Home CTAs from Onboarding has finished // https://app.asana.com/0/1157893581871903/1207769731595075/f suspend fun areBubbleDaxDialogsCompleted(): Boolean { - val noBrowserCtaExperiment = extendedOnboardingFeatureToggles.noBrowserCtas().isEnabled() - val bubbleCtasShown = daxDialogEndShown() && (daxDialogNetworkShown() || daxDialogOtherShown() || daxDialogTrackersFoundShown()) - return noBrowserCtaExperiment || bubbleCtasShown || hideTips() || !userStageStore.daxOnboardingActive() + return withContext(dispatchers.io()) { + val noBrowserCtaExperiment = extendedOnboardingFeatureToggles.noBrowserCtas().isEnabled() + val bubbleCtasShown = daxDialogEndShown() && (daxDialogNetworkShown() || daxDialogOtherShown() || daxDialogTrackersFoundShown()) + noBrowserCtaExperiment || bubbleCtasShown || hideTips() || !userStageStore.daxOnboardingActive() + } } private fun daxDialogSerpShown(): Boolean = dismissedCtaDao.exists(CtaId.DAX_DIALOG_SERP) diff --git a/app/src/test/java/com/duckduckgo/app/cta/ui/OnboardingDaxDialogTests.kt b/app/src/test/java/com/duckduckgo/app/cta/ui/OnboardingDaxDialogTests.kt index 2bdf27b047be..020ded0aeaa5 100644 --- a/app/src/test/java/com/duckduckgo/app/cta/ui/OnboardingDaxDialogTests.kt +++ b/app/src/test/java/com/duckduckgo/app/cta/ui/OnboardingDaxDialogTests.kt @@ -38,7 +38,6 @@ import com.duckduckgo.app.widget.ui.WidgetCapabilities import com.duckduckgo.brokensite.api.BrokenSitePrompt import com.duckduckgo.browser.api.UserBrowserProperties import com.duckduckgo.common.test.CoroutineTestRule -import com.duckduckgo.common.utils.DispatcherProvider import com.duckduckgo.duckplayer.api.DuckPlayer import com.duckduckgo.feature.toggles.api.Toggle import kotlinx.coroutines.test.runTest @@ -72,7 +71,6 @@ class OnboardingDaxDialogTests { private val onboardingStore: OnboardingStore = mock() private val userStageStore: UserStageStore = mock() private val tabRepository: TabRepository = mock() - private val dispatchers: DispatcherProvider = mock() private val duckDuckGoUrlDetector: DuckDuckGoUrlDetector = mock() private val extendedOnboardingFeatureToggles: ExtendedOnboardingFeatureToggles = mock() private val mockDuckPlayer: DuckPlayer = mock() @@ -95,7 +93,13 @@ class OnboardingDaxDialogTests { widgetCapabilities, dismissedCtaDao, userAllowListRepository, - settingsDataStore, onboardingStore, userStageStore, tabRepository, dispatchers, duckDuckGoUrlDetector, extendedOnboardingFeatureToggles, + settingsDataStore, + onboardingStore, + userStageStore, + tabRepository, + coroutineRule.testDispatcherProvider, + duckDuckGoUrlDetector, + extendedOnboardingFeatureToggles, subscriptions = mock(), mockDuckPlayer, mockHighlightsOnboardingExperimentManager,