From c611f39ec365cd0bcf1f0a4305c5c67c2c9a57b6 Mon Sep 17 00:00:00 2001 From: ganfra Date: Tue, 10 Sep 2024 09:07:00 +0200 Subject: [PATCH 1/2] Pinned messages : fix timeline provider subscription --- .../pinned/PinnedEventsTimelineProvider.kt | 44 ++++++++++++++----- 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/PinnedEventsTimelineProvider.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/PinnedEventsTimelineProvider.kt index 4a1deb729b..75b639de85 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/PinnedEventsTimelineProvider.kt +++ b/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> = MutableStateFlow(AsyncData.Uninitialized) + private val _timelineStateFlow: MutableStateFlow> = + MutableStateFlow(AsyncData.Uninitialized) override fun activeTimelineFlow(): StateFlow { 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) { From 99158dadc0841e7044ebe1add77eb3259b97dc69 Mon Sep 17 00:00:00 2001 From: ganfra Date: Tue, 10 Sep 2024 09:07:16 +0200 Subject: [PATCH 2/2] Pinned messages : allow action to continue when leaving the pinned messages list. --- .../impl/pinned/list/PinnedMessagesListPresenter.kt | 5 ++--- .../pinned/list/PinnedMessagesListPresenterTest.kt | 10 ++++++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/list/PinnedMessagesListPresenter.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/list/PinnedMessagesListPresenter.kt index f005690ef0..c25675c7f7 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/list/PinnedMessagesListPresenter.kt +++ b/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 { @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) } } diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/pinned/list/PinnedMessagesListPresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/pinned/list/PinnedMessagesListPresenterTest.kt index 03927ec994..0478e08934 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/pinned/list/PinnedMessagesListPresenterTest.kt +++ b/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, ) } }