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