From 275a6d3a80bdbe26be53defc1901ba699a5a82fb Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Mon, 31 Aug 2026 07:01:59 +1000 Subject: [PATCH] QA: let a launch extra state whether the app counts as updated The in-app review prompt gates which events may raise it on whether the app was updated rather than freshly installed: when updated, only the donate trigger qualifies; when freshly installed, the path and theme triggers do too. That fact is read from the package manager as `firstInstallTime != lastUpdateTime`, and a test harness cannot influence either value. Installing over an existing package always makes them differ, so the app always reads as updated and the fresh-install branch is unreachable from a test -- which leaves two of the three triggers with no way to be exercised. `sessionAppUpdated` states it instead: `true`, `false`, or `useActual` to drop the override and use the real answer. Absent leaves the stored value alone, like the other extras. Applying it also clears the stored review state, because that state is what the flag feeds -- the flag is only consulted when a fresh state is derived, so without the clear a device that had already run a review spec would keep its old state and ignore the extra. The reset happens once, on the launch carrying the extra, so a spec asserting the prompt appears only once still works across a later relaunch. A boolean rather than iOS's `customFirstInstallDateTime` date shape, because a date cannot express this here: the value compared against is `lastUpdateTime`, which the harness can neither read nor set, so supplying a first-install date can reliably produce only "updated" -- the state that already happens by default and is not the one the specs need. The two other readers of `firstInstallTime` want it as a date and are untouched. This changes no real-world behaviour. The gate and its inputs are exactly as they were on any build without the launch config, where the override reads null; all that changes is whether a test can choose which side of the gate to exercise. --- .../utilities/TextSecurePreferences.kt | 19 +++++++ .../securesms/qa/QaLaunchConfig.kt | 56 +++++++++++++++++++ .../securesms/reviews/InAppReviewManager.kt | 8 ++- .../reviews/InAppReviewManagerTest.kt | 56 ++++++++++++++++++- 4 files changed, 137 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/org/session/libsession/utilities/TextSecurePreferences.kt b/app/src/main/java/org/session/libsession/utilities/TextSecurePreferences.kt index e0525d4ecb..2bd02c519c 100644 --- a/app/src/main/java/org/session/libsession/utilities/TextSecurePreferences.kt +++ b/app/src/main/java/org/session/libsession/utilities/TextSecurePreferences.kt @@ -263,6 +263,17 @@ interface TextSecurePreferences { fun getDebugRefundInProgressOverride(): Boolean? fun setDebugRefundInProgressOverride(refunding: Boolean?) + /** + * Overrides whether the app considers itself to have been UPDATED rather than freshly installed, + * which gates which events may raise the in-app review prompt. + * + * Tri-state: `null` means "use the real package-manager answer". The real answer is + * `firstInstallTime != lastUpdateTime`, which a test harness cannot influence — it installs over an + * existing package, so the app always reads as updated and the fresh-install branch is unreachable. + */ + fun getDebugAppUpdated(): Boolean? + fun setDebugAppUpdated(updated: Boolean?) + /** * Mocked originating payment provider (a `BackendRequests.PAYMENT_PROVIDER_*` slug), or `null` for no * override so the fixture's own provider stands. @@ -454,6 +465,7 @@ interface TextSecurePreferences { const val DEBUG_PRO_PROFILE_FEATURES = "debug_pro_profile_features" const val DEBUG_SUBSCRIPTION_STATUS = "debug_subscription_status" const val DEBUG_PRO_ACCESS_OVERRIDE = "debug_pro_access_override" + const val DEBUG_APP_UPDATED = "debug_app_updated" const val DEBUG_PRO_ACCESS_EXPIRY = "debug_pro_access_expiry" const val DEBUG_PRO_PLAN_STATUS = "debug_pro_plan_status" const val DEBUG_FORCE_NO_BILLING = "debug_pro_has_billing" @@ -1327,6 +1339,13 @@ class AppTextSecurePreferences @Inject constructor( _events.tryEmit(TextSecurePreferences.DEBUG_SUBSCRIPTION_STATUS) } + override fun getDebugAppUpdated(): Boolean? = + getStringPreference(TextSecurePreferences.DEBUG_APP_UPDATED, null)?.toBooleanStrictOrNull() + + override fun setDebugAppUpdated(updated: Boolean?) { + setStringPreference(TextSecurePreferences.DEBUG_APP_UPDATED, updated?.toString()) + } + override fun getDebugProAccessOverride(): Boolean? = getStringPreference(TextSecurePreferences.DEBUG_PRO_ACCESS_OVERRIDE, null)?.toBooleanStrictOrNull() diff --git a/app/src/main/java/org/thoughtcrime/securesms/qa/QaLaunchConfig.kt b/app/src/main/java/org/thoughtcrime/securesms/qa/QaLaunchConfig.kt index 9bf6633cc8..69c240f04f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/qa/QaLaunchConfig.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/qa/QaLaunchConfig.kt @@ -148,6 +148,30 @@ object QaLaunchConfig { */ private const val EXTRA_FORCE_PRO_REVOCATION_REFRESH = "sessionForceProRevocationRefresh" + /** + * States whether the app should consider itself UPDATED rather than freshly installed, which decides + * which events may raise the in-app review prompt: when updated, only the donate trigger can; when + * freshly installed, the path and theme triggers can too. + * + * `true` | `false`. Absent leaves the stored override alone; `useActual` restores the real + * package-manager answer. + * + * It needs an extra because the real answer is `firstInstallTime != lastUpdateTime`, and a harness + * cannot influence either: installing over an existing package always makes them differ, so the app + * always reads as updated and the fresh-install branch is unreachable from a test. The two triggers + * behind it are not testable without this. + * + * Applying this also CLEARS the stored review state, because that state is what the flag feeds: the + * flag is only consulted when deriving a fresh state, so without the clear a device that had already + * run a review spec would keep its old state and ignore the extra. That reset happens once, on the + * launch carrying the extra — a later relaunch without it keeps whatever the app has since decided, + * so a spec asserting the prompt appears only once still works across a restart. + * + * Mirrors iOS's `customFirstInstallDateTime` in purpose but not in shape — see the commit for why a + * date cannot express this on Android. + */ + private const val EXTRA_APP_UPDATED = "sessionAppUpdated" + /** * When the mocked Pro access expires, overriding the fixed offset the fixture selected by * [EXTRA_PRO_BACKEND_STATUS] carries. iOS's `mockCurrentUserAccessExpiryTimestamp`, which is an @@ -280,6 +304,7 @@ object QaLaunchConfig { // After the status extra: it overrides the access half that one sets. applyProProof(intent, prefs) applyForceProRevocationRefresh(intent, prefs) + applyAppUpdated(intent, prefs) applyProAccessExpiry(intent, prefs) applyProLoadingState(intent, prefs) applyProRefundingStatus(intent, prefs) @@ -312,6 +337,7 @@ object QaLaunchConfig { EXTRA_PRO_BACKEND_STATUS, EXTRA_PRO_PROOF, EXTRA_FORCE_PRO_REVOCATION_REFRESH, + EXTRA_APP_UPDATED, EXTRA_PRO_ACCESS_EXPIRY, EXTRA_PRO_LOADING_STATE, EXTRA_PRO_REFUNDING_STATUS, @@ -639,6 +665,36 @@ object QaLaunchConfig { return true } + private fun applyAppUpdated(intent: Intent, prefs: TextSecurePreferences): Boolean { + if (!intent.hasExtra(EXTRA_APP_UPDATED)) { + return false + } + + val raw = intent.getStringExtra(EXTRA_APP_UPDATED).orEmpty().trim() + // null = drop the override and use the real package-manager answer. + val override: Boolean? = when (raw.lowercase()) { + "true" -> true + "false" -> false + USE_ACTUAL -> null + else -> { + Log.e( + TAG, + "Ignoring unknown '$EXTRA_APP_UPDATED' extra: '$raw'. Use true | false | $USE_ACTUAL." + ) + return false + } + } + + prefs.setDebugAppUpdated(override) + + // The flag is only read when a fresh review state is derived, so an existing stored state would + // silently outrank this extra. Clearing it is what makes the extra mean what it says. + prefs.inAppReviewState = null + + Log.i(TAG, "Set app-updated override to $override (from '$raw'); cleared the stored review state") + return true + } + private fun applyProProof(intent: Intent, prefs: TextSecurePreferences): Boolean { if (!intent.hasExtra(EXTRA_PRO_PROOF)) { // Absent leaves the stored override alone, like every other Pro extra here. diff --git a/app/src/main/java/org/thoughtcrime/securesms/reviews/InAppReviewManager.kt b/app/src/main/java/org/thoughtcrime/securesms/reviews/InAppReviewManager.kt index b00d41ffbb..4c2b1997b8 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/reviews/InAppReviewManager.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/reviews/InAppReviewManager.kt @@ -78,7 +78,13 @@ class InAppReviewManager @Inject constructor( if (storeReviewManager.supportsReviewFlow) { val pkg = context.packageManager.getPackageInfo(context.packageName, 0) InAppReviewState.WaitingForTrigger( - appUpdated = pkg.firstInstallTime != pkg.lastUpdateTime + // The QA override comes first, and only exists because the real answer is not + // reachable from a test: a harness installs over an existing package, so + // firstInstallTime and lastUpdateTime always differ and the fresh-install branch — + // the one that allows the path and theme triggers — can never be exercised. + // Null in any build without the launch config, so the real answer stands. + appUpdated = prefs.getDebugAppUpdated() + ?: (pkg.firstInstallTime != pkg.lastUpdateTime) ) } else { InAppReviewState.DismissedForever diff --git a/app/src/test/java/org/thoughtcrime/securesms/reviews/InAppReviewManagerTest.kt b/app/src/test/java/org/thoughtcrime/securesms/reviews/InAppReviewManagerTest.kt index 7fb7639772..456e2abdeb 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/reviews/InAppReviewManagerTest.kt +++ b/app/src/test/java/org/thoughtcrime/securesms/reviews/InAppReviewManagerTest.kt @@ -164,9 +164,62 @@ class InAppReviewManagerTest { } } +/** + * The QA override for the install-state gate. + * + * It exists because the real answer is unreachable from a test: a harness installs over an existing + * package, so firstInstallTime and lastUpdateTime always differ, the app always reads as updated, and the + * two triggers that only a fresh install allows can never be exercised. These pin that the override + * outranks the package manager. + * + * Only the POSITIVE direction is asserted. The negative one — that an override of `true` gates the theme + * and path triggers back out — is not here, because turbine's `expectNoEvents()` does not fail in this + * setup even when the event DOES raise the prompt: verified by mutation, and `advanceUntilIdle()` before + * it does not change that. A test that cannot fail is worse than an absent one, so it is absent. The + * pre-existing `should show prompt respectively on triggers on update` rests on the same call and is + * likely to share the weakness. + */ +@RunWith(JUnit4::class) +class InAppReviewManagerAppUpdatedOverrideTest { + @get:Rule + val mockLoggingRule = MockLoggingRule() + + @Test + fun `override false lets the fresh-install triggers fire even though the package says updated`() = + runTest { + // Exactly the harness's situation: installed over an existing package. + for (event in listOf( + InAppReviewManager.Event.ThemeChanged, + InAppReviewManager.Event.PathScreenVisited, + )) { + val manager = createManager(isFreshInstall = false, debugAppUpdated = false) + + manager.shouldShowPrompt.test { + assertFalse(awaitItem()) + manager.onEvent(event) + assertTrue(awaitItem()) + } + } + } + + @Test + fun `the donate trigger fires either way`() = runTest { + for (updated in listOf(true, false)) { + val manager = createManager(isFreshInstall = !updated, debugAppUpdated = updated) + + manager.shouldShowPrompt.test { + assertFalse(awaitItem()) + manager.onEvent(InAppReviewManager.Event.DonateButtonClicked) + assertTrue(awaitItem()) + } + } + } +} + fun TestScope.createManager( isFreshInstall: Boolean, - supportInAppReviewFlow: Boolean = true + supportInAppReviewFlow: Boolean = true, + debugAppUpdated: Boolean? = null, ): InAppReviewManager { val pm = mock { on { getPackageInfo(any(), any()) } doReturn PackageInfo().apply { @@ -190,6 +243,7 @@ fun TestScope.createManager( prefs = mock { on { inAppReviewState } doAnswer { reviewState } on { inAppReviewState = any() } doAnswer { reviewState = it.arguments[0] as? String } + on { getDebugAppUpdated() } doReturn debugAppUpdated }, json = Json { serializersModule += ReviewsSerializerModule().provideReviewsSerializersModule()