From 6aaa15b130faa743cd587c106b42ce918b305b64 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Mon, 7 Sep 2026 23:18:48 +0200 Subject: [PATCH] sync: make the add-account flow's own messages translatable Fourteen English sentences were built in the ViewModel and rendered verbatim as fatal, Working.message and both error fields, so they shipped untranslated to every locale. Three more sites passed through whatever text the server, the repository or a Throwable produced. Seventeen in all, and three of the fourteen were added by fixes earlier on this branch -- which is the argument for a type rather than a rule. This is the defect Outcome.Cause was introduced to fix, and it only ever covered the address step. AddAccountMessage does the same for the rest: a plain Kotlin sealed type in the app, with the resource mapping beside Cause's in the screen. The state fields carry it, so a literal is now a compile error rather than something review has to catch. Nothing lints for this -- HardcodedText reads XML layout attributes and this app has none -- so the type is the only guard there is. The passthroughs get causes of their own, following the rule Outcome.Cause already states: PollResult.Failed carries RATE_LIMITED, MAINTENANCE or SERVER_ERROR, keeping the 429/503 distinction that flattening to one message would lose, and CredentialFailed carries KEYSTORE_REFUSED or NOT_SAVED. Their reason and detail stay for logs and are never shown. A server's words are untranslatable and often a bare status line; a Throwable's are worse. Twenty keys, base locale only -- Weblate owns the rest and picks them up. The four tests that asserted on English prose now assert on the message, which for the cross-domain one is stricter: it pins the host into the argument instead of anywhere in a sentence. --- .../agendula/data/sync/AccountRepository.kt | 22 ++++- .../agendula/ui/accounts/AddAccountMessage.kt | 55 ++++++++++++ .../agendula/ui/accounts/AddAccountScreen.kt | 64 +++++++++++--- .../ui/accounts/AddAccountViewModel.kt | 85 +++++++++++-------- app/src/main/res/values/strings.xml | 20 +++++ .../ui/accounts/AddAccountViewModelTest.kt | 17 ++-- .../caldav/NextcloudLoginFlow.kt | 46 ++++++++-- 7 files changed, 249 insertions(+), 60 deletions(-) create mode 100644 app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountMessage.kt diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/sync/AccountRepository.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/sync/AccountRepository.kt index 14ea7dd..2c33b49 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/sync/AccountRepository.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/sync/AccountRepository.kt @@ -58,7 +58,21 @@ class AccountRepository @Inject constructor( sealed interface Outcome { data class Created(val accountId: Long) : Outcome data object AlreadyExists : Outcome - data class CredentialFailed(val reason: String) : Outcome + data class CredentialFailed(val cause: Cause, val detail: String = "") : Outcome + + /** + * Why an account could not be saved, in a form the UI can translate. + * + * ⚠️ The UI renders *this*, never [CredentialFailed.detail] — which is + * a `Throwable.message` and so an untranslated, often unreadable string. + */ + enum class Cause { + /** The Keystore refused to hold the password. */ + KEYSTORE_REFUSED, + + /** Anything else that stopped the write. */ + NOT_SAVED, + } } suspend fun all(): List = withContext(io) { database.accounts().all() } @@ -138,6 +152,7 @@ class AccountRepository @Inject constructor( // would sit in Settings failing to sync with nothing to explain it. rollback(accountId) return@withContext Outcome.CredentialFailed( + Outcome.Cause.KEYSTORE_REFUSED, "the device keystore would not store the password", ) } @@ -169,7 +184,10 @@ class AccountRepository @Inject constructor( appPassword: String, ): Outcome { if (!credentials.put(accountId, appPassword)) { - return Outcome.CredentialFailed("the device keystore would not store the password") + return Outcome.CredentialFailed( + Outcome.Cause.KEYSTORE_REFUSED, + "the device keystore would not store the password", + ) } accountState.setNeedsSignIn(accountId, false) database.accounts().recordSync(accountId, at = null, error = null) diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountMessage.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountMessage.kt new file mode 100644 index 0000000..7d044ca --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountMessage.kt @@ -0,0 +1,55 @@ +package de.jeanlucmakiola.agendula.ui.accounts + +import de.jeanlucmakiola.caldav.ServerQuirk + +/** + * Something the add-account flow has to tell the user, in a form the UI can + * translate. + * + * ⚠️ A type, not a `String`, and not a `@StringRes Int` either. The flow used to + * build its sentences in the ViewModel, so a dozen of them shipped in English to + * every locale — the same defect `CalDavDiscovery.Outcome.Cause` was introduced + * to fix, which it only ever fixed for the address step. Nothing lints for it: + * `HardcodedText` reads XML layout attributes, and this app has none. Making the + * state fields carry this makes a literal a compile error, which is the only + * guard available. + * + * A resource id would work too, but two of these need a format argument and an + * `Int` accepts any other `Int` — this way the argument travels with the message + * that needs it, and a test can assert on meaning rather than on prose. + */ +sealed interface AddAccountMessage { + + /** Something is happening, and the step says which. */ + sealed interface Progress : AddAccountMessage { + data object Discovering : Progress + data object SigningIn : Progress + data object ReadingLists : Progress + data object Saving : Progress + } + + /** Something went wrong, and the step says what. */ + sealed interface Problem : AddAccountMessage + + data object GoogleUnsupported : Problem + data object AlreadyExists : Problem + data object NoUsableLists : Problem + data object NotSaved : Problem + data object KeystoreRefused : Problem + data object CredentialsRejected : Problem + data object BrowserApprovalExpired : Problem + data object BrowserRateLimited : Problem + data object BrowserMaintenance : Problem + data object BrowserFailed : Problem + + /** + * A home set on a host the credential is not scoped to. + * + * The host travels with the message rather than being baked into a sentence, + * so the translation decides where it goes. + */ + data class OutsideCredentialScope(val host: String) : Problem + + /** A provider whose real requirement is not "wrong password". */ + data class Quirk(val quirk: ServerQuirk) : Problem +} diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountScreen.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountScreen.kt index 8d7256a..3db6cbd 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountScreen.kt @@ -143,7 +143,7 @@ internal fun AddAccountScreen( val fatal = state.fatal if (fatal != null) { Text( - fatal, + fatal.text(), style = MaterialTheme.typography.bodyLarge, color = MaterialTheme.colorScheme.onSurfaceVariant, textAlign = TextAlign.Center, @@ -286,7 +286,7 @@ private fun CredentialsStep(step: AddAccountStep.EnterCredentials, viewModel: Ad value = step.password, onValueChange = viewModel::onPasswordChanged, position = Position.Bottom, - error = step.error, + error = step.error?.text(), hint = stringResource(R.string.add_account_password_hint), keyboardType = KeyboardType.Password, onImeAction = viewModel::onCredentialsSubmitted, @@ -315,6 +315,53 @@ private val CalDavDiscovery.Outcome.Cause.message: Int CalDavDiscovery.Outcome.Cause.SERVER_ERROR -> R.string.add_account_error_server } +/** + * The same rule as [message], for the messages the flow itself produces. + * + * Kept here rather than on [AddAccountMessage] so the type stays a plain Kotlin + * one and the resource ids stay where the resources are. + */ +@Composable +private fun AddAccountMessage.text(): String = when (this) { + AddAccountMessage.Progress.Discovering -> + stringResource(R.string.add_account_working_discovering) + AddAccountMessage.Progress.SigningIn -> + stringResource(R.string.add_account_working_signing_in) + AddAccountMessage.Progress.ReadingLists -> + stringResource(R.string.add_account_working_reading_lists) + AddAccountMessage.Progress.Saving -> + stringResource(R.string.add_account_working_saving) + AddAccountMessage.GoogleUnsupported -> + stringResource(R.string.add_account_error_google_unsupported) + AddAccountMessage.AlreadyExists -> + stringResource(R.string.add_account_error_already_exists) + AddAccountMessage.NoUsableLists -> + stringResource(R.string.add_account_error_no_usable_lists) + AddAccountMessage.NotSaved -> stringResource(R.string.add_account_error_not_saved) + AddAccountMessage.KeystoreRefused -> stringResource(R.string.add_account_error_keystore) + AddAccountMessage.CredentialsRejected -> + stringResource(R.string.add_account_error_credentials_rejected) + AddAccountMessage.BrowserApprovalExpired -> + stringResource(R.string.add_account_browser_error_expired) + AddAccountMessage.BrowserRateLimited -> + stringResource(R.string.add_account_browser_error_rate_limited) + AddAccountMessage.BrowserMaintenance -> + stringResource(R.string.add_account_browser_error_maintenance) + AddAccountMessage.BrowserFailed -> + stringResource(R.string.add_account_browser_error_failed) + is AddAccountMessage.OutsideCredentialScope -> + stringResource(R.string.add_account_error_cross_domain, host) + is AddAccountMessage.Quirk -> when (quirk) { + ServerQuirk.FASTMAIL_APP_PASSWORD -> + stringResource(R.string.add_account_quirk_hint_fastmail) + ServerQuirk.ICLOUD_APP_SPECIFIC_PASSWORD -> + stringResource(R.string.add_account_quirk_hint_icloud) + // Neither reaches a credentials step: Google is refused before it, and + // the brute-force note is a pre-flight warning. + else -> "" + } +} + /** * The family's text input: a tonal grouped surface with a borderless field in * it, never Material's outlined box. @@ -479,7 +526,7 @@ private fun BrowserStep(step: AddAccountStep.WaitingForBrowser, viewModel: AddAc } else { stringResource(R.string.add_account_browser_failed_title) }, - text = step.error ?: stringResource(R.string.add_account_browser_body), + text = step.error?.text() ?: stringResource(R.string.add_account_browser_body), ) step.hostMismatch?.let { mismatch -> @@ -503,7 +550,7 @@ private fun WorkingStep(step: AddAccountStep.Working) { verticalArrangement = Arrangement.spacedBy(16.dp), ) { CircularProgressIndicator() - Text(step.message, style = MaterialTheme.typography.bodyLarge) + Text(step.message.text(), style = MaterialTheme.typography.bodyLarge) } } @@ -533,13 +580,10 @@ private fun ListsStep(step: AddAccountStep.ChooseLists, viewModel: AddAccountVie @Composable private fun QuirkNote(quirk: ServerQuirk) { val text = when (quirk) { - ServerQuirk.FASTMAIL_APP_PASSWORD -> - "Fastmail needs an app password, and CalDAV is not available on the Basic plan." + ServerQuirk.FASTMAIL_APP_PASSWORD -> stringResource(R.string.add_account_quirk_fastmail) ServerQuirk.ICLOUD_APP_SPECIFIC_PASSWORD -> - "iCloud needs an app-specific password, created at appleid.apple.com with two-factor on. " + - "Tasks stored there will not appear in Reminders." - ServerQuirk.GOOGLE_UNSUPPORTED -> - "Google Calendar does not support tasks over CalDAV." + stringResource(R.string.add_account_quirk_icloud) + ServerQuirk.GOOGLE_UNSUPPORTED -> stringResource(R.string.add_account_quirk_google) ServerQuirk.NEXTCLOUD_BRUTE_FORCE_PROTECTED -> return } QuirkNote(text) diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountViewModel.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountViewModel.kt index dc59e41..ce72c26 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountViewModel.kt @@ -36,13 +36,13 @@ sealed interface AddAccountStep { val error: CalDavDiscovery.Outcome.Cause? = null, ) : AddAccountStep - data class Working(val message: String) : AddAccountStep + data class Working(val message: AddAccountMessage.Progress) : AddAccountStep /** The server wants credentials and is not a Nextcloud we can hand to a browser. */ data class EnterCredentials( val username: String = "", val password: String = "", - val error: String? = null, + val error: AddAccountMessage? = null, ) : AddAccountStep /** @@ -53,7 +53,7 @@ sealed interface AddAccountStep { */ data class WaitingForBrowser( val hostMismatch: NextcloudLoginFlow.HostMismatch? = null, - val error: String? = null, + val error: AddAccountMessage? = null, ) : AddAccountStep data class ChooseLists( @@ -82,7 +82,7 @@ data class AddAccountUiState( */ val originMismatch: NextcloudLoginFlow.HostMismatch? = null, /** Set when the flow cannot continue at all; the UI offers only "start over". */ - val fatal: String? = null, + val fatal: AddAccountMessage? = null, /** Non-null once the browser flow has a URL to open. */ val openInBrowser: HttpUrl? = null, ) @@ -147,7 +147,7 @@ class AddAccountViewModel @Inject constructor( _state.update { it.copy( step = AddAccountStep.EnterServer(input = input), - fatal = FATAL_GOOGLE, + fatal = AddAccountMessage.GoogleUnsupported, quirk = quirk, ) } @@ -156,7 +156,7 @@ class AddAccountViewModel @Inject constructor( serverRoot = ServiceDiscovery.serverRootFor(input) - working("Looking for a CalDAV server") + working(AddAccountMessage.Progress.Discovering) viewModelScope.launch { when (val outcome = gateway.discover(typedInput)) { is CalDavDiscovery.Outcome.Found -> onDiscovered(outcome) @@ -186,7 +186,7 @@ class AddAccountViewModel @Inject constructor( username = step.username.trim() appPassword = step.password - working("Signing in") + working(AddAccountMessage.Progress.SigningIn) viewModelScope.launch { val credentials = serverRoot?.let { CalDavGateway.Credentials(username, appPassword, it) @@ -204,7 +204,7 @@ class AddAccountViewModel @Inject constructor( username = username, password = "", error = quirkHint() ?: crossDomainHint() - ?: "That username or password was not accepted.", + ?: AddAccountMessage.CredentialsRejected, ), ) } @@ -272,7 +272,7 @@ class AddAccountViewModel @Inject constructor( result.hostMismatch?.let { mismatch -> _state.update { it.copy(originMismatch = mismatch) } } - working("Reading your task lists") + working(AddAccountMessage.Progress.ReadingLists) val outcome = gateway.discover( // The server the credentials were *issued by*, not the // host the user typed — the two differ in exactly the @@ -304,10 +304,7 @@ class AddAccountViewModel @Inject constructor( hostsNeedingAuth = outcome.hosts val stranger = outsideCredentialScope(outcome.hosts) if (stranger != null) { - fatal( - "This server keeps some task lists on $stranger, " + - "which Agendula cannot sign in to yet.", - ) + fatal(AddAccountMessage.OutsideCredentialScope(stranger)) } else { backToServer(CalDavDiscovery.Outcome.Cause.SERVER_ERROR) } @@ -322,17 +319,29 @@ class AddAccountViewModel @Inject constructor( NextcloudLoginFlow.PollResult.Pending -> Unit is NextcloudLoginFlow.PollResult.Expired -> { - browserFailed(result.reason) + browserFailed(AddAccountMessage.BrowserApprovalExpired) return@launch } is NextcloudLoginFlow.PollResult.Failed -> { - browserFailed(result.reason) + // ⚠️ The cause, never result.reason — that is a bare + // status line or a parser's complaint, in whatever + // language the server chose. It is for logs. + browserFailed( + when (result.cause) { + NextcloudLoginFlow.PollResult.Cause.RATE_LIMITED -> + AddAccountMessage.BrowserRateLimited + NextcloudLoginFlow.PollResult.Cause.MAINTENANCE -> + AddAccountMessage.BrowserMaintenance + NextcloudLoginFlow.PollResult.Cause.SERVER_ERROR -> + AddAccountMessage.BrowserFailed + }, + ) return@launch } } } - browserFailed("The approval window closed before the server answered.") + browserFailed(AddAccountMessage.BrowserApprovalExpired) } } @@ -367,7 +376,7 @@ class AddAccountViewModel @Inject constructor( return } - working("Adding the account") + working(AddAccountMessage.Progress.Saving) viewModelScope.launch { val outcome = runCatching { repository.create( @@ -381,7 +390,8 @@ class AddAccountViewModel @Inject constructor( // A constraint violation or an IO failure in DataStore must not // take the app down and leave the spinner up forever. AccountRepository.Outcome.CredentialFailed( - it.message ?: "the account could not be saved", + AccountRepository.Outcome.Cause.NOT_SAVED, + it.message.orEmpty(), ) } when (outcome) { @@ -401,9 +411,15 @@ class AddAccountViewModel @Inject constructor( } AccountRepository.Outcome.AlreadyExists -> - fatal("That account is already set up.") + fatal(AddAccountMessage.AlreadyExists) - is AccountRepository.Outcome.CredentialFailed -> fatal(outcome.reason) + is AccountRepository.Outcome.CredentialFailed -> fatal( + when (outcome.cause) { + AccountRepository.Outcome.Cause.KEYSTORE_REFUSED -> + AddAccountMessage.KeystoreRefused + AccountRepository.Outcome.Cause.NOT_SAVED -> AddAccountMessage.NotSaved + }, + ) } } } @@ -461,7 +477,7 @@ class AddAccountViewModel @Inject constructor( private fun onDiscovered(outcome: CalDavDiscovery.Outcome.Found) { found = outcome if (outcome.collections.isEmpty()) { - fatal("Signed in, but this account has no task lists that Agendula can use.") + fatal(AddAccountMessage.NoUsableLists) return } _state.update { @@ -479,7 +495,7 @@ class AddAccountViewModel @Inject constructor( } } - private fun working(message: String) = + private fun working(message: AddAccountMessage.Progress) = _state.update { it.copy(step = AddAccountStep.Working(message), fatal = null) } /** @@ -488,7 +504,7 @@ class AddAccountViewModel @Inject constructor( * happens to render `fatal` first, and relying on that leaves the state * lying about what it is doing. */ - private fun fatal(reason: String) = _state.update { + private fun fatal(reason: AddAccountMessage) = _state.update { it.copy(step = AddAccountStep.EnterServer(input = typedInput), fatal = reason) } @@ -504,7 +520,7 @@ class AddAccountViewModel @Inject constructor( it.copy(step = AddAccountStep.EnterServer(input = typedInput, error = cause)) } - private fun browserFailed(reason: String) = _state.update { + private fun browserFailed(reason: AddAccountMessage) = _state.update { it.copy(step = AddAccountStep.WaitingForBrowser(error = reason), openInBrowser = null) } @@ -514,22 +530,22 @@ class AddAccountViewModel @Inject constructor( } /** The provider-specific reason a correct-looking password gets rejected. */ - private fun quirkHint(): String? = when (ServerQuirk.forInput(typedInput)) { - ServerQuirk.FASTMAIL_APP_PASSWORD -> - "Fastmail needs an app password, not your account password — and CalDAV is not on the Basic plan." - ServerQuirk.ICLOUD_APP_SPECIFIC_PASSWORD -> - "iCloud needs an app-specific password, which you create at appleid.apple.com with two-factor on." - else -> null - } + private fun quirkHint(): AddAccountMessage? = + when (val quirk = ServerQuirk.forInput(typedInput)) { + ServerQuirk.FASTMAIL_APP_PASSWORD, + ServerQuirk.ICLOUD_APP_SPECIFIC_PASSWORD, + -> AddAccountMessage.Quirk(quirk) + else -> null + } /** * A home set on a different registrable domain than the account's own is * legal (RFC 4791 §6.2.1) but unreachable for us: the credential is scoped to * one domain. Better to name it than to leave "wrong password" standing. */ - private fun crossDomainHint(): String? = + private fun crossDomainHint(): AddAccountMessage? = outsideCredentialScope(hostsNeedingAuth)?.let { - "This server keeps some task lists on $it, which Agendula cannot sign in to yet." + AddAccountMessage.OutsideCredentialScope(it) } /** @@ -563,8 +579,5 @@ class AddAccountViewModel @Inject constructor( /** The server-side lifetime is 1200s; this covers it and then stops. */ const val MAX_POLL_ATTEMPTS = 600 - const val FATAL_GOOGLE = - "Google Calendar does not support tasks over CalDAV — its own documentation says so — " + - "so Agendula cannot sync with it." } } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index a3da8ab..92c9a9d 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -333,6 +333,26 @@ That address is unencrypted. Agendula only sends your password over https. Signed in, but this account has no task lists. The server ran into a problem. Try again in a moment. + Looking for a CalDAV server + Signing in + Reading your task lists + Adding the account + Google Calendar does not support tasks over CalDAV \u2014 its own documentation says so \u2014 so Agendula cannot sync with it. + This server keeps some task lists on %1$s, which Agendula can\u2019t sign in to yet. + That account is already set up. + Signed in, but this account has no task lists Agendula can use. + The account couldn\u2019t be saved. + This device wouldn\u2019t store the password. + That username or password was not accepted. + The approval window closed before the server answered. + The server is turning away repeated attempts. Wait a few minutes and try again. + The server is in maintenance mode. Try again once it is back. + The server didn\u2019t finish signing you in. Try again in a moment. + Fastmail needs an app password, not your account password \u2014 and CalDAV is not on the Basic plan. + iCloud needs an app-specific password, which you create at appleid.apple.com with two-factor on. + Fastmail needs an app password, and CalDAV is not available on the Basic plan. + iCloud needs an app-specific password, created at appleid.apple.com with two-factor on. Tasks stored there will not appear in Reminders. + Google Calendar does not support tasks over CalDAV. Sign in again to keep syncing Sign in again %1$s \u00b7 %2$s diff --git a/app/src/test/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountViewModelTest.kt b/app/src/test/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountViewModelTest.kt index e98424b..c8c4bc1 100644 --- a/app/src/test/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountViewModelTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountViewModelTest.kt @@ -64,7 +64,7 @@ class AddAccountViewModelTest { // It supports neither VTODO nor MKCALENDAR — its own docs say so — so // a 401 the user cannot act on is the wrong answer. - assertThat(vm.state.value.fatal).contains("does not support tasks") + assertThat(vm.state.value.fatal).isEqualTo(AddAccountMessage.GoogleUnsupported) assertThat(gateway.discoveries).isEmpty() } @@ -410,7 +410,11 @@ class AddAccountViewModelTest { // Retrying cannot reach it, and each attempt mints another // password — so this is fatal rather than back-to-the-address. - assertThat(vm.state.value.fatal).contains("dav.elsewhere.org") + // Stronger than a substring: it pins the host into the argument + // rather than anywhere in a sentence, so the translation decides + // where it goes. + assertThat(vm.state.value.fatal) + .isEqualTo(AddAccountMessage.OutsideCredentialScope("dav.elsewhere.org")) } } @@ -444,7 +448,7 @@ class AddAccountViewModelTest { vm.onServerSubmitted() advanceUntilIdle() - assertThat(vm.state.value.fatal).contains("no task lists") + assertThat(vm.state.value.fatal).isEqualTo(AddAccountMessage.NoUsableLists) } } @@ -481,7 +485,7 @@ class AddAccountViewModelTest { } @Test - fun `a failure inside create leaves a readable message, not a spinner`() = + fun `a failure inside create leaves a translatable message, not a spinner`() = runTest(dispatcher) { creator.thrown = IllegalStateException("UNIQUE constraint failed") @@ -489,7 +493,10 @@ class AddAccountViewModelTest { vm.onSave() advanceUntilIdle() - assertThat(vm.state.value.fatal).contains("UNIQUE constraint failed") + // ⚠️ The exception's words are deliberately gone. They are + // untranslated and unreadable; the cause is what the user sees, + // and the detail is kept for logs. + assertThat(vm.state.value.fatal).isEqualTo(AddAccountMessage.NotSaved) assertThat(vm.state.value.step).isNotInstanceOf(AddAccountStep.Working::class.java) } diff --git a/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/NextcloudLoginFlow.kt b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/NextcloudLoginFlow.kt index e58d1c0..640fb62 100644 --- a/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/NextcloudLoginFlow.kt +++ b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/NextcloudLoginFlow.kt @@ -70,7 +70,27 @@ class NextcloudLoginFlow( /** 404: still waiting. Also what an expired or already-consumed flow returns. */ data object Pending : PollResult data class Expired(val reason: String) : PollResult - data class Failed(val reason: String) : PollResult + data class Failed(val cause: Cause, val reason: String) : PollResult + + /** + * Why the flow ended, in a form the UI can translate. + * + * ⚠️ The UI must render *this*, never [Failed.reason]. A server's own + * words are untranslatable, often in a language the user does not read, + * and here they are frequently a bare status line or a JSON parser's + * complaint. [reason] exists for logs, and is never shown — the same rule + * [CalDavDiscovery.Outcome.Cause] states for discovery. + */ + enum class Cause { + /** Answering, but turning away repeated attempts (429). */ + RATE_LIMITED, + + /** Down on purpose (503). */ + MAINTENANCE, + + /** The server's own error, or an answer we could not read. */ + SERVER_ERROR, + } } /** @@ -148,22 +168,34 @@ class NextcloudLoginFlow( val contentType = response.header("Content-Type").orEmpty() // A Cloudflare challenge is a 200 carrying HTML. if (!contentType.contains("application/json", ignoreCase = true)) { - PollResult.Failed("server answered 200 with $contentType, not JSON") + PollResult.Failed( + PollResult.Cause.SERVER_ERROR, + "server answered 200 with $contentType, not JSON", + ) } else { parseCredentials(flow, response.body?.string().orEmpty()) } } response.code == 429 -> - PollResult.Failed("the server is rate-limiting this address (429)") + PollResult.Failed( + PollResult.Cause.RATE_LIMITED, + "the server is rate-limiting this address (429)", + ) response.code == 503 -> - PollResult.Failed("the server is in maintenance mode (503)") + PollResult.Failed( + PollResult.Cause.MAINTENANCE, + "the server is in maintenance mode (503)", + ) - else -> PollResult.Failed("unexpected HTTP ${response.code}") + else -> PollResult.Failed( + PollResult.Cause.SERVER_ERROR, + "unexpected HTTP ${response.code}", + ) } } - }.getOrElse { PollResult.Failed(it.message ?: it.toString()) } + }.getOrElse { PollResult.Failed(PollResult.Cause.SERVER_ERROR, it.message ?: it.toString()) } } private fun parseCredentials(flow: Flow, body: String): PollResult = runCatching { @@ -188,7 +220,7 @@ class NextcloudLoginFlow( appPassword = root["appPassword"]?.jsonPrimitive?.content ?: error("no appPassword"), ), ) - }.getOrElse { PollResult.Failed(it.message ?: it.toString()) } + }.getOrElse { PollResult.Failed(PollResult.Cause.SERVER_ERROR, it.message ?: it.toString()) } /** * These URLs are generated from `overwrite.cli.url` / `overwriteprotocol` /