mirror of
https://github.com/d4rken-org/capod.git
synced 2026-09-14 18:26:11 -04:00
fix(upgrade): Only count a sponsor visit when the page actually opened
The sponsor unlock heuristic armed itself on every tap, regardless of whether a browser ever opened the page. An unrelated later background round-trip could then hand out supporter status with no page ever shown. WebpageTool.open() now reports whether an activity was actually started and the FOSS repo passes that through synchronously, so the ViewModel can only arm after a successful launch. A second tap while a launch is still pending is ignored, and the upgraded status view's donate button gets its own unarmed entry point - an existing supporter has nothing left to unlock, and re-persisting would rewrite their "supporter since" date.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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),
|
||||
|
||||
@@ -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()
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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"
|
||||
}
|
||||
}
|
||||
@@ -39,6 +39,7 @@ class FossUpgradeScreenHostTest : BaseTest() {
|
||||
|
||||
private fun mockRepo(): UpgradeRepoFoss = mockk<UpgradeRepoFoss>(relaxed = true).apply {
|
||||
every { upgradeInfo } returns MutableStateFlow(UpgradeRepoFoss.Info())
|
||||
every { openGithubSponsorsPage() } returns true
|
||||
coEvery { persistUpgrade() } answers { persisted++ }
|
||||
}
|
||||
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<UpgradeRepoFoss.Info> = MutableStateFlow(UpgradeRepoFoss.Info()),
|
||||
): UpgradeRepoFoss = mockk<UpgradeRepoFoss>(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
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user