mirror of
https://github.com/d4rken-org/capod.git
synced 2026-09-14 18:26:11 -04:00
fix(upgrade): Show a failed error-dialog fix action inline instead of a toast
The Google Play launch-failure message was shown via Toast, which Android caps at 2 lines: English lost the trailing "device.", French was cut mid-word and lost an entire condition. The strings are fine, the container was wrong. The fix action now rethrows after logging, the failure reaches the dialog, and the dialog renders the message inline while staying open. The dismiss button stays available, so the dialog is never latched. The message is passed per dispatch rather than read from the LocalizedError, so no future action button can surface the fix action's failure copy. The inline state is keyed on the throwable, not the LocalizedError, which is rebuilt with fresh action lambdas on every recomposition.
This commit is contained in:
+9
-6
@@ -1,12 +1,10 @@
|
||||
package eu.darken.capod.common.upgrade.core.billing
|
||||
|
||||
import android.app.Activity
|
||||
import android.content.ActivityNotFoundException
|
||||
import android.content.Context
|
||||
import android.content.Intent
|
||||
import android.net.Uri
|
||||
import android.provider.Settings
|
||||
import android.widget.Toast
|
||||
import eu.darken.capod.R
|
||||
import eu.darken.capod.common.debug.logging.Logging.Priority.ERROR
|
||||
import eu.darken.capod.common.debug.logging.log
|
||||
@@ -34,18 +32,23 @@ class GplayServiceUnavailableException(cause: Throwable) :
|
||||
try {
|
||||
activity.startActivity(intent)
|
||||
} catch (e: ActivityNotFoundException) {
|
||||
onLaunchFailed(activity, e)
|
||||
onLaunchFailed(e)
|
||||
throw e
|
||||
} catch (e: SecurityException) {
|
||||
// Play can be installed but unreachable: disabled app, work/restricted profile or a
|
||||
// ROM that guards the settings screen. The launch is denied, not unresolvable.
|
||||
onLaunchFailed(activity, e)
|
||||
onLaunchFailed(e)
|
||||
throw e
|
||||
}
|
||||
},
|
||||
// The failure is not presented here: it propagates to the dialog, which renders this inline.
|
||||
// A Toast caps at 2 lines and clipped this message (in French mid-word, dropping a whole
|
||||
// condition), so the container was the defect, not the wording.
|
||||
fixActionErrorMessage = context.getString(R.string.upgrades_gplay_not_installed_message),
|
||||
)
|
||||
|
||||
private fun onLaunchFailed(activity: Activity, e: Exception) {
|
||||
private fun onLaunchFailed(e: Exception) {
|
||||
log(ERROR) { "Can't launch settings intent for Google Play: $e" }
|
||||
Toast.makeText(activity, R.string.upgrades_gplay_not_installed_message, Toast.LENGTH_SHORT).show()
|
||||
}
|
||||
|
||||
companion object {
|
||||
|
||||
@@ -1,7 +1,10 @@
|
||||
package eu.darken.capod.common.error
|
||||
|
||||
import android.app.Activity
|
||||
import androidx.compose.foundation.layout.Column
|
||||
import androidx.compose.foundation.layout.padding
|
||||
import androidx.compose.material3.AlertDialog
|
||||
import androidx.compose.material3.MaterialTheme
|
||||
import androidx.compose.material3.Text
|
||||
import androidx.compose.material3.TextButton
|
||||
import androidx.compose.runtime.Composable
|
||||
@@ -10,8 +13,10 @@ import androidx.compose.runtime.getValue
|
||||
import androidx.compose.runtime.mutableStateOf
|
||||
import androidx.compose.runtime.remember
|
||||
import androidx.compose.runtime.setValue
|
||||
import androidx.compose.ui.Modifier
|
||||
import androidx.compose.ui.platform.LocalContext
|
||||
import androidx.compose.ui.res.stringResource
|
||||
import androidx.compose.ui.unit.dp
|
||||
import eu.darken.capod.R
|
||||
import eu.darken.capod.common.debug.logging.Logging.Priority.ERROR
|
||||
import eu.darken.capod.common.debug.logging.asLog
|
||||
@@ -48,24 +53,57 @@ private fun ComposeErrorDialog(
|
||||
// acknowledge-only shape. The activity is required to launch the fix.
|
||||
val hasFix = localizedError.fixAction != null && activity != null
|
||||
|
||||
// Keyed on the throwable, not the LocalizedError: the latter is rebuilt (with fresh action
|
||||
// lambdas, so never equal) on every recomposition, which would wipe the message immediately.
|
||||
var actionError by remember(throwable) { mutableStateOf<String?>(null) }
|
||||
|
||||
// errorMessage is per-dispatch, NOT read from localizedError: fixActionErrorMessage describes
|
||||
// only the fix action's failure, so every dispatch site passes its own copy (or none) and no
|
||||
// other action can ever surface it.
|
||||
fun dispatchAndDismiss(
|
||||
action: (Activity) -> Unit,
|
||||
errorMessage: String? = null,
|
||||
) {
|
||||
// Error actions are arbitrary code (intent launches): a throw here would crash the UI
|
||||
// thread from inside a click handler, and skipping onDismiss() would leave the dialog
|
||||
// latched on the current error with no way out.
|
||||
try {
|
||||
action(activity!!)
|
||||
} catch (e: Exception) {
|
||||
log(TAG, ERROR) { "Error action failed: ${e.asLog()}" }
|
||||
// A dispatch that ships its own failure copy keeps the dialog open and shows it inline
|
||||
// (no length cap, unlike a Toast). Never latched: the dismiss button stays available.
|
||||
errorMessage?.let {
|
||||
actionError = it
|
||||
return
|
||||
}
|
||||
}
|
||||
onDismiss()
|
||||
}
|
||||
|
||||
AlertDialog(
|
||||
onDismissRequest = onDismiss,
|
||||
title = { Text(text = localizedError.label) },
|
||||
text = { Text(text = localizedError.description) },
|
||||
text = {
|
||||
Column {
|
||||
Text(text = localizedError.description)
|
||||
actionError?.let {
|
||||
Text(
|
||||
text = it,
|
||||
color = MaterialTheme.colorScheme.error,
|
||||
modifier = Modifier.padding(top = 8.dp),
|
||||
)
|
||||
}
|
||||
}
|
||||
},
|
||||
confirmButton = {
|
||||
if (hasFix) {
|
||||
TextButton(
|
||||
onClick = {
|
||||
// Error actions are arbitrary code (intent launches): a throw here would
|
||||
// crash the UI thread from inside a click handler, and skipping onDismiss()
|
||||
// would leave the dialog latched on the current error with no way out.
|
||||
try {
|
||||
localizedError.fixAction!!.invoke(activity!!)
|
||||
} catch (e: Exception) {
|
||||
log(TAG, ERROR) { "Error action failed: ${e.asLog()}" }
|
||||
} finally {
|
||||
onDismiss()
|
||||
}
|
||||
dispatchAndDismiss(
|
||||
action = localizedError.fixAction!!,
|
||||
errorMessage = localizedError.fixActionErrorMessage,
|
||||
)
|
||||
},
|
||||
) {
|
||||
Text(text = localizedError.fixActionLabel ?: stringResource(android.R.string.ok))
|
||||
|
||||
@@ -14,6 +14,8 @@ data class LocalizedError(
|
||||
val description: String,
|
||||
val fixActionLabel: String? = null,
|
||||
val fixAction: ((Activity) -> Unit)? = null,
|
||||
/** Shown inline in the error dialog if the fix action fails, instead of a length-capped toast. */
|
||||
val fixActionErrorMessage: String? = null,
|
||||
) {
|
||||
fun asText() = "$label:\n$description"
|
||||
}
|
||||
|
||||
@@ -3,9 +3,12 @@ package eu.darken.capod.common.error
|
||||
import android.app.Activity
|
||||
import android.content.Context
|
||||
import androidx.compose.ui.test.assertCountEquals
|
||||
import androidx.compose.ui.test.assertIsDisplayed
|
||||
import androidx.compose.ui.test.onAllNodesWithText
|
||||
import androidx.compose.ui.test.onNodeWithText
|
||||
import androidx.compose.ui.test.performClick
|
||||
import androidx.test.core.app.ApplicationProvider
|
||||
import eu.darken.capod.R
|
||||
import eu.darken.capod.common.compose.PreviewWrapper
|
||||
import eu.darken.capod.common.flow.SingleEventFlow
|
||||
import io.kotest.matchers.shouldBe
|
||||
@@ -22,7 +25,11 @@ class ComposeErrorDialogGuardTest : BaseComposeRobolectricTest() {
|
||||
override val errorEvents = SingleEventFlow<Throwable>()
|
||||
}
|
||||
|
||||
private val dismissLabel: String
|
||||
get() = ApplicationProvider.getApplicationContext<Context>().getString(R.string.general_dismiss_action)
|
||||
|
||||
private class TestError(
|
||||
private val fixErrorMessage: String? = null,
|
||||
private val fixAction: (Activity) -> Unit,
|
||||
) : Exception(ERROR_BODY), HasLocalizedError {
|
||||
override fun getLocalizedError(context: Context): LocalizedError = LocalizedError(
|
||||
@@ -31,6 +38,7 @@ class ComposeErrorDialogGuardTest : BaseComposeRobolectricTest() {
|
||||
description = ERROR_BODY,
|
||||
fixActionLabel = FIX_LABEL,
|
||||
fixAction = fixAction,
|
||||
fixActionErrorMessage = fixErrorMessage,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -66,8 +74,42 @@ class ComposeErrorDialogGuardTest : BaseComposeRobolectricTest() {
|
||||
// caught and onDismiss still ran.
|
||||
composeRule.onAllNodesWithText(FIX_LABEL).assertCountEquals(0)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a throwing fix action with its own message keeps the dialog open and shows it inline`() {
|
||||
// A Toast caps at 2 lines and clipped this kind of message; the dialog body has no cap.
|
||||
showError(
|
||||
TestError(fixErrorMessage = FIX_ERROR_MESSAGE) {
|
||||
throw IllegalStateException("fix action exploded")
|
||||
}
|
||||
)
|
||||
|
||||
composeRule.onNodeWithText(FIX_LABEL).performClick()
|
||||
composeRule.waitForIdle()
|
||||
|
||||
composeRule.onNodeWithText(FIX_ERROR_MESSAGE).assertIsDisplayed()
|
||||
composeRule.onNodeWithText(FIX_LABEL).assertIsDisplayed()
|
||||
// Not latched: the way out stays available while the message is shown.
|
||||
composeRule.onNodeWithText(dismissLabel).performClick()
|
||||
composeRule.waitForIdle()
|
||||
composeRule.onAllNodesWithText(FIX_LABEL).assertCountEquals(0)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a fix action that succeeds never shows its failure message`() {
|
||||
// The message belongs to the fix action's failed dispatch, not to the error itself: a
|
||||
// dispatch that did not throw must keep the plain dismiss behaviour.
|
||||
showError(TestError(fixErrorMessage = FIX_ERROR_MESSAGE) { })
|
||||
|
||||
composeRule.onNodeWithText(FIX_LABEL).performClick()
|
||||
composeRule.waitForIdle()
|
||||
|
||||
composeRule.onAllNodesWithText(FIX_LABEL).assertCountEquals(0)
|
||||
composeRule.onAllNodesWithText(FIX_ERROR_MESSAGE).assertCountEquals(0)
|
||||
}
|
||||
}
|
||||
|
||||
private const val ERROR_TITLE = "Test error title"
|
||||
private const val ERROR_BODY = "Test error description"
|
||||
private const val FIX_LABEL = "Fix it"
|
||||
private const val FIX_ERROR_MESSAGE = "Fixing it did not work"
|
||||
|
||||
+26
-11
@@ -4,6 +4,7 @@ import android.app.Activity
|
||||
import android.content.ActivityNotFoundException
|
||||
import android.content.Intent
|
||||
import eu.darken.capod.R
|
||||
import io.kotest.assertions.throwables.shouldThrow
|
||||
import io.kotest.matchers.nulls.shouldNotBeNull
|
||||
import io.kotest.matchers.shouldBe
|
||||
import org.junit.Test
|
||||
@@ -17,7 +18,8 @@ import testhelpers.TestApplication
|
||||
|
||||
/**
|
||||
* The error dialog's "Google Play" button runs on an activity context: a device where the launch is
|
||||
* refused must get a toast, not a crash. The successful launch is covered by ComposeErrorDialogTest.
|
||||
* refused must get the failure reported through the dialog (which can show the full message), not a
|
||||
* clipped toast and not a crash. The successful launch is covered by ComposeErrorDialogTest.
|
||||
*/
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
@Config(sdk = [33], application = TestApplication::class)
|
||||
@@ -35,23 +37,36 @@ class GplayFixActionTest : BaseTest() {
|
||||
|
||||
private fun <T : Activity> activityOf(clazz: Class<T>): T = Robolectric.buildActivity(clazz).setup().get()
|
||||
|
||||
private fun assertToastInsteadOfCrash(activity: Activity) {
|
||||
val fixAction = GplayServiceUnavailableException(RuntimeException("Play hiccup"))
|
||||
.getLocalizedError(activity).fixAction.shouldNotBeNull()
|
||||
private fun fixActionOf(activity: Activity) = GplayServiceUnavailableException(RuntimeException("Play hiccup"))
|
||||
.getLocalizedError(activity).fixAction.shouldNotBeNull()
|
||||
|
||||
fixAction.invoke(activity)
|
||||
@Test
|
||||
fun `a denied launch reports through the dialog instead of a toast`() {
|
||||
val activity = activityOf(DeniedLaunchActivity::class.java)
|
||||
|
||||
val expected = activity.getString(R.string.upgrades_gplay_not_installed_message)
|
||||
ShadowToast.getTextOfLatestToast() shouldBe expected
|
||||
shouldThrow<SecurityException> { fixActionOf(activity).invoke(activity) }
|
||||
|
||||
// A toast caps at 2 lines and clipped this message (French lost a whole condition
|
||||
// mid-word) — the dialog renders it inline instead.
|
||||
ShadowToast.getLatestToast() shouldBe null
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a denied launch shows the not-installed toast instead of crashing`() {
|
||||
assertToastInsteadOfCrash(activityOf(DeniedLaunchActivity::class.java))
|
||||
fun `an unresolvable launch reports through the dialog instead of a toast`() {
|
||||
val activity = activityOf(MissingPlayActivity::class.java)
|
||||
|
||||
shouldThrow<ActivityNotFoundException> { fixActionOf(activity).invoke(activity) }
|
||||
|
||||
ShadowToast.getLatestToast() shouldBe null
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an unresolvable launch shows the not-installed toast instead of crashing`() {
|
||||
assertToastInsteadOfCrash(activityOf(MissingPlayActivity::class.java))
|
||||
fun `the failure message travels with the error for the dialog to show`() {
|
||||
val activity = activityOf(Activity::class.java)
|
||||
|
||||
val message = GplayServiceUnavailableException(RuntimeException("Play hiccup"))
|
||||
.getLocalizedError(activity).fixActionErrorMessage.shouldNotBeNull()
|
||||
|
||||
message shouldBe activity.getString(R.string.upgrades_gplay_not_installed_message)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user