commit c3b6de7ddf95801676c3d1900d2b699d5abd41c9
parent 40d6f18bfdc90b8dac1a5fdcca43fab4fc19b7c2
Author: Jonathan Almeida <jonalmeida942@gmail.com>
Date: Thu, 9 Oct 2025 02:06:51 +0000
Bug 1992918 - Use migrated nimbus data and eval JEXL with legacy criteria r=tcampbell,android-reviewers
We removed the LegacyReviewPromptTriggerTest after observing that we are
not meaningfully testing the conditions due to bypassing JEXL.
A follow-up bug will be filled to increase coverage of the criteria
system.
Differential Revision: https://phabricator.services.mozilla.com/D267898
Diffstat:
4 files changed, 34 insertions(+), 60 deletions(-)
diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/Components.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/Components.kt
@@ -290,8 +290,6 @@ class Components(private val context: Context) {
SetupChecklistTelemetryMiddleware(),
ReviewPromptMiddleware(
isReviewPromptFeatureEnabled = { settings.customReviewPromptFeatureEnabled },
- numberOfAppLaunches = { settings.numberOfAppLaunches },
- isDefaultBrowser = { settings.isDefaultBrowser },
isTelemetryEnabled = { settings.isTelemetryEnabled },
createJexlHelper = nimbus::createJexlHelper,
nimbusEventStore = nimbus.events,
diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/reviewprompt/ReviewPromptMiddleware.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/reviewprompt/ReviewPromptMiddleware.kt
@@ -26,29 +26,28 @@ private const val REVIEW_PROMPT_SHOWN_NIMBUS_EVENT_ID = "review_prompt_shown"
* [Middleware] evaluating the triggers to show a review prompt.
*
* @param isReviewPromptFeatureEnabled If returns false disables the entire feature and prompt will never be shown.
- * @param numberOfAppLaunches Returns number of app launches.
- * @param isDefaultBrowser Returns true if app is set as the default system browser.
* @param isTelemetryEnabled Returns true if the user allows sending technical and interaction telemetry.
* @param createJexlHelper Returns a helper for evaluating JEXL expressions.
* @param buildTriggerMainCriteria Builds a sequence of trigger's main criteria that all need to be true.
* @param buildTriggerSubCriteria Builds a sequence of trigger's sub-criteria.
+ * @param buildTriggerLegacyCriteria Builds a sequence of trigger's for the legacy behaviour.
* Only one of these needs to be true (in addition to the main criteria).
* @param nimbusEventStore [NimbusEventStore] used to record events evaluated in JEXL expressions.
*/
class ReviewPromptMiddleware(
private val isReviewPromptFeatureEnabled: () -> Boolean,
- private val numberOfAppLaunches: () -> Int,
- private val isDefaultBrowser: () -> Boolean,
private val isTelemetryEnabled: () -> Boolean,
private val createJexlHelper: () -> NimbusMessagingHelperInterface,
private val buildTriggerMainCriteria: (NimbusMessagingHelperInterface) -> Sequence<Boolean> =
- TiggerBuilder::mainCriteria,
+ TriggerBuilder::mainCriteria,
private val buildTriggerSubCriteria: (NimbusMessagingHelperInterface) -> Sequence<Boolean> =
- TiggerBuilder::subCriteria,
+ TriggerBuilder::subCriteria,
+ private val buildTriggerLegacyCriteria: (NimbusMessagingHelperInterface) -> Sequence<Boolean> =
+ TriggerBuilder::legacyCriteria,
private val nimbusEventStore: NimbusEventStore,
) : Middleware<AppState, AppAction> {
- private object TiggerBuilder {
+ private object TriggerBuilder {
fun mainCriteria(jexlHelper: NimbusMessagingHelperInterface): Sequence<Boolean> {
return sequence {
yield(hasNotBeenPromptedLastFourMonths(jexlHelper))
@@ -62,6 +61,16 @@ class ReviewPromptMiddleware(
yield(isDefaultBrowser(jexlHelper))
}
}
+
+ fun legacyCriteria(
+ jexlHelper: NimbusMessagingHelperInterface,
+ ): Sequence<Boolean> {
+ return sequence {
+ yield(hasNotBeenPromptedLastFourMonths(jexlHelper))
+ yield(hasBeenOpenedSeveralTimes(jexlHelper))
+ yield(isDefaultBrowser(jexlHelper))
+ }
+ }
}
override fun invoke(
@@ -91,17 +100,23 @@ class ReviewPromptMiddleware(
return
}
- if (!isReviewPromptFeatureEnabled()) {
- // Keep the simpler legacy behavior.
- if (legacyReviewPromptTrigger(numberOfAppLaunches, isDefaultBrowser)) {
- context.dispatch(ShowPlayStorePrompt)
- } else {
- context.dispatch(DoNotShowReviewPrompt)
+ createJexlHelper().use { jexlHelper ->
+ // Keep the legacy criteria around, but use the nimbus data and jexl to trigger.
+ // Leaving the original if-else logic and early return for readability.
+ if (!isReviewPromptFeatureEnabled()) {
+ // We build the legacy criteria using the same triggers as before.
+ val legacyCriteria = buildTriggerLegacyCriteria(
+ jexlHelper,
+ ).all { it }
+ if (legacyCriteria) {
+ context.dispatch(ShowPlayStorePrompt)
+ } else {
+ context.dispatch(DoNotShowReviewPrompt)
+ }
+ return@use
}
- return
- }
- createJexlHelper().use { jexlHelper ->
+ // Otherwise, we use the new criteria.
val allMainCriteriaSatisfied = buildTriggerMainCriteria(jexlHelper).all { it }
if (!allMainCriteriaSatisfied) {
context.dispatch(DoNotShowReviewPrompt)
@@ -170,16 +185,13 @@ internal fun usedAppOnAtLeastFourOfLastSevenDays(
return jexlHelper.evalJexlSafe("'app_opened'|eventCountNonZero('Days', 7) >= 4")
}
-private const val NUMBER_OF_LAUNCHES_REQUIRED = 5
-
/**
* Matches logic from ReviewPromptController.shouldShowPrompt, which has been deleted.
* Kept so we can fall back to it in case the custom review prompt is disabled with a kill-switch.
*/
@VisibleForTesting
-internal fun legacyReviewPromptTrigger(
- numberOfAppLaunches: () -> Int,
- isDefaultBrowser: () -> Boolean,
+internal fun hasBeenOpenedSeveralTimes(
+ jexlHelper: NimbusMessagingHelperInterface,
): Boolean {
- return isDefaultBrowser() && numberOfAppLaunches() >= NUMBER_OF_LAUNCHES_REQUIRED
+ return jexlHelper.evalJexlSafe("number_of_app_launches >= 5")
}
diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/reviewprompt/LegacyReviewPromptTriggerTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/reviewprompt/LegacyReviewPromptTriggerTest.kt
@@ -1,34 +0,0 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-package org.mozilla.fenix.reviewprompt
-
-import org.junit.Assert.assertFalse
-import org.junit.Assert.assertTrue
-import org.junit.Test
-
-class LegacyReviewPromptTriggerTest {
-
- private var numberOfAppLaunches = 5
- private var isDefaultBrowser = true
-
- @Test
- fun `WHEN app is the default browser AND was launched at least 5 times THEN legacy trigger is satisfied`() {
- assertTrue(legacyReviewPromptTrigger({ numberOfAppLaunches }, { isDefaultBrowser }))
- }
-
- @Test
- fun `WHEN app isn't the default browser THEN legacy trigger is not satisfied`() {
- isDefaultBrowser = false
-
- assertFalse(legacyReviewPromptTrigger({ numberOfAppLaunches }, { isDefaultBrowser }))
- }
-
- @Test
- fun `WHEN app was launched less than 5 times THEN legacy trigger is not satisfied`() {
- numberOfAppLaunches = 4
-
- assertFalse(legacyReviewPromptTrigger({ numberOfAppLaunches }, { isDefaultBrowser }))
- }
-}
diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/reviewprompt/ReviewPromptMiddlewareTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/reviewprompt/ReviewPromptMiddlewareTest.kt
@@ -28,8 +28,6 @@ class ReviewPromptMiddlewareTest {
middlewares = listOf(
ReviewPromptMiddleware(
isReviewPromptFeatureEnabled = { true },
- numberOfAppLaunches = { 5 },
- isDefaultBrowser = { true },
isTelemetryEnabled = { isTelemetryEnabled },
createJexlHelper = {
object : NimbusMessagingHelperInterface {
