Merge pull request #3432 from element-hq/feature/fga/pinned_messages_fix_timeline_provider

Feature/fga/pinned messages fix timeline provider

Author: ganfra

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

@@ -18,11 +18,13 @@ import io.element.android.libraries.matrix.api.room.MatrixRoom
 import io.element.android.libraries.matrix.api.timeline.Timeline
 import io.element.android.libraries.matrix.api.timeline.TimelineProvider
 import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.coroutineScope
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.combine
+import kotlinx.coroutines.flow.distinctUntilChanged
 import kotlinx.coroutines.flow.launchIn
-import kotlinx.coroutines.flow.onCompletion
+import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.onEach
 import javax.inject.Inject
 
@@ -32,7 +34,8 @@ class PinnedEventsTimelineProvider @Inject constructor(
     private val networkMonitor: NetworkMonitor,
     private val featureFlagService: FeatureFlagService,
 ) : TimelineProvider {
-    private val _timelineStateFlow: MutableStateFlow<AsyncData<Timeline>> = MutableStateFlow(AsyncData.Uninitialized)
+    private val _timelineStateFlow: MutableStateFlow<AsyncData<Timeline>> =
+        MutableStateFlow(AsyncData.Uninitialized)
 
     override fun activeTimelineFlow(): StateFlow<Timeline?> {
         return _timelineStateFlow
@@ -44,25 +47,46 @@ class PinnedEventsTimelineProvider @Inject constructor(
     val timelineStateFlow = _timelineStateFlow
 
     fun launchIn(scope: CoroutineScope) {
+        _timelineStateFlow.subscriptionCount
+            .map { count -> count > 0 }
+            .distinctUntilChanged()
+            .onEach { isActive ->
+                if (isActive) {
+                    onActive()
+                } else {
+                    onInactive()
+                }
+            }
+            .launchIn(scope)
+    }
+
+    private suspend fun onActive() = coroutineScope {
         combine(
             featureFlagService.isFeatureEnabledFlow(FeatureFlags.PinnedEvents),
             networkMonitor.connectivity
-        ) {
-          // do not use connectivity here as data can be loaded from cache, it's just to trigger retry if needed
-          isEnabled, _ ->
+        ) { isEnabled, _ ->
+            // do not use connectivity here as data can be loaded from cache, it's just to trigger retry if needed
             isEnabled
         }
             .onEach { isFeatureEnabled ->
                 if (isFeatureEnabled) {
                     loadTimelineIfNeeded()
                 } else {
-                    _timelineStateFlow.value = AsyncData.Uninitialized
+                    resetTimeline()
                 }
             }
-            .onCompletion {
-                invokeOnTimeline { close() }
-            }
-            .launchIn(scope)
+            .launchIn(this)
+    }
+
+    private suspend fun onInactive() {
+        resetTimeline()
+    }
+
+    private suspend fun resetTimeline() {
+        invokeOnTimeline {
+            close()
+        }
+        _timelineStateFlow.emit(AsyncData.Uninitialized)
     }
 
     suspend fun invokeOnTimeline(action: suspend Timeline.() -> Unit) {

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

@@ -15,7 +15,6 @@ import androidx.compose.runtime.getValue
 import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.produceState
 import androidx.compose.runtime.remember
-import androidx.compose.runtime.rememberCoroutineScope
 import androidx.compose.runtime.rememberUpdatedState
 import androidx.compose.runtime.setValue
 import dagger.assisted.Assisted
@@ -59,6 +58,7 @@ class PinnedMessagesListPresenter @AssistedInject constructor(
     private val timelineProvider: PinnedEventsTimelineProvider,
     private val snackbarDispatcher: SnackbarDispatcher,
     actionListPresenterFactory: ActionListPresenter.Factory,
+    private val appCoroutineScope: CoroutineScope,
 ) : Presenter<PinnedMessagesListState> {
     @AssistedFactory
     interface Factory {
@@ -93,10 +93,9 @@ class PinnedMessagesListPresenter @AssistedInject constructor(
             }
         )
 
-        val coroutineScope = rememberCoroutineScope()
         fun handleEvents(event: PinnedMessagesListEvents) {
             when (event) {
-                is PinnedMessagesListEvents.HandleAction -> coroutineScope.handleTimelineAction(event.action, event.event)
+                is PinnedMessagesListEvents.HandleAction -> appCoroutineScope.handleTimelineAction(event.action, event.event)
             }
         }
 

features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/pinned/list/PinnedMessagesListPresenterTest.kt

@@ -35,11 +35,14 @@ import io.element.android.tests.testutils.lambda.assert
 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.ExperimentalCoroutinesApi
 import kotlinx.coroutines.flow.flowOf
 import kotlinx.coroutines.test.TestScope
+import kotlinx.coroutines.test.advanceUntilIdle
 import kotlinx.coroutines.test.runTest
 import org.junit.Test
 
+@OptIn(ExperimentalCoroutinesApi::class)
 class PinnedMessagesListPresenterTest {
     @Test
     fun `present - initial state feature disabled`() = runTest {
@@ -155,6 +158,7 @@ class PinnedMessagesListPresenterTest {
             val filledState = awaitItem() as PinnedMessagesListState.Filled
             val eventItem = filledState.timelineItems.first() as TimelineItem.Event
             filledState.eventSink(PinnedMessagesListEvents.HandleAction(TimelineItemAction.Redact, eventItem))
+            advanceUntilIdle()
             cancelAndIgnoreRemainingEvents()
             assert(redactEventLambda)
                 .isCalledOnce()
@@ -184,9 +188,11 @@ class PinnedMessagesListPresenterTest {
 
             pinnedEventsTimeline.unpinEventLambda = successUnpinEventLambda
             filledState.eventSink(PinnedMessagesListEvents.HandleAction(TimelineItemAction.Unpin, eventItem))
+            advanceUntilIdle()
 
             pinnedEventsTimeline.unpinEventLambda = failureUnpinEventLambda
             filledState.eventSink(PinnedMessagesListEvents.HandleAction(TimelineItemAction.Unpin, eventItem))
+            advanceUntilIdle()
 
             cancelAndIgnoreRemainingEvents()
 
@@ -221,6 +227,7 @@ class PinnedMessagesListPresenterTest {
             val filledState = awaitItem() as PinnedMessagesListState.Filled
             val eventItem = filledState.timelineItems.first() as TimelineItem.Event
             filledState.eventSink(PinnedMessagesListEvents.HandleAction(TimelineItemAction.ViewInTimeline, eventItem))
+            advanceUntilIdle()
             cancelAndIgnoreRemainingEvents()
             assert(onViewInTimelineClickLambda)
                 .isCalledOnce()
@@ -249,6 +256,7 @@ class PinnedMessagesListPresenterTest {
             val filledState = awaitItem() as PinnedMessagesListState.Filled
             val eventItem = filledState.timelineItems.first() as TimelineItem.Event
             filledState.eventSink(PinnedMessagesListEvents.HandleAction(TimelineItemAction.ViewSource, eventItem))
+            advanceUntilIdle()
             cancelAndIgnoreRemainingEvents()
             assert(onShowEventDebugInfoClickLambda)
                 .isCalledOnce()
@@ -277,6 +285,7 @@ class PinnedMessagesListPresenterTest {
             val filledState = awaitItem() as PinnedMessagesListState.Filled
             val eventItem = filledState.timelineItems.first() as TimelineItem.Event
             filledState.eventSink(PinnedMessagesListEvents.HandleAction(TimelineItemAction.Forward, eventItem))
+            advanceUntilIdle()
             cancelAndIgnoreRemainingEvents()
             assert(onForwardEventClickLambda)
                 .isCalledOnce()
@@ -322,6 +331,7 @@ class PinnedMessagesListPresenterTest {
             timelineProvider = timelineProvider,
             snackbarDispatcher = SnackbarDispatcher(),
             actionListPresenterFactory = FakeActionListPresenter.Factory,
+            appCoroutineScope = this,
         )
     }
 }