Ensure labs feature are ordered as they are declared.

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<String, Boolean>()
+            mutableStateListOf<EnabledFeature>()
         }
-
         LaunchedEffect(Unit) {
-            for (feature in features.values) {
+            featureFlagService.getAvailableFeatures(isInLabs = true)
-                val isEnabled = featureFlagService.isFeatureEnabled(feature)
+                .forEach { feature ->
-                enabledFeatures[feature.key] = isEnabled
+                    enabledFeatures.add(EnabledFeature(feature, featureFlagService.isFeatureEnabled(feature)))
-            }
+                }
         }
-
         var isApplyingChanges by remember { mutableStateOf(false) }
-
+        val featureUiModels = createUiModels(enabledFeatures)
-        val featureUiModels = createUiModels(features, enabledFeatures)
 
         fun handleEvent(event: LabsEvents) {
             when (event) {
                 is LabsEvents.ToggleFeature -> coroutineScope.launch {
-                    val feature = features[event.feature.key] ?: return@launch
+                    val featureIndex = enabledFeatures.indexOfFirst { it.feature.key == event.feature.key }.takeIf { it != -1 } ?: return@launch
-                    val isEnabled = featureFlagService.isFeatureEnabled(feature)
+                    val enabledFeature = enabledFeatures[featureIndex]
-                    featureFlagService.setFeatureEnabled(feature = feature, enabled = !isEnabled)
+                    val feature = enabledFeature.feature
-                    enabledFeatures[feature.key] = !isEnabled
+                    val newValue = enabledFeature.isEnabled.not()
-
+                    if (featureFlagService.setFeatureEnabled(feature, newValue)) {
-                    when (feature.key) {
+                        enabledFeatures[featureIndex] = enabledFeatures[featureIndex].copy(isEnabled = newValue)
-                        FeatureFlags.Threads.key -> {
+                        when (feature.key) {
-                            // Threads require a cache clear to recreate the event cache
+                            FeatureFlags.Threads.key -> {
-                            clearCacheUseCase()
+                                // Threads require a cache clear to recreate the event cache
-                            isApplyingChanges = true
+                                clearCacheUseCase()
+                                isApplyingChanges = true
+                            }
                         }
                     }
                 }
             }
         }
-
         return LabsState(
             features = featureUiModels,
             isApplyingChanges = isApplyingChanges,
@@ -90,31 +82,29 @@ class LabsPresenter(
 
     @Composable
     private fun createUiModels(
-        features: SnapshotStateMap<String, Feature>,
+        enabledFeatures: SnapshotStateList<EnabledFeature>,
-        enabledFeatures: SnapshotStateMap<String, Boolean>
     ): ImmutableList<FeatureUiModel> {
-        return features.values.map { feature ->
+        return enabledFeatures.map { enabledFeature ->
-            key(feature.key) {
+            key(enabledFeature.feature.key) {
-                val isEnabled = enabledFeatures[feature.key].orFalse()
+                val title = when (enabledFeature.feature) {
-                val title = when (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
                     )
                 }
             }

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).hasSize(2)
-            assertThat(receivedFeatures.first().key).isEqualTo(availableFeatures.first().key)
+            assertThat(receivedFeatures[0].key).isEqualTo(availableFeatures[0].key)
-
+            assertThat(receivedFeatures[1].key).isEqualTo(availableFeatures[1].key)
-            cancelAndIgnoreRemainingEvents()
         }
     }
 
@@ -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()