Pinned events: make sure state is preserved

Author: ganfra

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/IsPinnedMessagesFeatureEnabled.kt

@@ -0,0 +1,54 @@
+/*
+ * Copyright (c) 2024 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.element.android.features.messages.impl.pinned
+
+import androidx.compose.runtime.Composable
+import androidx.compose.runtime.LaunchedEffect
+import androidx.compose.runtime.getValue
+import androidx.compose.runtime.mutableStateOf
+import androidx.compose.runtime.saveable.rememberSaveable
+import androidx.compose.runtime.setValue
+import com.squareup.anvil.annotations.ContributesBinding
+import io.element.android.libraries.di.AppScope
+import io.element.android.libraries.featureflag.api.FeatureFlagService
+import io.element.android.libraries.featureflag.api.FeatureFlags
+import kotlinx.coroutines.flow.launchIn
+import kotlinx.coroutines.flow.onEach
+import javax.inject.Inject
+
+fun interface IsPinnedMessagesFeatureEnabled {
+    @Composable
+    operator fun invoke(): Boolean
+}
+
+@ContributesBinding(AppScope::class)
+class DefaultIsPinnedMessagesFeatureEnabled @Inject constructor(
+    private val featureFlagService: FeatureFlagService,
+) : IsPinnedMessagesFeatureEnabled {
+    @Composable
+    override operator fun invoke(): Boolean {
+        var isFeatureEnabled by rememberSaveable {
+            mutableStateOf(false)
+        }
+        LaunchedEffect(Unit) {
+            featureFlagService.isFeatureEnabledFlow(FeatureFlags.PinnedEvents)
+                .onEach { isFeatureEnabled = it }
+                .launchIn(this)
+        }
+        return isFeatureEnabled
+    }
+}

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/banner/PinnedMessagesBannerPresenter.kt

@@ -26,10 +26,9 @@ import androidx.compose.runtime.remember
 import androidx.compose.runtime.rememberUpdatedState
 import androidx.compose.runtime.saveable.rememberSaveable
 import androidx.compose.runtime.setValue
+import io.element.android.features.messages.impl.pinned.IsPinnedMessagesFeatureEnabled
 import io.element.android.features.networkmonitor.api.NetworkMonitor
 import io.element.android.libraries.architecture.Presenter
-import io.element.android.libraries.featureflag.api.FeatureFlagService
-import io.element.android.libraries.featureflag.api.FeatureFlags
 import io.element.android.libraries.matrix.api.room.MatrixRoom
 import kotlinx.collections.immutable.ImmutableList
 import kotlinx.collections.immutable.persistentListOf
@@ -46,21 +45,20 @@ import kotlin.time.Duration.Companion.milliseconds
 class PinnedMessagesBannerPresenter @Inject constructor(
     private val room: MatrixRoom,
     private val itemFactory: PinnedMessagesBannerItemFactory,
-    private val featureFlagService: FeatureFlagService,
+    private val isFeatureEnabled: IsPinnedMessagesFeatureEnabled,
     private val networkMonitor: NetworkMonitor,
 ) : Presenter<PinnedMessagesBannerState> {
+    private val pinnedItems = mutableStateOf<ImmutableList<PinnedMessagesBannerItem>>(persistentListOf())
+
     @Composable
     override fun present(): PinnedMessagesBannerState {
-        val isFeatureEnabled by featureFlagService.isFeatureEnabledFlow(FeatureFlags.PinnedEvents).collectAsState(initial = false)
-        var hasTimelineFailedToLoad by rememberSaveable { mutableStateOf(false) }
-        var currentPinnedMessageIndex by rememberSaveable { mutableIntStateOf(-1) }
+        val isFeatureEnabled = isFeatureEnabled()
         val knownPinnedMessagesCount by remember {
             room.roomInfoFlow.map { roomInfo -> roomInfo.pinnedEventIds.size }
         }.collectAsState(initial = 0)
 
-        var pinnedItems by remember {
-            mutableStateOf<ImmutableList<PinnedMessagesBannerItem>>(persistentListOf())
-        }
+        var hasTimelineFailedToLoad by rememberSaveable { mutableStateOf(false) }
+        var currentPinnedMessageIndex by rememberSaveable { mutableIntStateOf(-1) }
 
         PinnedMessagesBannerItemsEffect(
             isFeatureEnabled = isFeatureEnabled,
@@ -69,7 +67,7 @@ class PinnedMessagesBannerPresenter @Inject constructor(
                 if (currentPinnedMessageIndex >= pinnedMessageCount || currentPinnedMessageIndex < 0) {
                     currentPinnedMessageIndex = pinnedMessageCount - 1
                 }
-                pinnedItems = newItems
+                pinnedItems.value = newItems
             },
             onTimelineFail = { hasTimelineFailed ->
                 hasTimelineFailedToLoad = hasTimelineFailed
@@ -82,7 +80,7 @@ class PinnedMessagesBannerPresenter @Inject constructor(
                     if (currentPinnedMessageIndex > 0) {
                         currentPinnedMessageIndex--
                     } else {
-                        currentPinnedMessageIndex = pinnedItems.size - 1
+                        currentPinnedMessageIndex = pinnedItems.value.size - 1
                     }
                 }
             }
@@ -92,7 +90,7 @@ class PinnedMessagesBannerPresenter @Inject constructor(
             isFeatureEnabled = isFeatureEnabled,
             hasTimelineFailed = hasTimelineFailedToLoad,
             realPinnedMessagesCount = knownPinnedMessagesCount,
-            pinnedItems = pinnedItems,
+            pinnedItems = pinnedItems.value,
             currentPinnedMessageIndex = currentPinnedMessageIndex,
             eventSink = ::handleEvent
         )
@@ -111,16 +109,14 @@ class PinnedMessagesBannerPresenter @Inject constructor(
         return when {
             !isFeatureEnabled -> PinnedMessagesBannerState.Hidden
             hasTimelineFailed -> PinnedMessagesBannerState.Hidden
+            currentPinnedMessage != null -> PinnedMessagesBannerState.Loaded(
+                currentPinnedMessage = currentPinnedMessage,
+                currentPinnedMessageIndex = currentPinnedMessageIndex,
+                knownPinnedMessagesCount = pinnedItems.size,
+                eventSink = eventSink
+            )
             realPinnedMessagesCount == 0 -> PinnedMessagesBannerState.Hidden
-            currentPinnedMessage == null -> PinnedMessagesBannerState.Loading(realPinnedMessagesCount = realPinnedMessagesCount)
-            else -> {
-                PinnedMessagesBannerState.Loaded(
-                    currentPinnedMessage = currentPinnedMessage,
-                    currentPinnedMessageIndex = currentPinnedMessageIndex,
-                    knownPinnedMessagesCount = pinnedItems.size,
-                    eventSink = eventSink
-                )
-            }
+            else -> PinnedMessagesBannerState.Loading(realPinnedMessagesCount = realPinnedMessagesCount)
         }
     }
 
@@ -136,8 +132,10 @@ class PinnedMessagesBannerPresenter @Inject constructor(
         val networkStatus by networkMonitor.connectivity.collectAsState()
 
         LaunchedEffect(isFeatureEnabled, networkStatus) {
-            if (!isFeatureEnabled) return@LaunchedEffect
-
+            if (!isFeatureEnabled) {
+                updatedOnItemsChange(persistentListOf())
+                return@LaunchedEffect
+            }
             val pinnedEventsTimeline = room.pinnedEventsTimeline()
                 .onFailure { updatedOnTimelineFail(true) }
                 .onSuccess { updatedOnTimelineFail(false) }

features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/pinned/banner/PinnedMessagesBannerPresenterTest.kt

@@ -20,8 +20,6 @@ import com.google.common.truth.Truth.assertThat
 import io.element.android.features.networkmonitor.api.NetworkMonitor
 import io.element.android.features.networkmonitor.test.FakeNetworkMonitor
 import io.element.android.libraries.eventformatter.test.FakePinnedMessagesBannerFormatter
-import io.element.android.libraries.featureflag.api.FeatureFlags
-import io.element.android.libraries.featureflag.test.FakeFeatureFlagService
 import io.element.android.libraries.matrix.api.room.MatrixRoom
 import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem
 import io.element.android.libraries.matrix.test.AN_EVENT_ID
@@ -195,15 +193,10 @@ class PinnedMessagesBannerPresenterTest {
         networkMonitor: NetworkMonitor = FakeNetworkMonitor(),
         isFeatureEnabled: Boolean = true,
     ): PinnedMessagesBannerPresenter {
-        val featureFlagService = FakeFeatureFlagService(
-            initialState = mapOf(
-                FeatureFlags.PinnedEvents.key to isFeatureEnabled
-            )
-        )
         return PinnedMessagesBannerPresenter(
             room = room,
             itemFactory = itemFactory,
-            featureFlagService = featureFlagService,
+            isFeatureEnabled = { isFeatureEnabled },
             networkMonitor = networkMonitor,
         )
     }