2
0
mirror of https://github.com/fork-maintainers/iceraven-browser synced 2024-11-15 18:12:54 +00:00

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).
This commit is contained in:
DreVla 2023-10-26 17:26:39 +03:00 committed by mergify[bot]
parent 6a38fc03aa
commit 6ab8aadd6b
6 changed files with 85 additions and 17 deletions

View File

@ -152,23 +152,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

View File

@ -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()

View File

@ -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
@ -1717,6 +1718,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.
*/

View File

@ -374,4 +374,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>

View File

@ -358,7 +358,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(
@ -373,7 +373,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,
)
@ -385,7 +385,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(
@ -401,7 +401,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,
)

View File

@ -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()
assertFalse(activity.settings().shouldShowReviewQualityCheckCFR)
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()
verify { mockSettings.shouldShowReviewQualityCheckCFR = false }
verify(exactly = 0) { mockSettings.reviewQualityCheckCfrDisplayTimeInMillis = any() }
}
fun handleTranslationsButtonClick() {