For #16133: Simplify OpenInAppOnboardingObserver condition (#16851)

app/src/main/java/org/mozilla/fenix/browser/OpenInAppOnboardingObserver.kt

@@ -6,6 +6,7 @@ package org.mozilla.fenix.browser
 
 import android.content.Context
 import android.view.ViewGroup
+import androidx.annotation.VisibleForTesting
 import androidx.navigation.NavController
 import mozilla.components.browser.session.Session
 import mozilla.components.feature.app.links.AppLinksUseCases
@@ -25,8 +26,10 @@ class OpenInAppOnboardingObserver(
     private val container: ViewGroup
 ) : Session.Observer {
 
-    private var sessionDomainForDisplayedBanner: String? = null
-    private var infoBanner: InfoBanner? = null
+    @VisibleForTesting
+    internal var sessionDomainForDisplayedBanner: String? = null
+    @VisibleForTesting
+    internal var infoBanner: InfoBanner? = null
 
     override fun onUrlChanged(session: Session, url: String) {
         sessionDomainForDisplayedBanner?.let {
@@ -36,15 +39,14 @@ class OpenInAppOnboardingObserver(
         }
     }
 
-    @Suppress("ComplexCondition")
     override fun onLoadingStateChanged(session: Session, loading: Boolean) {
+        if (loading || settings.openLinksInExternalApp || !settings.shouldShowOpenInAppCfr) {
+            return
+        }
+
         val appLink = appLinksUseCases.appLinkRedirect
 
-        if (!loading &&
-            !settings.openLinksInExternalApp &&
-            settings.shouldShowOpenInAppCfr &&
-            appLink(session.url).hasExternalApp()
-        ) {
+        if (appLink(session.url).hasExternalApp()) {
             infoBanner = InfoBanner(
                 context = context,
                 message = context.getString(R.string.open_in_app_cfr_info_message),

app/src/test/java/org/mozilla/fenix/browser/OpenInAppOnboardingObserverTest.kt

@@ -0,0 +1,113 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package org.mozilla.fenix.browser
+
+import android.content.Context
+import io.mockk.MockKAnnotations
+import io.mockk.every
+import io.mockk.impl.annotations.MockK
+import io.mockk.mockk
+import io.mockk.verify
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import mozilla.components.browser.session.Session
+import mozilla.components.feature.app.links.AppLinkRedirect
+import mozilla.components.feature.app.links.AppLinksUseCases
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
+import org.mozilla.fenix.utils.Settings
+
+@ExperimentalCoroutinesApi
+@RunWith(FenixRobolectricTestRunner::class)
+class OpenInAppOnboardingObserverTest {
+
+    @MockK(relaxed = true) private lateinit var context: Context
+    @MockK(relaxed = true) private lateinit var settings: Settings
+    @MockK(relaxed = true) private lateinit var session: Session
+    @MockK(relaxed = true) private lateinit var appLinksUseCases: AppLinksUseCases
+    @MockK(relaxed = true) private lateinit var applinksRedirect: AppLinkRedirect
+    @MockK(relaxed = true) private lateinit var getAppLinkRedirect: AppLinksUseCases.GetAppLinkRedirect
+    @MockK(relaxed = true) private lateinit var infoBanner: InfoBanner
+
+    @Before
+    fun setup() {
+        MockKAnnotations.init(this)
+    }
+
+    @Test
+    fun `do not show banner when openLinksInExternalApp is set to true`() {
+        every { settings.openLinksInExternalApp } returns true
+        every { settings.shouldShowOpenInAppCfr } returns true
+
+        val observer = OpenInAppOnboardingObserver(context, mockk(), settings, appLinksUseCases, mockk())
+        observer.onLoadingStateChanged(session, false)
+
+        verify(exactly = 0) { appLinksUseCases.appLinkRedirect }
+
+        every { settings.openLinksInExternalApp } returns false
+        observer.onLoadingStateChanged(session, false)
+
+        verify(exactly = 1) { appLinksUseCases.appLinkRedirect }
+    }
+
+    @Test
+    fun `do not show banner when shouldShowOpenInAppCfr is set to false`() {
+        every { settings.openLinksInExternalApp } returns false
+        every { settings.shouldShowOpenInAppCfr } returns false
+
+        val observer = OpenInAppOnboardingObserver(context, mockk(), settings, appLinksUseCases, mockk())
+        observer.onLoadingStateChanged(session, false)
+
+        verify(exactly = 0) { appLinksUseCases.appLinkRedirect }
+
+        every { settings.shouldShowOpenInAppCfr } returns true
+        observer.onLoadingStateChanged(session, false)
+
+        verify(exactly = 1) { appLinksUseCases.appLinkRedirect }
+    }
+
+    @Test
+    fun `do not show banner when URL is loading`() {
+        every { settings.openLinksInExternalApp } returns false
+        every { settings.shouldShowOpenInAppCfr } returns true
+
+        val observer = OpenInAppOnboardingObserver(context, mockk(), settings, appLinksUseCases, mockk())
+
+        observer.onLoadingStateChanged(session, true)
+
+        verify(exactly = 0) { appLinksUseCases.appLinkRedirect }
+
+        observer.onLoadingStateChanged(session, false)
+
+        verify(exactly = 1) { appLinksUseCases.appLinkRedirect }
+    }
+
+    @Test
+    fun `do not show banner when external app is not found`() {
+        every { settings.openLinksInExternalApp } returns false
+        every { settings.shouldShowOpenInAppCfr } returns true
+        every { appLinksUseCases.appLinkRedirect } returns getAppLinkRedirect
+        every { getAppLinkRedirect.invoke(any()) } returns applinksRedirect
+
+        val observer = OpenInAppOnboardingObserver(context, mockk(), settings, appLinksUseCases, mockk())
+        observer.onLoadingStateChanged(session, false)
+
+        verify(exactly = 0) { settings.shouldShowOpenInAppBanner }
+    }
+
+    @Test
+    fun `do not dismiss banner when URL is the same`() {
+        val observer = OpenInAppOnboardingObserver(context, mockk(), settings, appLinksUseCases, mockk())
+        observer.infoBanner = infoBanner
+        observer.sessionDomainForDisplayedBanner = "mozilla.com"
+        observer.onUrlChanged(session, "https://mozilla.com")
+
+        verify(exactly = 0) { infoBanner.dismiss() }
+
+        observer.onUrlChanged(session, "https://abc.com")
+        verify(exactly = 1) { infoBanner.dismiss() }
+    }
+}