From 31f8156dd69aaab078f92e9bd82035fd41018ee8 Mon Sep 17 00:00:00 2001
From: Alexandru2909 <aputanu@mozilla.com>
Date: Tue, 26 Sep 2023 16:40:36 +0300
Subject: [PATCH] Bug 1855209 - Show the shopping CFRs only once for loaded
 product pages

---
 .../toolbar/BrowserToolbarCFRPresenter.kt     | 18 +++++----
 .../toolbar/BrowserToolbarCFRPresenterTest.kt | 37 ++++++++++++++++++-
 2 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarCFRPresenter.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarCFRPresenter.kt
index 7542154f56..0f101ee539 100644
--- a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarCFRPresenter.kt
+++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarCFRPresenter.kt
@@ -23,6 +23,7 @@ import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.cancel
 import kotlinx.coroutines.flow.distinctUntilChanged
 import kotlinx.coroutines.flow.filter
+import kotlinx.coroutines.flow.firstOrNull
 import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.mapNotNull
 import kotlinx.coroutines.flow.transformWhile
@@ -107,15 +108,18 @@ class BrowserToolbarCFRPresenter(
 
             ToolbarCFR.SHOPPING, ToolbarCFR.SHOPPING_OPTED_IN -> {
                 scope = browserStore.flowScoped { flow ->
-                    flow.mapNotNull { it.selectedTab }
+                    val shouldShowCfr: Boolean? = flow.mapNotNull { it.selectedTab }
                         .filter { it.isProductUrl && it.content.progress == 100 && !it.content.loading }
                         .distinctUntilChanged()
-                        .collect {
-                            if (toolbar.findViewById<View>(R.id.mozac_browser_toolbar_page_actions).isVisible) {
-                                scope?.cancel()
-                                showShoppingCFR(getCFRToShow() == ToolbarCFR.SHOPPING_OPTED_IN)
-                            }
-                        }
+                        .map { toolbar.findViewById<View>(R.id.mozac_browser_toolbar_page_actions).isVisible }
+                        .filter { it }
+                        .firstOrNull()
+
+                    if (shouldShowCfr == true) {
+                        showShoppingCFR(getCFRToShow() == ToolbarCFR.SHOPPING_OPTED_IN)
+                    }
+
+                    scope?.cancel()
                 }
             }
 
diff --git a/app/src/test/java/org/mozilla/fenix/components/toolbar/BrowserToolbarCFRPresenterTest.kt b/app/src/test/java/org/mozilla/fenix/components/toolbar/BrowserToolbarCFRPresenterTest.kt
index 909b0adb77..9ccc68a1d3 100644
--- a/app/src/test/java/org/mozilla/fenix/components/toolbar/BrowserToolbarCFRPresenterTest.kt
+++ b/app/src/test/java/org/mozilla/fenix/components/toolbar/BrowserToolbarCFRPresenterTest.kt
@@ -16,6 +16,7 @@ import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.cancel
 import mozilla.components.browser.state.action.ContentAction
 import mozilla.components.browser.state.action.ShoppingProductAction
+import mozilla.components.browser.state.action.TabListAction
 import mozilla.components.browser.state.state.BrowserState
 import mozilla.components.browser.state.state.CustomTabSessionState
 import mozilla.components.browser.state.state.TabSessionState
@@ -255,7 +256,7 @@ class BrowserToolbarCFRPresenterTest {
     }
 
     @Test
-    fun `GIVEN the user opted in the shopping feature AND the opted in shopping CFR should be shown WHEN the tab is not loading THEN the CFR is shown`() {
+    fun `GIVEN the user opted in the shopping feature AND the opted in shopping CFR should be shown WHEN the tab finishes loading THEN the CFR is shown`() {
         val tab = createTab(url = "")
         val browserStore = createBrowserStore(
             tab = tab,
@@ -290,6 +291,40 @@ class BrowserToolbarCFRPresenterTest {
         verify { presenter.showShoppingCFR(eq(true)) }
     }
 
+    @Test
+    fun `GIVEN the user opted in the shopping feature AND the opted in shopping CFR should be shown WHEN opening a loaded product page THEN the CFR is shown`() {
+        val tab1 = createTab(url = "", id = "tab1")
+        val tab2 = createTab(url = "", id = "tab2")
+        val browserStore = BrowserStore(
+            initialState = BrowserState(
+                tabs = listOf(tab1, tab2),
+                selectedTabId = tab2.id,
+            ),
+        )
+
+        val presenter = createPresenter(
+            settings = mockk {
+                every { shouldShowTotalCookieProtectionCFR } returns false
+                every { shouldShowReviewQualityCheckCFR } returns true
+                every { shouldShowEraseActionCFR } returns false
+                every { reviewQualityCheckOptInTimeInMillis } returns System.currentTimeMillis() - Settings.TWO_DAYS_MS
+            },
+            browserStore = browserStore,
+        )
+        every { presenter.showShoppingCFR(any()) } just Runs
+
+        presenter.start()
+
+        assertNotNull(presenter.scope)
+
+        browserStore.dispatch(ShoppingProductAction.UpdateProductUrlStatusAction(tab1.id, true)).joinBlocking()
+        browserStore.dispatch(ContentAction.UpdateProgressAction(tab1.id, 100)).joinBlocking()
+        verify(exactly = 0) { presenter.showShoppingCFR(any()) }
+
+        browserStore.dispatch(TabListAction.SelectTabAction(tab1.id)).joinBlocking()
+        verify(exactly = 1) { presenter.showShoppingCFR(any()) }
+    }
+
     /**
      * Creates and return a [spyk] of a [BrowserToolbarCFRPresenter] that can handle actually showing CFRs.
      */