From e6badb1e04e9e73ea038b84334e82ea961b82f6f Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 15 May 2024 11:24:09 +0200 Subject: [PATCH] Use rawName instead of displayName in RoomDetailsEditPresenter #2844 --- .../impl/edit/RoomDetailsEditPresenter.kt | 93 ++++++++++++------- .../impl/edit/RoomDetailsEditState.kt | 3 +- .../impl/edit/RoomDetailsEditStateProvider.kt | 7 +- .../impl/edit/RoomDetailsEditView.kt | 7 +- .../edit/RoomDetailsEditPresenterTest.kt | 25 +++-- .../matrix/ui/room/MatrixRoomState.kt | 18 ++++ 6 files changed, 107 insertions(+), 46 deletions(-) diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditPresenter.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditPresenter.kt index df179bec48..600fee771f 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditPresenter.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditPresenter.kt @@ -37,6 +37,9 @@ import io.element.android.libraries.matrix.api.room.MatrixRoom import io.element.android.libraries.matrix.api.room.StateEventType import io.element.android.libraries.matrix.api.room.powerlevels.canSendState import io.element.android.libraries.matrix.ui.media.AvatarAction +import io.element.android.libraries.matrix.ui.room.avatarUrl +import io.element.android.libraries.matrix.ui.room.rawName +import io.element.android.libraries.matrix.ui.room.topic import io.element.android.libraries.mediapickers.api.PickerProvider import io.element.android.libraries.mediaupload.api.MediaPreProcessor import io.element.android.libraries.permissions.api.PermissionsEvents @@ -61,23 +64,38 @@ class RoomDetailsEditPresenter @Inject constructor( val cameraPermissionState = cameraPermissionPresenter.present() val roomSyncUpdateFlow = room.syncUpdateFlow.collectAsState() - // Since there is no way to obtain the new avatar uri after uploading a new avatar, - // just erase the local value when the room field has changed - var roomAvatarUri by rememberSaveable(room.avatarUrl) { mutableStateOf(room.avatarUrl?.toUri()) } + val roomAvatarUri = room.avatarUrl()?.toUri() + var roomAvatarUriEdited by rememberSaveable { mutableStateOf(null) } + LaunchedEffect(roomAvatarUri) { + // Every time the roomAvatar change (from sync), we can set the new avatar. + roomAvatarUriEdited = roomAvatarUri + } - var roomName by rememberSaveable { mutableStateOf(room.displayName.trim()) } - var roomTopic by rememberSaveable { mutableStateOf(room.topic?.trim()) } + val roomRawNameTrimmed = room.rawName().orEmpty().trim() + var roomRawNameEdited by rememberSaveable { mutableStateOf("") } + LaunchedEffect(roomRawNameTrimmed) { + // Every time the rawName change (from sync), we can set the new name. + roomRawNameEdited = roomRawNameTrimmed + } + val roomTopicTrimmed = room.topic().orEmpty().trim() + var roomTopicEdited by rememberSaveable { mutableStateOf("") } + LaunchedEffect(roomTopicTrimmed) { + // Every time the topic change (from sync), we can set the new topic. + roomTopicEdited = roomTopicTrimmed + } val saveButtonEnabled by remember( - roomSyncUpdateFlow.value, - roomName, - roomTopic, + roomRawNameTrimmed, + roomRawNameEdited, + roomTopicTrimmed, + roomTopicEdited, roomAvatarUri, + roomAvatarUriEdited, ) { derivedStateOf { - roomAvatarUri?.toString()?.trim() != room.avatarUrl?.toUri()?.toString()?.trim() || - roomName.trim() != room.displayName.trim() || - roomTopic.orEmpty().trim() != room.topic.orEmpty().trim() + roomRawNameTrimmed != roomRawNameEdited.trim() || + roomTopicTrimmed != roomTopicEdited.trim() || + roomAvatarUri != roomAvatarUriEdited } } @@ -85,17 +103,17 @@ class RoomDetailsEditPresenter @Inject constructor( var canChangeTopic by remember { mutableStateOf(false) } var canChangeAvatar by remember { mutableStateOf(false) } - LaunchedEffect(Unit) { + LaunchedEffect(roomSyncUpdateFlow.value) { canChangeName = room.canSendState(StateEventType.ROOM_NAME).getOrElse { false } canChangeTopic = room.canSendState(StateEventType.ROOM_TOPIC).getOrElse { false } canChangeAvatar = room.canSendState(StateEventType.ROOM_AVATAR).getOrElse { false } } val cameraPhotoPicker = mediaPickerProvider.registerCameraPhotoPicker( - onResult = { uri -> if (uri != null) roomAvatarUri = uri } + onResult = { uri -> if (uri != null) roomAvatarUriEdited = uri } ) val galleryImagePicker = mediaPickerProvider.registerGalleryImagePicker( - onResult = { uri -> if (uri != null) roomAvatarUri = uri } + onResult = { uri -> if (uri != null) roomAvatarUriEdited = uri } ) LaunchedEffect(cameraPermissionState.permissionGranted) { @@ -105,12 +123,12 @@ class RoomDetailsEditPresenter @Inject constructor( } } - val avatarActions by remember(roomAvatarUri) { + val avatarActions by remember(roomAvatarUriEdited) { derivedStateOf { listOfNotNull( AvatarAction.TakePhoto, AvatarAction.ChoosePhoto, - AvatarAction.Remove.takeIf { roomAvatarUri != null }, + AvatarAction.Remove.takeIf { roomAvatarUriEdited != null }, ).toImmutableList() } } @@ -119,7 +137,15 @@ class RoomDetailsEditPresenter @Inject constructor( val localCoroutineScope = rememberCoroutineScope() fun handleEvents(event: RoomDetailsEditEvents) { when (event) { - is RoomDetailsEditEvents.Save -> localCoroutineScope.saveChanges(roomName, roomTopic, roomAvatarUri, saveAction) + is RoomDetailsEditEvents.Save -> localCoroutineScope.saveChanges( + currentNameTrimmed = roomRawNameTrimmed, + newNameTrimmed = roomRawNameEdited.trim(), + currentTopicTrimmed = roomTopicTrimmed, + newTopicTrimmed = roomTopicEdited.trim(), + currentAvatar = roomAvatarUri, + newAvatarUri = roomAvatarUriEdited, + action = saveAction, + ) is RoomDetailsEditEvents.HandleAvatarAction -> { when (event.action) { AvatarAction.ChoosePhoto -> galleryImagePicker.launch() @@ -129,23 +155,23 @@ class RoomDetailsEditPresenter @Inject constructor( pendingPermissionRequest = true cameraPermissionState.eventSink(PermissionsEvents.RequestPermissions) } - AvatarAction.Remove -> roomAvatarUri = null + AvatarAction.Remove -> roomAvatarUriEdited = null } } - is RoomDetailsEditEvents.UpdateRoomName -> roomName = event.name - is RoomDetailsEditEvents.UpdateRoomTopic -> roomTopic = event.topic.takeUnless { it.isEmpty() } + is RoomDetailsEditEvents.UpdateRoomName -> roomRawNameEdited = event.name + is RoomDetailsEditEvents.UpdateRoomTopic -> roomTopicEdited = event.topic RoomDetailsEditEvents.CancelSaveChanges -> saveAction.value = AsyncAction.Uninitialized } } return RoomDetailsEditState( roomId = room.roomId, - roomName = roomName, + roomRawName = roomRawNameEdited, canChangeName = canChangeName, - roomTopic = roomTopic.orEmpty(), + roomTopic = roomTopicEdited, canChangeTopic = canChangeTopic, - roomAvatarUrl = roomAvatarUri, + roomAvatarUrl = roomAvatarUriEdited, canChangeAvatar = canChangeAvatar, avatarActions = avatarActions, saveButtonEnabled = saveButtonEnabled, @@ -156,25 +182,28 @@ class RoomDetailsEditPresenter @Inject constructor( } private fun CoroutineScope.saveChanges( - name: String, - topic: String?, - avatarUri: Uri?, + currentNameTrimmed: String, + newNameTrimmed: String, + currentTopicTrimmed: String, + newTopicTrimmed: String, + currentAvatar: Uri?, + newAvatarUri: Uri?, action: MutableState>, ) = launch { val results = mutableListOf>() suspend { - if (topic.orEmpty().trim() != room.topic.orEmpty().trim()) { - results.add(room.setTopic(topic.orEmpty()).onFailure { + if (newTopicTrimmed != currentTopicTrimmed) { + results.add(room.setTopic(newTopicTrimmed).onFailure { Timber.e(it, "Failed to set room topic") }) } - if (name.isNotEmpty() && name.trim() != room.displayName.trim()) { - results.add(room.setName(name).onFailure { + if (newNameTrimmed.isNotEmpty() && newNameTrimmed != currentNameTrimmed) { + results.add(room.setName(newNameTrimmed).onFailure { Timber.e(it, "Failed to set room name") }) } - if (avatarUri?.toString()?.trim() != room.avatarUrl?.trim()) { - results.add(updateAvatar(avatarUri).onFailure { + if (newAvatarUri != currentAvatar) { + results.add(updateAvatar(newAvatarUri).onFailure { Timber.e(it, "Failed to update avatar") }) } diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditState.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditState.kt index 7efe68163f..ec74905e38 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditState.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditState.kt @@ -25,7 +25,8 @@ import kotlinx.collections.immutable.ImmutableList data class RoomDetailsEditState( val roomId: RoomId, - val roomName: String, + /** The raw room name (i.e. the room name from the state event `m.room.name`), not the display name. */ + val roomRawName: String, val canChangeName: Boolean, val roomTopic: String, val canChangeTopic: Boolean, diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditStateProvider.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditStateProvider.kt index e98154cbf6..7b6e16352a 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditStateProvider.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditStateProvider.kt @@ -30,17 +30,18 @@ open class RoomDetailsEditStateProvider : PreviewParameterProvider Unit = {}, ) = RoomDetailsEditState( roomId = roomId, - roomName = roomName, + roomRawName = roomRawName, canChangeName = canChangeName, roomTopic = roomTopic, canChangeTopic = canChangeTopic, diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditView.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditView.kt index 0c172a452c..6e4c0bea25 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditView.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditView.kt @@ -119,7 +119,8 @@ fun RoomDetailsEditView( Spacer(modifier = Modifier.height(24.dp)) EditableAvatarView( matrixId = state.roomId.value, - displayName = state.roomName, + // As per Element Web, we use the raw name for the avatar as well + displayName = state.roomRawName, avatarUrl = state.roomAvatarUrl, avatarSize = AvatarSize.EditRoomDetails, onAvatarClicked = ::onAvatarClicked, @@ -130,7 +131,7 @@ fun RoomDetailsEditView( if (state.canChangeName) { LabelledTextField( label = stringResource(id = R.string.screen_room_details_room_name_label), - value = state.roomName, + value = state.roomRawName, placeholder = stringResource(CommonStrings.common_room_name_placeholder), singleLine = true, onValueChange = { state.eventSink(RoomDetailsEditEvents.UpdateRoomName(it)) }, @@ -138,7 +139,7 @@ fun RoomDetailsEditView( } else { LabelledReadOnlyField( title = stringResource(R.string.screen_room_details_room_name_label), - value = state.roomName + value = state.roomRawName ) } diff --git a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/edit/RoomDetailsEditPresenterTest.kt b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/edit/RoomDetailsEditPresenterTest.kt index f38689a5d7..a14d9c52c4 100644 --- a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/edit/RoomDetailsEditPresenterTest.kt +++ b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/edit/RoomDetailsEditPresenterTest.kt @@ -28,6 +28,7 @@ import io.element.android.libraries.architecture.AsyncAction import io.element.android.libraries.matrix.api.room.MatrixRoom import io.element.android.libraries.matrix.api.room.StateEventType import io.element.android.libraries.matrix.test.AN_AVATAR_URL +import io.element.android.libraries.matrix.test.room.aRoomInfo import io.element.android.libraries.matrix.ui.media.AvatarAction import io.element.android.libraries.mediapickers.test.FakePickerProvider import io.element.android.libraries.mediaupload.api.MediaUploadInfo @@ -90,7 +91,17 @@ class RoomDetailsEditPresenterTest { @Test fun `present - initial state is created from room info`() = runTest { - val room = aMatrixRoom(avatarUrl = AN_AVATAR_URL) + val room = aMatrixRoom( + avatarUrl = AN_AVATAR_URL, + displayName = "a display name", + ).also { + it.givenRoomInfo( + aRoomInfo( + name = "a display name", + rawName = "a raw name", + ) + ) + } val presenter = createRoomDetailsEditPresenter(room) moleculeFlow(RecompositionMode.Immediate) { @@ -98,7 +109,7 @@ class RoomDetailsEditPresenterTest { }.test { val initialState = awaitItem() assertThat(initialState.roomId).isEqualTo(room.roomId) - assertThat(initialState.roomName).isEqualTo(room.displayName) + assertThat(initialState.roomRawName).isEqualTo("a raw name") assertThat(initialState.roomAvatarUrl).isEqualTo(roomAvatarUri) assertThat(initialState.roomTopic).isEqualTo(room.topic.orEmpty()) assertThat(initialState.avatarActions).containsExactly( @@ -199,34 +210,34 @@ class RoomDetailsEditPresenterTest { }.test { val initialState = awaitItem() assertThat(initialState.roomTopic).isEqualTo("My topic") - assertThat(initialState.roomName).isEqualTo("Name") + assertThat(initialState.roomRawName).isEqualTo("Name") assertThat(initialState.roomAvatarUrl).isEqualTo(roomAvatarUri) initialState.eventSink(RoomDetailsEditEvents.UpdateRoomName("Name II")) awaitItem().apply { assertThat(roomTopic).isEqualTo("My topic") - assertThat(roomName).isEqualTo("Name II") + assertThat(roomRawName).isEqualTo("Name II") assertThat(roomAvatarUrl).isEqualTo(roomAvatarUri) } initialState.eventSink(RoomDetailsEditEvents.UpdateRoomName("Name III")) awaitItem().apply { assertThat(roomTopic).isEqualTo("My topic") - assertThat(roomName).isEqualTo("Name III") + assertThat(roomRawName).isEqualTo("Name III") assertThat(roomAvatarUrl).isEqualTo(roomAvatarUri) } initialState.eventSink(RoomDetailsEditEvents.UpdateRoomTopic("Another topic")) awaitItem().apply { assertThat(roomTopic).isEqualTo("Another topic") - assertThat(roomName).isEqualTo("Name III") + assertThat(roomRawName).isEqualTo("Name III") assertThat(roomAvatarUrl).isEqualTo(roomAvatarUri) } initialState.eventSink(RoomDetailsEditEvents.HandleAvatarAction(AvatarAction.Remove)) awaitItem().apply { assertThat(roomTopic).isEqualTo("Another topic") - assertThat(roomName).isEqualTo("Name III") + assertThat(roomRawName).isEqualTo("Name III") assertThat(roomAvatarUrl).isNull() } } diff --git a/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/room/MatrixRoomState.kt b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/room/MatrixRoomState.kt index aa3121146a..ab3c80a6e2 100644 --- a/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/room/MatrixRoomState.kt +++ b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/room/MatrixRoomState.kt @@ -62,3 +62,21 @@ fun MatrixRoom.isOwnUserAdmin(): Boolean { val powerLevel = roomInfo?.userPowerLevels?.get(sessionId) ?: 0L return RoomMember.Role.forPowerLevel(powerLevel) == RoomMember.Role.ADMIN } + +@Composable +fun MatrixRoom.rawName(): String? { + val roomInfo by roomInfoFlow.collectAsState(initial = null) + return roomInfo?.rawName +} + +@Composable +fun MatrixRoom.topic(): String? { + val roomInfo by roomInfoFlow.collectAsState(initial = null) + return roomInfo?.topic +} + +@Composable +fun MatrixRoom.avatarUrl(): String? { + val roomInfo by roomInfoFlow.collectAsState(initial = null) + return roomInfo?.avatarUrl +}