Bug 1861173 - Change timing of the Review Checker onboarding CFR

The current onboarding CFR is set to show the first time someone
lands on a product detail page. If the user dismisses the CFR and
does not opt-in, we show it on the next product detail page after at
least 24 hours have elapsed.
This patch changes this to showing the CFR up to 3X (instead of 2)
with at least 12 hrs in-between (instead of 24).

(cherry picked from commit dddeecc082a9be8dcc53caddc5bec79753fff9bb)

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

@@ -150,23 +150,28 @@ class BrowserToolbarCFRPresenter(
         }
     }
 
+    /**
+     * Decides which Shopping CFR needs to be displayed depending on
+     * participation of user in Review Checker and time elapsed
+     * from last CFR display.
+     */
     private fun whichShoppingCFR(): ToolbarCFR {
         fun Long.isInitialized(): Boolean = this != 0L
-        fun Long.afterOneDay(): Boolean = this.isInitialized() &&
-            System.currentTimeMillis() - this > Settings.ONE_DAY_MS
+        fun Long.afterTwelveHours(): Boolean = this.isInitialized() &&
+            System.currentTimeMillis() - this > Settings.TWELVE_HOURS_MS
 
         val optInTime = settings.reviewQualityCheckOptInTimeInMillis
         val firstCfrShownTime = settings.reviewQualityCheckCfrDisplayTimeInMillis
 
         return when {
-            // First CFR should be displayed on first product page visit
+            // Try Review Checker CFR should be displayed on first product page visit
             !firstCfrShownTime.isInitialized() ->
                 ToolbarCFR.SHOPPING
-            // First CFR should be displayed again 24 hours later only for not opted in users
-            !optInTime.isInitialized() && firstCfrShownTime.afterOneDay() ->
+            // Try Review Checker CFR should be displayed again 12 hours later only for not opted in users
+            !optInTime.isInitialized() && firstCfrShownTime.afterTwelveHours() ->
                 ToolbarCFR.SHOPPING
-            // Second CFR should be shown 24 hours after opt in
-            optInTime.afterOneDay() ->
+            // Already Opted In CFR should be shown 12 hours after opt in
+            optInTime.afterTwelveHours() ->
                 ToolbarCFR.SHOPPING_OPTED_IN
             else -> {
                 ToolbarCFR.NONE

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

@@ -70,6 +70,8 @@ interface BrowserToolbarController {
     fun handleTranslationsButtonClick()
 }
 
+private const val MAX_DISPLAY_NUMBER_SHOPPING_CFR = 3
+
 @Suppress("LongParameterList")
 class DefaultBrowserToolbarController(
     private val store: BrowserStore,
@@ -229,9 +231,16 @@ class DefaultBrowserToolbarController(
         internal const val TELEMETRY_BROWSER_IDENTIFIER = "browserMenu"
     }
 
+    /**
+     * Stop showing the CFR after being displayed three times with
+     * with at least 12 hrs in-between.
+     * As described in: https://bugzilla.mozilla.org/show_bug.cgi?id=1861173#c0
+     */
     private fun updateShoppingCfrSettings() = with(activity.settings()) {
-        if (reviewQualityCheckCfrDisplayTimeInMillis != 0L) {
-            // We want to show the first CFR a second time if the user doesn't opt in the feature
+        reviewQualityCheckCFRClosedCounter.inc()
+        if (reviewQualityCheckCfrDisplayTimeInMillis != 0L &&
+            reviewQualityCheckCFRClosedCounter >= MAX_DISPLAY_NUMBER_SHOPPING_CFR
+        ) {
             shouldShowReviewQualityCheckCFR = false
         } else {
             reviewQualityCheckCfrDisplayTimeInMillis = System.currentTimeMillis()

app/src/main/java/org/mozilla/fenix/utils/Settings.kt

@@ -80,6 +80,7 @@ class Settings(private val appContext: Context) : PreferencesHolder {
         const val FOUR_HOURS_MS = 60 * 60 * 4 * 1000L
         const val ONE_MINUTE_MS = 60 * 1000L
         const val ONE_HOUR_MS = 60 * ONE_MINUTE_MS
+        const val TWELVE_HOURS_MS = 60 * 60 * 12 * 1000L
         const val ONE_DAY_MS = 60 * 60 * 24 * 1000L
         const val TWO_DAYS_MS = 2 * ONE_DAY_MS
         const val THREE_DAYS_MS = 3 * ONE_DAY_MS
@@ -1749,6 +1750,15 @@ class Settings(private val appContext: Context) : PreferencesHolder {
         default = 0L,
     )
 
+    /**
+     * Counts how many times any Review Checker CFR was closed after being presented to the user.
+     * When closed 3 times, the CFR will not be shown anymore.
+     */
+    var reviewQualityCheckCFRClosedCounter by intPreference(
+        appContext.getPreferenceKey(R.string.pref_key_review_quality_cfr_shown_counter),
+        default = 0,
+    )
+
     /**
      * Get the current mode for how https-only is enabled.
      */

app/src/main/res/values/preference_keys.xml

@@ -378,4 +378,5 @@
     <string name="pref_key_should_show_review_quality_cfr">pref_key_should_show_review_quality_cfr</string>
     <string name="pref_key_should_show_review_quality_opt_in_time">pref_key_should_show_review_quality_opt_in_time</string>
     <string name="pref_key_should_show_review_quality_cfr_displayed_time">pref_key_should_show_review_quality_cfr_displayed_time</string>
+    <string name="pref_key_review_quality_cfr_shown_counter">pref_key_review_quality_cfr_shown_counter</string>
 </resources>

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

@@ -359,7 +359,7 @@ class BrowserToolbarCFRPresenterTest {
     }
 
     @Test
-    fun `GIVEN the first CFR was displayed less than 24h ago AND the user did not opt in to the shopping feature WHEN opening a loaded product page THEN no shopping CFR is shown`() {
+    fun `GIVEN the first CFR was displayed less than 12h ago AND the user did not opt in to the shopping feature WHEN opening a loaded product page THEN no shopping CFR is shown`() {
         val tab1 = createTab(url = "", id = "tab1")
         val browserStore = BrowserStore(
             initialState = BrowserState(
@@ -374,7 +374,7 @@ class BrowserToolbarCFRPresenterTest {
                 every { shouldShowReviewQualityCheckCFR } returns true
                 every { shouldShowEraseActionCFR } returns false
                 every { reviewQualityCheckOptInTimeInMillis } returns 0L
-                every { reviewQualityCheckCfrDisplayTimeInMillis } returns System.currentTimeMillis() - 19 * 60 * 60 * 1000L
+                every { reviewQualityCheckCfrDisplayTimeInMillis } returns System.currentTimeMillis() - (11 * 60 * 60 * 1000L)
             },
             browserStore = browserStore,
         )
@@ -386,7 +386,7 @@ class BrowserToolbarCFRPresenterTest {
     }
 
     @Test
-    fun `GIVEN the first CFR was displayed 24h ago AND the user did not opt in to the shopping feature WHEN opening a loaded product page THEN the first shopping CFR is shown`() {
+    fun `GIVEN the first CFR was displayed 12h ago AND the user did not opt in to the shopping feature WHEN opening a loaded product page THEN the first shopping CFR is shown`() {
         val tab1 = createTab(url = "", id = "tab1")
         val tab2 = createTab(url = "", id = "tab2")
         val browserStore = BrowserStore(
@@ -402,7 +402,7 @@ class BrowserToolbarCFRPresenterTest {
                 every { shouldShowReviewQualityCheckCFR } returns true
                 every { shouldShowEraseActionCFR } returns false
                 every { reviewQualityCheckOptInTimeInMillis } returns 0L
-                every { reviewQualityCheckCfrDisplayTimeInMillis } returns System.currentTimeMillis() - Settings.ONE_DAY_MS
+                every { reviewQualityCheckCfrDisplayTimeInMillis } returns System.currentTimeMillis() - Settings.TWELVE_HOURS_MS
             },
             browserStore = browserStore,
         )

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

@@ -55,6 +55,7 @@ import org.mozilla.fenix.ext.settings
 import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
 import org.mozilla.fenix.home.HomeFragment
 import org.mozilla.fenix.home.HomeScreenViewModel
+import org.mozilla.fenix.utils.Settings
 
 @RunWith(FenixRobolectricTestRunner::class)
 class DefaultBrowserToolbarControllerTest {
@@ -381,9 +382,10 @@ class DefaultBrowserToolbarControllerTest {
     }
 
     @Test
-    fun handleShoppingCfrActionClick() {
+    fun handleShoppingCfrActionClickAfterShowingThreeTimes() {
         val controller = createController()
         every { activity.settings().reviewQualityCheckCfrDisplayTimeInMillis } returns System.currentTimeMillis()
+        every { activity.settings().reviewQualityCheckCFRClosedCounter } returns 3
 
         controller.handleShoppingCfrActionClick()
 
@@ -394,13 +396,54 @@ class DefaultBrowserToolbarControllerTest {
     }
 
     @Test
-    fun handleShoppingCfrDismiss() {
+    fun handleShoppingCfrDismissOnce() {
         val controller = createController()
-        every { activity.settings().reviewQualityCheckCfrDisplayTimeInMillis } returns System.currentTimeMillis()
+        val mockSettings = mockk<Settings> {
+            every { reviewQualityCheckCfrDisplayTimeInMillis } returns System.currentTimeMillis()
+            every { reviewQualityCheckCfrDisplayTimeInMillis = any() } just Runs
+            every { reviewQualityCheckCFRClosedCounter } returns 1
+            every { shouldShowReviewQualityCheckCFR } returns true
+        }
+        every { activity.settings() } returns mockSettings
+
+        controller.handleShoppingCfrDismiss()
+
+        verify(exactly = 0) { mockSettings.shouldShowReviewQualityCheckCFR = false }
+        verify { mockSettings.reviewQualityCheckCfrDisplayTimeInMillis = any() }
+    }
+
+    @Test
+    fun handleShoppingCfrDismissTwice() {
+        val controller = createController()
+        val mockSettings = mockk<Settings> {
+            every { reviewQualityCheckCfrDisplayTimeInMillis } returns System.currentTimeMillis()
+            every { reviewQualityCheckCfrDisplayTimeInMillis = any() } just Runs
+            every { reviewQualityCheckCFRClosedCounter } returns 2
+            every { shouldShowReviewQualityCheckCFR } returns true
+        }
+        every { activity.settings() } returns mockSettings
+
+        controller.handleShoppingCfrDismiss()
+
+        verify(exactly = 0) { mockSettings.shouldShowReviewQualityCheckCFR = false }
+        verify { mockSettings.reviewQualityCheckCfrDisplayTimeInMillis = any() }
+    }
+
+    @Test
+    fun handleShoppingCfrDismissThreeTimes() {
+        val controller = createController()
+        val mockSettings = mockk<Settings> {
+            every { reviewQualityCheckCfrDisplayTimeInMillis } returns System.currentTimeMillis()
+            every { reviewQualityCheckCFRClosedCounter } returns 3
+            every { shouldShowReviewQualityCheckCFR } returns true
+            every { shouldShowReviewQualityCheckCFR = any() } just Runs
+        }
+        every { activity.settings() } returns mockSettings
 
         controller.handleShoppingCfrDismiss()
 
-        assertFalse(activity.settings().shouldShowReviewQualityCheckCFR)
+        verify { mockSettings.shouldShowReviewQualityCheckCFR = false }
+        verify(exactly = 0) { mockSettings.reviewQualityCheckCfrDisplayTimeInMillis = any() }
     }
 
     fun handleTranslationsButtonClick() {