diff --git a/app/src/foss/java/eu/darken/capod/common/upgrade/core/UpgradeRepoFoss.kt b/app/src/foss/java/eu/darken/capod/common/upgrade/core/UpgradeRepoFoss.kt index 3546afa1..40f48184 100644 --- a/app/src/foss/java/eu/darken/capod/common/upgrade/core/UpgradeRepoFoss.kt +++ b/app/src/foss/java/eu/darken/capod/common/upgrade/core/UpgradeRepoFoss.kt @@ -13,7 +13,6 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.shareIn -import kotlinx.coroutines.launch import java.time.Instant import java.util.UUID import javax.inject.Inject @@ -49,9 +48,11 @@ class UpgradeRepoFoss @Inject constructor( .setupCommonEventHandlers(TAG) { "upgradeInfo" } .shareIn(appScope, SharingStarted.WhileSubscribed(3000L, 0L), replay = 1) - fun openGithubSponsorsPage() = appScope.launch { + // Synchronous so the caller learns whether the page actually opened: the FOSS unlock heuristic + // only arms on a successful launch, and a fire-and-forget coroutine can't report that back. + fun openGithubSponsorsPage(): Boolean { log(TAG) { "openGithubSponsorsPage()" } - webpageTool.open(upgradeSite) + return webpageTool.open(upgradeSite) } // Writes capod's RETAINED persistence schema: existing supporter records are serialized with diff --git a/app/src/foss/java/eu/darken/capod/common/upgrade/ui/UpgradeScreen.kt b/app/src/foss/java/eu/darken/capod/common/upgrade/ui/UpgradeScreen.kt index fc5f1602..93c7bce5 100644 --- a/app/src/foss/java/eu/darken/capod/common/upgrade/ui/UpgradeScreen.kt +++ b/app/src/foss/java/eu/darken/capod/common/upgrade/ui/UpgradeScreen.kt @@ -95,6 +95,7 @@ fun UpgradeScreenHost( supporterSince = state?.supporterSince, snackbarHostState = snackbarHostState, onGithubSponsors = vm::goGithubSponsors, + onOpenSponsors = vm::openSponsors, onShowUpgradeOptions = vm::onShowUpgradeOptions, onNavigateUp = vm::navUp, ) @@ -106,6 +107,7 @@ internal fun UpgradeScreen( supporterSince: Instant? = null, snackbarHostState: SnackbarHostState = remember { SnackbarHostState() }, onGithubSponsors: () -> Unit = {}, + onOpenSponsors: () -> Unit = {}, onShowUpgradeOptions: () -> Unit = {}, onNavigateUp: () -> Unit = {}, ) { @@ -140,7 +142,7 @@ internal fun UpgradeScreen( FossUpgradeView.STATUS_UPGRADED -> UpgradeStatusUpgradedContent( paddingValues = paddingValues, supporterSince = supporterSince, - onGithubSponsors = onGithubSponsors, + onOpenSponsors = onOpenSponsors, ) } } @@ -234,7 +236,7 @@ private fun UpgradeStatusFreeContent( private fun UpgradeStatusUpgradedContent( paddingValues: PaddingValues, supporterSince: Instant? = null, - onGithubSponsors: () -> Unit, + onOpenSponsors: () -> Unit, ) { UpgradeScreenContent( paddingValues = paddingValues, @@ -273,7 +275,7 @@ private fun UpgradeStatusUpgradedContent( ) { UpgradeSectionBody(text = stringResource(R.string.upgrade_screen_recurring_body)) OutlinedButton( - onClick = onGithubSponsors, + onClick = onOpenSponsors, modifier = Modifier .fillMaxWidth() .testTag(UpgradeScreenTags.FOSS_DONATE), diff --git a/app/src/foss/java/eu/darken/capod/common/upgrade/ui/UpgradeViewModel.kt b/app/src/foss/java/eu/darken/capod/common/upgrade/ui/UpgradeViewModel.kt index 47882106..880e76a7 100644 --- a/app/src/foss/java/eu/darken/capod/common/upgrade/ui/UpgradeViewModel.kt +++ b/app/src/foss/java/eu/darken/capod/common/upgrade/ui/UpgradeViewModel.kt @@ -106,9 +106,28 @@ class UpgradeViewModel @Inject constructor( handle[KEY_SHOW_UPGRADE_OPTIONS] = true } + /** Armed variant: the pitch's sponsor button, which starts the return-after-5s unlock heuristic. */ fun goGithubSponsors() { log(TAG) { "goGithubSponsors()" } + if (hasPendingSponsorLaunch()) { + log(TAG) { "A sponsor launch is already awaiting its return" } + return + } + // Only arm the heuristic if the page actually opened; otherwise an unrelated later + // background/foreground round-trip would grant supporter status with no page ever shown. + if (!upgradeRepo.openGithubSponsorsPage()) { + log(TAG) { "Sponsor page didn't open; not arming the unlock heuristic" } + return + } handle[KEY_SPONSOR_PRESSED_AT] = SystemClock.elapsedRealtime() + } + + /** + * Unarmed variant: the status view's donate button. An existing supporter re-visiting the page + * must not re-arm the unlock heuristic — there is nothing left to unlock. + */ + fun openSponsors() { + log(TAG) { "openSponsors()" } upgradeRepo.openGithubSponsorsPage() } diff --git a/app/src/main/java/eu/darken/capod/common/WebpageTool.kt b/app/src/main/java/eu/darken/capod/common/WebpageTool.kt index 19cf4ecb..3e993ef6 100644 --- a/app/src/main/java/eu/darken/capod/common/WebpageTool.kt +++ b/app/src/main/java/eu/darken/capod/common/WebpageTool.kt @@ -15,14 +15,18 @@ class WebpageTool @Inject constructor( @ApplicationContext private val context: Context, ) { - fun open(address: String) { + // Returns whether an activity was actually started, so callers that gate behaviour on the page + // having opened (e.g. the FOSS sponsor unlock heuristic) don't fire when no browser handled it. + fun open(address: String): Boolean { val intent = Intent(Intent.ACTION_VIEW, address.toUri()).apply { addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) } - try { + return try { context.startActivity(intent) + true } catch (e: Exception) { log(ERROR) { "Failed to launch: ${e.asLog()}" } + false } } diff --git a/app/src/test/java/eu/darken/capod/common/WebpageToolTest.kt b/app/src/test/java/eu/darken/capod/common/WebpageToolTest.kt new file mode 100644 index 00000000..e47a17c7 --- /dev/null +++ b/app/src/test/java/eu/darken/capod/common/WebpageToolTest.kt @@ -0,0 +1,57 @@ +package eu.darken.capod.common + +import android.app.Application +import android.content.ActivityNotFoundException +import android.content.ContextWrapper +import android.content.Intent +import androidx.core.net.toUri +import androidx.test.core.app.ApplicationProvider +import io.kotest.matchers.shouldBe +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.Shadows.shadowOf +import org.robolectric.annotation.Config +import testhelpers.BaseTest +import testhelpers.TestApplication + +/** + * The return value is a contract, not a convenience: the FOSS sponsor unlock heuristic only arms + * when the page actually opened. Every swallow point has to report the failure back. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [33], application = TestApplication::class) +class WebpageToolTest : BaseTest() { + + private val application: Application + get() = ApplicationProvider.getApplicationContext() + + @Test + fun `a launched page reports success`() { + WebpageTool(application).open(URL) shouldBe true + + shadowOf(application).nextStartedActivity!!.data shouldBe URL.toUri() + } + + @Test + fun `a missing browser reports failure`() { + val context = object : ContextWrapper(application) { + override fun startActivity(intent: Intent) = throw ActivityNotFoundException("No browser") + } + + WebpageTool(context).open(URL) shouldBe false + } + + @Test + fun `a denied launch reports failure`() { + val context = object : ContextWrapper(application) { + override fun startActivity(intent: Intent) = throw SecurityException("Permission Denial") + } + + WebpageTool(context).open(URL) shouldBe false + } + + companion object { + private const val URL = "https://github.com/sponsors/d4rken" + } +} diff --git a/app/src/testFoss/java/eu/darken/capod/common/upgrade/ui/FossUpgradeScreenHostTest.kt b/app/src/testFoss/java/eu/darken/capod/common/upgrade/ui/FossUpgradeScreenHostTest.kt index e1cb5abe..9b870d7d 100644 --- a/app/src/testFoss/java/eu/darken/capod/common/upgrade/ui/FossUpgradeScreenHostTest.kt +++ b/app/src/testFoss/java/eu/darken/capod/common/upgrade/ui/FossUpgradeScreenHostTest.kt @@ -39,6 +39,7 @@ class FossUpgradeScreenHostTest : BaseTest() { private fun mockRepo(): UpgradeRepoFoss = mockk(relaxed = true).apply { every { upgradeInfo } returns MutableStateFlow(UpgradeRepoFoss.Info()) + every { openGithubSponsorsPage() } returns true coEvery { persistUpgrade() } answers { persisted++ } } diff --git a/app/src/testFoss/java/eu/darken/capod/common/upgrade/ui/FossUpgradeScreenTest.kt b/app/src/testFoss/java/eu/darken/capod/common/upgrade/ui/FossUpgradeScreenTest.kt index a5bc947d..a52093b5 100644 --- a/app/src/testFoss/java/eu/darken/capod/common/upgrade/ui/FossUpgradeScreenTest.kt +++ b/app/src/testFoss/java/eu/darken/capod/common/upgrade/ui/FossUpgradeScreenTest.kt @@ -12,6 +12,7 @@ import androidx.test.core.app.ApplicationProvider import androidx.compose.ui.semantics.SemanticsActions import eu.darken.capod.R import eu.darken.capod.common.compose.PreviewWrapper +import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Test import testhelpers.compose.BaseComposeRobolectricTest @@ -127,9 +128,16 @@ class FossUpgradeScreenTest : BaseComposeRobolectricTest() { @Test fun `recurring donation button invokes the sponsors callback`() { var clicked = false + // The armed pitch callback must stay untouched here: a supporter donating again has nothing + // left to unlock, so the donate button goes through the unarmed callback. + var armed = false composeRule.setUpgradeContent { - UpgradeScreen(view = FossUpgradeView.STATUS_UPGRADED, onGithubSponsors = { clicked = true }) + UpgradeScreen( + view = FossUpgradeView.STATUS_UPGRADED, + onGithubSponsors = { armed = true }, + onOpenSponsors = { clicked = true }, + ) } composeRule.onNodeWithTag(UpgradeScreenTags.FOSS_DONATE) @@ -137,6 +145,7 @@ class FossUpgradeScreenTest : BaseComposeRobolectricTest() { composeRule.runOnIdle { assertTrue(clicked) + assertFalse(armed) } } } diff --git a/app/src/testFoss/java/eu/darken/capod/common/upgrade/ui/FossUpgradeViewModelTest.kt b/app/src/testFoss/java/eu/darken/capod/common/upgrade/ui/FossUpgradeViewModelTest.kt index e7bf6ca2..c358e658 100644 --- a/app/src/testFoss/java/eu/darken/capod/common/upgrade/ui/FossUpgradeViewModelTest.kt +++ b/app/src/testFoss/java/eu/darken/capod/common/upgrade/ui/FossUpgradeViewModelTest.kt @@ -11,6 +11,7 @@ import io.kotest.matchers.shouldBe import io.mockk.coVerify import io.mockk.every import io.mockk.mockk +import io.mockk.verify import kotlinx.coroutines.CoroutineStart import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.async @@ -61,6 +62,7 @@ class FossUpgradeViewModelTest : BaseTest() { info: MutableStateFlow = MutableStateFlow(UpgradeRepoFoss.Info()), ): UpgradeRepoFoss = mockk(relaxed = true).apply { every { upgradeInfo } returns info + every { openGithubSponsorsPage() } returns true } private fun buildVm( @@ -309,4 +311,69 @@ class FossUpgradeViewModelTest : BaseTest() { // Consumed: a later resume must not re-run the unlock. recreatedVm.hasPendingSponsorLaunch() shouldBe false } + + @Test + fun `a sponsor page that never opened arms nothing and a later retry still works`() = runTest2( + context = testDispatcher, + ) { + // A silently failed launch must not leave the heuristic armed: an unrelated later + // background round-trip would otherwise hand out supporter status for free. + val repo = mockRepo() + every { repo.openGithubSponsorsPage() } returns false + val vm = buildVm(repo = repo) + + vm.goGithubSponsors() + advanceUntilIdle() + + vm.hasPendingSponsorLaunch() shouldBe false + + // And the failure must not brick the button either — the next working attempt arms as usual. + every { repo.openGithubSponsorsPage() } returns true + vm.goGithubSponsors() + advanceUntilIdle() + + vm.hasPendingSponsorLaunch() shouldBe true + } + + @Test + fun `a second sponsor tap while a launch is pending opens the page only once`() = runTest2( + context = testDispatcher, + ) { + val repo = mockRepo() + val vm = buildVm(repo = repo) + + vm.goGithubSponsors() + vm.goGithubSponsors() + advanceUntilIdle() + + verify(exactly = 1) { repo.openGithubSponsorsPage() } + } + + @Test + fun `a long donate visit from the status view does not re-persist the upgrade`() = runTest2( + context = testDispatcher, + ) { + // The status view's donate button is unarmed on purpose: a supporter browsing the sponsors + // page for a while must not run the unlock heuristic again and rewrite their upgrade date. + val repo = mockRepo(MutableStateFlow(upgradedInfo())) + val vm = buildVm(repo = repo) + + val state = async { vm.state.first { it != null }!! } + vm.bindRoute(Nav.Main.Upgrade(manage = true)) + advanceUntilIdle() + state.await().supporterSince shouldBe Instant.EPOCH + + vm.openSponsors() + advanceUntilIdle() + + verify(exactly = 1) { repo.openGithubSponsorsPage() } + vm.hasPendingSponsorLaunch() shouldBe false + + ShadowSystemClock.advanceBy(Duration.ofSeconds(6)) + vm.checkSponsorReturn() + advanceUntilIdle() + + coVerify(exactly = 0) { repo.persistUpgrade() } + vm.state.value!!.supporterSince shouldBe Instant.EPOCH + } }