From 0f8cf5024455b5c7935a016992742cb651dfb737 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Thu, 6 Jul 2023 15:52:46 +0100 Subject: [PATCH 1/2] Room details: don't allow edits in DMs If the room is a DM, we won't allow any editing functionality regardless of power levels. If there is no topic set, then the entire section is hidden, like in rooms without a topic where you lack the power level to change it. Closes #799 --- .../roomdetails/impl/RoomDetailsPresenter.kt | 28 ++++----- .../roomdetails/RoomDetailsPresenterTests.kt | 58 +++++++++++++++++++ 2 files changed, 68 insertions(+), 18 deletions(-) diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt index 2c70e66ff6..4ef482ffe9 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt @@ -22,7 +22,7 @@ import androidx.compose.runtime.State import androidx.compose.runtime.collectAsState import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.getValue -import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.produceState import androidx.compose.runtime.remember import io.element.android.features.leaveroom.api.LeaveRoomEvent import io.element.android.features.leaveroom.api.LeaveRoomPresenter @@ -55,14 +55,14 @@ class RoomDetailsPresenter @Inject constructor( val canEditTopic by getCanSendStateEvent(membersState, StateEventType.ROOM_TOPIC) val dmMember by room.getDirectRoomMember(membersState) val roomMemberDetailsPresenter = roomMemberDetailsPresenter(dmMember) - val roomType = getRoomType(dmMember) + val roomType by getRoomType(dmMember) - val topicState = remember(canEditTopic, room.topic) { + val topicState = remember(canEditTopic, room.topic, roomType) { val topic = room.topic when { !topic.isNullOrBlank() -> RoomTopicState.ExistingTopic(topic) - canEditTopic -> RoomTopicState.CanAddTopic + canEditTopic && roomType is RoomDetailsType.Room -> RoomTopicState.CanAddTopic else -> RoomTopicState.Hidden } } @@ -85,8 +85,8 @@ class RoomDetailsPresenter @Inject constructor( memberCount = room.joinedMemberCount, isEncrypted = room.isEncrypted, canInvite = canInvite, - canEdit = canEditAvatar || canEditName || canEditTopic, - roomType = roomType.value, + canEdit = (canEditAvatar || canEditName || canEditTopic) && roomType == RoomDetailsType.Room, + roomType = roomType, roomMemberDetailsState = roomMemberDetailsState, leaveRoomState = leaveRoomState, eventSink = ::handleEvents, @@ -112,20 +112,12 @@ class RoomDetailsPresenter @Inject constructor( } @Composable - private fun getCanInvite(membersState: MatrixRoomMembersState): State { - val canInvite = remember(membersState) { mutableStateOf(false) } - LaunchedEffect(membersState) { - canInvite.value = room.canInvite().getOrElse { false } - } - return canInvite + private fun getCanInvite(membersState: MatrixRoomMembersState) = produceState(false, membersState) { + value = room.canInvite().getOrElse { false } } @Composable - private fun getCanSendStateEvent(membersState: MatrixRoomMembersState, type: StateEventType): State { - val canSendEvent = remember(membersState) { mutableStateOf(false) } - LaunchedEffect(membersState) { - canSendEvent.value = room.canSendStateEvent(type).getOrElse { false } - } - return canSendEvent + private fun getCanSendStateEvent(membersState: MatrixRoomMembersState, type: StateEventType) = produceState(false, membersState) { + value = room.canSendStateEvent(type).getOrElse { false } } } diff --git a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt index f9d947615a..60e8d248b5 100644 --- a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt +++ b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt @@ -173,6 +173,64 @@ class RoomDetailsPresenterTests { } } + @Test + fun `present - initial state when user can edit attributes in a DM`() = runTest { + val myRoomMember = aRoomMember(A_SESSION_ID) + val otherRoomMember = aRoomMember(A_USER_ID_2) + val room = aMatrixRoom( + isEncrypted = true, + isDirect = true, + ).apply { + val roomMembers = listOf(myRoomMember, otherRoomMember) + givenRoomMembersState(MatrixRoomMembersState.Ready(roomMembers)) + + givenCanSendStateResult(StateEventType.ROOM_TOPIC, Result.success(true)) + givenCanSendStateResult(StateEventType.ROOM_NAME, Result.success(true)) + givenCanSendStateResult(StateEventType.ROOM_AVATAR, Result.success(true)) + givenCanInviteResult(Result.success(false)) + } + val presenter = aRoomDetailsPresenter(room) + moleculeFlow(RecompositionClock.Immediate) { + presenter.present() + }.test { + // Initially false + assertThat(awaitItem().canEdit).isFalse() + // Then the asynchronous check completes, but editing is still disallowed because it's a DM + val settledState = awaitItem() + assertThat(settledState.canEdit).isFalse() + // If there is a topic, it's visible + assertThat(settledState.roomTopic).isEqualTo(RoomTopicState.ExistingTopic(room.topic!!)) + + cancelAndIgnoreRemainingEvents() + } + } + @Test + fun `present - initial state when in a DM with no topic`() = runTest { + val myRoomMember = aRoomMember(A_SESSION_ID) + val otherRoomMember = aRoomMember(A_USER_ID_2) + val room = aMatrixRoom( + isEncrypted = true, + isDirect = true, + topic = null, + ).apply { + val roomMembers = listOf(myRoomMember, otherRoomMember) + givenRoomMembersState(MatrixRoomMembersState.Ready(roomMembers)) + + givenCanSendStateResult(StateEventType.ROOM_TOPIC, Result.success(true)) + } + val presenter = aRoomDetailsPresenter(room) + moleculeFlow(RecompositionClock.Immediate) { + presenter.present() + }.test { + skipItems(1) + + // There's no topic, so we hide the entire UI for DMs + assertThat(awaitItem().roomTopic).isEqualTo(RoomTopicState.Hidden) + + cancelAndIgnoreRemainingEvents() + } + } + @Test fun `present - initial state when user can edit all attributes`() = runTest { val room = aMatrixRoom().apply { From 4f014fcde26bae70cd5441642c35e35e38670653 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Thu, 6 Jul 2023 16:22:49 +0100 Subject: [PATCH 2/2] Add test for leaving rooms Completely unrelated to what I was doing, but might appease the code coverage gods? --- .../roomdetails/RoomDetailsPresenterTests.kt | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt index 60e8d248b5..ccd1476c3a 100644 --- a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt +++ b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt @@ -20,7 +20,10 @@ import app.cash.molecule.RecompositionClock import app.cash.molecule.moleculeFlow import app.cash.turbine.test import com.google.common.truth.Truth.assertThat +import io.element.android.features.leaveroom.api.LeaveRoomEvent +import io.element.android.features.leaveroom.api.LeaveRoomPresenter import io.element.android.features.leaveroom.fake.LeaveRoomPresenterFake +import io.element.android.features.roomdetails.impl.RoomDetailsEvent import io.element.android.features.roomdetails.impl.RoomDetailsPresenter import io.element.android.features.roomdetails.impl.RoomDetailsType import io.element.android.features.roomdetails.impl.RoomTopicState @@ -44,13 +47,13 @@ import org.junit.Test @ExperimentalCoroutinesApi class RoomDetailsPresenterTests { - private fun aRoomDetailsPresenter(room: MatrixRoom): RoomDetailsPresenter { + private fun aRoomDetailsPresenter(room: MatrixRoom, leaveRoomPresenter: LeaveRoomPresenter = LeaveRoomPresenterFake()): RoomDetailsPresenter { val roomMemberDetailsPresenterFactory = object : RoomMemberDetailsPresenter.Factory { override fun create(roomMemberId: UserId): RoomMemberDetailsPresenter { return RoomMemberDetailsPresenter(FakeMatrixClient(), room, roomMemberId) } } - return RoomDetailsPresenter(room, roomMemberDetailsPresenterFactory, LeaveRoomPresenterFake()) + return RoomDetailsPresenter(room, roomMemberDetailsPresenterFactory, leaveRoomPresenter) } @Test @@ -305,6 +308,22 @@ class RoomDetailsPresenterTests { cancelAndIgnoreRemainingEvents() } } + + @Test + fun `present - leave room event is passed on to leave room presenter`() = runTest { + val leaveRoomPresenter = LeaveRoomPresenterFake() + val room = aMatrixRoom() + val presenter = aRoomDetailsPresenter(room, leaveRoomPresenter) + moleculeFlow(RecompositionClock.Immediate) { + presenter.present() + }.test { + awaitItem().eventSink(RoomDetailsEvent.LeaveRoom) + + assertThat(leaveRoomPresenter.events).contains(LeaveRoomEvent.ShowConfirmation(room.roomId)) + + cancelAndIgnoreRemainingEvents() + } + } } fun aMatrixRoom(