commit 5cd562920cc58b7952797bc281c59d9cc0f6437f parent 19305ddf53eb09529dd2873d609a946a656d852f Author: twhite <towhite@mozilla.com> Date: Wed, 1 Oct 2025 09:59:44 +0000 Bug 1984491 - Allow Nimbus to define the maximum number of times to display the Terms of Use prompt. r=android-reviewers,rebecatudor273 Differential Revision: https://phabricator.services.mozilla.com/D266804 Diffstat:
9 files changed, 96 insertions(+), 10 deletions(-)
diff --git a/mobile/android/fenix/app/nimbus.fml.yaml b/mobile/android/fenix/app/nimbus.fml.yaml @@ -655,6 +655,10 @@ features: description: When the feature is enabled then the prompt can show for users. type: Boolean default: false + max-display-count: + description: The maximum number of times to show the prompt. + type: Int + default: 2 tab-strip: description: Enables tab strip. diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/termsofuse/TermsOfUseManager.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/termsofuse/TermsOfUseManager.kt @@ -4,6 +4,7 @@ package org.mozilla.fenix.termsofuse +import androidx.annotation.VisibleForTesting import org.mozilla.fenix.utils.Settings import org.mozilla.fenix.utils.Settings.Companion.FIVE_DAYS_MS import org.mozilla.fenix.utils.Settings.Companion.THIRTY_SECONDS_MS @@ -34,6 +35,7 @@ class TermsOfUseManager(private val settings: Settings) { * This function returns `true` if: * - The user has not accepted the Terms of Use. * - The terms of use prompt feature flag is enabled. + * - The prompt has not already been displayed the maximum number of times. * - The user has not postponed accepting the Terms of Use or it's been at least 5 days since they did. * - This is the first time checking to see if we should show the prompt since starting the app * OR the [ignoreFirstCheckSinceStartingApp] flag is true (we should ignore this when checking from homepage). @@ -44,12 +46,14 @@ class TermsOfUseManager(private val settings: Settings) { * @return `true` if the Terms of Use bottom sheet should be shown; otherwise, `false`. */ @Suppress("ReturnCount") - private fun shouldShowTermsOfUsePrompt( + @VisibleForTesting + internal fun shouldShowTermsOfUsePrompt( ignoreFirstCheckSinceStartingApp: Boolean = false, currentTimeInMillis: Long = System.currentTimeMillis(), ): Boolean { if (settings.hasAcceptedTermsOfService) return false if (!settings.isTermsOfUsePromptEnabled) return false + if (settings.termsOfUsePromptDisplayedCount >= settings.termsOfUseMaxDisplayCount) return false val isFirstCheck = isFirstCheckSinceStartingApp isFirstCheckSinceStartingApp = false diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/termsofuse/store/TermsOfUsePromptPreferencesMiddleware.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/termsofuse/store/TermsOfUsePromptPreferencesMiddleware.kt @@ -40,8 +40,8 @@ class TermsOfUsePromptPreferencesMiddleware( is TermsOfUsePromptAction.OnPromptDismissed -> repository.updateLastTermsOfUsePromptTimeInMillis() - // no-ops - is TermsOfUsePromptAction.OnImpression -> {} + is TermsOfUsePromptAction.OnImpression -> + repository.incrementTermsOfUsePromptDisplayedCount() } next(action) diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/termsofuse/store/TermsOfUsePromptRepository.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/termsofuse/store/TermsOfUsePromptRepository.kt @@ -36,6 +36,11 @@ interface TermsOfUsePromptRepository { * Updates the 'has clicked the term of use prompt "remind me later" action' preference to true. */ fun updateHasClickedTermOfUsePromptRemindMeLaterPreference() + + /** + * Increments the number of times the Terms of Use prompt has been displayed by 1. + */ + fun incrementTermsOfUsePromptDisplayedCount() } /** @@ -65,4 +70,8 @@ class DefaultTermsOfUsePromptRepository( override fun updateHasClickedTermOfUsePromptRemindMeLaterPreference() { settings.hasClickedTermOfUsePromptRemindMeLater = true } + + override fun incrementTermsOfUsePromptDisplayedCount() { + settings.termsOfUsePromptDisplayedCount++ + } } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/utils/Settings.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/utils/Settings.kt @@ -604,6 +604,19 @@ class Settings(private val appContext: Context) : PreferencesHolder { ) /** + * The maximum number of times the Terms of Use prompt should be displayed. + */ + var termsOfUseMaxDisplayCount = FxNimbus.features.termsOfUsePrompt.value().maxDisplayCount + + /** + * The total number of times the Terms of Use prompt has been displayed. + */ + var termsOfUsePromptDisplayedCount by intPreference( + key = appContext.getPreferenceKey(R.string.pref_key_terms_prompt_displayed_count), + default = 0, + ) + + /** * Timestamp in milliseconds when the terms of use prompt was last shown to the user. * A value of 0L indicates that the prompt has never been shown. */ diff --git a/mobile/android/fenix/app/src/main/res/values/preference_keys.xml b/mobile/android/fenix/app/src/main/res/values/preference_keys.xml @@ -84,6 +84,7 @@ <string name="pref_key_translations_offer" translatable="false">pref_key_translations_offer</string> <string name="pref_key_terms_accepted" translatable="false">pref_key_terms_accepted</string> <string name="pref_key_terms_prompt_enabled" translatable="false">pref_key_terms_prompt_enabled</string> + <string name="pref_key_terms_prompt_displayed_count" translatable="false">pref_key_terms_prompt_displayed_count</string> <string name="pref_key_terms_last_prompt_time" translatable="false">pref_key_terms_last_prompt_time</string> <string name="pref_key_terms_postponed" translatable="false">pref_key_terms_postponed</string> <string name="pref_key_debug_terms_trigger_time" translatable="false">pref_key_debug_terms_trigger_time</string> diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/termsofuse/TermsOfUseManagerTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/termsofuse/TermsOfUseManagerTest.kt @@ -141,4 +141,42 @@ class TermsOfUseManagerTest { assertTrue(termsOfUseManager.shouldShowTermsOfUsePromptOnHomepage()) } + + @Test + fun `GIVEN other conditions satisfied WHEN prompt has not been displayed THEN shouldShowTermsOfUsePrompt returns true`() { + settings.hasAcceptedTermsOfService = false + settings.isTermsOfUsePromptEnabled = true + settings.hasPostponedAcceptingTermsOfUse = false + // Prompt display count configuration. + settings.termsOfUsePromptDisplayedCount = 0 + settings.termsOfUseMaxDisplayCount = 2 + + termsOfUseManager.onStart() + + assertTrue(termsOfUseManager.shouldShowTermsOfUsePrompt()) + } + + @Test + fun `GIVEN other conditions satisfied WHEN prompt has been displayed the maximum number of times THEN shouldShowTermsOfUsePrompt returns false`() { + settings.hasAcceptedTermsOfService = false + settings.isTermsOfUsePromptEnabled = true + settings.hasPostponedAcceptingTermsOfUse = false + // Prompt display count configuration. + settings.termsOfUsePromptDisplayedCount = 2 + settings.termsOfUseMaxDisplayCount = 2 + + assertFalse(termsOfUseManager.shouldShowTermsOfUsePrompt()) + } + + @Test + fun `GIVEN other conditions satisfied WHEN prompt has been displayed more than the maximum number of times THEN shouldShowTermsOfUsePrompt returns false`() { + settings.hasAcceptedTermsOfService = false + settings.isTermsOfUsePromptEnabled = true + settings.hasPostponedAcceptingTermsOfUse = false + // Prompt display count configuration. + settings.termsOfUsePromptDisplayedCount = 3 + settings.termsOfUseMaxDisplayCount = 2 + + assertFalse(termsOfUseManager.shouldShowTermsOfUsePrompt()) + } } diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/termsofuse/store/TermsOfUsePromptPreferencesMiddlewareTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/termsofuse/store/TermsOfUsePromptPreferencesMiddlewareTest.kt @@ -5,6 +5,7 @@ package org.mozilla.fenix.termsofuse.store import io.mockk.mockk +import junit.framework.TestCase.assertEquals import junit.framework.TestCase.assertFalse import junit.framework.TestCase.assertTrue import mozilla.components.lib.state.MiddlewareContext @@ -50,6 +51,7 @@ class TermsOfUsePromptPreferencesMiddlewareTest { assertFalse(settings.hasClickedTermOfUsePromptLink) assertFalse(settings.hasClickedTermOfUsePromptRemindMeLater) assertFalse(settings.lastTermsOfUsePromptTimeInMillis > 0) + assertEquals(0, settings.termsOfUsePromptDisplayedCount) } @Test @@ -67,6 +69,7 @@ class TermsOfUsePromptPreferencesMiddlewareTest { assertFalse(settings.hasClickedTermOfUsePromptLink) assertTrue(settings.hasClickedTermOfUsePromptRemindMeLater) assertFalse(settings.lastTermsOfUsePromptTimeInMillis > 0) + assertEquals(0, settings.termsOfUsePromptDisplayedCount) } @Test @@ -84,6 +87,7 @@ class TermsOfUsePromptPreferencesMiddlewareTest { assertFalse(settings.hasClickedTermOfUsePromptLink) assertFalse(settings.hasClickedTermOfUsePromptRemindMeLater) assertFalse(settings.lastTermsOfUsePromptTimeInMillis > 0) + assertEquals(0, settings.termsOfUsePromptDisplayedCount) } @Test @@ -101,6 +105,7 @@ class TermsOfUsePromptPreferencesMiddlewareTest { assertFalse(settings.hasClickedTermOfUsePromptLink) assertFalse(settings.hasClickedTermOfUsePromptRemindMeLater) assertTrue(settings.lastTermsOfUsePromptTimeInMillis > 0) + assertEquals(0, settings.termsOfUsePromptDisplayedCount) } @Test @@ -118,6 +123,7 @@ class TermsOfUsePromptPreferencesMiddlewareTest { assertTrue(settings.hasClickedTermOfUsePromptLink) assertFalse(settings.hasClickedTermOfUsePromptRemindMeLater) assertFalse(settings.lastTermsOfUsePromptTimeInMillis > 0) + assertEquals(0, settings.termsOfUsePromptDisplayedCount) } @Test @@ -135,6 +141,7 @@ class TermsOfUsePromptPreferencesMiddlewareTest { assertTrue(settings.hasClickedTermOfUsePromptLink) assertFalse(settings.hasClickedTermOfUsePromptRemindMeLater) assertFalse(settings.lastTermsOfUsePromptTimeInMillis > 0) + assertEquals(0, settings.termsOfUsePromptDisplayedCount) } @Test @@ -152,23 +159,25 @@ class TermsOfUsePromptPreferencesMiddlewareTest { assertTrue(settings.hasClickedTermOfUsePromptLink) assertFalse(settings.hasClickedTermOfUsePromptRemindMeLater) assertFalse(settings.lastTermsOfUsePromptTimeInMillis > 0) + assertEquals(0, settings.termsOfUsePromptDisplayedCount) } @Test - fun `WHEN action is noop THEN the repository preferences are not updated`() { - assertNoOpAction(TermsOfUsePromptAction.OnImpression(Surface.HOMEPAGE_NEW_TAB)) - } - - private fun assertNoOpAction(action: TermsOfUsePromptAction) { + fun `WHEN the OnImpression action is received THEN the expected preference is updated`() { assertAllPrefsDefault() middleware.invoke( context = context, next = {}, - action = action, + action = TermsOfUsePromptAction.OnImpression(Surface.HOMEPAGE_NEW_TAB), ) - assertAllPrefsDefault() + assertFalse(settings.hasAcceptedTermsOfService) + assertFalse(settings.hasPostponedAcceptingTermsOfUse) + assertFalse(settings.hasClickedTermOfUsePromptLink) + assertFalse(settings.hasClickedTermOfUsePromptRemindMeLater) + assertFalse(settings.lastTermsOfUsePromptTimeInMillis > 0) + assertEquals(1, settings.termsOfUsePromptDisplayedCount) } private fun assertAllPrefsDefault() { @@ -177,5 +186,6 @@ class TermsOfUsePromptPreferencesMiddlewareTest { assertFalse(settings.hasClickedTermOfUsePromptLink) assertFalse(settings.hasClickedTermOfUsePromptRemindMeLater) assertFalse(settings.lastTermsOfUsePromptTimeInMillis > 0) + assertEquals(0, settings.termsOfUsePromptDisplayedCount) } } diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/termsofuse/store/TermsOfUsePromptRepositoryTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/termsofuse/store/TermsOfUsePromptRepositoryTest.kt @@ -47,4 +47,11 @@ class TermsOfUsePromptRepositoryTest { repository.updateLastTermsOfUsePromptTimeInMillis() assertTrue(settings.lastTermsOfUsePromptTimeInMillis > 0) } + + @Test + fun `WHEN incrementTermsOfUsePromptDisplayedCount is called THEN the preference is updated`() { + assertEquals(0, settings.termsOfUsePromptDisplayedCount) + repository.incrementTermsOfUsePromptDisplayedCount() + assertEquals(1, settings.termsOfUsePromptDisplayedCount) + } }