Respect child space order

Change-Id: Ibf350b03ff01b9ba31898d25cc7e4dab33bcde07

features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/datasource/SpaceListDataSource.kt

@@ -30,6 +30,7 @@ import io.element.android.libraries.designsystem.components.avatar.AvatarSize
 import io.element.android.libraries.eventformatter.api.RoomLastMessageFormatter
 import io.element.android.libraries.matrix.api.MatrixClient
 import io.element.android.libraries.matrix.api.core.RoomId
+import io.element.android.libraries.matrix.api.room.MatrixSpaceChildInfo
 import io.element.android.libraries.matrix.api.roomlist.RoomListService
 import io.element.android.libraries.matrix.api.roomlist.RoomSummary
 import kotlinx.collections.immutable.ImmutableList
@@ -113,22 +114,24 @@ class SpaceListDataSource @Inject constructor(
     // TODO what can we cache something here?
     private suspend fun buildSpaceHierarchy(spaceSummaries: List<RoomListRoomSummary>): ImmutableList<SpaceHierarchyItem> {
         // Map spaceId -> list of child spaces
-        val spaceHierarchyMap = HashMap<String, MutableList<RoomListRoomSummary>>()
+        val spaceHierarchyMap = HashMap<String, MutableList<Pair<MatrixSpaceChildInfo, RoomListRoomSummary>>>()
         // Map spaceId -> list of regular child rooms
-        val regularChildren = HashMap<String, MutableList<String>>()
+        val regularChildren = HashMap<String, MutableList<MatrixSpaceChildInfo>>()
         val rootSpaces = HashSet<RoomListRoomSummary>(spaceSummaries)
         spaceSummaries.forEach { parentSpace ->
             val spaceInfo = client.getRoom(parentSpace.roomId)
             val spaceChildren = spaceInfo?.spaceChildren
-            spaceChildren?.forEach childLoop@{ childId ->
-                val child = spaceSummaries.find { it.roomId.value == childId }
+            spaceChildren?.forEach childLoop@{ spaceChild ->
+                val child = spaceSummaries.find { it.roomId.value == spaceChild.roomId }
                 if (child == null) {
                     // Treat as regular child, since it doesn't appear to be a space (at least none known to us at this point)
-                    regularChildren[parentSpace.roomId.value] = regularChildren[parentSpace.roomId.value]?.apply { add(childId) } ?: mutableListOf(childId)
+                    regularChildren[parentSpace.roomId.value] = regularChildren[parentSpace.roomId.value]?.apply { add(spaceChild) } ?: mutableListOf(spaceChild)
                     return@childLoop
                 }
-                rootSpaces.removeAll { it.roomId.value == childId }
-                spaceHierarchyMap[parentSpace.roomId.value] = spaceHierarchyMap[parentSpace.roomId.value]?.apply { add(child) } ?: mutableListOf(child)
+                rootSpaces.removeAll { it.roomId.value == spaceChild.roomId }
+                spaceHierarchyMap[parentSpace.roomId.value] = spaceHierarchyMap[parentSpace.roomId.value]?.apply {
+                    add(Pair(spaceChild, child))
+                } ?: mutableListOf(Pair(spaceChild, child))
             }
         }
 
@@ -142,16 +145,16 @@ class SpaceListDataSource @Inject constructor(
     private fun createSpaceHierarchyItem(
         spaceSummary: RoomListRoomSummary,
         order: String?,
-        hierarchy: HashMap<String, MutableList<RoomListRoomSummary>>,
-        regularChildren: HashMap<String, MutableList<String>>,
+        hierarchy: HashMap<String, MutableList<Pair<MatrixSpaceChildInfo, RoomListRoomSummary>>>,
+        regularChildren: HashMap<String, MutableList<MatrixSpaceChildInfo>>,
         forbiddenChildren: List<String> = emptyList(),
     ): SpaceHierarchyItem {
-        val children = hierarchy[spaceSummary.id]?.mapNotNull {
-            if (it.roomId.value in forbiddenChildren) {
-                Timber.w("Detected space loop: ${spaceSummary.id} -> ${it.roomId.value}")
+        val children = hierarchy[spaceSummary.id]?.mapNotNull { (spaceChildInfo, child) ->
+            if (child.roomId.value in forbiddenChildren) {
+                Timber.w("Detected space loop: ${spaceSummary.id} -> ${child.roomId.value}")
                 null
             } else {
-                createSpaceHierarchyItem(it, null, hierarchy, regularChildren, forbiddenChildren + listOf(spaceSummary.roomId.value))
+                createSpaceHierarchyItem(child, spaceChildInfo.order, hierarchy, regularChildren, forbiddenChildren + listOf(spaceSummary.roomId.value))
             }
         }?.sortedWith(SpaceComparator)?.toImmutableList() ?: persistentListOf()
         return SpaceHierarchyItem(
@@ -160,7 +163,7 @@ class SpaceListDataSource @Inject constructor(
             spaces = children,
             flattenedRooms = (
                 // All direct children rooms
-                regularChildren[spaceSummary.id].orEmpty()
+                regularChildren[spaceSummary.id].orEmpty().map { it.roomId }
                     // All indirect children rooms
                     + children.flatMap { it.flattenedRooms }
             ).toImmutableList(),

features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/datasource/TopLevelSpaceComparator.kt

@@ -27,7 +27,9 @@ object SpaceComparator : Comparator<SpaceListDataSource.SpaceHierarchyItem> {
         } else {
             if (leftOrder == null) {
                 if (rightOrder == null) {
-                    compareValues(left?.info?.name, right?.info?.name)
+                    // Spec says to fallback to roomId, but we at SchildiChat find lowercase names more suitable
+                    //compareValues(left?.info?.roomId?.value, right?.info?.roomId?.value)
+                    compareValues(left?.info?.name?.lowercase(), right?.info?.name?.lowercase())
                 } else {
                     1
                 }

gradle/libs.versions.toml

@@ -152,7 +152,7 @@ jsoup = "org.jsoup:jsoup:1.17.2"
 appyx_core = { module = "com.bumble.appyx:core", version.ref = "appyx" }
 molecule-runtime = "app.cash.molecule:molecule-runtime:1.3.2"
 timber = "com.jakewharton.timber:timber:5.0.1"
-matrix_sdk = "chat.schildi.rustcomponents:sdk-android:0.2.1"
+matrix_sdk = "chat.schildi.rustcomponents:sdk-android:0.2.2"
 matrix_richtexteditor = { module = "io.element.android:wysiwyg", version.ref = "wysiwyg" }
 matrix_richtexteditor_compose = { module = "io.element.android:wysiwyg-compose", version.ref = "wysiwyg" }
 sqldelight-driver-android = { module = "app.cash.sqldelight:android-driver", version.ref = "sqldelight" }

libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt

@@ -49,7 +49,7 @@ interface MatrixRoom : Closeable {
     val isEncrypted: Boolean
     val isDirect: Boolean
     val isPublic: Boolean
-    val spaceChildren: List<String>
+    val spaceChildren: List<MatrixSpaceChildInfo>
     val rootSpaceOrder: String?
     val activeMemberCount: Long
     val joinedMemberCount: Long

libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixSpaceChildInfo.kt

@@ -0,0 +1,10 @@
+package io.element.android.libraries.matrix.api.room
+
+import androidx.compose.runtime.Immutable
+
+@Immutable
+data class MatrixSpaceChildInfo(
+    val roomId: String,
+    val order: String?,
+    val suggested: Boolean,
+)

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/MatrixSpaceChildInfoMapper.kt

@@ -0,0 +1,14 @@
+package io.element.android.libraries.matrix.impl.room
+
+import io.element.android.libraries.matrix.api.room.MatrixSpaceChildInfo
+import org.matrix.rustcomponents.sdk.SpaceChildInfo
+
+object MatrixSpaceChildInfoMapper {
+    fun map(spaceChildInfo: SpaceChildInfo): MatrixSpaceChildInfo = spaceChildInfo.let {
+        return MatrixSpaceChildInfo(
+            roomId = it.roomId,
+            order = it.order,
+            suggested = it.suggested,
+        )
+    }
+}

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt

@@ -37,6 +37,7 @@ import io.element.android.libraries.matrix.api.room.MatrixRoom
 import io.element.android.libraries.matrix.api.room.MatrixRoomInfo
 import io.element.android.libraries.matrix.api.room.MatrixRoomMembersState
 import io.element.android.libraries.matrix.api.room.MatrixRoomNotificationSettingsState
+import io.element.android.libraries.matrix.api.room.MatrixSpaceChildInfo
 import io.element.android.libraries.matrix.api.room.Mention
 import io.element.android.libraries.matrix.api.room.MessageEventType
 import io.element.android.libraries.matrix.api.room.StateEventType
@@ -185,8 +186,8 @@ class RustMatrixRoom(
     override val isPublic: Boolean
         get() = innerRoom.isPublic()
 
-    override val spaceChildren: List<String>
-        get() = innerRoom.spaceChildren()
+    override val spaceChildren: List<MatrixSpaceChildInfo>
+        get() = innerRoom.spaceChildren().map(MatrixSpaceChildInfoMapper::map)
 
     override val isDirect: Boolean
         get() = innerRoom.isDirect()

libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt

@@ -34,6 +34,7 @@ import io.element.android.libraries.matrix.api.room.MatrixRoom
 import io.element.android.libraries.matrix.api.room.MatrixRoomInfo
 import io.element.android.libraries.matrix.api.room.MatrixRoomMembersState
 import io.element.android.libraries.matrix.api.room.MatrixRoomNotificationSettingsState
+import io.element.android.libraries.matrix.api.room.MatrixSpaceChildInfo
 import io.element.android.libraries.matrix.api.room.Mention
 import io.element.android.libraries.matrix.api.room.MessageEventType
 import io.element.android.libraries.matrix.api.room.RoomMember
@@ -72,7 +73,7 @@ class FakeMatrixRoom(
     override val alias: String? = null,
     override val alternativeAliases: List<String> = emptyList(),
     override val isPublic: Boolean = true,
-    override val spaceChildren: List<String> = emptyList(),
+    override val spaceChildren: List<MatrixSpaceChildInfo> = emptyList(),
     override val rootSpaceOrder: String? = null,
     override val isDirect: Boolean = false,
     override val isOneToOne: Boolean = false,