From 395bfa6b8b1c8b4516a540b975e01c2d70ca0c4f Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Fri, 2 Oct 2026 22:18:03 +0200 Subject: [PATCH] feat(subgroups): say which group the parent is, not only which id The parent card on a room's detail screen read "Parent group" over sixteen hex characters. Every other card naming a room names it -- `SubgroupCard` has shown `room?.chatRoom?.subject` since subgroups landed -- and the one card pointing the other way up the lineage did not, although the room it points at is usually right there in the database. So: "Parent group" moves to the overline, the group's name becomes the headline, and the truncated id moves underneath, where it can still be read against what the lineage says. The label keeps its place rather than being replaced, because a card carrying a name and nothing else says nothing about which way the lineage runs, and the signing key's card -- same icon column, same shape -- sits directly above this one. **The name can only come from the room, and there is not always one.** A birth certificate names the child and never the certifier, so a member of the subgroup who is not also in the group above holds a verified lineage with nothing to call it by. That case keeps the card it already had: the id as the headline, no supporting line. It is not a corner -- it is the ordinary position of anyone welcomed into a subgroup rather than founding one -- so the room is a nullable field rather than something to wait for. `SubgroupManager.parentGroupsOf` is `parentsOf` with each id looked up, returning `Parent(chatRoomId, room)` the way `subgroupsOf` returns `Subgroup`. The verified set and its canonical order are untouched; a parent whose room is missing stays in the list. The id-only `parentsOf` stays as it is, because `offersTo` compares ids and should not pay for rooms it will not read. Three tests: a parent this device is in is named by its room, one it is not in is still listed with nothing to name it, and two parents keep the lineage's own order -- asserted against `parentsOf` rather than a written-out order, since canonical means sorted by id and nobody should have to know how two derived ids sort. 459 common tests, 985 jvm tests, `m3Audit` meets every budget. Co-Authored-By: Claude Opus 5 --- .../repository/DatabaseChatRepository.kt | 4 +- .../compose/managers/SubgroupManager.kt | 29 +++++++++ .../compose/repository/ChatRepository.kt | 10 +++- .../ui/composable/ChatRoomDetailScreen.kt | 59 ++++++++++++++----- .../ui/view/model/ChatRoomDetailViewModel.kt | 2 +- .../ui/view/state/ChatRoomDetailUIState.kt | 7 ++- .../managers/SubgroupManagerJvmTest.kt | 48 +++++++++++++++ 7 files changed, 138 insertions(+), 21 deletions(-) diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/repository/DatabaseChatRepository.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/repository/DatabaseChatRepository.kt index 3463a4e5..70e05e89 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/repository/DatabaseChatRepository.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/repository/DatabaseChatRepository.kt @@ -192,8 +192,8 @@ class DatabaseChatRepository( false } - override suspend fun parentsOf(chatRoomId: String): List = try { - SubgroupManager.parentsOf(database, chatRoomId) + override suspend fun parentsOf(chatRoomId: String): List = try { + SubgroupManager.parentGroupsOf(database, chatRoomId) } catch (e: Throwable) { logger.e("Error reading the parents of $chatRoomId", e) emptyList() diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/managers/SubgroupManager.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/managers/SubgroupManager.kt index a10c237c..7ca84bcf 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/managers/SubgroupManager.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/managers/SubgroupManager.kt @@ -842,6 +842,35 @@ object SubgroupManager { database.groupKeyStateDao().getLocalByChatRoomId(chatRoomId)?.parentChatRoomIds() .orEmpty() + /** + * A group this room is a child of, plus the room itself where this device + * has it. + * + * [room] is the only place a parent's *name* can come from: a certificate + * names the child, never the certifier, so a device that is not in the + * parent room knows the lineage and not what to call it. Null is therefore + * the ordinary case for a subgroup member who is not also a parent member, + * and the id is all such a screen can honestly show. + */ + data class Parent( + val chatRoomId: HexKey, + val room: LocalChatRoom?, + ) + + /** + * [parentsOf], with each parent's room where this device holds one. + * + * The same verified set in the same order -- this only looks each id up, and + * a parent whose room is missing stays in the list rather than being dropped. + */ + suspend fun parentGroupsOf(database: MantraDatabase, chatRoomId: String): List = + parentsOf(database, chatRoomId).map { parentChatRoomId -> + Parent( + chatRoomId = parentChatRoomId, + room = database.chatRoomDao().findChatRoomById(parentChatRoomId) + ) + } + /** * What a certificate arriving in a room is, or null if it is nothing this * room should believe. diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/repository/ChatRepository.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/repository/ChatRepository.kt index 319e3782..222cdea5 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/repository/ChatRepository.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/repository/ChatRepository.kt @@ -78,8 +78,12 @@ interface ChatRepository { * may be adopted by another later. A member welcomed in after the founding * holds no state and gets an empty list even when the room really is a * subgroup. + * + * Each carries the parent's room where this device has one, which is the + * only place its name can come from -- a certificate names the child and + * never the certifier. */ - suspend fun parentsOf(chatRoomId: String): List + suspend fun parentsOf(chatRoomId: String): List /** * One event the group signed, as the group signed it, or null when this @@ -323,7 +327,9 @@ interface ChatRepository { override suspend fun subgroupsOf(chatRoomId: String): List = emptyList() - override suspend fun parentsOf(chatRoomId: String): List = emptyList() + override suspend fun parentsOf( + chatRoomId: String + ): List = emptyList() override suspend fun groupSignedEvent( chatRoomId: String, diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/ChatRoomDetailScreen.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/ChatRoomDetailScreen.kt index 9125dfbb..f3bcf1ea 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/ChatRoomDetailScreen.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/ChatRoomDetailScreen.kt @@ -280,15 +280,40 @@ fun ChatRoomDetailScreen( // later. They are a set rather than a path -- neither // is "the" parent and the order says nothing about // which came first -- so each gets an equal row. - chatRoomDetailUIState.parentChatRoomIds.forEach { parentChatRoomId -> - item(key = "parent-$parentChatRoomId") { + chatRoomDetailUIState.parents.forEach { parent -> + item(key = "parent-${parent.chatRoomId}") { + // The parent's own name, which only its room + // can give: a certificate names the child and + // never the certifier, so a member of the + // subgroup who is not also in the parent has + // the id and nothing else to call it by. + val name = parent.room + ?.chatRoom + ?.subject + ?.takeIf { it.isNotBlank() } + + // Shown either way -- as the headline where + // there is no name, and under it where there + // is, since it is the part that can be + // compared against what the lineage says. + val identifier: @Composable () -> Unit = { + Text( + text = parent.chatRoomId + .take(16) + .inComparableGroups(), + fontFamily = FontFamily.Monospace, + maxLines = 1, + overflow = TextOverflow.MiddleEllipsis + ) + } + Card( modifier = Modifier.fillMaxWidth(), onClick = { onNavigateToRoute.invoke( ChatRoomDetailRoute( activeUserPublicKey = activeUserPublicKey, - chatRoomId = parentChatRoomId, + chatRoomId = parent.chatRoomId, relayHint = relayHint ) ) @@ -307,19 +332,25 @@ fun ChatRoomDetailScreen( contentDescription = Decorative ) }, - headlineContent = { + // The label keeps its place above the + // name rather than being replaced by + // it: a card carrying a group's name + // and nothing else says nothing about + // which way the lineage runs, and the + // signing key's card -- same icon + // column, same shape -- sits directly + // above this one. + overlineContent = { Text(text = stringResource(Res.string.parent_group)) }, - supportingContent = { - Text( - text = parentChatRoomId - .take(16) - .inComparableGroups(), - fontFamily = FontFamily.Monospace, - maxLines = 1, - overflow = TextOverflow.MiddleEllipsis - ) - } + headlineContent = { + if (name != null) { + Text(text = name) + } else { + identifier() + } + }, + supportingContent = identifier.takeIf { name != null } ) } } diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/ChatRoomDetailViewModel.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/ChatRoomDetailViewModel.kt index 6b7c16f3..8b79af88 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/ChatRoomDetailViewModel.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/ChatRoomDetailViewModel.kt @@ -82,7 +82,7 @@ class ChatRoomDetailViewModel( groupKeyState = chatRepository.groupKeyState(chatRoomId), signedGroupKeyStateEvent = chatRepository.signedGroupKeyStateEvent(chatRoomId), subgroups = chatRepository.subgroupsOf(chatRoomId), - parentChatRoomIds = chatRepository.parentsOf(chatRoomId), + parents = chatRepository.parentsOf(chatRoomId), canAddSubgroup = localChatRoom.chatRoom.mlsGroupState != null && chatRepository.canSign(chatRoomId), adoptionOffers = chatRepository.adoptionOffersFor(chatRoomId), diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/state/ChatRoomDetailUIState.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/state/ChatRoomDetailUIState.kt index d357797e..9b976403 100755 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/state/ChatRoomDetailUIState.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/state/ChatRoomDetailUIState.kt @@ -41,15 +41,18 @@ sealed interface ChatRoomDetailUIState { */ val subgroups: List = emptyList(), /** - * The group this room is a subgroup of, or null when it is nobody's + * The groups this room is a subgroup of, empty when it is nobody's * child. * * Verified: it comes off the room's key state, which only carries a * parent whose certificate was checked. A member welcomed in after the * founding holds no state and so sees nothing here, even in a room that * really is a subgroup. + * + * Each carries the parent's room where this device holds one, so the + * card can name the group rather than only identify it. */ - val parentChatRoomIds: List = emptyList(), + val parents: List = emptyList(), /** * Whether this device could open a subgroup here at all. * diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/managers/SubgroupManagerJvmTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/managers/SubgroupManagerJvmTest.kt index 32ee1bd3..ca78097c 100644 --- a/composeApp/src/jvmTest/kotlin/press/mantra/compose/managers/SubgroupManagerJvmTest.kt +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/managers/SubgroupManagerJvmTest.kt @@ -777,6 +777,54 @@ class SubgroupManagerJvmTest { Unit } + @Test + fun `a parent this device is in is named by its room`() = runBlocking { + parentWith(members = listOf(alice, bob)) + childWithKeyState(parents = listOf(parentRoomId)) + + val parent = SubgroupManager.parentGroupsOf(db, childRoomId).single() + + assertEquals(parentRoomId, parent.chatRoomId) + // The room, and nothing else, is where a parent's name comes from: a + // certificate names the child and never the certifier. + assertEquals("Ekklesia", parent.room?.chatRoom?.subject) + } + + @Test + fun `a parent this device is not in is still listed, with nothing to name it`() = runBlocking { + // No parent room: the ordinary position of a subgroup member who is not + // also in the group above. The lineage is known -- it was checked + // against that group's own signature -- so the parent stays in the list + // and only its name is missing. + childWithKeyState(parents = listOf(parentRoomId)) + + val parent = SubgroupManager.parentGroupsOf(db, childRoomId).single() + + assertEquals(parentRoomId, parent.chatRoomId) + assertNull(parent.room) + } + + @Test + fun `two parents keep the lineage's own order, named or not`() = runBlocking { + parentWith(members = listOf(alice, bob)) + childWithKeyState(parents = listOf(parentRoomId, siblingRoomId)) + + val parents = SubgroupManager.parentGroupsOf(db, childRoomId) + + // The same set in the same order -- canonical, which is sorted by id and + // so says nothing about which parent came first. Asserted against + // `parentsOf` rather than against a written-out order, since the ids are + // derived from keys and nobody should have to know how two of them sort. + assertEquals( + SubgroupManager.parentsOf(db, childRoomId), + parents.map { it.chatRoomId } + ) + assertEquals( + mapOf(parentRoomId to "Ekklesia", siblingRoomId to null), + parents.associate { it.chatRoomId to it.room?.chatRoom?.subject } + ) + } + private suspend fun refuseAdoptionOf(adopting: LocalChatRoom): String? = SubgroupManager.refuseAdoption( database = db,