diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenter.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenter.kt index 7106b53503..85aadbb06b 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenter.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenter.kt @@ -13,17 +13,18 @@ import androidx.compose.runtime.MutableState import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue import androidx.compose.runtime.key -import androidx.compose.runtime.mutableStateMapOf +import androidx.compose.runtime.mutableStateListOf import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.produceState import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue -import androidx.compose.runtime.snapshots.SnapshotStateMap +import androidx.compose.runtime.snapshots.SnapshotStateList import dev.zacsweers.metro.Inject import io.element.android.features.enterprise.api.EnterpriseService import io.element.android.features.preferences.impl.developer.tracing.toLogLevel import io.element.android.features.preferences.impl.developer.tracing.toLogLevelItem +import io.element.android.features.preferences.impl.model.EnabledFeature import io.element.android.features.preferences.impl.tasks.ClearCacheUseCase import io.element.android.features.preferences.impl.tasks.ComputeCacheSizeUseCase import io.element.android.features.rageshake.api.preferences.RageshakePreferencesState @@ -31,15 +32,14 @@ import io.element.android.libraries.architecture.AsyncAction import io.element.android.libraries.architecture.AsyncData import io.element.android.libraries.architecture.Presenter import io.element.android.libraries.architecture.runCatchingUpdatingState -import io.element.android.libraries.core.bool.orFalse import io.element.android.libraries.core.extensions.runCatchingExceptions import io.element.android.libraries.core.meta.BuildMeta import io.element.android.libraries.core.meta.BuildType -import io.element.android.libraries.featureflag.api.Feature import io.element.android.libraries.featureflag.api.FeatureFlagService import io.element.android.libraries.featureflag.api.FeatureFlags import io.element.android.libraries.featureflag.ui.model.FeatureUiModel import io.element.android.libraries.preferences.api.store.AppPreferencesStore +import kotlinx.collections.immutable.ImmutableList import kotlinx.collections.immutable.persistentListOf import kotlinx.collections.immutable.toImmutableList import kotlinx.coroutines.CoroutineScope @@ -61,12 +61,8 @@ class DeveloperSettingsPresenter( @Composable override fun present(): DeveloperSettingsState { val rageshakeState = rageshakePresenter.present() - - val features = remember { - mutableStateMapOf() - } val enabledFeatures = remember { - mutableStateMapOf() + mutableStateListOf() } val cacheSize = remember { mutableStateOf>(AsyncData.Uninitialized) @@ -89,13 +85,12 @@ class DeveloperSettingsPresenter( val tracingLogPacks by produceState(persistentListOf()) { appPreferencesStore.getTracingLogPacksFlow() // Sort the entries alphabetically by its title - .map { it.sortedBy { it.title } } + .map { it.sortedBy { pack -> pack.title } } .collectLatest { value = it.toImmutableList() } } LaunchedEffect(Unit) { featureFlagService.getAvailableFeatures() - .filter { it.isInLabs.not() && it.isFinished.not() } .run { // Never display room directory search in release builds for Play Store if (buildMeta.flavorDescription == "GooglePlay" && buildMeta.buildType == BuildType.RELEASE) { @@ -105,11 +100,10 @@ class DeveloperSettingsPresenter( } } .forEach { feature -> - features[feature.key] = feature - enabledFeatures[feature.key] = featureFlagService.isFeatureEnabled(feature) + enabledFeatures.add(EnabledFeature(feature, featureFlagService.isFeatureEnabled(feature))) } } - val featureUiModels = createUiModels(features, enabledFeatures) + val featureUiModels = createUiModels(enabledFeatures) val coroutineScope = rememberCoroutineScope() // Compute cache size each time the clear cache action value is changed LaunchedEffect(clearCacheAction.value.isSuccess()) { @@ -119,10 +113,9 @@ class DeveloperSettingsPresenter( fun handleEvents(event: DeveloperSettingsEvents) { when (event) { is DeveloperSettingsEvents.UpdateEnabledFeature -> coroutineScope.updateEnabledFeature( - features, - enabledFeatures, - event.feature, - event.isEnabled, + enabledFeatures = enabledFeatures, + featureKey = event.feature.key, + enabled = event.isEnabled, triggerClearCache = { handleEvents(DeveloperSettingsEvents.ClearCache) } ) is DeveloperSettingsEvents.SetCustomElementCallBaseUrl -> coroutineScope.launch { @@ -154,7 +147,7 @@ class DeveloperSettingsPresenter( } return DeveloperSettingsState( - features = featureUiModels.toImmutableList(), + features = featureUiModels, cacheSize = cacheSize.value, clearCacheAction = clearCacheAction.value, rageshakeState = rageshakeState, @@ -172,35 +165,33 @@ class DeveloperSettingsPresenter( @Composable private fun createUiModels( - features: SnapshotStateMap, - enabledFeatures: SnapshotStateMap - ): List { - return features.values.map { feature -> - key(feature.key) { - val isEnabled = enabledFeatures[feature.key].orFalse() - remember(feature, isEnabled) { + enabledFeatures: SnapshotStateList, + ): ImmutableList { + return enabledFeatures.map { enabledFeature -> + key(enabledFeature.feature.key) { + remember(enabledFeature) { FeatureUiModel( - key = feature.key, - title = feature.title, - description = feature.description, + key = enabledFeature.feature.key, + title = enabledFeature.feature.title, + description = enabledFeature.feature.description, icon = null, - isEnabled = isEnabled + isEnabled = enabledFeature.isEnabled ) } } - } + }.toImmutableList() } private fun CoroutineScope.updateEnabledFeature( - features: SnapshotStateMap, - enabledFeatures: SnapshotStateMap, - featureUiModel: FeatureUiModel, + enabledFeatures: SnapshotStateList, + featureKey: String, enabled: Boolean, @Suppress("UNUSED_PARAMETER") triggerClearCache: () -> Unit, ) = launch { - val feature = features[featureUiModel.key] ?: return@launch + val featureIndex = enabledFeatures.indexOfFirst { it.feature.key == featureKey }.takeIf { it != -1 } ?: return@launch + val feature = enabledFeatures[featureIndex].feature if (featureFlagService.setFeatureEnabled(feature, enabled)) { - enabledFeatures[featureUiModel.key] = enabled + enabledFeatures[featureIndex] = enabledFeatures[featureIndex].copy(isEnabled = enabled) } } diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsState.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsState.kt index 389297a46b..e69f315a36 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsState.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsState.kt @@ -26,7 +26,9 @@ data class DeveloperSettingsState( val isEnterpriseBuild: Boolean, val showColorPicker: Boolean, val eventSink: (DeveloperSettingsEvents) -> Unit -) +) { + val showLoader = clearCacheAction is AsyncAction.Loading +} data class CustomElementCallBaseUrlState( val baseUrl: String?, diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsView.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsView.kt index e00716e2ce..d60b210381 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsView.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsView.kt @@ -7,6 +7,7 @@ package io.element.android.features.preferences.impl.developer +import androidx.activity.compose.BackHandler import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.foundation.progressSemantics @@ -22,6 +23,7 @@ import io.element.android.compound.theme.ElementTheme import io.element.android.features.preferences.impl.R import io.element.android.features.preferences.impl.developer.tracing.LogLevelItem import io.element.android.features.rageshake.api.preferences.RageshakePreferencesView +import io.element.android.libraries.designsystem.components.ProgressDialog import io.element.android.libraries.designsystem.components.list.ListItemContent import io.element.android.libraries.designsystem.components.preferences.PreferenceCategory import io.element.android.libraries.designsystem.components.preferences.PreferenceDropdown @@ -50,9 +52,20 @@ fun DeveloperSettingsView( onBackClick: () -> Unit, modifier: Modifier = Modifier, ) { + if (state.showLoader) { + ProgressDialog() + } + BackHandler( + enabled = !state.showLoader, + onBack = onBackClick, + ) PreferencePage( modifier = modifier, - onBackClick = onBackClick, + onBackClick = { + if (!state.showLoader) { + onBackClick() + } + }, title = stringResource(id = CommonStrings.common_developer_options) ) { // Note: this is OK to hardcode strings in this debug screen. diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/labs/LabsPresenter.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/labs/LabsPresenter.kt index 5e74454d75..9ebd9e56e3 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/labs/LabsPresenter.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/labs/LabsPresenter.kt @@ -11,20 +11,19 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.key -import androidx.compose.runtime.mutableStateMapOf +import androidx.compose.runtime.mutableStateListOf import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue -import androidx.compose.runtime.snapshots.SnapshotStateMap +import androidx.compose.runtime.snapshots.SnapshotStateList import dev.zacsweers.metro.Inject import io.element.android.compound.tokens.generated.CompoundIcons import io.element.android.features.preferences.impl.R +import io.element.android.features.preferences.impl.model.EnabledFeature import io.element.android.features.preferences.impl.tasks.ClearCacheUseCase import io.element.android.libraries.architecture.Presenter -import io.element.android.libraries.core.bool.orFalse import io.element.android.libraries.designsystem.theme.components.IconSource -import io.element.android.libraries.featureflag.api.Feature import io.element.android.libraries.featureflag.api.FeatureFlagService import io.element.android.libraries.featureflag.api.FeatureFlags import io.element.android.libraries.featureflag.ui.model.FeatureUiModel @@ -42,46 +41,38 @@ class LabsPresenter( @Composable override fun present(): LabsState { val coroutineScope = rememberCoroutineScope() - val features = remember { - val entries = featureFlagService.getAvailableFeatures() - .filter { it.isInLabs && !it.isFinished } - .map { it.key to it } - mutableStateMapOf(*entries.toTypedArray()) - } val enabledFeatures = remember { - mutableStateMapOf() + mutableStateListOf() } - LaunchedEffect(Unit) { - for (feature in features.values) { - val isEnabled = featureFlagService.isFeatureEnabled(feature) - enabledFeatures[feature.key] = isEnabled - } + featureFlagService.getAvailableFeatures(isInLabs = true) + .forEach { feature -> + enabledFeatures.add(EnabledFeature(feature, featureFlagService.isFeatureEnabled(feature))) + } } - var isApplyingChanges by remember { mutableStateOf(false) } - - val featureUiModels = createUiModels(features, enabledFeatures) + val featureUiModels = createUiModels(enabledFeatures) fun handleEvent(event: LabsEvents) { when (event) { is LabsEvents.ToggleFeature -> coroutineScope.launch { - val feature = features[event.feature.key] ?: return@launch - val isEnabled = featureFlagService.isFeatureEnabled(feature) - featureFlagService.setFeatureEnabled(feature = feature, enabled = !isEnabled) - enabledFeatures[feature.key] = !isEnabled - - when (feature.key) { - FeatureFlags.Threads.key -> { - // Threads require a cache clear to recreate the event cache - clearCacheUseCase() - isApplyingChanges = true + val featureIndex = enabledFeatures.indexOfFirst { it.feature.key == event.feature.key }.takeIf { it != -1 } ?: return@launch + val enabledFeature = enabledFeatures[featureIndex] + val feature = enabledFeature.feature + val newValue = enabledFeature.isEnabled.not() + if (featureFlagService.setFeatureEnabled(feature, newValue)) { + enabledFeatures[featureIndex] = enabledFeatures[featureIndex].copy(isEnabled = newValue) + when (feature.key) { + FeatureFlags.Threads.key -> { + // Threads require a cache clear to recreate the event cache + clearCacheUseCase() + isApplyingChanges = true + } } } } } } - return LabsState( features = featureUiModels, isApplyingChanges = isApplyingChanges, @@ -91,31 +82,29 @@ class LabsPresenter( @Composable private fun createUiModels( - features: SnapshotStateMap, - enabledFeatures: SnapshotStateMap + enabledFeatures: SnapshotStateList, ): ImmutableList { - return features.values.map { feature -> - key(feature.key) { - val isEnabled = enabledFeatures[feature.key].orFalse() - val title = when (feature) { + return enabledFeatures.map { enabledFeature -> + key(enabledFeature.feature.key) { + val title = when (enabledFeature.feature) { FeatureFlags.Threads -> stringProvider.getString(R.string.screen_labs_enable_threads) - else -> feature.title + else -> enabledFeature.feature.title } - val description = when (feature) { + val description = when (enabledFeature.feature) { FeatureFlags.Threads -> stringProvider.getString(R.string.screen_labs_enable_threads_description) - else -> feature.description + else -> enabledFeature.feature.description } - val icon = when (feature) { + val icon = when (enabledFeature.feature) { FeatureFlags.Threads -> CompoundIcons.Threads() else -> null } - remember(feature, isEnabled) { + remember(enabledFeature) { FeatureUiModel( - key = feature.key, + key = enabledFeature.feature.key, title = title, description = description, icon = icon?.let(IconSource::Vector), - isEnabled = isEnabled + isEnabled = enabledFeature.isEnabled ) } } diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/model/EnabledFeature.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/model/EnabledFeature.kt new file mode 100644 index 0000000000..715184c56a --- /dev/null +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/model/EnabledFeature.kt @@ -0,0 +1,15 @@ +/* + * Copyright 2025 New Vector Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial + * Please see LICENSE files in the repository root for full details. + */ + +package io.element.android.features.preferences.impl.model + +import io.element.android.libraries.featureflag.api.Feature + +data class EnabledFeature( + val feature: Feature, + val isEnabled: Boolean, +) diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootPresenter.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootPresenter.kt index 3da72f982e..2a8eeaea1f 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootPresenter.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootPresenter.kt @@ -112,7 +112,7 @@ class PreferencesRootPresenter( .launchIn(this) } - val showLabsItem = remember { featureFlagService.getAvailableFeatures().any { it.isInLabs && !it.isFinished } } + val showLabsItem = remember { featureFlagService.getAvailableFeatures(isInLabs = true).isNotEmpty() } val directLogoutState = directLogoutPresenter.present() diff --git a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenterTest.kt b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenterTest.kt index 6761a37fc6..f13d5b3cd8 100644 --- a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenterTest.kt +++ b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenterTest.kt @@ -21,6 +21,7 @@ import io.element.android.libraries.architecture.AsyncAction import io.element.android.libraries.architecture.AsyncData import io.element.android.libraries.core.meta.BuildMeta import io.element.android.libraries.core.meta.BuildType +import io.element.android.libraries.featureflag.api.Feature import io.element.android.libraries.featureflag.api.FeatureFlags import io.element.android.libraries.featureflag.test.FakeFeature import io.element.android.libraries.featureflag.test.FakeFeatureFlagService @@ -41,15 +42,17 @@ class DeveloperSettingsPresenterTest { @Test fun `present - ensures initial states are correct`() = runTest { - val availableFeatures = listOf( - FakeFeature( - key = "feature_1", - title = "Feature 1", - isInLabs = false, + val getAvailableFeaturesResult = lambdaRecorder> { _, _ -> + listOf( + FakeFeature( + key = "feature_1", + title = "Feature 1", + isInLabs = false, + ) ) - ) + } val presenter = createDeveloperSettingsPresenter( - featureFlagService = FakeFeatureFlagService(providedAvailableFeatures = availableFeatures) + featureFlagService = FakeFeatureFlagService(getAvailableFeaturesResult = getAvailableFeaturesResult) ) presenter.test { awaitItem().also { state -> @@ -73,6 +76,8 @@ class DeveloperSettingsPresenterTest { awaitItem().also { state -> assertThat(state.cacheSize).isInstanceOf(AsyncData.Success::class.java) } + getAvailableFeaturesResult.assertions().isCalledOnce() + .with(value(false), value(false)) } } @@ -203,50 +208,17 @@ class DeveloperSettingsPresenterTest { } } - @Test - fun `present - won't display features in labs or finished`() = runTest { - val availableFeatures = listOf( - // Only this feature should be displayed - FakeFeature( - key = "feature_1", - title = "Feature 1", - isInLabs = false, - ), - FakeFeature( - key = "feature_2", - title = "Feature 2", - isInLabs = true, - ), - FakeFeature( - key = "feature_3", - title = "Feature 3", - isInLabs = false, - isFinished = true, - ) - ) - - val presenter = createDeveloperSettingsPresenter( - featureFlagService = FakeFeatureFlagService( - providedAvailableFeatures = availableFeatures, - ) - ) - presenter.test { - skipItems(2) - awaitItem().also { state -> - assertThat(state.features).hasSize(1) - } - } - } - private fun createDeveloperSettingsPresenter( featureFlagService: FakeFeatureFlagService = FakeFeatureFlagService( - providedAvailableFeatures = listOf( - FakeFeature( - key = "feature_1", - title = "Feature 1", - isInLabs = false, + getAvailableFeaturesResult = { _, _ -> + listOf( + FakeFeature( + key = "feature_1", + title = "Feature 1", + isInLabs = false, + ) ) - ) + } ), cacheSizeUseCase: FakeComputeCacheSizeUseCase = FakeComputeCacheSizeUseCase(), clearCacheUseCase: FakeClearCacheUseCase = FakeClearCacheUseCase(), diff --git a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/labs/LabsPresenterTest.kt b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/labs/LabsPresenterTest.kt index 5117a9fe94..699e8102d5 100644 --- a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/labs/LabsPresenterTest.kt +++ b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/labs/LabsPresenterTest.kt @@ -15,13 +15,15 @@ import io.element.android.libraries.featureflag.api.FeatureFlags import io.element.android.libraries.featureflag.test.FakeFeature import io.element.android.libraries.featureflag.test.FakeFeatureFlagService import io.element.android.services.toolbox.test.strings.FakeStringProvider +import io.element.android.tests.testutils.lambda.lambdaRecorder +import io.element.android.tests.testutils.lambda.value import io.element.android.tests.testutils.test import kotlinx.coroutines.test.runTest import org.junit.Test class LabsPresenterTest { @Test - fun `present - ensures only unfinished features in labs are displayed`() = runTest { + fun `present - ensures features are displayed in the correct order`() = runTest { val availableFeatures = listOf( FakeFeature( key = "feature_1", @@ -30,24 +32,23 @@ class LabsPresenterTest { ), FakeFeature( key = "feature_2", - title = "Feature 2", - isInLabs = false, - ), - FakeFeature( - key = "feature_3", title = "Feature 3", isInLabs = true, - isFinished = true, ) ) + val getAvailableFeaturesResult = lambdaRecorder> { _, _ -> + availableFeatures + } createLabsPresenter( - availableFeatures = availableFeatures, + getAvailableFeaturesResult = getAvailableFeaturesResult, ).test { + skipItems(1) val receivedFeatures = awaitItem().features - assertThat(receivedFeatures).hasSize(1) - assertThat(receivedFeatures.first().key).isEqualTo(availableFeatures.first().key) - - cancelAndIgnoreRemainingEvents() + assertThat(receivedFeatures).hasSize(2) + assertThat(receivedFeatures[0].key).isEqualTo(availableFeatures[0].key) + assertThat(receivedFeatures[1].key).isEqualTo(availableFeatures[1].key) + getAvailableFeaturesResult.assertions().isCalledOnce() + .with(value(false), value(true)) } } @@ -61,19 +62,15 @@ class LabsPresenterTest { ), ) createLabsPresenter( - availableFeatures = availableFeatures, + getAvailableFeaturesResult = { _, _ -> availableFeatures }, ).test { + skipItems(1) val initialItem = awaitItem() val feature = initialItem.features.first() assertThat(feature.isEnabled).isFalse() - - // Wait until the data finished loading - skipItems(1) - // Toggle the feature, should be true now initialItem.eventSink(LabsEvents.ToggleFeature(feature)) assertThat(awaitItem().features.first().isEnabled).isTrue() - // Toggle the feature, should be false now initialItem.eventSink(LabsEvents.ToggleFeature(feature)) assertThat(awaitItem().features.first().isEnabled).isFalse() @@ -92,21 +89,17 @@ class LabsPresenterTest { val clearCacheUseCase = FakeClearCacheUseCase() createLabsPresenter( - availableFeatures = availableFeatures, + getAvailableFeaturesResult = { _, _ -> availableFeatures }, clearCacheUseCase = clearCacheUseCase, ).test { + skipItems(1) val initialItem = awaitItem() val feature = initialItem.features.first() assertThat(feature.isEnabled).isFalse() assertThat(initialItem.isApplyingChanges).isFalse() - - // Wait until the data finished loading - skipItems(1) - // Toggle the feature initialItem.eventSink(LabsEvents.ToggleFeature(feature)) assertThat(awaitItem().features.first().isEnabled).isTrue() - // The clear cache use case should have been called assertThat(awaitItem().isApplyingChanges).isTrue() assertThat(clearCacheUseCase.executeHasBeenCalled).isTrue() @@ -114,12 +107,12 @@ class LabsPresenterTest { } private fun createLabsPresenter( - availableFeatures: List = emptyList(), + getAvailableFeaturesResult: (Boolean, Boolean) -> List = { _, _ -> emptyList() }, clearCacheUseCase: ClearCacheUseCase = FakeClearCacheUseCase(), ): LabsPresenter { return LabsPresenter( stringProvider = FakeStringProvider(), - featureFlagService = FakeFeatureFlagService(providedAvailableFeatures = availableFeatures), + featureFlagService = FakeFeatureFlagService(getAvailableFeaturesResult = getAvailableFeaturesResult), clearCacheUseCase = clearCacheUseCase, ) } diff --git a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootPresenterTest.kt b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootPresenterTest.kt index f65589beb6..80d4706c33 100644 --- a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootPresenterTest.kt +++ b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootPresenterTest.kt @@ -189,14 +189,16 @@ class PreferencesRootPresenterTest { fun `present - labs can be shown if any feature flag is in labs and not finished`() = runTest { createPresenter( featureFlagService = FakeFeatureFlagService( - providedAvailableFeatures = listOf( - FakeFeature( - key = "feature_1", - title = "Feature 1", - isInLabs = true, - isFinished = false, + getAvailableFeaturesResult = { _, _ -> + listOf( + FakeFeature( + key = "feature_1", + title = "Feature 1", + isInLabs = true, + isFinished = false, + ) ) - ) + } ), matrixClient = FakeMatrixClient( canDeactivateAccountResult = { true }, @@ -212,20 +214,16 @@ class PreferencesRootPresenterTest { fun `present - labs can't be shown if all feature flags in labs are finished`() = runTest { createPresenter( featureFlagService = FakeFeatureFlagService( - providedAvailableFeatures = listOf( - FakeFeature( - key = "feature_1", - title = "Feature 1", - isInLabs = true, - isFinished = true, - ) - ) + getAvailableFeaturesResult = { _, _ -> + emptyList() + } ), matrixClient = FakeMatrixClient( canDeactivateAccountResult = { true }, accountManagementUrlResult = { Result.success(null) }, ), ).test { + skipItems(1) assertThat(awaitItem().showLabsItem).isFalse() cancelAndIgnoreRemainingEvents() } diff --git a/libraries/featureflag/api/src/main/kotlin/io/element/android/libraries/featureflag/api/FeatureFlagService.kt b/libraries/featureflag/api/src/main/kotlin/io/element/android/libraries/featureflag/api/FeatureFlagService.kt index 1358909771..2623faa3ab 100644 --- a/libraries/featureflag/api/src/main/kotlin/io/element/android/libraries/featureflag/api/FeatureFlagService.kt +++ b/libraries/featureflag/api/src/main/kotlin/io/element/android/libraries/featureflag/api/FeatureFlagService.kt @@ -35,7 +35,12 @@ interface FeatureFlagService { suspend fun setFeatureEnabled(feature: Feature, enabled: Boolean): Boolean /** - * @return the list of available (not finished) features that can be toggled. + * @return the list of available features that can be toggled. + * @param includeFinishedFeatures whether to include finished features, default is false + * @param isInLabs whether the user is in labs (to include lab features), default is false */ - fun getAvailableFeatures(): List + fun getAvailableFeatures( + includeFinishedFeatures: Boolean = false, + isInLabs: Boolean = false, + ): List } diff --git a/libraries/featureflag/impl/build.gradle.kts b/libraries/featureflag/impl/build.gradle.kts index c54f95a293..f44032661c 100644 --- a/libraries/featureflag/impl/build.gradle.kts +++ b/libraries/featureflag/impl/build.gradle.kts @@ -31,4 +31,5 @@ dependencies { testCommonDependencies(libs) testImplementation(projects.libraries.matrix.test) + testImplementation(projects.libraries.featureflag.test) } diff --git a/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/DefaultFeatureFlagService.kt b/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/DefaultFeatureFlagService.kt index 01c1e8a258..cecd0ca0a8 100644 --- a/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/DefaultFeatureFlagService.kt +++ b/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/DefaultFeatureFlagService.kt @@ -14,7 +14,6 @@ import dev.zacsweers.metro.SingleIn import io.element.android.libraries.core.meta.BuildMeta import io.element.android.libraries.featureflag.api.Feature import io.element.android.libraries.featureflag.api.FeatureFlagService -import io.element.android.libraries.featureflag.api.FeatureFlags import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.flowOf @@ -24,25 +23,30 @@ import kotlinx.coroutines.flow.flowOf class DefaultFeatureFlagService( private val providers: Set<@JvmSuppressWildcards FeatureFlagProvider>, private val buildMeta: BuildMeta, + private val featuresProvider: FeaturesProvider, ) : FeatureFlagService { override fun isFeatureEnabledFlow(feature: Feature): Flow { return providers.filter { it.hasFeature(feature) } - .sortedByDescending(FeatureFlagProvider::priority) - .firstOrNull() + .maxByOrNull(FeatureFlagProvider::priority) ?.isFeatureEnabledFlow(feature) ?: flowOf(feature.defaultValue(buildMeta)) } override suspend fun setFeatureEnabled(feature: Feature, enabled: Boolean): Boolean { return providers.filterIsInstance() - .sortedBy(FeatureFlagProvider::priority) - .firstOrNull() + .maxByOrNull(FeatureFlagProvider::priority) ?.setFeatureEnabled(feature, enabled) ?.let { true } ?: false } - override fun getAvailableFeatures(): List { - return FeatureFlags.entries.filter { !it.isFinished } + override fun getAvailableFeatures( + includeFinishedFeatures: Boolean, + isInLabs: Boolean, + ): List { + return featuresProvider.provide().filter { flag -> + (includeFinishedFeatures || !flag.isFinished) && + flag.isInLabs == isInLabs + } } } diff --git a/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/FeaturesProvider.kt b/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/FeaturesProvider.kt new file mode 100644 index 0000000000..8d580006e7 --- /dev/null +++ b/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/FeaturesProvider.kt @@ -0,0 +1,24 @@ +/* + * Copyright 2025 New Vector Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial + * Please see LICENSE files in the repository root for full details. + */ + +package io.element.android.libraries.featureflag.impl + +import dev.zacsweers.metro.AppScope +import dev.zacsweers.metro.ContributesBinding +import dev.zacsweers.metro.Inject +import io.element.android.libraries.featureflag.api.Feature +import io.element.android.libraries.featureflag.api.FeatureFlags + +fun interface FeaturesProvider { + fun provide(): List +} + +@ContributesBinding(AppScope::class) +@Inject +class DefaultFeaturesProvider : FeaturesProvider { + override fun provide(): List = FeatureFlags.entries +} diff --git a/libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/DefaultFeatureFlagServiceTest.kt b/libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/DefaultFeatureFlagServiceTest.kt index 097e318609..438dbe1ecc 100644 --- a/libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/DefaultFeatureFlagServiceTest.kt +++ b/libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/DefaultFeatureFlagServiceTest.kt @@ -9,26 +9,47 @@ package io.element.android.libraries.featureflag.impl import app.cash.turbine.test import com.google.common.truth.Truth.assertThat -import io.element.android.libraries.featureflag.api.FeatureFlags +import io.element.android.libraries.core.meta.BuildMeta +import io.element.android.libraries.featureflag.api.Feature +import io.element.android.libraries.featureflag.test.FakeFeature import io.element.android.libraries.matrix.test.core.aBuildMeta import kotlinx.coroutines.test.runTest import org.junit.Test class DefaultFeatureFlagServiceTest { + private val aFeature = FakeFeature( + key = "test_feature", + title = "Test Feature", + ) + @Test fun `given service without provider when feature is checked then it returns the default value`() = runTest { + val featureWithDefaultToFalse = FakeFeature( + key = "test_feature", + title = "Test Feature", + defaultValue = { false } + ) + val featureWithDefaultToTrue = FakeFeature( + key = "test_feature_2", + title = "Test Feature 2", + defaultValue = { true } + ) val buildMeta = aBuildMeta() - val featureFlagService = DefaultFeatureFlagService(emptySet(), buildMeta) - featureFlagService.isFeatureEnabledFlow(FeatureFlags.Space).test { - assertThat(awaitItem()).isEqualTo(FeatureFlags.Space.defaultValue(buildMeta)) + val featureFlagService = createDefaultFeatureFlagService(buildMeta = buildMeta) + featureFlagService.isFeatureEnabledFlow(featureWithDefaultToFalse).test { + assertThat(awaitItem()).isFalse() + cancelAndIgnoreRemainingEvents() + } + featureFlagService.isFeatureEnabledFlow(featureWithDefaultToTrue).test { + assertThat(awaitItem()).isTrue() cancelAndIgnoreRemainingEvents() } } @Test fun `given service without provider when set enabled feature is called then it returns false`() = runTest { - val featureFlagService = DefaultFeatureFlagService(emptySet(), aBuildMeta()) - val result = featureFlagService.setFeatureEnabled(FeatureFlags.Space, true) + val featureFlagService = createDefaultFeatureFlagService() + val result = featureFlagService.setFeatureEnabled(aFeature, true) assertThat(result).isFalse() } @@ -36,8 +57,11 @@ class DefaultFeatureFlagServiceTest { fun `given service with a runtime provider when set enabled feature is called then it returns true`() = runTest { val buildMeta = aBuildMeta() val featureFlagProvider = FakeMutableFeatureFlagProvider(0, buildMeta) - val featureFlagService = DefaultFeatureFlagService(setOf(featureFlagProvider), buildMeta) - val result = featureFlagService.setFeatureEnabled(FeatureFlags.Space, true) + val featureFlagService = createDefaultFeatureFlagService( + providers = setOf(featureFlagProvider), + buildMeta = buildMeta, + ) + val result = featureFlagService.setFeatureEnabled(aFeature, true) assertThat(result).isTrue() } @@ -45,11 +69,14 @@ class DefaultFeatureFlagServiceTest { fun `given service with a runtime provider and feature enabled when feature is checked then it returns the correct value`() = runTest { val buildMeta = aBuildMeta() val featureFlagProvider = FakeMutableFeatureFlagProvider(0, buildMeta) - val featureFlagService = DefaultFeatureFlagService(setOf(featureFlagProvider), buildMeta) - featureFlagService.setFeatureEnabled(FeatureFlags.Space, true) - featureFlagService.isFeatureEnabledFlow(FeatureFlags.Space).test { + val featureFlagService = createDefaultFeatureFlagService( + providers = setOf(featureFlagProvider), + buildMeta = buildMeta + ) + featureFlagService.setFeatureEnabled(aFeature, true) + featureFlagService.isFeatureEnabledFlow(aFeature).test { assertThat(awaitItem()).isTrue() - featureFlagService.setFeatureEnabled(FeatureFlags.Space, false) + featureFlagService.setFeatureEnabled(aFeature, false) assertThat(awaitItem()).isFalse() } } @@ -59,11 +86,84 @@ class DefaultFeatureFlagServiceTest { val buildMeta = aBuildMeta() val lowPriorityFeatureFlagProvider = FakeMutableFeatureFlagProvider(LOW_PRIORITY, buildMeta) val highPriorityFeatureFlagProvider = FakeMutableFeatureFlagProvider(HIGH_PRIORITY, buildMeta) - val featureFlagService = DefaultFeatureFlagService(setOf(lowPriorityFeatureFlagProvider, highPriorityFeatureFlagProvider), buildMeta) - lowPriorityFeatureFlagProvider.setFeatureEnabled(FeatureFlags.Space, false) - highPriorityFeatureFlagProvider.setFeatureEnabled(FeatureFlags.Space, true) - featureFlagService.isFeatureEnabledFlow(FeatureFlags.Space).test { + val featureFlagService = createDefaultFeatureFlagService( + providers = setOf(lowPriorityFeatureFlagProvider, highPriorityFeatureFlagProvider), + buildMeta = buildMeta + ) + lowPriorityFeatureFlagProvider.setFeatureEnabled(aFeature, false) + highPriorityFeatureFlagProvider.setFeatureEnabled(aFeature, true) + featureFlagService.isFeatureEnabledFlow(aFeature).test { assertThat(awaitItem()).isTrue() } } + + @Test + fun `getAvailableFeatures should return expected features`() { + val aFinishedLabFeature = FakeFeature( + key = "finished_lab_feature", + title = "Finished Lab Feature", + isFinished = true, + isInLabs = true, + ) + val aFinishedDevFeature = FakeFeature( + key = "finished_dev_feature", + title = "Finished Dev Feature", + isFinished = true, + isInLabs = false, + ) + val anUnfinishedLabFeature = FakeFeature( + key = "unfinished_lab_feature", + title = "Unfinished Lab Feature", + isFinished = false, + isInLabs = true, + ) + val anUnfinishedDevFeature = FakeFeature( + key = "unfinished_dev_feature", + title = "Unfinished Dev Feature", + isFinished = false, + isInLabs = false, + ) + val featureFlagService = createDefaultFeatureFlagService( + features = listOf( + aFinishedLabFeature, + aFinishedDevFeature, + anUnfinishedLabFeature, + anUnfinishedDevFeature, + ), + ) + assertThat( + featureFlagService.getAvailableFeatures( + includeFinishedFeatures = false, + isInLabs = true, + ) + ).containsExactly(anUnfinishedLabFeature) + assertThat( + featureFlagService.getAvailableFeatures( + includeFinishedFeatures = true, + isInLabs = true, + ) + ).containsExactly(aFinishedLabFeature, anUnfinishedLabFeature) + assertThat( + featureFlagService.getAvailableFeatures( + includeFinishedFeatures = false, + isInLabs = false, + ) + ).containsExactly(anUnfinishedDevFeature) + assertThat( + featureFlagService.getAvailableFeatures( + includeFinishedFeatures = true, + isInLabs = false, + ) + ).containsExactly(aFinishedDevFeature, anUnfinishedDevFeature) + } } + +private fun createDefaultFeatureFlagService( + providers: Set = emptySet(), + buildMeta: BuildMeta = aBuildMeta(), + features: List = emptyList(), +) = DefaultFeatureFlagService( + providers = providers, + buildMeta = buildMeta, + featuresProvider = { features } +) diff --git a/libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/DefaultFeaturesProviderTest.kt b/libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/DefaultFeaturesProviderTest.kt new file mode 100644 index 0000000000..8b746c2e89 --- /dev/null +++ b/libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/DefaultFeaturesProviderTest.kt @@ -0,0 +1,24 @@ +/* + * Copyright 2025 New Vector Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial + * Please see LICENSE files in the repository root for full details. + */ + +package io.element.android.libraries.featureflag.impl + +import com.google.common.truth.Truth.assertThat +import io.element.android.libraries.featureflag.api.FeatureFlags +import org.junit.Test + +class DefaultFeaturesProviderTest { + @Test + fun `provide should return all features`() { + val provider = DefaultFeaturesProvider() + val features = provider.provide() + assertThat(features.size).isEqualTo(FeatureFlags.entries.size) + FeatureFlags.entries.forEach { + assertThat(features.contains(it)).isTrue() + } + } +} diff --git a/libraries/featureflag/test/src/main/java/io/element/android/libraries/featureflag/test/FakeFeatureFlagService.kt b/libraries/featureflag/test/src/main/java/io/element/android/libraries/featureflag/test/FakeFeatureFlagService.kt index 49fc095ed4..456936e501 100644 --- a/libraries/featureflag/test/src/main/java/io/element/android/libraries/featureflag/test/FakeFeatureFlagService.kt +++ b/libraries/featureflag/test/src/main/java/io/element/android/libraries/featureflag/test/FakeFeatureFlagService.kt @@ -17,7 +17,7 @@ import kotlinx.coroutines.flow.MutableStateFlow class FakeFeatureFlagService( initialState: Map = emptyMap(), private val buildMeta: BuildMeta = aBuildMeta(), - var providedAvailableFeatures: List = emptyList(), + private val getAvailableFeaturesResult: (Boolean, Boolean) -> List = { _, _ -> emptyList() }, ) : FeatureFlagService { private val enabledFeatures = initialState .mapValues { MutableStateFlow(it.value) } @@ -33,7 +33,10 @@ class FakeFeatureFlagService( return enabledFeatures.getOrPut(feature.key) { MutableStateFlow(feature.defaultValue(buildMeta)) } } - override fun getAvailableFeatures(): List { - return providedAvailableFeatures + override fun getAvailableFeatures( + includeFinishedFeatures: Boolean, + isInLabs: Boolean, + ): List { + return getAvailableFeaturesResult(includeFinishedFeatures, isInLabs) } } diff --git a/tests/uitests/src/test/snapshots/images/features.preferences.impl.developer_DeveloperSettingsView_Day_1_en.png b/tests/uitests/src/test/snapshots/images/features.preferences.impl.developer_DeveloperSettingsView_Day_1_en.png index 176ba2c9ea..6678623bc5 100644 --- a/tests/uitests/src/test/snapshots/images/features.preferences.impl.developer_DeveloperSettingsView_Day_1_en.png +++ b/tests/uitests/src/test/snapshots/images/features.preferences.impl.developer_DeveloperSettingsView_Day_1_en.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:d4d03e543f38658338d7fedcc7f290ad973a68e1ec4a828d4651c188769e6769 -size 46130 +oid sha256:84e4c7fd597e4cdcb043514b370b0fd1e3b65f42b8c7811a206433ad422841fe +size 42445 diff --git a/tests/uitests/src/test/snapshots/images/features.preferences.impl.developer_DeveloperSettingsView_Night_1_en.png b/tests/uitests/src/test/snapshots/images/features.preferences.impl.developer_DeveloperSettingsView_Night_1_en.png index 2f74fe63fc..536d5a22c8 100644 --- a/tests/uitests/src/test/snapshots/images/features.preferences.impl.developer_DeveloperSettingsView_Night_1_en.png +++ b/tests/uitests/src/test/snapshots/images/features.preferences.impl.developer_DeveloperSettingsView_Night_1_en.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:c8b5df08672db5d6c6df1daba79166bb3251edc6e612b29eaad881a2b2dc44d3 -size 44788 +oid sha256:8880ddfe80d8062bcaceac1e9d1407e24d365e9c6b69fc8ce65e3ade973e8acf +size 41037