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 b1e3fc7b77..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,45 +41,38 @@ class LabsPresenter( @Composable override fun present(): LabsState { val coroutineScope = rememberCoroutineScope() - val features = remember { - val entries = featureFlagService.getAvailableFeatures(isInLabs = true) - .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, @@ -90,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/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..c9af521960 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 @@ -21,7 +21,7 @@ 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 +30,18 @@ class LabsPresenterTest { ), FakeFeature( key = "feature_2", - title = "Feature 2", - isInLabs = false, - ), - FakeFeature( - key = "feature_3", title = "Feature 3", isInLabs = true, - isFinished = true, ) ) createLabsPresenter( availableFeatures = availableFeatures, ).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) } } @@ -63,17 +57,13 @@ class LabsPresenterTest { createLabsPresenter( availableFeatures = 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() @@ -95,18 +85,14 @@ class LabsPresenterTest { availableFeatures = 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()