From 78d11995c8827ddacf4ce64994d05b6ea464b495 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wolf-Martell=20Montw=C3=A9?= Date: Mon, 3 Nov 2025 10:41:49 +0100 Subject: [PATCH 01/10] docs: add documentation for Outcome --- .../net/thunderbird/core/outcome/Outcome.kt | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/core/outcome/src/commonMain/kotlin/net/thunderbird/core/outcome/Outcome.kt b/core/outcome/src/commonMain/kotlin/net/thunderbird/core/outcome/Outcome.kt index 656da2364a8..4aaf1422650 100644 --- a/core/outcome/src/commonMain/kotlin/net/thunderbird/core/outcome/Outcome.kt +++ b/core/outcome/src/commonMain/kotlin/net/thunderbird/core/outcome/Outcome.kt @@ -1,20 +1,56 @@ package net.thunderbird.core.outcome +/** + * A sealed interface representing the outcome of an operation. + * + * @param SUCCESS The type of the value when the operation succeeds. + * @param FAILURE The type of the error when the operation fails. + */ sealed interface Outcome { + + /** + * A successful outcome with a value of type [SUCCESS]. + * + * @param data The value of the successful outcome. + */ data class Success(val data: SUCCESS) : Outcome + + /** + * A failed outcome with an error of type [FAILURE]. + * + * @param error The error of the failed outcome. + * @param cause The cause of the failed outcome. + */ data class Failure( val error: FAILURE, val cause: Any? = null, ) : Outcome + /** + * Whether the outcome is a success. + */ val isSuccess: Boolean get() = this is Success + /** + * Whether the outcome is a failure. + */ val isFailure: Boolean get() = this is Failure companion object { + /** + * Create a [Success] outcome with the given value. + * + * @param data The value of the successful outcome. + */ fun success(data: SUCCESS): Outcome = Success(data) + + /** + * Create a [Failure] outcome with the given error. + * + * @param error The error of the failed outcome. + */ fun failure(error: FAILURE): Outcome = Failure(error) } } From 41c8e786da8dd4640f10c6ff658722b13c31d13b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wolf-Martell=20Montw=C3=A9?= Date: Mon, 3 Nov 2025 11:33:38 +0100 Subject: [PATCH 02/10] refactor: change feature:account:core module to kmp --- feature/account/core/build.gradle.kts | 15 +++++++++++---- .../account/core/AccountCoreExternalContract.kt | 1 + .../feature/account/core/AccountCoreModule.kt | 0 .../core/data/DefaultAccountProfileRepository.kt | 0 4 files changed, 12 insertions(+), 4 deletions(-) rename feature/account/core/src/{main => commonMain}/kotlin/net/thunderbird/feature/account/core/AccountCoreExternalContract.kt (99%) rename feature/account/core/src/{main => commonMain}/kotlin/net/thunderbird/feature/account/core/AccountCoreModule.kt (100%) rename feature/account/core/src/{main => commonMain}/kotlin/net/thunderbird/feature/account/core/data/DefaultAccountProfileRepository.kt (100%) diff --git a/feature/account/core/build.gradle.kts b/feature/account/core/build.gradle.kts index 3ed1d376274..f96d2aee3fe 100644 --- a/feature/account/core/build.gradle.kts +++ b/feature/account/core/build.gradle.kts @@ -1,8 +1,15 @@ plugins { - id(ThunderbirdPlugins.Library.jvm) - alias(libs.plugins.android.lint) + id(ThunderbirdPlugins.Library.kmp) } -dependencies { - api(projects.feature.account.api) +android { + namespace = "net.thunderbird.feature.account.core" +} + +kotlin { + sourceSets { + commonMain.dependencies { + api(projects.feature.account.api) + } + } } diff --git a/feature/account/core/src/main/kotlin/net/thunderbird/feature/account/core/AccountCoreExternalContract.kt b/feature/account/core/src/commonMain/kotlin/net/thunderbird/feature/account/core/AccountCoreExternalContract.kt similarity index 99% rename from feature/account/core/src/main/kotlin/net/thunderbird/feature/account/core/AccountCoreExternalContract.kt rename to feature/account/core/src/commonMain/kotlin/net/thunderbird/feature/account/core/AccountCoreExternalContract.kt index 207441ffdcd..f7336529cd2 100644 --- a/feature/account/core/src/main/kotlin/net/thunderbird/feature/account/core/AccountCoreExternalContract.kt +++ b/feature/account/core/src/commonMain/kotlin/net/thunderbird/feature/account/core/AccountCoreExternalContract.kt @@ -7,6 +7,7 @@ import net.thunderbird.feature.account.profile.AccountProfile interface AccountCoreExternalContract { interface AccountProfileLocalDataSource { + fun getById(accountId: AccountId): Flow suspend fun update(accountProfile: AccountProfile) diff --git a/feature/account/core/src/main/kotlin/net/thunderbird/feature/account/core/AccountCoreModule.kt b/feature/account/core/src/commonMain/kotlin/net/thunderbird/feature/account/core/AccountCoreModule.kt similarity index 100% rename from feature/account/core/src/main/kotlin/net/thunderbird/feature/account/core/AccountCoreModule.kt rename to feature/account/core/src/commonMain/kotlin/net/thunderbird/feature/account/core/AccountCoreModule.kt diff --git a/feature/account/core/src/main/kotlin/net/thunderbird/feature/account/core/data/DefaultAccountProfileRepository.kt b/feature/account/core/src/commonMain/kotlin/net/thunderbird/feature/account/core/data/DefaultAccountProfileRepository.kt similarity index 100% rename from feature/account/core/src/main/kotlin/net/thunderbird/feature/account/core/data/DefaultAccountProfileRepository.kt rename to feature/account/core/src/commonMain/kotlin/net/thunderbird/feature/account/core/data/DefaultAccountProfileRepository.kt From ca25df2bbad9fcbeb9ce2750ec43cf56c8e2da76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wolf-Martell=20Montw=C3=A9?= Date: Mon, 3 Nov 2025 11:38:23 +0100 Subject: [PATCH 03/10] feat(account): add getAll to AccountProfileRepository --- .../DefaultAccountProfileLocalDataSource.kt | 9 ++ ...efaultAccountProfileLocalDataSourceTest.kt | 58 ++++++--- .../profile/AccountProfileRepository.kt | 21 ++++ .../core/AccountCoreExternalContract.kt | 21 ++++ .../data/DefaultAccountProfileRepository.kt | 5 + .../DefaultAccountProfileRepositoryTest.kt | 116 ++++++++++++++++++ .../core/data/FakeAccountProfileDataSource.kt | 25 ++++ 7 files changed, 240 insertions(+), 15 deletions(-) create mode 100644 feature/account/core/src/commonTest/kotlin/net/thunderbird/feature/account/core/data/DefaultAccountProfileRepositoryTest.kt create mode 100644 feature/account/core/src/commonTest/kotlin/net/thunderbird/feature/account/core/data/FakeAccountProfileDataSource.kt diff --git a/app-common/src/main/kotlin/net/thunderbird/app/common/account/data/DefaultAccountProfileLocalDataSource.kt b/app-common/src/main/kotlin/net/thunderbird/app/common/account/data/DefaultAccountProfileLocalDataSource.kt index 03bc948d037..99794cfb138 100644 --- a/app-common/src/main/kotlin/net/thunderbird/app/common/account/data/DefaultAccountProfileLocalDataSource.kt +++ b/app-common/src/main/kotlin/net/thunderbird/app/common/account/data/DefaultAccountProfileLocalDataSource.kt @@ -14,6 +14,15 @@ internal class DefaultAccountProfileLocalDataSource( private val dataMapper: AccountProfileDataMapper, ) : AccountProfileLocalDataSource { + override fun getAll(): Flow> { + return accountManager.getAll() + .map { accounts -> + accounts.map { dto -> + dataMapper.toDomain(dto.profile) + } + } + } + override fun getById(accountId: AccountId): Flow { return accountManager.getById(accountId) .map { account -> diff --git a/app-common/src/test/kotlin/net/thunderbird/app/common/account/data/DefaultAccountProfileLocalDataSourceTest.kt b/app-common/src/test/kotlin/net/thunderbird/app/common/account/data/DefaultAccountProfileLocalDataSourceTest.kt index 0f5d4af2519..4696e87c4f4 100644 --- a/app-common/src/test/kotlin/net/thunderbird/app/common/account/data/DefaultAccountProfileLocalDataSourceTest.kt +++ b/app-common/src/test/kotlin/net/thunderbird/app/common/account/data/DefaultAccountProfileLocalDataSourceTest.kt @@ -24,15 +24,47 @@ import org.junit.Test class DefaultAccountProfileLocalDataSourceTest { + @Test + fun `getAll should return all account profiles`() = runTest { + // Arrange + val accountId1 = AccountIdFactory.create() + val legacyAccount1 = createLegacyAccount(accountId1) + val accountProfile1 = createAccountProfile(accountId1) + + val accountId2 = AccountIdFactory.create() + val legacyAccount2 = createLegacyAccount(accountId2) + val accountProfile2 = createAccountProfile(accountId2) + + val testSubject = createTestSubject(listOf(legacyAccount1, legacyAccount2)) + + // Act / Assert + testSubject.getAll().test { + val profiles = awaitItem() + assertThat(profiles).isEqualTo(listOf(accountProfile1, accountProfile2)) + } + } + + @Test + fun `getAll should return empty list when no accounts found`() = runTest { + // Arrange + val testSubject = createTestSubject(emptyList()) + + // Act / Assert + testSubject.getAll().test { + val result = awaitItem() + assertThat(result).isEqualTo(emptyList()) + } + } + @Test fun `getById should return account profile`() = runTest { - // arrange + // Arrange val accountId = AccountIdFactory.create() val legacyAccount = createLegacyAccount(accountId) val accountProfile = createAccountProfile(accountId) - val testSubject = createTestSubject(legacyAccount) + val testSubject = createTestSubject(listOf(legacyAccount)) - // act & assert + // Act / Assert testSubject.getById(accountId).test { assertThat(awaitItem()).isEqualTo(accountProfile) } @@ -40,11 +72,11 @@ class DefaultAccountProfileLocalDataSourceTest { @Test fun `getById should return null when account is not found`() = runTest { - // arrange + // Arrange val accountId = AccountIdFactory.create() - val testSubject = createTestSubject(null) + val testSubject = createTestSubject(emptyList()) - // act & assert + // Act / Assert testSubject.getById(accountId).test { assertThat(awaitItem()).isEqualTo(null) } @@ -52,7 +84,7 @@ class DefaultAccountProfileLocalDataSourceTest { @Test fun `update should save account profile`() = runTest { - // arrange + // Arrange val accountId = AccountIdFactory.create() val legacyAccount = createLegacyAccount(accountId) val accountProfile = createAccountProfile(accountId) @@ -60,9 +92,9 @@ class DefaultAccountProfileLocalDataSourceTest { val updatedName = "updatedName" val updatedAccountProfile = accountProfile.copy(name = updatedName) - val testSubject = createTestSubject(legacyAccount) + val testSubject = createTestSubject(listOf(legacyAccount)) - // act & assert + // Act / Assert testSubject.getById(accountId).test { assertThat(awaitItem()).isEqualTo(accountProfile) @@ -139,15 +171,11 @@ class DefaultAccountProfileLocalDataSourceTest { } private fun createTestSubject( - legacyAccount: LegacyAccount?, + accounts: List, ): DefaultAccountProfileLocalDataSource { return DefaultAccountProfileLocalDataSource( accountManager = FakeLegacyAccountManager( - initialAccounts = if (legacyAccount != null) { - listOf(legacyAccount) - } else { - emptyList() - }, + initialAccounts = accounts, ), dataMapper = DefaultAccountProfileDataMapper( avatarMapper = DefaultAccountAvatarDataMapper(), diff --git a/feature/account/api/src/commonMain/kotlin/net/thunderbird/feature/account/profile/AccountProfileRepository.kt b/feature/account/api/src/commonMain/kotlin/net/thunderbird/feature/account/profile/AccountProfileRepository.kt index 4272db38739..e081899a144 100644 --- a/feature/account/api/src/commonMain/kotlin/net/thunderbird/feature/account/profile/AccountProfileRepository.kt +++ b/feature/account/api/src/commonMain/kotlin/net/thunderbird/feature/account/profile/AccountProfileRepository.kt @@ -3,9 +3,30 @@ package net.thunderbird.feature.account.profile import kotlinx.coroutines.flow.Flow import net.thunderbird.feature.account.AccountId +/** + * Repository interface for managing account profiles. + */ interface AccountProfileRepository { + /** + * Gets all account profiles as a flow. + * + * @return A flow emitting a list of all account profiles. + */ + fun getAll(): Flow> + + /** + * Gets an account profile by its ID as a flow. + * + * @param accountId The ID of the account. + * @return A flow emitting the account profile or null if not found. + */ fun getById(accountId: AccountId): Flow + /** + * Updates the given account profile. + * + * @param accountProfile The account profile to update. + */ suspend fun update(accountProfile: AccountProfile) } diff --git a/feature/account/core/src/commonMain/kotlin/net/thunderbird/feature/account/core/AccountCoreExternalContract.kt b/feature/account/core/src/commonMain/kotlin/net/thunderbird/feature/account/core/AccountCoreExternalContract.kt index f7336529cd2..be8b5515634 100644 --- a/feature/account/core/src/commonMain/kotlin/net/thunderbird/feature/account/core/AccountCoreExternalContract.kt +++ b/feature/account/core/src/commonMain/kotlin/net/thunderbird/feature/account/core/AccountCoreExternalContract.kt @@ -6,10 +6,31 @@ import net.thunderbird.feature.account.profile.AccountProfile interface AccountCoreExternalContract { + /** + * Local data source for account profiles. + */ interface AccountProfileLocalDataSource { + /** + * Gets all account profiles as a flow. + * + * @return A flow emitting a list of all account profiles. + */ + fun getAll(): Flow> + + /** + * Gets an account profile by its ID as a flow. + * + * @param accountId The ID of the account. + * @return A flow emitting the account profile or null if not found. + */ fun getById(accountId: AccountId): Flow + /** + * Updates the given account profile. + * + * @param accountProfile The account profile to update. + */ suspend fun update(accountProfile: AccountProfile) } } diff --git a/feature/account/core/src/commonMain/kotlin/net/thunderbird/feature/account/core/data/DefaultAccountProfileRepository.kt b/feature/account/core/src/commonMain/kotlin/net/thunderbird/feature/account/core/data/DefaultAccountProfileRepository.kt index e4e67342c6b..8d89ba95f56 100644 --- a/feature/account/core/src/commonMain/kotlin/net/thunderbird/feature/account/core/data/DefaultAccountProfileRepository.kt +++ b/feature/account/core/src/commonMain/kotlin/net/thunderbird/feature/account/core/data/DefaultAccountProfileRepository.kt @@ -11,6 +11,11 @@ class DefaultAccountProfileRepository( private val localDataSource: AccountProfileLocalDataSource, ) : AccountProfileRepository { + override fun getAll(): Flow> { + return localDataSource.getAll() + .distinctUntilChanged() + } + override fun getById(accountId: AccountId): Flow { return localDataSource.getById(accountId) .distinctUntilChanged() diff --git a/feature/account/core/src/commonTest/kotlin/net/thunderbird/feature/account/core/data/DefaultAccountProfileRepositoryTest.kt b/feature/account/core/src/commonTest/kotlin/net/thunderbird/feature/account/core/data/DefaultAccountProfileRepositoryTest.kt new file mode 100644 index 00000000000..363369c2f2e --- /dev/null +++ b/feature/account/core/src/commonTest/kotlin/net/thunderbird/feature/account/core/data/DefaultAccountProfileRepositoryTest.kt @@ -0,0 +1,116 @@ +package net.thunderbird.feature.account.core.data + +import app.cash.turbine.test +import assertk.assertThat +import assertk.assertions.isEqualTo +import kotlin.collections.emptyList +import kotlin.test.Test +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.test.runTest +import net.thunderbird.feature.account.AccountIdFactory +import net.thunderbird.feature.account.profile.AccountAvatar +import net.thunderbird.feature.account.profile.AccountProfile + +class DefaultAccountProfileRepositoryTest { + + @Test + fun `getAll should return distinct values`() = runTest { + // Arrange + val profiles: MutableStateFlow> = MutableStateFlow(emptyList()) + val list1 = listOf(PROFILE_1) + val list2 = listOf(PROFILE_1, PROFILE_2) + val testSubject = DefaultAccountProfileRepository( + localDataSource = FakeAccountProfileDataSource( + profiles = profiles, + ), + ) + + // Act / Assert + testSubject.getAll().test { + val emptyResult = awaitItem() + assertThat(emptyResult).isEqualTo(emptyList()) + + profiles.value = list1 + val firstResult = awaitItem() + assertThat(firstResult).isEqualTo(list1) + + profiles.value = list1 + expectNoEvents() + + profiles.value = list2 + val secondResult = awaitItem() + assertThat(secondResult).isEqualTo(list2) + + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `getById should return distinct values`() = runTest { + // Arrange + val profiles: MutableStateFlow> = MutableStateFlow(emptyList()) + val list1 = listOf(PROFILE_1, PROFILE_2) + val testSubject = DefaultAccountProfileRepository( + localDataSource = FakeAccountProfileDataSource( + profiles = profiles, + ), + ) + + // Act / Assert + testSubject.getById(PROFILE_ID_2).test { + val nullResult = awaitItem() + assertThat(nullResult).isEqualTo(null) + + profiles.value = list1 + val firstResult = awaitItem() + assertThat(firstResult).isEqualTo(PROFILE_2) + + profiles.value = list1 + expectNoEvents() + + profiles.value = emptyList() + val secondResult = awaitItem() + assertThat(secondResult).isEqualTo(null) + + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `update should call local data source update`() = runTest { + // Arrange + var updatedProfile: AccountProfile? = null + val testSubject = DefaultAccountProfileRepository( + localDataSource = object : FakeAccountProfileDataSource() { + override suspend fun update(accountProfile: AccountProfile) { + updatedProfile = accountProfile + } + }, + ) + val profile = PROFILE_1.copy(name = "Updated Name") + + // Act + testSubject.update(profile) + + // Assert + assertThat(updatedProfile).isEqualTo(profile) + } + + private companion object { + val PROFILE_ID_1 = AccountIdFactory.create() + val PROFILE_ID_2 = AccountIdFactory.create() + + val PROFILE_1 = AccountProfile( + id = PROFILE_ID_1, + name = "Profile 1", + color = 0xFF0000, + avatar = AccountAvatar.Icon(name = "icon-1"), + ) + val PROFILE_2 = AccountProfile( + id = PROFILE_ID_2, + name = "Profile 2", + color = 0x00FF00, + avatar = AccountAvatar.Monogram(value = "AB"), + ) + } +} diff --git a/feature/account/core/src/commonTest/kotlin/net/thunderbird/feature/account/core/data/FakeAccountProfileDataSource.kt b/feature/account/core/src/commonTest/kotlin/net/thunderbird/feature/account/core/data/FakeAccountProfileDataSource.kt new file mode 100644 index 00000000000..34f0d253720 --- /dev/null +++ b/feature/account/core/src/commonTest/kotlin/net/thunderbird/feature/account/core/data/FakeAccountProfileDataSource.kt @@ -0,0 +1,25 @@ +package net.thunderbird.feature.account.core.data + +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.map +import net.thunderbird.feature.account.AccountId +import net.thunderbird.feature.account.core.AccountCoreExternalContract +import net.thunderbird.feature.account.profile.AccountProfile + +open class FakeAccountProfileDataSource( + val profiles: MutableStateFlow> = MutableStateFlow(emptyList()), +) : AccountCoreExternalContract.AccountProfileLocalDataSource { + + override fun getAll(): Flow> = profiles + + override fun getById(accountId: AccountId): Flow { + return profiles.map { list -> + list.find { it.id == accountId } + } + } + + override suspend fun update(accountProfile: AccountProfile) { + TODO("Not yet implemented") + } +} From 810591b40f1f0a9003fc149f0c0c7d010af1e050 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wolf-Martell=20Montw=C3=A9?= Date: Mon, 3 Nov 2025 11:48:11 +0100 Subject: [PATCH 04/10] feat(account): inject account colors --- app-common/build.gradle.kts | 2 + .../app/common/account/AccountColorPicker.kt | 33 ++-- .../common/account/AppCommonAccountModule.kt | 15 +- .../common/account/AccountColorPickerTest.kt | 165 ++++++++++++++++++ .../data/FakeAccountProfileRepository.kt | 22 +++ .../assertk/assertions/ListExtensions.kt | 3 + .../assertk/assertions/ValueExtensions.kt | 21 +++ .../src/main/res/values/account_colors.xml | 6 - .../usecase/FakeAccountProfileRepository.kt | 6 +- 9 files changed, 247 insertions(+), 26 deletions(-) create mode 100644 app-common/src/test/kotlin/net/thunderbird/app/common/account/AccountColorPickerTest.kt create mode 100644 app-common/src/test/kotlin/net/thunderbird/app/common/account/data/FakeAccountProfileRepository.kt create mode 100644 core/testing/src/commonMain/kotlin/assertk/assertions/ValueExtensions.kt diff --git a/app-common/build.gradle.kts b/app-common/build.gradle.kts index 373e8e396a0..d2685ffbbbf 100644 --- a/app-common/build.gradle.kts +++ b/app-common/build.gradle.kts @@ -55,6 +55,8 @@ dependencies { implementation(libs.androidx.work.runtime) implementation(libs.androidx.lifecycle.process) + implementation(libs.kotlinx.collections.immutable) testImplementation(projects.feature.account.fake) + testImplementation(projects.core.testing) } diff --git a/app-common/src/main/kotlin/net/thunderbird/app/common/account/AccountColorPicker.kt b/app-common/src/main/kotlin/net/thunderbird/app/common/account/AccountColorPicker.kt index eb9793c8c92..f92cd6fa5d3 100644 --- a/app-common/src/main/kotlin/net/thunderbird/app/common/account/AccountColorPicker.kt +++ b/app-common/src/main/kotlin/net/thunderbird/app/common/account/AccountColorPicker.kt @@ -1,27 +1,26 @@ package net.thunderbird.app.common.account -import android.content.res.Resources -import app.k9mail.core.ui.legacy.theme2.common.R -import net.thunderbird.core.android.account.LegacyAccountDtoManager +import kotlinx.collections.immutable.ImmutableList +import kotlinx.coroutines.flow.first +import net.thunderbird.feature.account.profile.AccountProfileRepository internal class AccountColorPicker( - private val accountManager: LegacyAccountDtoManager, - private val resources: Resources, + private val repository: AccountProfileRepository, + private val accountColors: ImmutableList, ) { - fun pickColor(): Int { - val accounts = accountManager.getAccounts() - val usedAccountColors = accounts.map { it.chipColor }.toSet() - val accountColors = resources.getIntArray(R.array.account_colors).toList() + suspend fun pickColor(): Int { + val profiles = repository.getAll().first() + val usedCounts = profiles.groupingBy { it.color }.eachCount() - val availableColors = accountColors - usedAccountColors - if (availableColors.isEmpty()) { - return accountColors.random() + val minCount = accountColors.minOf { usedCounts[it] ?: 0 } + val candidates = accountColors.filter { + (usedCounts[it] ?: 0) == minCount } - val defaultAccountColors = resources.getIntArray(R.array.default_account_colors) - return availableColors.shuffled().minByOrNull { color -> - val index = defaultAccountColors.indexOf(color) - if (index != -1) index else defaultAccountColors.size - } ?: error("availableColors must not be empty") + return if (candidates.isNotEmpty()) { + candidates.shuffled().first() + } else { + accountColors.shuffled().first() + } } } diff --git a/app-common/src/main/kotlin/net/thunderbird/app/common/account/AppCommonAccountModule.kt b/app-common/src/main/kotlin/net/thunderbird/app/common/account/AppCommonAccountModule.kt index 610f2263df9..0e8d056b602 100644 --- a/app-common/src/main/kotlin/net/thunderbird/app/common/account/AppCommonAccountModule.kt +++ b/app-common/src/main/kotlin/net/thunderbird/app/common/account/AppCommonAccountModule.kt @@ -1,6 +1,8 @@ package net.thunderbird.app.common.account import app.k9mail.feature.account.setup.AccountSetupExternalContract +import kotlinx.collections.immutable.ImmutableList +import kotlinx.collections.immutable.toImmutableList import net.thunderbird.app.common.account.data.DefaultAccountProfileLocalDataSource import net.thunderbird.app.common.account.data.DefaultLegacyAccountManager import net.thunderbird.core.android.account.AccountDefaultsProvider @@ -13,8 +15,11 @@ import net.thunderbird.feature.account.core.featureAccountCoreModule import net.thunderbird.feature.account.storage.legacy.featureAccountStorageLegacyModule import net.thunderbird.feature.mail.account.api.AccountManager import org.koin.android.ext.koin.androidApplication +import org.koin.android.ext.koin.androidContext +import org.koin.core.qualifier.named import org.koin.dsl.binds import org.koin.dsl.module +import app.k9mail.core.ui.legacy.theme2.common.R as ThemeCommonR internal val appCommonAccountModule = module { includes( @@ -43,10 +48,16 @@ internal val appCommonAccountModule = module { ) } + factory>(named("AccountColors")) { + androidContext().resources.getIntArray( + ThemeCommonR.array.account_colors, + ).toList().toImmutableList() + } + factory { AccountColorPicker( - accountManager = get(), - resources = get(), + repository = get(), + accountColors = get(named("AccountColors")), ) } diff --git a/app-common/src/test/kotlin/net/thunderbird/app/common/account/AccountColorPickerTest.kt b/app-common/src/test/kotlin/net/thunderbird/app/common/account/AccountColorPickerTest.kt new file mode 100644 index 00000000000..c16028daefe --- /dev/null +++ b/app-common/src/test/kotlin/net/thunderbird/app/common/account/AccountColorPickerTest.kt @@ -0,0 +1,165 @@ +package net.thunderbird.app.common.account + +import assertk.assertThat +import assertk.assertions.isEqualTo +import assertk.assertions.isOneOf +import kotlin.test.Test +import kotlinx.collections.immutable.persistentListOf +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.test.runTest +import net.thunderbird.app.common.account.data.FakeAccountProfileRepository +import net.thunderbird.feature.account.AccountIdFactory +import net.thunderbird.feature.account.profile.AccountAvatar +import net.thunderbird.feature.account.profile.AccountProfile + +class AccountColorPickerTest { + + @Test + fun `should pick random color when none used`() = runTest { + // Arrange + val profiles: MutableStateFlow> = MutableStateFlow(emptyList()) + val testSubject = AccountColorPicker( + repository = FakeAccountProfileRepository(profiles), + accountColors = ACCOUNT_COLORS, + ) + + // Act + val result = testSubject.pickColor() + + // Assert + assertThat(result).isOneOf(COLOR_RED, COLOR_GREEN, COLOR_BLUE) + } + + @Test + fun `should pick one of the available colors when some are used`() = runTest { + // Arrange + val profiles: MutableStateFlow> = MutableStateFlow( + listOf( + ACCOUNT_PROFILE_GREEN_1, + ), + ) + val testSubject = AccountColorPicker( + repository = FakeAccountProfileRepository(profiles), + accountColors = ACCOUNT_COLORS, + ) + + // Act + val result = testSubject.pickColor() + + // Assert + assertThat(result).isOneOf(COLOR_RED, COLOR_BLUE) + } + + @Test + fun `should pick last available color when others are used`() = runTest { + // Arrange + val profiles: MutableStateFlow> = MutableStateFlow( + listOf( + ACCOUNT_PROFILE_RED_1, + ACCOUNT_PROFILE_GREEN_1, + ), + ) + val testSubject = AccountColorPicker( + repository = FakeAccountProfileRepository(profiles), + accountColors = ACCOUNT_COLORS, + ) + + // Act + val result = testSubject.pickColor() + + // Assert + assertThat(result).isEqualTo(COLOR_BLUE) + } + + @Test + fun `should pick random color when all colors are used equally`() = runTest { + // Arrange + val profiles: MutableStateFlow> = MutableStateFlow( + listOf( + ACCOUNT_PROFILE_RED_1, + ACCOUNT_PROFILE_GREEN_1, + ACCOUNT_PROFILE_BLUE_1, + ), + ) + val testSubject = AccountColorPicker( + repository = FakeAccountProfileRepository(profiles), + accountColors = ACCOUNT_COLORS, + ) + + // Act + val result = testSubject.pickColor() + + // Assert + assertThat(result).isOneOf(COLOR_RED, COLOR_GREEN, COLOR_BLUE) + } + + @Test + fun `should pick from least used colors when colors are used multiple times`() = runTest { + // Arrange + val profiles: MutableStateFlow> = MutableStateFlow( + listOf( + ACCOUNT_PROFILE_RED_1, + ACCOUNT_PROFILE_RED_2, + ACCOUNT_PROFILE_GREEN_1, + ACCOUNT_PROFILE_GREEN_2, + ACCOUNT_PROFILE_BLUE_1, + ), + ) + val testSubject = AccountColorPicker( + repository = FakeAccountProfileRepository(profiles), + accountColors = ACCOUNT_COLORS, + ) + + // Act + val result = testSubject.pickColor() + + // Assert + assertThat(result).isEqualTo(COLOR_BLUE) + } + + private companion object { + const val COLOR_RED = 0xFF0000 + const val COLOR_GREEN = 0x00FF00 + const val COLOR_BLUE = 0x0000FF + + val ACCOUNT_COLORS = persistentListOf( + COLOR_RED, + COLOR_GREEN, + COLOR_BLUE, + ) + + val ACCOUNT_PROFILE_RED_1 = AccountProfile( + id = AccountIdFactory.create(), + name = "Account Red 1", + color = COLOR_RED, + avatar = AccountAvatar.Icon(name = "icon1"), + ) + val ACCOUNT_PROFILE_RED_2 = AccountProfile( + id = AccountIdFactory.create(), + name = "Account Red 2", + color = COLOR_RED, + avatar = AccountAvatar.Icon(name = "icon4"), + ) + + val ACCOUNT_PROFILE_GREEN_1 = AccountProfile( + id = AccountIdFactory.create(), + name = "Account Green 1", + color = COLOR_GREEN, + avatar = AccountAvatar.Icon(name = "icon2"), + ) + + val ACCOUNT_PROFILE_GREEN_2 = AccountProfile( + id = AccountIdFactory.create(), + name = "Account Green 2", + color = COLOR_GREEN, + avatar = AccountAvatar.Icon(name = "icon5"), + ) + + val ACCOUNT_PROFILE_BLUE_1 = AccountProfile( + id = AccountIdFactory.create(), + name = "Account Blue 1", + color = COLOR_BLUE, + avatar = AccountAvatar.Icon(name = "icon3"), + ) + } +} diff --git a/app-common/src/test/kotlin/net/thunderbird/app/common/account/data/FakeAccountProfileRepository.kt b/app-common/src/test/kotlin/net/thunderbird/app/common/account/data/FakeAccountProfileRepository.kt new file mode 100644 index 00000000000..4042894ae2d --- /dev/null +++ b/app-common/src/test/kotlin/net/thunderbird/app/common/account/data/FakeAccountProfileRepository.kt @@ -0,0 +1,22 @@ +package net.thunderbird.app.common.account.data + +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow +import net.thunderbird.feature.account.AccountId +import net.thunderbird.feature.account.profile.AccountProfile +import net.thunderbird.feature.account.profile.AccountProfileRepository + +class FakeAccountProfileRepository( + val profiles: MutableStateFlow> = MutableStateFlow(emptyList()), +) : AccountProfileRepository { + + override fun getAll(): Flow> = profiles + + override fun getById(accountId: AccountId): Flow { + TODO("Not yet implemented") + } + + override suspend fun update(accountProfile: AccountProfile) { + TODO("Not yet implemented") + } +} diff --git a/core/testing/src/commonMain/kotlin/assertk/assertions/ListExtensions.kt b/core/testing/src/commonMain/kotlin/assertk/assertions/ListExtensions.kt index 2ab42b25287..4940a3f533f 100644 --- a/core/testing/src/commonMain/kotlin/assertk/assertions/ListExtensions.kt +++ b/core/testing/src/commonMain/kotlin/assertk/assertions/ListExtensions.kt @@ -4,6 +4,9 @@ import assertk.Assert import assertk.assertions.support.expected import assertk.assertions.support.show +/** + * Asserts that the list contains no duplicate elements. + */ fun Assert>.containsNoDuplicates() = given { actual -> val seen: MutableSet = mutableSetOf() val duplicates = actual.filter { !seen.add(it) } diff --git a/core/testing/src/commonMain/kotlin/assertk/assertions/ValueExtensions.kt b/core/testing/src/commonMain/kotlin/assertk/assertions/ValueExtensions.kt new file mode 100644 index 00000000000..5b0f9a3d65d --- /dev/null +++ b/core/testing/src/commonMain/kotlin/assertk/assertions/ValueExtensions.kt @@ -0,0 +1,21 @@ +package assertk.assertions + +import assertk.Assert +import assertk.assertions.support.expected +import assertk.assertions.support.show + +/** + * Asserts that the value is one of the expected values. + */ +fun Assert.isOneOf(vararg expectedValues: T) = given { actual -> + if (expectedValues.none { it == actual }) { + expected("to be one of ${show(expectedValues.toList())} but was ${show(actual)}") + } +} + +/** + * Asserts that the value is one of the expected values. + */ +inline fun Assert.isOneOf(expectedValues: Collection) { + isOneOf(*expectedValues.toTypedArray()) +} diff --git a/core/ui/legacy/theme2/common/src/main/res/values/account_colors.xml b/core/ui/legacy/theme2/common/src/main/res/values/account_colors.xml index 976b4ce3eb7..747546bfa67 100644 --- a/core/ui/legacy/theme2/common/src/main/res/values/account_colors.xml +++ b/core/ui/legacy/theme2/common/src/main/res/values/account_colors.xml @@ -21,10 +21,4 @@ @color/material_deep_purple_600 @color/material_blue_gray_700 - - - @color/material_blue_700 - @color/material_pink_500 - @color/material_amber_600 - diff --git a/feature/account/settings/impl/src/test/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/FakeAccountProfileRepository.kt b/feature/account/settings/impl/src/test/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/FakeAccountProfileRepository.kt index 245c122f997..3e2d472cd00 100644 --- a/feature/account/settings/impl/src/test/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/FakeAccountProfileRepository.kt +++ b/feature/account/settings/impl/src/test/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/FakeAccountProfileRepository.kt @@ -12,9 +12,13 @@ internal class FakeAccountProfileRepository( initialAccountProfile: AccountProfile? = null, ) : AccountProfileRepository { - private val accountProfileState = MutableStateFlow(initialAccountProfile) + private val accountProfileState = MutableStateFlow(initialAccountProfile) private val accountProfile: StateFlow = accountProfileState + override fun getAll(): Flow> { + TODO("Not yet implemented") + } + override fun getById(accountId: AccountId): Flow { return accountProfile } From 5456201dd5e35470977376a1b50668dc26631518 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wolf-Martell=20Montw=C3=A9?= Date: Tue, 4 Nov 2025 12:10:16 +0100 Subject: [PATCH 05/10] refactor(account-setting): use injected account colors to remove legacy theme dependency --- feature/account/settings/impl/build.gradle.kts | 1 - .../feature/account/settings/AccountSettingsModule.kt | 2 ++ .../settings/impl/ui/general/GeneralResourceProvider.kt | 5 +---- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/feature/account/settings/impl/build.gradle.kts b/feature/account/settings/impl/build.gradle.kts index 44002a750cc..cbf9eff838c 100644 --- a/feature/account/settings/impl/build.gradle.kts +++ b/feature/account/settings/impl/build.gradle.kts @@ -28,7 +28,6 @@ dependencies { implementation(projects.core.logging.implLegacy) implementation(projects.core.ui.compose.designsystem) implementation(projects.core.ui.compose.navigation) - implementation(projects.core.ui.legacy.theme2.common) debugImplementation(projects.core.ui.setting.implDialog) diff --git a/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/AccountSettingsModule.kt b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/AccountSettingsModule.kt index 455b1915a48..3dccba6c3ce 100644 --- a/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/AccountSettingsModule.kt +++ b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/AccountSettingsModule.kt @@ -11,6 +11,7 @@ import net.thunderbird.feature.account.settings.impl.ui.general.GeneralResourceP import net.thunderbird.feature.account.settings.impl.ui.general.GeneralSettingsViewModel import org.koin.android.ext.koin.androidContext import org.koin.core.module.dsl.viewModel +import org.koin.core.qualifier.named import org.koin.dsl.module val featureAccountSettingsModule = module { @@ -19,6 +20,7 @@ val featureAccountSettingsModule = module { factory { GeneralResourceProvider( context = androidContext(), + colors = get(named("AccountColors")), ) } diff --git a/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/ui/general/GeneralResourceProvider.kt b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/ui/general/GeneralResourceProvider.kt index c2890c12228..e1ca931e1e6 100644 --- a/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/ui/general/GeneralResourceProvider.kt +++ b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/ui/general/GeneralResourceProvider.kt @@ -6,14 +6,13 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color import androidx.compose.ui.graphics.vector.ImageVector import kotlinx.collections.immutable.ImmutableList -import kotlinx.collections.immutable.toImmutableList import net.thunderbird.feature.account.settings.R import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.ResourceProvider import net.thunderbird.feature.account.settings.impl.ui.general.components.GeneralSettingsProfileView -import app.k9mail.core.ui.legacy.theme2.common.R as ThunderbirdCommonR internal class GeneralResourceProvider( private val context: Context, + override val colors: ImmutableList, ) : ResourceProvider.GeneralResourceProvider { override fun profileUi( @@ -59,6 +58,4 @@ internal class GeneralResourceProvider( context.getString(R.string.account_settings_general_color_description) } override val colorIcon: () -> ImageVector? = { null } - override val colors: ImmutableList = context.resources.getIntArray(ThunderbirdCommonR.array.account_colors) - .toList().toImmutableList() } From 27b4a5a0c0a801cdcca136844bdd630fa4a1d739 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wolf-Martell=20Montw=C3=A9?= Date: Tue, 4 Nov 2025 13:42:32 +0100 Subject: [PATCH 06/10] refactor(account-setting): rename SettingsError to AccountSettingError --- .../impl/domain/AccountSettingsDomainContract.kt | 15 +++++---------- .../impl/domain/usecase/GetAccountName.kt | 7 +++---- .../impl/domain/usecase/GetGeneralSettings.kt | 7 +++---- .../impl/domain/usecase/UpdateGeneralSettings.kt | 12 ++++++------ .../impl/ui/general/GeneralSettingsViewModel.kt | 6 +++--- .../impl/domain/usecase/GetAccountNameTest.kt | 4 ++-- .../impl/domain/usecase/GetGeneralSettingsTest.kt | 4 ++-- .../domain/usecase/UpdateGeneralSettingsTest.kt | 6 +++--- 8 files changed, 27 insertions(+), 34 deletions(-) diff --git a/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/AccountSettingsDomainContract.kt b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/AccountSettingsDomainContract.kt index 977aa0485db..08a8179fd53 100644 --- a/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/AccountSettingsDomainContract.kt +++ b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/AccountSettingsDomainContract.kt @@ -9,28 +9,23 @@ import net.thunderbird.core.outcome.Outcome import net.thunderbird.core.ui.setting.SettingValue import net.thunderbird.core.ui.setting.Settings import net.thunderbird.feature.account.AccountId -import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.SettingsError - -internal typealias AccountNameOutcome = Outcome -internal typealias AccountSettingsOutcome = Outcome internal interface AccountSettingsDomainContract { interface UseCase { - fun interface GetAccountName { - operator fun invoke(accountId: AccountId): Flow + operator fun invoke(accountId: AccountId): Flow> } fun interface GetGeneralSettings { - operator fun invoke(accountId: AccountId): Flow + operator fun invoke(accountId: AccountId): Flow> } fun interface UpdateGeneralSettings { suspend operator fun invoke( accountId: AccountId, setting: SettingValue<*>, - ): Outcome + ): Outcome } } @@ -57,9 +52,9 @@ internal interface AccountSettingsDomainContract { } } - sealed interface SettingsError { + sealed interface AccountSettingError { data class NotFound( val message: String, - ) : SettingsError + ) : AccountSettingError } } diff --git a/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/GetAccountName.kt b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/GetAccountName.kt index 54025e23ad7..924f7b1597e 100644 --- a/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/GetAccountName.kt +++ b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/GetAccountName.kt @@ -5,21 +5,20 @@ import kotlinx.coroutines.flow.map import net.thunderbird.core.outcome.Outcome import net.thunderbird.feature.account.AccountId import net.thunderbird.feature.account.profile.AccountProfileRepository -import net.thunderbird.feature.account.settings.impl.domain.AccountNameOutcome -import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract +import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.AccountSettingError import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.UseCase internal class GetAccountName( private val repository: AccountProfileRepository, ) : UseCase.GetAccountName { - override fun invoke(accountId: AccountId): Flow { + override fun invoke(accountId: AccountId): Flow> { return repository.getById(accountId).map { profile -> if (profile != null) { Outcome.success(profile.name) } else { Outcome.failure( - AccountSettingsDomainContract.SettingsError.NotFound( + AccountSettingError.NotFound( message = "Account profile not found for accountId: ${accountId.asRaw()}", ), ) diff --git a/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/GetGeneralSettings.kt b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/GetGeneralSettings.kt index 6e214b56ea8..5f1457952c1 100644 --- a/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/GetGeneralSettings.kt +++ b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/GetGeneralSettings.kt @@ -18,10 +18,9 @@ import net.thunderbird.feature.account.profile.AccountAvatar import net.thunderbird.feature.account.profile.AccountProfile import net.thunderbird.feature.account.profile.AccountProfileRepository import net.thunderbird.feature.account.settings.AccountSettingsFeatureFlags +import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.AccountSettingError import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.ResourceProvider -import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.SettingsError import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.UseCase -import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsOutcome import net.thunderbird.feature.account.settings.impl.domain.entity.GeneralPreference import net.thunderbird.feature.account.settings.impl.domain.entity.generateId @@ -31,13 +30,13 @@ internal class GetGeneralSettings( private val monogramCreator: AvatarMonogramCreator, private val featureFlagProvider: FeatureFlagProvider, ) : UseCase.GetGeneralSettings { - override fun invoke(accountId: AccountId): Flow { + override fun invoke(accountId: AccountId): Flow> { return repository.getById(accountId).map { profile -> if (profile != null) { Outcome.success(generateSettings(accountId, profile)) } else { Outcome.failure( - SettingsError.NotFound( + AccountSettingError.NotFound( message = "Account profile not found for accountId: ${accountId.asRaw()}", ), ) diff --git a/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/UpdateGeneralSettings.kt b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/UpdateGeneralSettings.kt index 6941309b658..8262cf13649 100644 --- a/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/UpdateGeneralSettings.kt +++ b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/UpdateGeneralSettings.kt @@ -8,7 +8,7 @@ import net.thunderbird.feature.account.AccountId import net.thunderbird.feature.account.profile.AccountAvatar import net.thunderbird.feature.account.profile.AccountProfile import net.thunderbird.feature.account.profile.AccountProfileRepository -import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.SettingsError +import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.AccountSettingError import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.UseCase import net.thunderbird.feature.account.settings.impl.domain.entity.GeneralPreference import net.thunderbird.feature.account.settings.impl.domain.entity.generateId @@ -19,7 +19,7 @@ internal class UpdateGeneralSettings( override suspend fun invoke( accountId: AccountId, setting: SettingValue<*>, - ): Outcome { + ): Outcome { return when (setting.id) { GeneralPreference.PROFILE_INDICATOR.generateId(accountId) -> { val option = setting.value as CompactOption<*> @@ -27,7 +27,7 @@ internal class UpdateGeneralSettings( if (avatar == null) { return Outcome.failure( - SettingsError.NotFound( + AccountSettingError.NotFound( message = "Invalid avatar option selected for accountId: $accountId", ), ) @@ -50,7 +50,7 @@ internal class UpdateGeneralSettings( } else -> Outcome.failure( - SettingsError.NotFound( + AccountSettingError.NotFound( message = "Unknown setting id: ${setting.id}", ), ) @@ -60,10 +60,10 @@ internal class UpdateGeneralSettings( private suspend fun updateAccountProfile( accountId: AccountId, update: AccountProfile.() -> AccountProfile, - ): Outcome { + ): Outcome { val accountProfile = repository.getById(accountId).firstOrNull() ?: return Outcome.failure( - SettingsError.NotFound( + AccountSettingError.NotFound( message = "Account profile not found for accountId: $accountId", ), ) diff --git a/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/ui/general/GeneralSettingsViewModel.kt b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/ui/general/GeneralSettingsViewModel.kt index b075693d76c..fe5507a3f67 100644 --- a/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/ui/general/GeneralSettingsViewModel.kt +++ b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/ui/general/GeneralSettingsViewModel.kt @@ -7,7 +7,7 @@ import net.thunderbird.core.logging.legacy.Log import net.thunderbird.core.outcome.handle import net.thunderbird.core.ui.setting.SettingValue import net.thunderbird.feature.account.AccountId -import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.SettingsError +import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.AccountSettingError import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.UseCase import net.thunderbird.feature.account.settings.impl.ui.general.GeneralSettingsContract.Effect import net.thunderbird.feature.account.settings.impl.ui.general.GeneralSettingsContract.Event @@ -66,9 +66,9 @@ internal class GeneralSettingsViewModel( } } - private fun handleError(error: SettingsError) { + private fun handleError(error: AccountSettingError) { when (error) { - is SettingsError.NotFound -> Log.w(error.message) + is AccountSettingError.NotFound -> Log.w(error.message) } } } diff --git a/feature/account/settings/impl/src/test/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/GetAccountNameTest.kt b/feature/account/settings/impl/src/test/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/GetAccountNameTest.kt index 02f22093a0f..0c79f6d96da 100644 --- a/feature/account/settings/impl/src/test/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/GetAccountNameTest.kt +++ b/feature/account/settings/impl/src/test/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/GetAccountNameTest.kt @@ -10,7 +10,7 @@ import net.thunderbird.core.outcome.Outcome import net.thunderbird.feature.account.AccountIdFactory import net.thunderbird.feature.account.profile.AccountAvatar import net.thunderbird.feature.account.profile.AccountProfile -import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.SettingsError +import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.AccountSettingError import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.UseCase class GetAccountNameTest { @@ -49,7 +49,7 @@ class GetAccountNameTest { assertThat(outcome).isInstanceOf(Outcome.Failure::class) val failure = outcome as Outcome.Failure - assertThat(failure.error).isInstanceOf(SettingsError.NotFound::class) + assertThat(failure.error).isInstanceOf(AccountSettingError.NotFound::class) } } diff --git a/feature/account/settings/impl/src/test/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/GetGeneralSettingsTest.kt b/feature/account/settings/impl/src/test/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/GetGeneralSettingsTest.kt index b0425c0f36f..a4cd365c8cb 100644 --- a/feature/account/settings/impl/src/test/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/GetGeneralSettingsTest.kt +++ b/feature/account/settings/impl/src/test/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/GetGeneralSettingsTest.kt @@ -13,8 +13,8 @@ import net.thunderbird.core.ui.setting.SettingValue import net.thunderbird.feature.account.AccountIdFactory import net.thunderbird.feature.account.profile.AccountAvatar import net.thunderbird.feature.account.profile.AccountProfile +import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.AccountSettingError import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.ResourceProvider -import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.SettingsError import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.UseCase import net.thunderbird.feature.account.settings.impl.domain.entity.GeneralPreference import net.thunderbird.feature.account.settings.impl.domain.entity.generateId @@ -99,7 +99,7 @@ internal class GetGeneralSettingsTest { testSubject(accountId).test { assertThat(awaitItem()).isEqualTo( Outcome.failure( - SettingsError.NotFound( + AccountSettingError.NotFound( message = "Account profile not found for accountId: ${accountId.asRaw()}", ), ), diff --git a/feature/account/settings/impl/src/test/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/UpdateGeneralSettingsTest.kt b/feature/account/settings/impl/src/test/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/UpdateGeneralSettingsTest.kt index 53b6e6768b7..89ef9412a25 100644 --- a/feature/account/settings/impl/src/test/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/UpdateGeneralSettingsTest.kt +++ b/feature/account/settings/impl/src/test/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/UpdateGeneralSettingsTest.kt @@ -14,7 +14,7 @@ import net.thunderbird.core.ui.setting.SettingValue.CompactSelectSingleOption.Co import net.thunderbird.feature.account.AccountIdFactory import net.thunderbird.feature.account.profile.AccountAvatar import net.thunderbird.feature.account.profile.AccountProfile -import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.SettingsError +import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.AccountSettingError import net.thunderbird.feature.account.settings.impl.domain.entity.GeneralPreference import net.thunderbird.feature.account.settings.impl.domain.entity.generateId @@ -120,7 +120,7 @@ class UpdateGeneralSettingsTest { // Assert assertThat(result).isInstanceOf(Outcome.Failure::class) - assertThat((result as Outcome.Failure).error).isInstanceOf(SettingsError.NotFound::class) + assertThat((result as Outcome.Failure).error).isInstanceOf(AccountSettingError.NotFound::class) } @Test @@ -198,7 +198,7 @@ class UpdateGeneralSettingsTest { // Assert assertThat(result).isInstanceOf(Outcome.Failure::class) - assertThat((result as Outcome.Failure).error).isInstanceOf(SettingsError.NotFound::class) + assertThat((result as Outcome.Failure).error).isInstanceOf(AccountSettingError.NotFound::class) // Ensure no change was made assertThat(repository.getById(accountId).firstOrNull()).isEqualTo(accountProfile) } From 8c1492d9d6f77533f611fba373a1c97188271680 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wolf-Martell=20Montw=C3=A9?= Date: Tue, 4 Nov 2025 13:51:43 +0100 Subject: [PATCH 07/10] refactor(account-setting): change update use case to command pattern --- .../domain/AccountSettingsDomainContract.kt | 10 +- .../domain/usecase/UpdateGeneralSettings.kt | 44 ++------ .../ui/general/GeneralSettingsViewModel.kt | 24 ++++- .../usecase/UpdateGeneralSettingsTest.kt | 102 +++--------------- .../general/GeneralSettingsViewModelTest.kt | 41 ++++--- 5 files changed, 83 insertions(+), 138 deletions(-) diff --git a/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/AccountSettingsDomainContract.kt b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/AccountSettingsDomainContract.kt index 08a8179fd53..eb607d5571d 100644 --- a/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/AccountSettingsDomainContract.kt +++ b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/AccountSettingsDomainContract.kt @@ -6,9 +6,9 @@ import androidx.compose.ui.graphics.vector.ImageVector import kotlinx.collections.immutable.ImmutableList import kotlinx.coroutines.flow.Flow import net.thunderbird.core.outcome.Outcome -import net.thunderbird.core.ui.setting.SettingValue import net.thunderbird.core.ui.setting.Settings import net.thunderbird.feature.account.AccountId +import net.thunderbird.feature.account.profile.AccountAvatar internal interface AccountSettingsDomainContract { @@ -24,11 +24,17 @@ internal interface AccountSettingsDomainContract { fun interface UpdateGeneralSettings { suspend operator fun invoke( accountId: AccountId, - setting: SettingValue<*>, + command: UpdateGeneralSettingCommand, ): Outcome } } + sealed interface UpdateGeneralSettingCommand { + data class UpdateName(val value: String) : UpdateGeneralSettingCommand + data class UpdateColor(val value: Int) : UpdateGeneralSettingCommand + data class UpdateAvatar(val value: AccountAvatar) : UpdateGeneralSettingCommand + } + interface ResourceProvider { interface GeneralResourceProvider { fun profileUi( diff --git a/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/UpdateGeneralSettings.kt b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/UpdateGeneralSettings.kt index 8262cf13649..9b727f46fe1 100644 --- a/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/UpdateGeneralSettings.kt +++ b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/UpdateGeneralSettings.kt @@ -2,58 +2,32 @@ package net.thunderbird.feature.account.settings.impl.domain.usecase import kotlinx.coroutines.flow.firstOrNull import net.thunderbird.core.outcome.Outcome -import net.thunderbird.core.ui.setting.SettingValue -import net.thunderbird.core.ui.setting.SettingValue.CompactSelectSingleOption.CompactOption import net.thunderbird.feature.account.AccountId -import net.thunderbird.feature.account.profile.AccountAvatar import net.thunderbird.feature.account.profile.AccountProfile import net.thunderbird.feature.account.profile.AccountProfileRepository import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.AccountSettingError +import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.UpdateGeneralSettingCommand import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.UseCase -import net.thunderbird.feature.account.settings.impl.domain.entity.GeneralPreference -import net.thunderbird.feature.account.settings.impl.domain.entity.generateId internal class UpdateGeneralSettings( private val repository: AccountProfileRepository, ) : UseCase.UpdateGeneralSettings { override suspend fun invoke( accountId: AccountId, - setting: SettingValue<*>, + command: UpdateGeneralSettingCommand, ): Outcome { - return when (setting.id) { - GeneralPreference.PROFILE_INDICATOR.generateId(accountId) -> { - val option = setting.value as CompactOption<*> - val avatar = option.value as? AccountAvatar - - if (avatar == null) { - return Outcome.failure( - AccountSettingError.NotFound( - message = "Invalid avatar option selected for accountId: $accountId", - ), - ) - } - updateAccountProfile(accountId) { - copy(avatar = avatar) - } + return when (command) { + is UpdateGeneralSettingCommand.UpdateName -> updateAccountProfile(accountId) { + copy(name = command.value) } - GeneralPreference.NAME.generateId(accountId) -> { - updateAccountProfile(accountId) { - copy(name = setting.value as String) - } + is UpdateGeneralSettingCommand.UpdateColor -> updateAccountProfile(accountId) { + copy(color = command.value) } - GeneralPreference.COLOR.generateId(accountId) -> { - updateAccountProfile(accountId) { - copy(color = setting.value as Int) - } + is UpdateGeneralSettingCommand.UpdateAvatar -> updateAccountProfile(accountId) { + copy(avatar = command.value) } - - else -> Outcome.failure( - AccountSettingError.NotFound( - message = "Unknown setting id: ${setting.id}", - ), - ) } } diff --git a/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/ui/general/GeneralSettingsViewModel.kt b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/ui/general/GeneralSettingsViewModel.kt index fe5507a3f67..f0994930729 100644 --- a/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/ui/general/GeneralSettingsViewModel.kt +++ b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/ui/general/GeneralSettingsViewModel.kt @@ -7,8 +7,12 @@ import net.thunderbird.core.logging.legacy.Log import net.thunderbird.core.outcome.handle import net.thunderbird.core.ui.setting.SettingValue import net.thunderbird.feature.account.AccountId +import net.thunderbird.feature.account.profile.AccountAvatar import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.AccountSettingError +import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.UpdateGeneralSettingCommand import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.UseCase +import net.thunderbird.feature.account.settings.impl.domain.entity.GeneralPreference +import net.thunderbird.feature.account.settings.impl.domain.entity.generateId import net.thunderbird.feature.account.settings.impl.ui.general.GeneralSettingsContract.Effect import net.thunderbird.feature.account.settings.impl.ui.general.GeneralSettingsContract.Event import net.thunderbird.feature.account.settings.impl.ui.general.GeneralSettingsContract.State @@ -55,14 +59,28 @@ internal class GeneralSettingsViewModel( override fun event(event: Event) { when (event) { - is Event.OnSettingValueChange -> updatePreference(event.setting) + is Event.OnSettingValueChange -> updateSetting(event.setting) is Event.OnBackPressed -> emitEffect(Effect.NavigateBack) } } - private fun updatePreference(setting: SettingValue<*>) { + private fun updateSetting(setting: SettingValue<*>) { + val (id, value) = setting.let { it.id to it.value } + viewModelScope.launch { - updateGeneralSettings(accountId, setting) + val command = when (id) { + GeneralPreference.COLOR.generateId(accountId) -> { + UpdateGeneralSettingCommand.UpdateColor(value as Int) + } + GeneralPreference.NAME.generateId(accountId) -> { + UpdateGeneralSettingCommand.UpdateName(value as String) + } + GeneralPreference.PROFILE_INDICATOR.generateId(accountId) -> { + UpdateGeneralSettingCommand.UpdateAvatar(value as AccountAvatar) + } + else -> null + } + command?.let { updateGeneralSettings(accountId, it) } } } diff --git a/feature/account/settings/impl/src/test/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/UpdateGeneralSettingsTest.kt b/feature/account/settings/impl/src/test/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/UpdateGeneralSettingsTest.kt index 89ef9412a25..98592293128 100644 --- a/feature/account/settings/impl/src/test/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/UpdateGeneralSettingsTest.kt +++ b/feature/account/settings/impl/src/test/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/UpdateGeneralSettingsTest.kt @@ -4,19 +4,14 @@ import assertk.assertThat import assertk.assertions.isEqualTo import assertk.assertions.isInstanceOf import kotlin.test.Test -import kotlinx.collections.immutable.persistentListOf import kotlinx.coroutines.flow.firstOrNull import kotlinx.coroutines.test.runTest import net.thunderbird.core.outcome.Outcome -import net.thunderbird.core.ui.setting.SettingValue -import net.thunderbird.core.ui.setting.SettingValue.CompactSelectSingleOption -import net.thunderbird.core.ui.setting.SettingValue.CompactSelectSingleOption.CompactOption import net.thunderbird.feature.account.AccountIdFactory import net.thunderbird.feature.account.profile.AccountAvatar import net.thunderbird.feature.account.profile.AccountProfile import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.AccountSettingError -import net.thunderbird.feature.account.settings.impl.domain.entity.GeneralPreference -import net.thunderbird.feature.account.settings.impl.domain.entity.generateId +import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.UpdateGeneralSettingCommand class UpdateGeneralSettingsTest { @@ -31,20 +26,14 @@ class UpdateGeneralSettingsTest { avatar = AccountAvatar.Icon(name = "star"), ) val newName = "Updated Account Name" - val setting = SettingValue.Text( - id = GeneralPreference.NAME.generateId(accountId), - title = { "Name" }, - description = { "Account name" }, - icon = { null }, - value = newName, - ) + val command = UpdateGeneralSettingCommand.UpdateName(newName) val repository = FakeAccountProfileRepository( initialAccountProfile = accountProfile, ) val testSubject = UpdateGeneralSettings(repository) // Act - val result = testSubject(accountId, setting) + val result = testSubject(accountId, command) // Assert assertThat(result).isInstanceOf(Outcome.Success::class) @@ -65,22 +54,9 @@ class UpdateGeneralSettingsTest { ) val newName = "Updated Account Name" val newColor = 0x00FF00 - val settings = listOf( - SettingValue.Text( - id = GeneralPreference.NAME.generateId(accountId), - title = { "Name" }, - description = { "Account name" }, - icon = { null }, - value = newName, - ), - SettingValue.Color( - id = GeneralPreference.COLOR.generateId(accountId), - title = { "Color" }, - description = { "Account color" }, - icon = { null }, - value = newColor, - colors = persistentListOf(0xFF0000, 0x00FF00, 0x0000FF), - ), + val commands = listOf( + UpdateGeneralSettingCommand.UpdateName(newName), + UpdateGeneralSettingCommand.UpdateColor(newColor), ) val repository = FakeAccountProfileRepository( initialAccountProfile = accountProfile, @@ -88,8 +64,8 @@ class UpdateGeneralSettingsTest { val testSubject = UpdateGeneralSettings(repository) // Act - settings.forEach { setting -> - testSubject(accountId, setting) + commands.forEach { command -> + testSubject(accountId, command) } // Assert @@ -105,18 +81,12 @@ class UpdateGeneralSettingsTest { fun `should emit NotFound when account profile not found`() = runTest { // Arrange val accountId = AccountIdFactory.create() - val setting = SettingValue.Text( - id = GeneralPreference.NAME.generateId(accountId), - title = { "Name" }, - description = { "Account name" }, - icon = { null }, - value = "Updated Account Name", - ) + val command = UpdateGeneralSettingCommand.UpdateName("Updated Account Name") val repository = FakeAccountProfileRepository() val testSubject = UpdateGeneralSettings(repository) // Act - val result = testSubject(accountId, setting) + val result = testSubject(accountId, command) // Assert assertThat(result).isInstanceOf(Outcome.Failure::class) @@ -124,7 +94,7 @@ class UpdateGeneralSettingsTest { } @Test - fun `should update avatar when PROFILE_INDICATOR CompactOption is selected`() = runTest { + fun `should update avatar when UpdateAvatar command is used`() = runTest { // Arrange val accountId = AccountIdFactory.create() val accountProfile = AccountProfile( @@ -134,27 +104,11 @@ class UpdateGeneralSettingsTest { avatar = AccountAvatar.Icon(name = "star"), ) val imageAvatar = AccountAvatar.Image(uri = "avatar://uri") - val imageOption: CompactOption = CompactOption( - id = "${accountId.asRaw()}-profile-indicator-image", - title = { "Image" }, - value = imageAvatar, - ) - val iconOption: CompactOption = CompactOption( - id = "${accountId.asRaw()}-profile-indicator-icon", - title = { "Icon" }, - value = AccountAvatar.Icon("user"), - ) - val setting: CompactSelectSingleOption = CompactSelectSingleOption( - id = GeneralPreference.PROFILE_INDICATOR.generateId(accountId), - title = { "Profile Indicator" }, - value = imageOption, - options = persistentListOf(imageOption, iconOption), - ) val repository = FakeAccountProfileRepository(initialAccountProfile = accountProfile) val testSubject = UpdateGeneralSettings(repository) // Act - val result = testSubject(accountId, setting) + val result = testSubject(accountId, UpdateGeneralSettingCommand.UpdateAvatar(imageAvatar)) // Assert assertThat(result).isInstanceOf(Outcome.Success::class) @@ -164,42 +118,18 @@ class UpdateGeneralSettingsTest { } @Test - fun `should return failure when PROFILE_INDICATOR value is invalid`() = runTest { + fun `should emit NotFound when updating avatar for non-existing account`() = runTest { // Arrange val accountId = AccountIdFactory.create() - val accountProfile = AccountProfile( - id = accountId, - name = "Test Account", - color = 0xFF0000, - avatar = AccountAvatar.Icon(name = "star"), - ) - // Construct a CompactSelectSingleOption with wrong inner value type - val invalidOption: CompactOption = CompactOption( - id = "${accountId.asRaw()}-profile-indicator-invalid", - title = { "Invalid" }, - value = "invalid", - ) - val otherOption: CompactOption = CompactOption( - id = "${accountId.asRaw()}-profile-indicator-icon", - title = { "Icon" }, - value = AccountAvatar.Icon("user"), - ) - val invalidSetting: CompactSelectSingleOption = CompactSelectSingleOption( - id = GeneralPreference.PROFILE_INDICATOR.generateId(accountId), - title = { "Profile Indicator" }, - value = invalidOption, - options = persistentListOf(invalidOption, otherOption), - ) - val repository = FakeAccountProfileRepository(initialAccountProfile = accountProfile) + val imageAvatar = AccountAvatar.Image(uri = "avatar://uri") + val repository = FakeAccountProfileRepository() val testSubject = UpdateGeneralSettings(repository) // Act - val result = testSubject(accountId, invalidSetting) + val result = testSubject(accountId, UpdateGeneralSettingCommand.UpdateAvatar(imageAvatar)) // Assert assertThat(result).isInstanceOf(Outcome.Failure::class) assertThat((result as Outcome.Failure).error).isInstanceOf(AccountSettingError.NotFound::class) - // Ensure no change was made - assertThat(repository.getById(accountId).firstOrNull()).isEqualTo(accountProfile) } } diff --git a/feature/account/settings/impl/src/test/kotlin/net/thunderbird/feature/account/settings/impl/ui/general/GeneralSettingsViewModelTest.kt b/feature/account/settings/impl/src/test/kotlin/net/thunderbird/feature/account/settings/impl/ui/general/GeneralSettingsViewModelTest.kt index 419c099826f..5a618186c02 100644 --- a/feature/account/settings/impl/src/test/kotlin/net/thunderbird/feature/account/settings/impl/ui/general/GeneralSettingsViewModelTest.kt +++ b/feature/account/settings/impl/src/test/kotlin/net/thunderbird/feature/account/settings/impl/ui/general/GeneralSettingsViewModelTest.kt @@ -22,11 +22,23 @@ import net.thunderbird.core.ui.setting.Settings import net.thunderbird.core.ui.setting.emptySettings import net.thunderbird.feature.account.AccountId import net.thunderbird.feature.account.AccountIdFactory +import net.thunderbird.feature.account.settings.impl.domain.entity.GeneralPreference +import net.thunderbird.feature.account.settings.impl.domain.entity.generateId import net.thunderbird.feature.account.settings.impl.ui.general.GeneralSettingsContract.Effect import net.thunderbird.feature.account.settings.impl.ui.general.GeneralSettingsContract.State import org.junit.Before import org.junit.Rule +private fun createSettings(accountId: AccountId): Settings = persistentListOf( + SettingValue.Text( + id = GeneralPreference.NAME.generateId(accountId), + title = { "Title" }, + description = { "Description" }, + icon = { null }, + value = "Test", + ), +) + class GeneralSettingsViewModelTest { @get:Rule @@ -87,7 +99,7 @@ class GeneralSettingsViewModelTest { subtitle = "Subtitle", settings = emptySettings(), ) - val settings = FakeData.settings + val settings = createSettings(accountId) generalSettingsRobot(accountId, initialState, settings) { verifyGeneralSettingsLoaded(settings) @@ -120,6 +132,7 @@ private class GeneralSettingsRobot( ) { private lateinit var settingsState: MutableStateFlow private lateinit var turbines: MviTurbines + private var lastSetting: SettingValue<*>? = null private val viewModel: GeneralSettingsContract.ViewModel by lazy { GeneralSettingsViewModel( @@ -133,17 +146,20 @@ private class GeneralSettingsRobot( Outcome.success(it) } }, - updateGeneralSettings = { _, setting -> - settingsState.value = settingsState.value.map { existingSetting -> - if (existingSetting is SettingValue<*> && existingSetting.id == setting.id) { - println("Updating setting: ${setting.id}") - println("Old setting: $existingSetting") - println("New setting: $setting") - setting - } else { - existingSetting - } - }.toImmutableList() + updateGeneralSettings = { _, _ -> + val newSetting = lastSetting + if (newSetting != null) { + settingsState.value = settingsState.value.map { existingSetting -> + if (existingSetting is SettingValue<*> && existingSetting.id == newSetting.id) { + println("Updating setting: ${newSetting.id}") + println("Old setting: $existingSetting") + println("New setting: $newSetting") + newSetting + } else { + existingSetting + } + }.toImmutableList() + } Outcome.success(Unit) }, initialState = initialState, @@ -186,6 +202,7 @@ private class GeneralSettingsRobot( } fun updateSetting(setting: SettingValue<*>) { + lastSetting = setting viewModel.event(GeneralSettingsContract.Event.OnSettingValueChange(setting)) } From 527254263ec5046c07292fa62a80817df7ea33de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wolf-Martell=20Montw=C3=A9?= Date: Tue, 4 Nov 2025 14:09:41 +0100 Subject: [PATCH 08/10] feat(account-setting): add account name and avatar validators --- .../account/settings/impl/build.gradle.kts | 1 + .../account/settings/AccountSettingsModule.kt | 11 +++++ .../domain/AccountSettingsDomainContract.kt | 9 ++++ .../domain/usecase/ValidateAccountName.kt | 21 +++++++++ .../domain/usecase/ValidateAvatarMonogram.kt | 25 ++++++++++ .../ui/general/GeneralSettingsContract.kt | 6 +++ .../ui/general/GeneralSettingsValidator.kt | 13 +++++ .../domain/usecase/ValidateAccountNameTest.kt | 38 +++++++++++++++ .../usecase/ValidateAvatarMonogramTest.kt | 47 +++++++++++++++++++ 9 files changed, 171 insertions(+) create mode 100644 feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/ValidateAccountName.kt create mode 100644 feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/ValidateAvatarMonogram.kt create mode 100644 feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/ui/general/GeneralSettingsValidator.kt create mode 100644 feature/account/settings/impl/src/test/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/ValidateAccountNameTest.kt create mode 100644 feature/account/settings/impl/src/test/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/ValidateAvatarMonogramTest.kt diff --git a/feature/account/settings/impl/build.gradle.kts b/feature/account/settings/impl/build.gradle.kts index cbf9eff838c..98d26dd6838 100644 --- a/feature/account/settings/impl/build.gradle.kts +++ b/feature/account/settings/impl/build.gradle.kts @@ -22,6 +22,7 @@ dependencies { implementation(projects.core.featureflag) implementation(projects.core.outcome) + implementation(projects.core.validation) implementation(projects.core.ui.setting.api) diff --git a/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/AccountSettingsModule.kt b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/AccountSettingsModule.kt index 3dccba6c3ce..c953e3a816a 100644 --- a/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/AccountSettingsModule.kt +++ b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/AccountSettingsModule.kt @@ -7,7 +7,11 @@ import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomai import net.thunderbird.feature.account.settings.impl.domain.usecase.GetAccountName import net.thunderbird.feature.account.settings.impl.domain.usecase.GetGeneralSettings import net.thunderbird.feature.account.settings.impl.domain.usecase.UpdateGeneralSettings +import net.thunderbird.feature.account.settings.impl.domain.usecase.ValidateAccountName +import net.thunderbird.feature.account.settings.impl.domain.usecase.ValidateAvatarMonogram import net.thunderbird.feature.account.settings.impl.ui.general.GeneralResourceProvider +import net.thunderbird.feature.account.settings.impl.ui.general.GeneralSettingsContract +import net.thunderbird.feature.account.settings.impl.ui.general.GeneralSettingsValidator import net.thunderbird.feature.account.settings.impl.ui.general.GeneralSettingsViewModel import org.koin.android.ext.koin.androidContext import org.koin.core.module.dsl.viewModel @@ -45,6 +49,13 @@ val featureAccountSettingsModule = module { ) } + factory { + GeneralSettingsValidator( + accountNameValidator = ValidateAccountName(), + avatarMonogramValidator = ValidateAvatarMonogram(), + ) + } + viewModel { params -> GeneralSettingsViewModel( accountId = params.get(), diff --git a/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/AccountSettingsDomainContract.kt b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/AccountSettingsDomainContract.kt index eb607d5571d..73d6a30efcd 100644 --- a/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/AccountSettingsDomainContract.kt +++ b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/AccountSettingsDomainContract.kt @@ -7,6 +7,7 @@ import kotlinx.collections.immutable.ImmutableList import kotlinx.coroutines.flow.Flow import net.thunderbird.core.outcome.Outcome import net.thunderbird.core.ui.setting.Settings +import net.thunderbird.core.validation.ValidationOutcome import net.thunderbird.feature.account.AccountId import net.thunderbird.feature.account.profile.AccountAvatar @@ -27,6 +28,14 @@ internal interface AccountSettingsDomainContract { command: UpdateGeneralSettingCommand, ): Outcome } + + fun interface ValidateAccountName { + operator fun invoke(name: String): ValidationOutcome + } + + fun interface ValidateAvatarMonogram { + operator fun invoke(monogram: String): ValidationOutcome + } } sealed interface UpdateGeneralSettingCommand { diff --git a/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/ValidateAccountName.kt b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/ValidateAccountName.kt new file mode 100644 index 00000000000..1e704c23d2b --- /dev/null +++ b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/ValidateAccountName.kt @@ -0,0 +1,21 @@ +package net.thunderbird.feature.account.settings.impl.domain.usecase + +import net.thunderbird.core.outcome.Outcome +import net.thunderbird.core.validation.ValidationError +import net.thunderbird.core.validation.ValidationOutcome +import net.thunderbird.core.validation.ValidationSuccess +import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.UseCase + +internal class ValidateAccountName : UseCase.ValidateAccountName { + + override fun invoke(name: String): ValidationOutcome { + return when { + name.isBlank() -> Outcome.Failure(ValidateAccountNameError.EmptyName) + else -> ValidationSuccess + } + } + + sealed interface ValidateAccountNameError : ValidationError { + data object EmptyName : ValidateAccountNameError + } +} diff --git a/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/ValidateAvatarMonogram.kt b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/ValidateAvatarMonogram.kt new file mode 100644 index 00000000000..19eb51bcb00 --- /dev/null +++ b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/ValidateAvatarMonogram.kt @@ -0,0 +1,25 @@ +package net.thunderbird.feature.account.settings.impl.domain.usecase + +import net.thunderbird.core.outcome.Outcome +import net.thunderbird.core.validation.ValidationError +import net.thunderbird.core.validation.ValidationOutcome +import net.thunderbird.core.validation.ValidationSuccess +import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.UseCase + +internal class ValidateAvatarMonogram : UseCase.ValidateAvatarMonogram { + + override fun invoke(monogram: String): ValidationOutcome = when { + monogram.isBlank() -> Outcome.Failure(ValidateAvatarMonogramError.EmptyMonogram) + monogram.length > MAX_MONOGRAM_LENGTH -> Outcome.Failure(ValidateAvatarMonogramError.TooLongMonogram) + else -> ValidationSuccess + } + + sealed interface ValidateAvatarMonogramError : ValidationError { + data object EmptyMonogram : ValidateAvatarMonogramError + data object TooLongMonogram : ValidateAvatarMonogramError + } + + private companion object { + const val MAX_MONOGRAM_LENGTH = 3 + } +} diff --git a/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/ui/general/GeneralSettingsContract.kt b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/ui/general/GeneralSettingsContract.kt index ee63daa4fed..29458c6c6df 100644 --- a/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/ui/general/GeneralSettingsContract.kt +++ b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/ui/general/GeneralSettingsContract.kt @@ -5,6 +5,7 @@ import app.k9mail.core.ui.compose.common.mvi.UnidirectionalViewModel import net.thunderbird.core.ui.setting.SettingValue import net.thunderbird.core.ui.setting.Settings import net.thunderbird.core.ui.setting.emptySettings +import net.thunderbird.core.validation.ValidationOutcome internal interface GeneralSettingsContract { @@ -27,4 +28,9 @@ internal interface GeneralSettingsContract { sealed interface Effect { object NavigateBack : Effect } + + interface Validator { + fun validateName(name: String): ValidationOutcome + fun validateMonogram(monogram: String): ValidationOutcome + } } diff --git a/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/ui/general/GeneralSettingsValidator.kt b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/ui/general/GeneralSettingsValidator.kt new file mode 100644 index 00000000000..1a329f148dd --- /dev/null +++ b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/ui/general/GeneralSettingsValidator.kt @@ -0,0 +1,13 @@ +package net.thunderbird.feature.account.settings.impl.ui.general + +import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.UseCase.ValidateAccountName +import net.thunderbird.feature.account.settings.impl.domain.AccountSettingsDomainContract.UseCase.ValidateAvatarMonogram + +internal class GeneralSettingsValidator( + private val accountNameValidator: ValidateAccountName, + private val avatarMonogramValidator: ValidateAvatarMonogram, +) : GeneralSettingsContract.Validator { + override fun validateName(name: String) = accountNameValidator(name) + + override fun validateMonogram(monogram: String) = avatarMonogramValidator(monogram) +} diff --git a/feature/account/settings/impl/src/test/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/ValidateAccountNameTest.kt b/feature/account/settings/impl/src/test/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/ValidateAccountNameTest.kt new file mode 100644 index 00000000000..3b1dc31fc9d --- /dev/null +++ b/feature/account/settings/impl/src/test/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/ValidateAccountNameTest.kt @@ -0,0 +1,38 @@ +package net.thunderbird.feature.account.settings.impl.domain.usecase + +import assertk.assertThat +import assertk.assertions.isInstanceOf +import assertk.assertions.prop +import kotlin.test.Test +import net.thunderbird.core.outcome.Outcome +import net.thunderbird.core.validation.ValidationError + +class ValidateAccountNameTest { + + private val testSubject = ValidateAccountName() + + @Test + fun `should succeed when name is valid`() { + val result = testSubject("Valid Name") + + assertThat(result).isInstanceOf>() + } + + @Test + fun `should fail when name is empty`() { + val result = testSubject("") + + assertThat(result).isInstanceOf>() + .prop(Outcome.Failure::error) + .isInstanceOf() + } + + @Test + fun `should fail when name is blank`() { + val result = testSubject(" ") + + assertThat(result).isInstanceOf>() + .prop(Outcome.Failure::error) + .isInstanceOf() + } +} diff --git a/feature/account/settings/impl/src/test/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/ValidateAvatarMonogramTest.kt b/feature/account/settings/impl/src/test/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/ValidateAvatarMonogramTest.kt new file mode 100644 index 00000000000..8aab636f804 --- /dev/null +++ b/feature/account/settings/impl/src/test/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/ValidateAvatarMonogramTest.kt @@ -0,0 +1,47 @@ +package net.thunderbird.feature.account.settings.impl.domain.usecase + +import assertk.assertThat +import assertk.assertions.isInstanceOf +import assertk.assertions.prop +import kotlin.test.Test +import net.thunderbird.core.outcome.Outcome +import net.thunderbird.core.validation.ValidationError + +class ValidateAvatarMonogramTest { + + private val testSubject = ValidateAvatarMonogram() + + @Test + fun `should succeed when monogram length is between 1 and 3`() { + assertThat(testSubject("A")).isInstanceOf>() + assertThat(testSubject("AB")).isInstanceOf>() + assertThat(testSubject("ABC")).isInstanceOf>() + } + + @Test + fun `should fail when monogram is empty`() { + val result = testSubject("") + + assertThat(result).isInstanceOf>() + .prop(Outcome.Failure::error) + .isInstanceOf() + } + + @Test + fun `should fail when monogram is blank`() { + val result = testSubject(" ") + + assertThat(result).isInstanceOf>() + .prop(Outcome.Failure::error) + .isInstanceOf() + } + + @Test + fun `should fail when monogram is longer than 3 characters`() { + val result = testSubject("ABCD") + + assertThat(result).isInstanceOf>() + .prop(Outcome.Failure::error) + .isInstanceOf() + } +} From 0125f50172c6da329b6df40cf6188d8da99aef75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wolf-Martell=20Montw=C3=A9?= Date: Tue, 4 Nov 2025 10:36:55 +0100 Subject: [PATCH 09/10] feat(core-validation): add integer input field --- .../validation/input/IntegerInputField.kt | 76 +++++++++++++++++++ .../core/validation/input/InputFieldTest.kt | 10 +++ 2 files changed, 86 insertions(+) create mode 100644 core/validation/src/commonMain/kotlin/net/thunderbird/core/validation/input/IntegerInputField.kt diff --git a/core/validation/src/commonMain/kotlin/net/thunderbird/core/validation/input/IntegerInputField.kt b/core/validation/src/commonMain/kotlin/net/thunderbird/core/validation/input/IntegerInputField.kt new file mode 100644 index 00000000000..d8edbfcd816 --- /dev/null +++ b/core/validation/src/commonMain/kotlin/net/thunderbird/core/validation/input/IntegerInputField.kt @@ -0,0 +1,76 @@ +package net.thunderbird.core.validation.input + +import net.thunderbird.core.outcome.Outcome +import net.thunderbird.core.validation.ValidationError +import net.thunderbird.core.validation.ValidationOutcome + +class IntegerInputField( + override val value: Int? = null, + override val error: ValidationError? = null, + override val isValid: Boolean = false, +) : InputField { + + override fun updateValue(value: Int?): IntegerInputField { + return IntegerInputField( + value = value, + error = null, + isValid = false, + ) + } + + override fun updateError(error: ValidationError?): IntegerInputField { + return IntegerInputField( + value = value, + error = error, + isValid = false, + ) + } + + override fun updateValidity(isValid: Boolean): IntegerInputField { + if (isValid == this.isValid) return this + + return IntegerInputField( + value = value, + error = null, + isValid = isValid, + ) + } + + override fun updateFromValidationOutcome(outcome: ValidationOutcome): IntegerInputField { + return when (outcome) { + is Outcome.Success -> IntegerInputField( + value = value, + error = null, + isValid = true, + ) + + is Outcome.Failure -> IntegerInputField( + value = value, + error = outcome.error, + isValid = false, + ) + } + } + + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (javaClass != other?.javaClass) return false + + other as IntegerInputField + + if (value != other.value) return false + if (error != other.error) return false + return isValid == other.isValid + } + + override fun hashCode(): Int { + var result = value?.hashCode() ?: 0 + result = 31 * result + (error?.hashCode() ?: 0) + result = 31 * result + isValid.hashCode() + return result + } + + override fun toString(): String { + return "IntegerInputField(value=$value, error=$error, isValid=$isValid)" + } +} diff --git a/core/validation/src/commonTest/kotlin/net/thunderbird/core/validation/input/InputFieldTest.kt b/core/validation/src/commonTest/kotlin/net/thunderbird/core/validation/input/InputFieldTest.kt index 5dedac1473f..e924b755027 100644 --- a/core/validation/src/commonTest/kotlin/net/thunderbird/core/validation/input/InputFieldTest.kt +++ b/core/validation/src/commonTest/kotlin/net/thunderbird/core/validation/input/InputFieldTest.kt @@ -243,6 +243,16 @@ class InputFieldTest( initialIsValid = false, updatedValue = 456L, ), + InputFieldTestData( + name = "IntegerInputField", + createInitialInput = { value, error, isValid -> IntegerInputField(value, error, isValid) }, + initialState = IntegerInputField(), + initialValue = 234, + initialValueEmpty = null, + initialError = null, + initialIsValid = false, + updatedValue = 567, + ), InputFieldTestData( name = "BooleanInputField", createInitialInput = { value, error, isValid -> BooleanInputField(value, error, isValid) }, From c5e31cbe6d18319200276fade86d353e616eb5b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wolf-Martell=20Montw=C3=A9?= Date: Tue, 4 Nov 2025 16:01:00 +0100 Subject: [PATCH 10/10] refactor(account-setting): change the init to launchIn --- .../ui/general/GeneralSettingsViewModel.kt | 45 ++++++++++--------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/ui/general/GeneralSettingsViewModel.kt b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/ui/general/GeneralSettingsViewModel.kt index f0994930729..9303b6fe6f3 100644 --- a/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/ui/general/GeneralSettingsViewModel.kt +++ b/feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/ui/general/GeneralSettingsViewModel.kt @@ -2,6 +2,8 @@ package net.thunderbird.feature.account.settings.impl.ui.general import androidx.lifecycle.viewModelScope import app.k9mail.core.ui.compose.common.mvi.BaseViewModel +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.launch import net.thunderbird.core.logging.legacy.Log import net.thunderbird.core.outcome.handle @@ -26,23 +28,30 @@ internal class GeneralSettingsViewModel( ) : BaseViewModel(initialState), GeneralSettingsContract.ViewModel { init { - viewModelScope.launch { - getAccountName(accountId).collect { outcome -> + observeAccountName() + observeGeneralSettings() + } + + override fun event(event: Event) { + when (event) { + is Event.OnBackPressed -> emitEffect(Effect.NavigateBack) + is Event.OnSettingValueChange -> updateSetting(event.setting) + } + } + + private fun observeAccountName() { + getAccountName(accountId) + .onEach { outcome -> outcome.handle( - onSuccess = { accountName -> - updateState { state -> - state.copy( - subtitle = accountName, - ) - } - }, + onSuccess = { updateState { state -> state.copy(subtitle = it) } }, onFailure = { handleError(it) }, ) - } - } + }.launchIn(viewModelScope) + } - viewModelScope.launch { - getGeneralSettings(accountId).collect { outcome -> + private fun observeGeneralSettings() { + getGeneralSettings(accountId) + .onEach { outcome -> outcome.handle( onSuccess = { settings -> updateState { state -> @@ -53,15 +62,7 @@ internal class GeneralSettingsViewModel( }, onFailure = { handleError(it) }, ) - } - } - } - - override fun event(event: Event) { - when (event) { - is Event.OnSettingValueChange -> updateSetting(event.setting) - is Event.OnBackPressed -> emitEffect(Effect.NavigateBack) - } + }.launchIn(viewModelScope) } private fun updateSetting(setting: SettingValue<*>) {