feat(settings): let the user set a default mail account
Persist a defaultAccountId preference (SettingsRepository/AppSettings, following the existing key/field/setter pattern), add a "Default account" switch to AccountSettingsScreen, and prefer it in ComposeViewModel's from-account fallback (fromAccountId -> valid default -> first account). Deleting the default account clears the preference (SettingsRepository .clearDefaultAccountId), and a stale/foreign id is validated against the current account list before use so it can never crash or point at a missing account. Closes #163 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -28,6 +28,7 @@ import org.libremail.R
|
||||
import org.libremail.contacts.ContactsRepository
|
||||
import org.libremail.data.local.AccountDatabase
|
||||
import org.libremail.data.settings.AccountSettingsRepository
|
||||
import org.libremail.data.settings.SettingsRepository
|
||||
import org.libremail.data.settings.SignatureRepository
|
||||
import org.libremail.domain.model.Account
|
||||
import org.libremail.domain.model.AuthType
|
||||
@@ -88,6 +89,7 @@ class ComposeScreenTest {
|
||||
contactsRepository = ContactsRepository(context),
|
||||
accountSettingsRepository = AccountSettingsRepository(database.accountSettingsDao()),
|
||||
signatureRepository = SignatureRepository(database.signatureDao()),
|
||||
settingsRepository = SettingsRepository(context),
|
||||
)
|
||||
composeTestRule.setContent {
|
||||
LibreMailTheme(darkTheme = false, dynamicColor = false) {
|
||||
|
||||
@@ -19,6 +19,7 @@ import org.libremail.R
|
||||
import org.libremail.data.local.AccountDatabase
|
||||
import org.libremail.data.local.toEntity
|
||||
import org.libremail.data.settings.AccountSettingsRepository
|
||||
import org.libremail.data.settings.SettingsRepository
|
||||
import org.libremail.data.settings.SignatureRepository
|
||||
import org.libremail.data.sync.SyncScheduler
|
||||
import org.libremail.domain.model.Account
|
||||
@@ -71,6 +72,7 @@ class AccountSettingsScreenTest {
|
||||
accountSettingsRepository = repository,
|
||||
signatureRepository = SignatureRepository(db.signatureDao()),
|
||||
syncScheduler = SyncScheduler(Provider { WorkManager.getInstance(context) }),
|
||||
settingsRepository = SettingsRepository(context),
|
||||
)
|
||||
composeTestRule.setContent {
|
||||
LibreMailTheme(darkTheme = false, dynamicColor = false) {
|
||||
|
||||
@@ -57,6 +57,14 @@ data class AppSettings(
|
||||
*/
|
||||
val retentionCount: Int = 0,
|
||||
val retentionMonths: Int = 0,
|
||||
/**
|
||||
* The user-chosen default account (issue #163): the account [org.libremail.ui.compose.ComposeViewModel]
|
||||
* uses when compose opens without an explicit account context (e.g. the unified-inbox FAB, or a
|
||||
* `mailto:`/share intent with no account hint). Null means no default is set. A value here is not
|
||||
* guaranteed to still name an existing account (see [SettingsRepository.clearDefaultAccountId]) —
|
||||
* treat it as a hint to validate against the current account list, never as a trusted id.
|
||||
*/
|
||||
val defaultAccountId: String? = null,
|
||||
)
|
||||
|
||||
private object Keys {
|
||||
@@ -71,6 +79,7 @@ private object Keys {
|
||||
val FETCH_POLICY = stringPreferencesKey("fetch_policy")
|
||||
val RETENTION_COUNT = intPreferencesKey("retention_count")
|
||||
val RETENTION_MONTHS = intPreferencesKey("retention_months")
|
||||
val DEFAULT_ACCOUNT_ID = stringPreferencesKey("default_account_id")
|
||||
val BATTERY_PROMPT_HANDLED = booleanPreferencesKey("battery_prompt_handled")
|
||||
val CONTACTS_PROMPT_HANDLED = booleanPreferencesKey("contacts_prompt_handled")
|
||||
val CONTACTS_PERMISSION_REQUESTED = booleanPreferencesKey("contacts_permission_requested")
|
||||
@@ -96,6 +105,7 @@ internal fun Preferences.toAppSettings(): AppSettings = AppSettings(
|
||||
?: FetchPolicy.WIFI_ONLY,
|
||||
retentionCount = this[Keys.RETENTION_COUNT] ?: 0,
|
||||
retentionMonths = this[Keys.RETENTION_MONTHS] ?: 0,
|
||||
defaultAccountId = this[Keys.DEFAULT_ACCOUNT_ID],
|
||||
)
|
||||
|
||||
@Singleton
|
||||
@@ -175,6 +185,29 @@ class SettingsRepository @Inject constructor(@ApplicationContext private val con
|
||||
context.settingsDataStore.edit { it[Keys.RETENTION_MONTHS] = value.coerceAtLeast(0) }
|
||||
}
|
||||
|
||||
/**
|
||||
* Sets (or, when [value] is null, clears) the user-chosen default account (#163). Used directly by
|
||||
* the per-account "set as default" toggle; deleting an account should go through
|
||||
* [clearDefaultAccountId] instead so it can't clobber a different account's default.
|
||||
*/
|
||||
suspend fun setDefaultAccountId(value: String?) {
|
||||
context.settingsDataStore.edit {
|
||||
if (value != null) it[Keys.DEFAULT_ACCOUNT_ID] = value else it.remove(Keys.DEFAULT_ACCOUNT_ID)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Clears the default-account preference, but only if it currently points at [accountId]. Called
|
||||
* when that account is deleted, so a stale id is never left behind — and so deleting some other
|
||||
* (non-default) account never disturbs an unrelated default. No-op if [accountId] isn't the current
|
||||
* default.
|
||||
*/
|
||||
suspend fun clearDefaultAccountId(accountId: String) {
|
||||
context.settingsDataStore.edit {
|
||||
if (it[Keys.DEFAULT_ACCOUNT_ID] == accountId) it.remove(Keys.DEFAULT_ACCOUNT_ID)
|
||||
}
|
||||
}
|
||||
|
||||
private suspend fun put(key: Preferences.Key<Boolean>, value: Boolean) {
|
||||
context.settingsDataStore.edit { it[key] = value }
|
||||
}
|
||||
|
||||
@@ -20,6 +20,7 @@ import org.libremail.contacts.ContactSuggestion
|
||||
import org.libremail.contacts.ContactsRepository
|
||||
import org.libremail.data.SignatureBlock
|
||||
import org.libremail.data.settings.AccountSettingsRepository
|
||||
import org.libremail.data.settings.SettingsRepository
|
||||
import org.libremail.data.settings.SignatureRepository
|
||||
import org.libremail.domain.model.Account
|
||||
import org.libremail.domain.model.Draft
|
||||
@@ -62,6 +63,7 @@ class ComposeViewModel @Inject constructor(
|
||||
private val contactsRepository: ContactsRepository,
|
||||
private val accountSettingsRepository: AccountSettingsRepository,
|
||||
private val signatureRepository: SignatureRepository,
|
||||
private val settingsRepository: SettingsRepository,
|
||||
) : ViewModel() {
|
||||
|
||||
private val draftId: String? =
|
||||
@@ -120,7 +122,15 @@ class ComposeViewModel @Inject constructor(
|
||||
// signature. Reply/forward drafts already carry theirs, so they take the draft branch above.
|
||||
viewModelScope.launch {
|
||||
val available = accountRepository.observeAccounts().first { it.isNotEmpty() }
|
||||
val effectiveId = _state.value.fromAccountId ?: available.first().id
|
||||
// The persisted default (#163) only counts if it still names an account that exists.
|
||||
// Deleting the default account normally clears this via
|
||||
// SettingsRepository.clearDefaultAccountId, but a stale id could still reach here (e.g.
|
||||
// a Backup restore onto a device that never had the account) — validate rather than
|
||||
// trust it, so it just falls through to the incidental "first account alphabetically"
|
||||
// behavior instead of crashing or composing from a nonexistent account.
|
||||
val defaultAccountId = settingsRepository.settings.first().defaultAccountId
|
||||
val validDefaultAccountId = defaultAccountId?.takeIf { id -> available.any { it.id == id } }
|
||||
val effectiveId = _state.value.fromAccountId ?: validDefaultAccountId ?: available.first().id
|
||||
applySignature(effectiveId)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -39,6 +39,7 @@ fun AccountSettingsScreen(
|
||||
val settings by viewModel.settings.collectAsStateWithLifecycle()
|
||||
val signatureCount by viewModel.signatureCount.collectAsStateWithLifecycle()
|
||||
val defaultSignatureName by viewModel.defaultSignatureName.collectAsStateWithLifecycle()
|
||||
val isDefaultAccount by viewModel.isDefaultAccount.collectAsStateWithLifecycle()
|
||||
val context = LocalContext.current
|
||||
val fallbackTitle = stringResource(R.string.settings_account_title)
|
||||
|
||||
@@ -63,6 +64,14 @@ fun AccountSettingsScreen(
|
||||
.padding(padding)
|
||||
.verticalScroll(rememberScrollState()),
|
||||
) {
|
||||
SwitchRow(
|
||||
title = stringResource(R.string.settings_account_set_default),
|
||||
checked = isDefaultAccount,
|
||||
onCheckedChange = viewModel::setDefaultAccount,
|
||||
subtitle = stringResource(R.string.settings_account_set_default_summary),
|
||||
)
|
||||
HorizontalDivider()
|
||||
|
||||
SectionHeader(stringResource(R.string.settings_signature))
|
||||
SwitchRow(
|
||||
title = stringResource(R.string.settings_signature_enable),
|
||||
|
||||
@@ -11,6 +11,7 @@ import kotlinx.coroutines.flow.map
|
||||
import kotlinx.coroutines.flow.stateIn
|
||||
import kotlinx.coroutines.launch
|
||||
import org.libremail.data.settings.AccountSettingsRepository
|
||||
import org.libremail.data.settings.SettingsRepository
|
||||
import org.libremail.data.settings.SignatureRepository
|
||||
import org.libremail.data.sync.SyncScheduler
|
||||
import org.libremail.domain.model.Account
|
||||
@@ -27,6 +28,7 @@ class AccountSettingsViewModel @Inject constructor(
|
||||
private val accountSettingsRepository: AccountSettingsRepository,
|
||||
signatureRepository: SignatureRepository,
|
||||
private val syncScheduler: SyncScheduler,
|
||||
private val settingsRepository: SettingsRepository,
|
||||
) : ViewModel() {
|
||||
|
||||
private val accountId: String =
|
||||
@@ -39,6 +41,11 @@ class AccountSettingsViewModel @Inject constructor(
|
||||
val settings: StateFlow<AccountSettings> = accountSettingsRepository.observe(accountId)
|
||||
.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), AccountSettings(accountId))
|
||||
|
||||
/** Whether this account is the user-chosen default (issue #163) — see [SettingsRepository]. */
|
||||
val isDefaultAccount: StateFlow<Boolean> = settingsRepository.settings
|
||||
.map { it.defaultAccountId == accountId }
|
||||
.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), false)
|
||||
|
||||
private val signaturesFlow = signatureRepository.observeForAccount(accountId)
|
||||
|
||||
val signatureCount: StateFlow<Int> = signaturesFlow
|
||||
@@ -78,9 +85,27 @@ class AccountSettingsViewModel @Inject constructor(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Makes this account the default (used by compose's from-account fallback, #163), or clears the
|
||||
* default when turned off. Only one account is default at a time — persisting this account's id
|
||||
* implicitly un-defaults whichever one held it before.
|
||||
*/
|
||||
fun setDefaultAccount(isDefault: Boolean) {
|
||||
viewModelScope.launch {
|
||||
if (isDefault) {
|
||||
settingsRepository.setDefaultAccountId(accountId)
|
||||
} else {
|
||||
settingsRepository.clearDefaultAccountId(accountId)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fun removeAccount(onRemoved: () -> Unit) {
|
||||
viewModelScope.launch {
|
||||
accountRepository.deleteAccount(accountId)
|
||||
// Don't strand the preference on a deleted account; only clears it if this account was
|
||||
// actually the default (see SettingsRepository.clearDefaultAccountId).
|
||||
settingsRepository.clearDefaultAccountId(accountId)
|
||||
onRemoved()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -252,6 +252,8 @@
|
||||
|
||||
<!-- Per-account settings -->
|
||||
<string name="settings_account_title">Account</string>
|
||||
<string name="settings_account_set_default">Default account</string>
|
||||
<string name="settings_account_set_default_summary">Use this account for new messages when none is specified</string>
|
||||
<string name="settings_signature">Signature</string>
|
||||
<string name="settings_signature_enable">Append signature automatically</string>
|
||||
<string name="settings_signature_hint">Your signature</string>
|
||||
|
||||
@@ -0,0 +1,40 @@
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
package org.libremail.data.settings
|
||||
|
||||
import androidx.datastore.preferences.core.emptyPreferences
|
||||
import androidx.datastore.preferences.core.preferencesOf
|
||||
import androidx.datastore.preferences.core.stringPreferencesKey
|
||||
import org.junit.Test
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertNull
|
||||
|
||||
/**
|
||||
* The user-chosen default account (#163), covered at the `toAppSettings()` mapping layer:
|
||||
* [SettingsRepository] itself needs a real `Context` for its DataStore, so (as with [AppSettingsTest]'s
|
||||
* coverage of [FetchPolicy]) the JVM-testable seam is the plain `Preferences` -> [AppSettings] mapping
|
||||
* that [SettingsRepository.setDefaultAccountId] and [SettingsRepository.clearDefaultAccountId] are
|
||||
* ultimately read back through.
|
||||
*/
|
||||
class SettingsRepositoryTest {
|
||||
|
||||
private val defaultAccountIdKey = stringPreferencesKey("default_account_id")
|
||||
|
||||
@Test
|
||||
fun `in-memory default has no default account`() {
|
||||
assertNull(AppSettings().defaultAccountId)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a persisted default account id round-trips`() {
|
||||
val prefs = preferencesOf(defaultAccountIdKey to "imap:me@example.org")
|
||||
|
||||
assertEquals("imap:me@example.org", prefs.toAppSettings().defaultAccountId)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an absent key reads back as no default (the cleared state)`() {
|
||||
// setDefaultAccountId(null) and clearDefaultAccountId both remove the key rather than store a
|
||||
// sentinel value, so "cleared" and "never set" are indistinguishable and both map to null here.
|
||||
assertNull(emptyPreferences().toAppSettings().defaultAccountId)
|
||||
}
|
||||
}
|
||||
@@ -10,6 +10,7 @@ import io.mockk.slot
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.flow.flowOf
|
||||
import kotlinx.coroutines.test.UnconfinedTestDispatcher
|
||||
import kotlinx.coroutines.test.resetMain
|
||||
import kotlinx.coroutines.test.runTest
|
||||
@@ -18,6 +19,8 @@ import org.junit.After
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
import org.libremail.data.settings.AccountSettingsRepository
|
||||
import org.libremail.data.settings.AppSettings
|
||||
import org.libremail.data.settings.SettingsRepository
|
||||
import org.libremail.data.settings.SignatureRepository
|
||||
import org.libremail.domain.model.Account
|
||||
import org.libremail.domain.model.AccountSettings
|
||||
@@ -66,6 +69,7 @@ class ComposeViewModelTest {
|
||||
signatures: Map<String, Signature> = emptyMap(),
|
||||
settings: Map<String, AccountSettings> = emptyMap(),
|
||||
mailRepository: MailRepository = mockk(relaxed = true),
|
||||
defaultAccountId: String? = null,
|
||||
): ComposeViewModel {
|
||||
val accountRepository = mockk<AccountRepository>()
|
||||
every { accountRepository.observeAccounts() } returns MutableStateFlow(accounts)
|
||||
@@ -76,6 +80,8 @@ class ComposeViewModelTest {
|
||||
}
|
||||
val signatureRepository = mockk<SignatureRepository>()
|
||||
coEvery { signatureRepository.getDefault(any()) } answers { signatures[firstArg<String>()] }
|
||||
val settingsRepository = mockk<SettingsRepository>()
|
||||
every { settingsRepository.settings } returns flowOf(AppSettings(defaultAccountId = defaultAccountId))
|
||||
return ComposeViewModel(
|
||||
savedStateHandle = savedState,
|
||||
mailRepository = mailRepository,
|
||||
@@ -83,6 +89,7 @@ class ComposeViewModelTest {
|
||||
contactsRepository = mockk(relaxed = true),
|
||||
accountSettingsRepository = accountSettingsRepository,
|
||||
signatureRepository = signatureRepository,
|
||||
settingsRepository = settingsRepository,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -120,6 +127,40 @@ class ComposeViewModelTest {
|
||||
assertEquals("\n\n-- \nBest, Bob", vm.state.value.body)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `uses the persisted default account when no explicit from-account is given`() = runTest(testDispatcher) {
|
||||
val vm = viewModel(accounts = listOf(alice, bob), defaultAccountId = bob.id)
|
||||
|
||||
assertEquals(bob.id, vm.state.value.fromAccountId)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `falls back to the first account when the persisted default no longer exists`() = runTest(testDispatcher) {
|
||||
// Simulates a default that outlived its account (deletion, or a Backup restore onto a device
|
||||
// that never had it) — must fall back to the incidental first-alphabetically account, not crash.
|
||||
val vm = viewModel(accounts = listOf(alice, bob), defaultAccountId = "imap:deleted-account")
|
||||
|
||||
assertEquals(alice.id, vm.state.value.fromAccountId)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `falls back to the first account when no default is set`() = runTest(testDispatcher) {
|
||||
val vm = viewModel(accounts = listOf(alice, bob), defaultAccountId = null)
|
||||
|
||||
assertEquals(alice.id, vm.state.value.fromAccountId)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an explicit from-account wins over the persisted default`() = runTest(testDispatcher) {
|
||||
val vm = viewModel(
|
||||
accounts = listOf(alice, bob),
|
||||
savedState = SavedStateHandle(mapOf(Routes.COMPOSE_ARG_FROM to alice.id)),
|
||||
defaultAccountId = bob.id,
|
||||
)
|
||||
|
||||
assertEquals(alice.id, vm.state.value.fromAccountId)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `does not append a signature when the account disabled it`() = runTest(testDispatcher) {
|
||||
val vm = viewModel(
|
||||
|
||||
Reference in New Issue
Block a user