Bug 1863504 - Track Shopping CFR shown to user when it is displayed

Rather than tracking that the CFR was shown to the user when the
user explicitly interacts with it (dismiss or action tap),
we will track it when the CFR was shown, not considering how the
user dismissed it.

app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarCFRPresenter.kt

@@ -69,7 +69,7 @@ internal var CFR_MINIMUM_NUMBER_OPENED_TABS = 5
  * @param isPrivate Whether or not the session is private.
  * @param sessionId optional custom tab id used to identify the custom tab in which to show a CFR.
  * @param onShoppingCfrActionClicked Triggered when the user taps on the shopping CFR action.
- * @param onShoppingCfrDismiss Triggered when the user closes the shopping CFR using the "X" button.
+ * @param onShoppingCfrDisplayed Triggered when CFR is displayed to the user.
  * @param shoppingExperienceFeature Used to determine if [ShoppingExperienceFeature] is enabled.
  */
 @Suppress("LongParameterList")
@@ -81,7 +81,7 @@ class BrowserToolbarCFRPresenter(
     private val isPrivate: Boolean,
     private val sessionId: String? = null,
     private val onShoppingCfrActionClicked: () -> Unit,
-    private val onShoppingCfrDismiss: () -> Unit,
+    private val onShoppingCfrDisplayed: () -> Unit,
     private val shoppingExperienceFeature: ShoppingExperienceFeature = DefaultShoppingExperienceFeature(),
 ) {
     @VisibleForTesting
@@ -348,14 +348,7 @@ class BrowserToolbarCFRPresenter(
                 dismissOnBackPress = true,
                 dismissOnClickOutside = true,
             ),
-            onDismiss = {
-                when (it) {
-                    true -> {
-                        onShoppingCfrDismiss()
-                    }
-                    false -> {}
-                }
-            },
+            onDismiss = {},
             text = {
                 FirefoxTheme {
                     Text(
@@ -392,6 +385,7 @@ class BrowserToolbarCFRPresenter(
         ).run {
             Shopping.addressBarFeatureCalloutDisplayed.record()
             popup = this
+            onShoppingCfrDisplayed()
             show()
         }
     }

app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt

@@ -60,9 +60,9 @@ interface BrowserToolbarController {
     fun handleShoppingCfrActionClick()
 
     /**
-     * @see [BrowserToolbarInteractor.onShoppingCfrDismiss]
+     * @see [BrowserToolbarInteractor.onShoppingCfrDisplayed]
      */
-    fun handleShoppingCfrDismiss()
+    fun handleShoppingCfrDisplayed()
 
     /**
      * @see [BrowserToolbarInteractor.onTranslationsButtonClicked]
@@ -211,13 +211,12 @@ class DefaultBrowserToolbarController(
     }
 
     override fun handleShoppingCfrActionClick() {
-        updateShoppingCfrSettings()
         navController.navigate(
             BrowserFragmentDirections.actionBrowserFragmentToReviewQualityCheckDialogFragment(),
         )
     }
 
-    override fun handleShoppingCfrDismiss() {
+    override fun handleShoppingCfrDisplayed() {
         updateShoppingCfrSettings()
     }
 

app/src/main/java/org/mozilla/fenix/components/toolbar/ToolbarIntegration.kt

@@ -103,7 +103,7 @@ class DefaultToolbarIntegration(
         isPrivate = isPrivate,
         sessionId = sessionId,
         onShoppingCfrActionClicked = interactor::onShoppingCfrActionClicked,
-        onShoppingCfrDismiss = interactor::onShoppingCfrDismiss,
+        onShoppingCfrDisplayed = interactor::onShoppingCfrDisplayed,
     )
 
     init {

app/src/main/java/org/mozilla/fenix/components/toolbar/interactor/BrowserToolbarInteractor.kt

@@ -39,9 +39,9 @@ interface BrowserToolbarInteractor {
     fun onShoppingCfrActionClicked()
 
     /**
-     * Updates the settings for the shopping CFR. Called when the user dismisses the shopping cfr action.
+     * Updates the settings for the shopping CFR. Called when the user is shown the CFR.
      */
-    fun onShoppingCfrDismiss()
+    fun onShoppingCfrDisplayed()
 
     /**
      * Opens the translation bottom sheet. Called when the user interacts with the translation
@@ -107,8 +107,8 @@ class DefaultBrowserToolbarInteractor(
         browserToolbarController.handleShoppingCfrActionClick()
     }
 
-    override fun onShoppingCfrDismiss() {
-        browserToolbarController.handleShoppingCfrDismiss()
+    override fun onShoppingCfrDisplayed() {
+        browserToolbarController.handleShoppingCfrDisplayed()
     }
 
     override fun onTranslationsButtonClicked() {

app/src/test/java/org/mozilla/fenix/components/toolbar/BrowserToolbarCFRPresenterTest.kt

@@ -478,7 +478,7 @@ class BrowserToolbarCFRPresenterTest {
             sessionId = sessionId,
             isPrivate = isPrivate,
             onShoppingCfrActionClicked = {},
-            onShoppingCfrDismiss = {},
+            onShoppingCfrDisplayed = {},
             shoppingExperienceFeature = shoppingExperienceFeature,
         ),
     )

app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarControllerTest.kt

@@ -382,21 +382,18 @@ class DefaultBrowserToolbarControllerTest {
     }
 
     @Test
-    fun handleShoppingCfrActionClickAfterShowingThreeTimes() {
+    fun handleShoppingCfrActionClick() {
         val controller = createController()
-        every { activity.settings().reviewQualityCheckCfrDisplayTimeInMillis } returns System.currentTimeMillis()
-        every { activity.settings().reviewQualityCheckCFRClosedCounter } returns 3
 
         controller.handleShoppingCfrActionClick()
 
         verify {
-            activity.settings().shouldShowReviewQualityCheckCFR = false
             navController.navigate(BrowserFragmentDirections.actionBrowserFragmentToReviewQualityCheckDialogFragment())
         }
     }
 
     @Test
-    fun handleShoppingCfrDismissOnce() {
+    fun handleShoppingCfrDisplayedOnce() {
         val controller = createController()
         val mockSettings = mockk<Settings> {
             every { reviewQualityCheckCfrDisplayTimeInMillis } returns System.currentTimeMillis()
@@ -407,14 +404,14 @@ class DefaultBrowserToolbarControllerTest {
         }
         every { activity.settings() } returns mockSettings
 
-        controller.handleShoppingCfrDismiss()
+        controller.handleShoppingCfrDisplayed()
 
         verify(exactly = 0) { mockSettings.shouldShowReviewQualityCheckCFR = false }
         verify { mockSettings.reviewQualityCheckCfrDisplayTimeInMillis = any() }
     }
 
     @Test
-    fun handleShoppingCfrDismissTwice() {
+    fun handleShoppingCfrDisplayedTwice() {
         val controller = createController()
         val mockSettings = mockk<Settings> {
             every { reviewQualityCheckCfrDisplayTimeInMillis } returns System.currentTimeMillis()
@@ -425,14 +422,14 @@ class DefaultBrowserToolbarControllerTest {
         }
         every { activity.settings() } returns mockSettings
 
-        controller.handleShoppingCfrDismiss()
+        controller.handleShoppingCfrDisplayed()
 
         verify(exactly = 0) { mockSettings.shouldShowReviewQualityCheckCFR = false }
         verify { mockSettings.reviewQualityCheckCfrDisplayTimeInMillis = any() }
     }
 
     @Test
-    fun handleShoppingCfrDismissThreeTimes() {
+    fun handleShoppingCfrDisplayedThreeTimes() {
         val controller = createController()
         val mockSettings = mockk<Settings> {
             every { reviewQualityCheckCfrDisplayTimeInMillis } returns System.currentTimeMillis()
@@ -443,7 +440,7 @@ class DefaultBrowserToolbarControllerTest {
         }
         every { activity.settings() } returns mockSettings
 
-        controller.handleShoppingCfrDismiss()
+        controller.handleShoppingCfrDisplayed()
 
         verify { mockSettings.shouldShowReviewQualityCheckCFR = false }
         verify(exactly = 0) { mockSettings.reviewQualityCheckCfrDisplayTimeInMillis = any() }