knock requests : rework knock requests service to avoid reloading of data (and weird ui glitch because of them)

features/knockrequests/impl/src/main/kotlin/io/element/android/features/knockrequests/impl/banner/KnockRequestsBannerPresenter.kt

@@ -19,11 +19,6 @@ import io.element.android.features.knockrequests.impl.data.KnockRequestsService
 import io.element.android.libraries.architecture.Presenter
 import io.element.android.libraries.core.coroutine.mapState
 import io.element.android.libraries.core.extensions.firstIfSingle
-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 io.element.android.libraries.matrix.ui.room.canHandleKnockRequestsAsState
-import io.element.android.libraries.matrix.ui.room.canInviteAsState
 import kotlinx.collections.immutable.toImmutableList
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.delay
@@ -33,10 +28,8 @@ import javax.inject.Inject
 private const val ACCEPT_ERROR_DISPLAY_DURATION = 1500L
 
 class KnockRequestsBannerPresenter @Inject constructor(
-    private val room: MatrixRoom,
     private val knockRequestsService: KnockRequestsService,
     private val appCoroutineScope: CoroutineScope,
-    private val featureFlagService: FeatureFlagService,
 ) : Presenter<KnockRequestsBannerState> {
     @Composable
     override fun present(): KnockRequestsBannerState {
@@ -48,15 +41,12 @@ class KnockRequestsBannerPresenter @Inject constructor(
             }
         }.collectAsState()
 
-        val syncUpdateFlow = room.syncUpdateFlow.collectAsState()
+        val permissions by knockRequestsService.permissionsFlow.collectAsState()
-        val canAccept by room.canInviteAsState(syncUpdateFlow.value)
-        val canHandleKnockRequests by room.canHandleKnockRequestsAsState(syncUpdateFlow.value)
         val showAcceptError = remember { mutableStateOf(false) }
-        val isKnockRequestsEnabled by featureFlagService.isFeatureEnabledFlow(FeatureFlags.Knock).collectAsState(false)
 
         val shouldShowBanner by remember {
             derivedStateOf {
-                isKnockRequestsEnabled && canHandleKnockRequests && knockRequests.isNotEmpty()
+                permissions.canHandle && knockRequests.isNotEmpty()
             }
         }
 
@@ -79,7 +69,7 @@ class KnockRequestsBannerPresenter @Inject constructor(
         return KnockRequestsBannerState(
             knockRequests = knockRequests,
             displayAcceptError = showAcceptError.value,
-            canAccept = canAccept,
+            canAccept = permissions.canAccept,
             isVisible = shouldShowBanner,
             eventSink = ::handleEvents,
         )

features/knockrequests/impl/src/main/kotlin/io/element/android/features/knockrequests/impl/data/KnockRequestPermissions.kt

@@ -0,0 +1,32 @@
+/*
+ * Copyright 2024 New Vector Ltd.
+ *
+ * SPDX-License-Identifier: AGPL-3.0-only
+ * Please see LICENSE in the repository root for full details.
+ */
+
+package io.element.android.features.knockrequests.impl.data
+
+import io.element.android.libraries.matrix.api.room.MatrixRoom
+import io.element.android.libraries.matrix.api.room.powerlevels.canBan
+import io.element.android.libraries.matrix.api.room.powerlevels.canInvite
+import io.element.android.libraries.matrix.api.room.powerlevels.canKick
+import kotlinx.coroutines.flow.Flow
+import kotlinx.coroutines.flow.map
+
+data class KnockRequestPermissions(
+    val canAccept: Boolean,
+    val canDecline: Boolean,
+    val canBan: Boolean,
+) {
+    val canHandle = canAccept || canDecline || canBan
+}
+
+fun MatrixRoom.knockRequestPermissionsFlow(): Flow<KnockRequestPermissions> {
+    return syncUpdateFlow.map {
+        val canAccept = canInvite().getOrDefault(false)
+        val canDecline = canKick().getOrDefault(false)
+        val canBan = canBan().getOrDefault(false)
+        KnockRequestPermissions(canAccept, canDecline, canBan)
+    }
+}

features/knockrequests/impl/src/main/kotlin/io/element/android/features/knockrequests/impl/data/KnockRequestWrapper.kt

@@ -12,23 +12,23 @@ import io.element.android.libraries.matrix.api.core.UserId
 import io.element.android.libraries.matrix.api.room.knock.KnockRequest
 
 class KnockRequestWrapper(
-    private val knockRequest: KnockRequest,
+    private val inner: KnockRequest,
     dateFormatter: (Long?) -> String? = { null }
 ) : KnockRequestPresentable {
-    override val eventId: EventId = knockRequest.eventId
+    override val eventId: EventId = inner.eventId
-    override val userId: UserId = knockRequest.userId
+    override val userId: UserId = inner.userId
-    override val displayName: String? = knockRequest.displayName
+    override val displayName: String? = inner.displayName
-    override val avatarUrl: String? = knockRequest.avatarUrl
+    override val avatarUrl: String? = inner.avatarUrl
-    override val reason: String? = knockRequest.reason?.trim()
+    override val reason: String? = inner.reason?.trim()
-    override val formattedDate: String? = dateFormatter(knockRequest.timestamp)
+    override val formattedDate: String? = dateFormatter(inner.timestamp)
 
-    val isSeen: Boolean = knockRequest.isSeen
+    val isSeen: Boolean = inner.isSeen
 
-    suspend fun accept(): Result<Unit> = knockRequest.accept()
+    suspend fun accept(): Result<Unit> = inner.accept()
 
-    suspend fun decline(reason: String?): Result<Unit> = knockRequest.decline(reason)
+    suspend fun decline(reason: String?): Result<Unit> = inner.decline(reason)
 
-    suspend fun declineAndBan(reason: String?): Result<Unit> = knockRequest.declineAndBan(reason)
+    suspend fun declineAndBan(reason: String?): Result<Unit> = inner.declineAndBan(reason)
 
-    suspend fun markAsSeen(): Result<Unit> = knockRequest.markAsSeen()
+    suspend fun markAsSeen(): Result<Unit> = inner.markAsSeen()
 }

features/knockrequests/impl/src/main/kotlin/io/element/android/features/knockrequests/impl/data/KnockRequestsModule.kt

@@ -12,6 +12,8 @@ import dagger.Module
 import dagger.Provides
 import io.element.android.libraries.di.RoomScope
 import io.element.android.libraries.di.SingleIn
+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
 
 @Module
@@ -19,7 +21,12 @@ import io.element.android.libraries.matrix.api.room.MatrixRoom
 object KnockRequestsModule {
     @Provides
     @SingleIn(RoomScope::class)
-    fun knockRequestsService(room: MatrixRoom): KnockRequestsService {
+    fun knockRequestsService(room: MatrixRoom, featureFlagService: FeatureFlagService): KnockRequestsService {
-        return KnockRequestsService(room.knockRequestsFlow, room.roomCoroutineScope)
+        return KnockRequestsService(
+            knockRequestsFlow = room.knockRequestsFlow,
+            permissionsFlow = room.knockRequestPermissionsFlow(),
+            isKnockFeatureEnabledFlow = featureFlagService.isFeatureEnabledFlow(FeatureFlags.Knock),
+            coroutineScope = room.roomCoroutineScope
+        )
     }
 }

features/knockrequests/impl/src/main/kotlin/io/element/android/features/knockrequests/impl/data/KnockRequestsService.kt

@@ -10,6 +10,7 @@ package io.element.android.features.knockrequests.impl.data
 import io.element.android.libraries.architecture.AsyncData
 import io.element.android.libraries.matrix.api.core.EventId
 import io.element.android.libraries.matrix.api.room.knock.KnockRequest
+import kotlinx.collections.immutable.persistentListOf
 import kotlinx.collections.immutable.toImmutableList
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.async
@@ -19,27 +20,40 @@ import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.SharingStarted
 import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.flow.getAndUpdate
-import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.stateIn
 import kotlinx.coroutines.supervisorScope
 
 class KnockRequestsService(
     knockRequestsFlow: Flow<List<KnockRequest>>,
+    permissionsFlow: Flow<KnockRequestPermissions>,
+    isKnockFeatureEnabledFlow: Flow<Boolean>,
     coroutineScope: CoroutineScope,
 ) {
     // Keep track of the knock requests that have been handled, so we don't have to wait for sync to remove them.
     private val handledKnockRequestIds = MutableStateFlow<Set<EventId>>(emptySet())
 
     val knockRequestsFlow = combine(
-        knockRequestsFlow.wrapped(),
+        isKnockFeatureEnabledFlow,
+        knockRequestsFlow,
         handledKnockRequestIds,
-    ) { knockRequests, handledKnockIds ->
+    ) { isKnockEnabled, knockRequests, handledKnockIds ->
-        val presentableKnockRequests = knockRequests
+        if (!isKnockEnabled) {
-            .filter { it.eventId !in handledKnockIds }
+            AsyncData.Success(persistentListOf())
-            .toImmutableList()
+        } else {
-        AsyncData.Success(presentableKnockRequests)
+            val presentableKnockRequests = knockRequests
+                .filter { it.eventId !in handledKnockIds }
+                .map { inner -> KnockRequestWrapper(inner) }
+                .toImmutableList()
+            AsyncData.Success(presentableKnockRequests)
+        }
     }.stateIn(coroutineScope, SharingStarted.Lazily, AsyncData.Loading())
 
+    val permissionsFlow = permissionsFlow.stateIn(
+        scope = coroutineScope,
+        started = SharingStarted.Lazily,
+        initialValue = KnockRequestPermissions(canAccept = false, canDecline = false, canBan = false)
+    )
+
     private fun knockRequestsList() = knockRequestsFlow.value.dataOrNull().orEmpty()
 
     private fun getKnockRequestById(eventId: EventId): KnockRequestWrapper? {
@@ -128,8 +142,4 @@ class KnockRequestsService(
     }
 
     private fun knockRequestNotFoundResult() = Result.failure<Unit>(IllegalArgumentException("Knock request not found"))
-
-    private fun Flow<List<KnockRequest>>.wrapped() = map { knockRequests ->
-        knockRequests.map { KnockRequestWrapper(it) }
-    }
 }

features/knockrequests/impl/src/main/kotlin/io/element/android/features/knockrequests/impl/list/KnockRequestsListPresenter.kt

@@ -20,16 +20,11 @@ import io.element.android.features.knockrequests.impl.data.KnockRequestsService
 import io.element.android.libraries.architecture.AsyncAction
 import io.element.android.libraries.architecture.Presenter
 import io.element.android.libraries.architecture.runUpdatingState
-import io.element.android.libraries.matrix.api.room.MatrixRoom
-import io.element.android.libraries.matrix.ui.room.canBanAsState
-import io.element.android.libraries.matrix.ui.room.canInviteAsState
-import io.element.android.libraries.matrix.ui.room.canKickAsState
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.launch
 import javax.inject.Inject
 
 class KnockRequestsListPresenter @Inject constructor(
-    private val room: MatrixRoom,
     private val knockRequestsService: KnockRequestsService,
 ) : Presenter<KnockRequestsListState> {
     @Composable
@@ -37,11 +32,7 @@ class KnockRequestsListPresenter @Inject constructor(
         val asyncAction = remember { mutableStateOf<AsyncAction<Unit>>(AsyncAction.Uninitialized) }
         var actionTarget by remember { mutableStateOf<KnockRequestsActionTarget>(KnockRequestsActionTarget.None) }
 
-        val syncUpdateFlow = room.syncUpdateFlow.collectAsState()
+        val permissions by knockRequestsService.permissionsFlow.collectAsState()
-        val canBan by room.canBanAsState(syncUpdateFlow.value)
-        val canDecline by room.canKickAsState(syncUpdateFlow.value)
-        val canAccept by room.canInviteAsState(syncUpdateFlow.value)
-
         val knockRequests by knockRequestsService.knockRequestsFlow.collectAsState()
 
         val coroutineScope = rememberCoroutineScope()
@@ -79,10 +70,8 @@ class KnockRequestsListPresenter @Inject constructor(
         return KnockRequestsListState(
             knockRequests = knockRequests,
             actionTarget = actionTarget,
+            permissions = permissions,
             asyncAction = asyncAction.value,
-            canAccept = canAccept,
-            canDecline = canDecline,
-            canBan = canBan,
             eventSink = ::handleEvents
         )
     }

features/knockrequests/impl/src/main/kotlin/io/element/android/features/knockrequests/impl/list/KnockRequestsListState.kt

@@ -8,6 +8,7 @@
 package io.element.android.features.knockrequests.impl.list
 
 import androidx.compose.runtime.Immutable
+import io.element.android.features.knockrequests.impl.data.KnockRequestPermissions
 import io.element.android.features.knockrequests.impl.data.KnockRequestPresentable
 import io.element.android.libraries.architecture.AsyncAction
 import io.element.android.libraries.architecture.AsyncData
@@ -17,12 +18,10 @@ data class KnockRequestsListState(
     val knockRequests: AsyncData<ImmutableList<KnockRequestPresentable>>,
     val actionTarget: KnockRequestsActionTarget,
     val asyncAction: AsyncAction<Unit>,
-    val canAccept: Boolean,
+    val permissions: KnockRequestPermissions,
-    val canDecline: Boolean,
-    val canBan: Boolean,
     val eventSink: (KnockRequestsListEvents) -> Unit,
 ) {
-    val canAcceptAll = canAccept && knockRequests is AsyncData.Success && knockRequests.data.size > 1
+    val canAcceptAll = permissions.canAccept && knockRequests is AsyncData.Success && knockRequests.data.size > 1
 }
 
 @Immutable

features/knockrequests/impl/src/main/kotlin/io/element/android/features/knockrequests/impl/list/KnockRequestsListStateProvider.kt

@@ -8,6 +8,7 @@
 package io.element.android.features.knockrequests.impl.list
 
 import androidx.compose.ui.tooling.preview.PreviewParameterProvider
+import io.element.android.features.knockrequests.impl.data.KnockRequestPermissions
 import io.element.android.features.knockrequests.impl.data.KnockRequestPresentable
 import io.element.android.features.knockrequests.impl.data.aKnockRequest
 import io.element.android.libraries.architecture.AsyncAction
@@ -82,7 +83,11 @@ open class KnockRequestsListStateProvider : PreviewParameterProvider<KnockReques
                         aKnockRequest()
                     )
                 ),
-                canAccept = false,
+                permissions = KnockRequestPermissions(
+                    canAccept = false,
+                    canDecline = true,
+                    canBan = true,
+                ),
                 actionTarget = KnockRequestsActionTarget.AcceptAll,
                 asyncAction = AsyncAction.Failure(Throwable("Failed to accept all")),
             ),
@@ -92,7 +97,11 @@ open class KnockRequestsListStateProvider : PreviewParameterProvider<KnockReques
                         aKnockRequest()
                     )
                 ),
-                canDecline = false,
+                permissions = KnockRequestPermissions(
+                    canAccept = true,
+                    canDecline = false,
+                    canBan = true,
+                ),
             ),
             aKnockRequestsListState(
                 knockRequests = AsyncData.Success(
@@ -100,8 +109,11 @@ open class KnockRequestsListStateProvider : PreviewParameterProvider<KnockReques
                         aKnockRequest()
                     )
                 ),
