commit 63d556f30e3aa8cc551b1c24aba31cec1f9f1116
parent b7e0462f57d5f90d28295ac43278f6c021d3522c
Author: Marcin Koziński <mkozinski@mozilla.com>
Date: Fri, 17 Oct 2025 11:56:39 +0000
Bug 1986398 - Refactor recordReviewPromptEvent to extract determining if prompt was displayed r=android-reviewers,twhite
Differential Revision: https://phabricator.services.mozilla.com/D263363
Diffstat:
2 files changed, 99 insertions(+), 32 deletions(-)
diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/PlayStoreReviewPromptController.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/PlayStoreReviewPromptController.kt
@@ -18,6 +18,10 @@ import mozilla.components.support.base.log.logger.Logger
import org.mozilla.fenix.BrowserDirection
import org.mozilla.fenix.GleanMetrics.ReviewPrompt
import org.mozilla.fenix.HomeActivity
+import org.mozilla.fenix.components.ReviewPromptAttemptResult.Displayed
+import org.mozilla.fenix.components.ReviewPromptAttemptResult.Error
+import org.mozilla.fenix.components.ReviewPromptAttemptResult.NotDisplayed
+import org.mozilla.fenix.components.ReviewPromptAttemptResult.Unknown
import org.mozilla.fenix.settings.SupportUtils
import java.text.SimpleDateFormat
import java.util.Date
@@ -44,12 +48,12 @@ class PlayStoreReviewPromptController(
val reviewInfoFlow = withContext(Dispatchers.IO) { manager.requestReviewFlow() }
reviewInfoFlow.addOnCompleteListener(activity) {
- val resultString = if (it.isSuccessful) {
+ val result = if (it.isSuccessful) {
logger.info("Review flow launched.")
// Launch the in-app flow.
manager.launchReviewFlow(activity, it.result)
- it.result.toString()
+ ReviewPromptAttemptResult.from(it.result.toString())
} else {
// Launch the Play store flow.
val reviewErrorCode =
@@ -58,11 +62,11 @@ class PlayStoreReviewPromptController(
onError()
- "reviewErrorCode=$reviewErrorCode"
+ Error
}
recordReviewPromptEvent(
- reviewInfoAsString = resultString,
+ promptAttemptResult = result,
numberOfAppLaunches = numberOfAppLaunches(),
now = Date(),
)
@@ -105,35 +109,72 @@ class PlayStoreReviewPromptController(
}
/**
+ * Result of an attempt to show a Play Store In-App Review Prompt.
+ */
+@VisibleForTesting
+enum class ReviewPromptAttemptResult {
+ /**
+ * Attempted completed without error, but the API didn't allow to display the prompt.
+ */
+ NotDisplayed,
+
+ /**
+ * Prompt has been shown.
+ */
+ Displayed,
+
+ /**
+ * There was an error, for example this is a device without Play Store.
+ */
+ Error,
+
+ /**
+ * Attempt completed without error, but we weren't able to determine if prompt has been shown or not.
+ */
+ Unknown,
+ ;
+
+ companion object {
+ /**
+ * The docs for [ReviewManager.launchReviewFlow] state 'In some circumstances the review
+ * flow will not be shown to the user, e.g. they have already seen it recently, so do not assume that
+ * calling this method will always display the review dialog.'
+ * However, investigation has shown that a [ReviewInfo] instance with the flag:
+ * - 'isNoOp=true' indicates that the prompt has NOT been displayed.
+ * - 'isNoOp=false' indicates that a prompt has been displayed.
+ * [ReviewManager.launchReviewFlow] will modify the ReviewInfo instance which can be used to determine
+ * which of these flags is present.
+ *
+ * The internals of ReviewInfo cannot be accessed directly or cast nicely, so let's simply use
+ * the object as a string.
+ */
+ fun from(reviewInfoAsString: String): ReviewPromptAttemptResult {
+ return when {
+ reviewInfoAsString.contains("isNoOp=true") -> NotDisplayed
+ reviewInfoAsString.contains("isNoOp=false") -> Displayed
+ // ReviewInfo is susceptible to changes outside of our control hence the catch-all 'else' statement.
+ else -> Unknown
+ }
+ }
+ }
+}
+
+/**
* Records a [ReviewPrompt] with the required data.
- *
- * **Note:** The docs for [ReviewManager.launchReviewFlow] state 'In some circumstances the review
- * flow will not be shown to the user, e.g. they have already seen it recently, so do not assume that
- * calling this method will always display the review dialog.'
- * However, investigation has shown that a [ReviewInfo] instance with the flag:
- * - 'isNoOp=true' indicates that the prompt has NOT been displayed.
- * - 'isNoOp=false' indicates that a prompt has been displayed.
- * [ReviewManager.launchReviewFlow] will modify the ReviewInfo instance which can be used to determine
- * which of these flags is present.
*/
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
fun recordReviewPromptEvent(
- reviewInfoAsString: String,
+ promptAttemptResult: ReviewPromptAttemptResult,
numberOfAppLaunches: Int,
now: Date,
) {
val formattedLocalDatetime =
SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss", Locale.getDefault()).format(now)
- // The internals of ReviewInfo cannot be accessed directly or cast nicely, so lets simply use
- // the object as a string.
- // ReviewInfo is susceptible to changes outside of our control hence the catch-all 'else' statement.
- val promptWasDisplayed = if (reviewInfoAsString.contains("isNoOp=true")) {
- "false"
- } else if (reviewInfoAsString.contains("isNoOp=false")) {
- "true"
- } else {
- "error"
+ val promptWasDisplayed = when (promptAttemptResult) {
+ NotDisplayed -> "false"
+ Displayed -> "true"
+ Error, Unknown -> "error"
}
ReviewPrompt.promptAttempt.record(
diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/PlayStoreReviewPromptControllerTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/PlayStoreReviewPromptControllerTest.kt
@@ -27,6 +27,9 @@ import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.mozilla.fenix.GleanMetrics.ReviewPrompt
+import org.mozilla.fenix.components.ReviewPromptAttemptResult.Displayed
+import org.mozilla.fenix.components.ReviewPromptAttemptResult.NotDisplayed
+import org.mozilla.fenix.components.ReviewPromptAttemptResult.Unknown
import org.mozilla.fenix.helpers.FenixGleanTestRule
import org.robolectric.RobolectricTestRunner
import java.text.SimpleDateFormat
@@ -104,35 +107,58 @@ class PlayStoreReviewPromptControllerTest {
}
@Test
+ fun `WHEN review info contains 'isNoOp=false' THEN prompt was displayed`() {
+ val displayState = ReviewPromptAttemptResult.from(createReviewInfoString("isNoOp=false"))
+
+ assertEquals(Displayed, displayState)
+ }
+
+ @Test
+ fun `WHEN review info contains 'isNoOp=true' THEN prompt wasn't displayed`() {
+ val displayState = ReviewPromptAttemptResult.from(createReviewInfoString("isNoOp=true"))
+
+ assertEquals(NotDisplayed, displayState)
+ }
+
+ @Test
+ fun `WHEN review info doesn't contain 'isNoOp' THEN prompt display state is unknown`() {
+ val displayState = ReviewPromptAttemptResult.from(createReviewInfoString())
+
+ assertEquals(Unknown, displayState)
+ }
+
+ private fun createReviewInfoString(optionalArg: String? = ""): String {
+ return "ReviewInfo{pendingIntent=PendingIntent{5b613b1: android.os.BinderProxy@46c8096}, $optionalArg}"
+ }
+
+ @Test
fun reviewPromptWasDisplayed() {
- testRecordReviewPromptEventRecordsTheExpectedData("isNoOp=false", "true")
+ testRecordReviewPromptEventRecordsTheExpectedData(Displayed, "true")
}
@Test
fun reviewPromptWasNotDisplayed() {
- testRecordReviewPromptEventRecordsTheExpectedData("isNoOp=true", "false")
+ testRecordReviewPromptEventRecordsTheExpectedData(NotDisplayed, "false")
}
@Test
fun reviewPromptDisplayStateUnknown() {
- testRecordReviewPromptEventRecordsTheExpectedData(expected = "error")
+ testRecordReviewPromptEventRecordsTheExpectedData(Unknown, "error")
}
private fun testRecordReviewPromptEventRecordsTheExpectedData(
- reviewInfoArg: String = "",
- expected: String,
+ promptDisplayState: ReviewPromptAttemptResult,
+ promptWasDisplayed: String,
) {
val numberOfAppLaunches = 1
- val reviewInfoAsString =
- "ReviewInfo{pendingIntent=PendingIntent{5b613b1: android.os.BinderProxy@46c8096}, $reviewInfoArg}"
val datetime = Date(TEST_TIME_NOW)
val formattedNowLocalDatetime = SIMPLE_DATE_FORMAT.format(datetime)
assertNull(ReviewPrompt.promptAttempt.testGetValue())
- recordReviewPromptEvent(reviewInfoAsString, numberOfAppLaunches, datetime)
+ recordReviewPromptEvent(promptDisplayState, numberOfAppLaunches, datetime)
val reviewPromptData = ReviewPrompt.promptAttempt.testGetValue()!!.last().extra!!
- assertEquals(expected, reviewPromptData["prompt_was_displayed"])
+ assertEquals(promptWasDisplayed, reviewPromptData["prompt_was_displayed"])
assertEquals(numberOfAppLaunches, reviewPromptData["number_of_app_launches"]!!.toInt())
assertEquals(formattedNowLocalDatetime, reviewPromptData["local_datetime"])
}
