Use rawName instead of displayName in RoomDetailsEditPresenter #2844

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<Uri?>(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<AsyncAction<Unit>>,
     ) = launch {
         val results = mutableListOf<Result<Unit>>()
         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")
                 })
             }

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,

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

@@ -30,17 +30,18 @@ open class RoomDetailsEditStateProvider : PreviewParameterProvider<RoomDetailsEd
         get() = sequenceOf(
             aRoomDetailsEditState(),
             aRoomDetailsEditState(roomTopic = ""),
+            aRoomDetailsEditState(roomRawName = ""),
             aRoomDetailsEditState(roomAvatarUrl = Uri.parse("example://uri")),
             aRoomDetailsEditState(canChangeName = true, canChangeTopic = false, canChangeAvatar = true, saveButtonEnabled = false),
             aRoomDetailsEditState(canChangeName = false, canChangeTopic = true, canChangeAvatar = false, saveButtonEnabled = false),
             aRoomDetailsEditState(saveAction = AsyncAction.Loading),
-            aRoomDetailsEditState(saveAction = AsyncAction.Failure(Throwable("Whelp")))
+            aRoomDetailsEditState(saveAction = AsyncAction.Failure(Throwable("Whelp"))),
         )
 }
 
 private fun aRoomDetailsEditState(
     roomId: RoomId = RoomId("!aRoomId:aDomain"),
-    roomName: String = "Marketing",
+    roomRawName: String = "Marketing",
     canChangeName: Boolean = true,
     roomTopic: String = "a room topic that is quite long so should wrap onto multiple lines",
     canChangeTopic: Boolean = true,
@@ -53,7 +54,7 @@ private fun aRoomDetailsEditState(
     eventSink: (RoomDetailsEditEvents) -> Unit = {},
 ) = RoomDetailsEditState(
     roomId = roomId,
-    roomName = roomName,
+    roomRawName = roomRawName,
     canChangeName = canChangeName,
     roomTopic = roomTopic,
     canChangeTopic = canChangeTopic,

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
                 )
             }
 

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()
             }
         }

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
+}