-                canAccept = false,
+                permissions = KnockRequestPermissions(
-                canDecline = false,
+                    canAccept = false,
+                    canDecline = false,
+                    canBan = true,
+                ),
             ),
             aKnockRequestsListState(
                 knockRequests = AsyncData.Success(
@@ -109,7 +121,11 @@ open class KnockRequestsListStateProvider : PreviewParameterProvider<KnockReques
                         aKnockRequest()
                     )
                 ),
-                canBan = false,
+                permissions = KnockRequestPermissions(
+                    canAccept = true,
+                    canDecline = true,
+                    canBan = false,
+                ),
             ),
         )
 }
@@ -118,16 +134,16 @@ fun aKnockRequestsListState(
     knockRequests: AsyncData<ImmutableList<KnockRequestPresentable>> = AsyncData.Success(persistentListOf()),
     actionTarget: KnockRequestsActionTarget = KnockRequestsActionTarget.None,
     asyncAction: AsyncAction<Unit> = AsyncAction.Uninitialized,
-    canAccept: Boolean = true,
+    permissions: KnockRequestPermissions = KnockRequestPermissions(
-    canDecline: Boolean = true,
+        canAccept = true,
-    canBan: Boolean = true,
+        canDecline = true,
+        canBan = true,
+    ),
     eventSink: (KnockRequestsListEvents) -> Unit = {},
 ) = KnockRequestsListState(
     knockRequests = knockRequests,
     actionTarget = actionTarget,
     asyncAction = asyncAction,
-    canAccept = canAccept,
+    permissions = permissions,
-    canDecline = canDecline,
-    canBan = canBan,
     eventSink = eventSink,
 )

features/knockrequests/impl/src/main/kotlin/io/element/android/features/knockrequests/impl/list/KnockRequestsListView.kt

@@ -126,9 +126,9 @@ private fun KnockRequestsListContent(
                 } else {
                     KnockRequestsList(
                         knockRequests = knockRequests,
-                        canAccept = state.canAccept,
+                        canAccept = state.permissions.canAccept,
-                        canDecline = state.canDecline,
+                        canDecline = state.permissions.canDecline,
-                        canBan = state.canBan,
+                        canBan = state.permissions.canBan,
                         onAcceptClick = ::onAcceptClick,
                         onDeclineClick = ::onDeclineClick,
                         onBanClick = ::onBanClick,

features/knockrequests/impl/src/test/kotlin/io/element/android/features/knockrequests/impl/banner/KnockRequestsBannerPresenterTest.kt

@@ -8,14 +8,12 @@
 package io.element.android.features.knockrequests.impl.banner
 
 import com.google.common.truth.Truth.assertThat
+import io.element.android.features.knockrequests.impl.data.KnockRequestPermissions
 import io.element.android.features.knockrequests.impl.data.KnockRequestsService
-import io.element.android.libraries.featureflag.api.FeatureFlags
-import io.element.android.libraries.featureflag.test.FakeFeatureFlagService
 import io.element.android.libraries.matrix.api.room.knock.KnockRequest
 import io.element.android.libraries.matrix.test.A_USER_ID
 import io.element.android.libraries.matrix.test.A_USER_ID_2
 import io.element.android.libraries.matrix.test.A_USER_ID_3
-import io.element.android.libraries.matrix.test.room.FakeMatrixRoom
 import io.element.android.libraries.matrix.test.room.knock.FakeKnockRequest
 import io.element.android.tests.testutils.lambda.assert
 import io.element.android.tests.testutils.lambda.lambdaRecorder
@@ -234,19 +232,12 @@ private fun TestScope.createKnockRequestsBannerPresenter(
 ): KnockRequestsBannerPresenter {
     val knockRequestsService = KnockRequestsService(
         knockRequestsFlow = knockRequestsFlow,
-        coroutineScope = backgroundScope
+        coroutineScope = backgroundScope,
-    )
+        isKnockFeatureEnabledFlow = flowOf(isFeatureEnabled),
-    val featureFlagService = FakeFeatureFlagService(
+        permissionsFlow = flowOf(KnockRequestPermissions(canAcceptKnockRequests, canAcceptKnockRequests, canAcceptKnockRequests)),
-        initialState = mapOf(
-            FeatureFlags.Knock.key to isFeatureEnabled
-        )
     )
     return KnockRequestsBannerPresenter(
-        room = FakeMatrixRoom(
-            canInviteResult = { Result.success(canAcceptKnockRequests) }
-        ),
         knockRequestsService = knockRequestsService,
         appCoroutineScope = this,
-        featureFlagService = featureFlagService
     )
 }

features/knockrequests/impl/src/test/kotlin/io/element/android/features/knockrequests/impl/list/KnockRequestsListPresenterTest.kt

@@ -10,13 +10,13 @@
 package io.element.android.features.knockrequests.impl.list
 
 import com.google.common.truth.Truth.assertThat
+import io.element.android.features.knockrequests.impl.data.KnockRequestPermissions
 import io.element.android.features.knockrequests.impl.data.KnockRequestsService
 import io.element.android.libraries.architecture.AsyncAction
 import io.element.android.libraries.architecture.AsyncData
 import io.element.android.libraries.matrix.api.room.knock.KnockRequest
 import io.element.android.libraries.matrix.test.AN_EVENT_ID
 import io.element.android.libraries.matrix.test.AN_EVENT_ID_2
-import io.element.android.libraries.matrix.test.room.FakeMatrixRoom
 import io.element.android.libraries.matrix.test.room.knock.FakeKnockRequest
 import io.element.android.tests.testutils.lambda.assert
 import io.element.android.tests.testutils.lambda.lambdaRecorder
@@ -35,15 +35,15 @@ class KnockRequestsListPresenterTest {
         presenter.test {
             awaitItem().also { state ->
                 assertThat(state.knockRequests).isInstanceOf(AsyncData.Loading::class.java)
-                assertThat(state.canAccept).isFalse()
+                assertThat(state.permissions.canAccept).isFalse()
-                assertThat(state.canDecline).isFalse()
+                assertThat(state.permissions.canDecline).isFalse()
-                assertThat(state.canBan).isFalse()
+                assertThat(state.permissions.canBan).isFalse()
             }
             awaitItem().also { state ->
                 assertThat(state.knockRequests).isInstanceOf(AsyncData.Loading::class.java)
-                assertThat(state.canAccept).isTrue()
+                assertThat(state.permissions.canAccept).isTrue()
-                assertThat(state.canDecline).isTrue()
+                assertThat(state.permissions.canDecline).isTrue()
-                assertThat(state.canBan).isTrue()
+                assertThat(state.permissions.canBan).isTrue()
             }
             awaitItem().also { state ->
                 assertThat(state.knockRequests).isInstanceOf(AsyncData.Success::class.java)
@@ -293,18 +293,12 @@ class KnockRequestsListPresenterTest {
         canBan: Boolean = true,
         knockRequestsFlow: Flow<List<KnockRequest>> = flowOf(emptyList())
     ): KnockRequestsListPresenter {
-        val room = FakeMatrixRoom(
-            canInviteResult = { Result.success(canAccept) },
-            canKickResult = { Result.success(canDecline) },
-            canBanResult = { Result.success(canBan) }
-        )
         val knockRequestsService = KnockRequestsService(
             knockRequestsFlow = knockRequestsFlow,
-            coroutineScope = backgroundScope
+            coroutineScope = backgroundScope,
-        )
+            isKnockFeatureEnabledFlow = flowOf(true),
-        return KnockRequestsListPresenter(
+            permissionsFlow = flowOf(KnockRequestPermissions(canAccept, canDecline, canBan)),
-            room = room,
-            knockRequestsService = knockRequestsService,
         )
+        return KnockRequestsListPresenter(knockRequestsService = knockRequestsService)
     }
 }

features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt

@@ -48,7 +48,6 @@ import io.element.android.services.analytics.api.AnalyticsService
 import io.element.android.services.analyticsproviders.api.trackers.captureInteraction
 import kotlinx.collections.immutable.toPersistentList
 import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.flow.collect
 import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.launch