From 2966af938a2720f7b58bb1fa5c9cd74e1f567ddc Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Mon, 28 Sep 2026 11:29:29 -0400 Subject: [PATCH 1/2] fix(chat): keep group members a FullRefresh leaves out A FullRefresh replaced the chat's cached members with the metadata's member list. For a group that list is only a page of the roster, so the refresh deleted every other member the device held from roster changes, message loads and feed pages. When one of them sent the last message, the Chats list preview lost the "Name: " prefix until something cached them again. Groups now merge on FullRefresh, as RosterStateHolder.refetch already does, and departures come through MemberLeft. DMs still replace. A member row with no profile row maps to UserProfile.Empty, whose name is "", so the preview read ": text". formatPreview now treats a blank name as missing, as iOS does. --- .../shared/chat/ui/ChatSummaryMapping.kt | 5 +- .../shared/chat/ui/ChatSummaryPreviewTest.kt | 30 +++- .../internal/delegates/EventStreamDelegate.kt | 20 ++- .../shared/chat/FullRefreshMembersTest.kt | 147 ++++++++++++++++++ 4 files changed, 194 insertions(+), 8 deletions(-) create mode 100644 apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/FullRefreshMembersTest.kt diff --git a/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/ChatSummaryMapping.kt b/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/ChatSummaryMapping.kt index 06dd020cb3..2cd77cd633 100644 --- a/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/ChatSummaryMapping.kt +++ b/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/ChatSummaryMapping.kt @@ -61,8 +61,11 @@ private fun ChatSummary.formatPreview( // Only a group attributes, and only someone else's message: "You:" already covers the viewer's, // and a DM's counterparty is the row's own name. Null when the roster subset does not have the // sender — the list does not fetch profiles, so an unattributed body is the honest fallback. + // A blank name counts as missing: a cached member with no profile row maps to an empty one, + // which would otherwise read as ": gm". val senderName = if (isGroup && !sentBySelf) { - metadata.members.firstOrNull { it.userId == lastMsg.senderId }?.userProfile?.displayName + metadata.members.firstOrNull { it.userId == lastMsg.senderId } + ?.userProfile?.displayName?.takeIf { it.isNotBlank() } } else { null } diff --git a/apps/flipcash/shared/chat-ui/src/test/kotlin/com/flipcash/shared/chat/ui/ChatSummaryPreviewTest.kt b/apps/flipcash/shared/chat-ui/src/test/kotlin/com/flipcash/shared/chat/ui/ChatSummaryPreviewTest.kt index 70892c27d2..1ba006deb6 100644 --- a/apps/flipcash/shared/chat-ui/src/test/kotlin/com/flipcash/shared/chat/ui/ChatSummaryPreviewTest.kt +++ b/apps/flipcash/shared/chat-ui/src/test/kotlin/com/flipcash/shared/chat/ui/ChatSummaryPreviewTest.kt @@ -114,13 +114,14 @@ class ChatSummaryPreviewTest { private fun groupSummary( content: List, senderId: ID?, + otherName: String = "Alice", ) = ChatSummary( metadata = ChatMetadata( chatId = ChatId(byteArrayOf(9)), type = ChatType.GROUP, members = listOf( ChatMember(userId = self, userProfile = profile("Me"), pointers = emptyList()), - ChatMember(userId = other, userProfile = profile("Alice"), pointers = emptyList()), + ChatMember(userId = other, userProfile = profile(otherName), pointers = emptyList()), ), lastMessage = ChatMessage( messageId = 42, @@ -135,8 +136,11 @@ class ChatSummaryPreviewTest { unreadCount = 0, ) - private fun groupReference(content: List, senderId: ID? = other) = - groupSummary(content, senderId) + private fun groupReference( + content: List, + senderId: ID? = other, + otherName: String = "Alice", + ) = groupSummary(content, senderId, otherName) .toConversationReference(selfId = self, tokensByMint = emptyMap(), resources = resources) @Test @@ -208,6 +212,26 @@ class ChatSummaryPreviewTest { ) } + @Test + fun `a member cached without a profile is not prefixed with an empty name`() { + // A member row with no profile row maps to UserProfile.Empty, whose name is "". + assertEquals( + "gm", + groupReference(listOf(MessageContent.Text("gm")), otherName = "").lastMessagePreview, + ) + } + + @Test + fun `group cash from a member cached without a profile previews as the amount alone`() { + val cash = MessageContent.Cash( + intentId = listOf(3), + amount = 25.toFiat(), + mint = Mint.usdf, + ) + + assertEquals("$25.00", groupReference(listOf(cash), otherName = " ").lastMessagePreview) + } + @Test fun `a DM is not prefixed with the counterparty's name`() { assertEquals("gm", preview(listOf(MessageContent.Text("gm")))) diff --git a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/EventStreamDelegate.kt b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/EventStreamDelegate.kt index 2cb0403d0a..ec31862713 100644 --- a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/EventStreamDelegate.kt +++ b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/EventStreamDelegate.kt @@ -10,6 +10,7 @@ import com.flipcash.services.controllers.ChatMessagingController import com.flipcash.services.controllers.EventStreamingController import com.flipcash.services.models.chat.ChatId import com.flipcash.services.models.chat.ChatMessage +import com.flipcash.services.models.chat.ChatType import com.flipcash.services.models.chat.ChatUpdate import com.flipcash.services.models.chat.Emoji import com.flipcash.services.models.chat.EmojiReaction @@ -383,10 +384,21 @@ class EventStreamDelegate @Inject constructor( when (metaUpdate) { is MetadataUpdate.FullRefresh -> { metadataDataSource.upsert(metaUpdate.metadata) - memberDataSource.replaceMembers( - metaUpdate.metadata.chatId, - metaUpdate.metadata.members, - ) + // A group's members here are a page of its roster, so replacing would drop + // members the device holds from roster changes and message loads — and with + // them the names its previews and transcript show. Merge, as + // RosterStateHolder.refetch does; departures come through MemberLeft. + if (metaUpdate.metadata.type == ChatType.GROUP) { + memberDataSource.upsert( + metaUpdate.metadata.chatId, + metaUpdate.metadata.members, + ) + } else { + memberDataSource.replaceMembers( + metaUpdate.metadata.chatId, + metaUpdate.metadata.members, + ) + } metaUpdate.metadata.lastMessage?.let { msg -> messageDataSource.upsert(metaUpdate.metadata.chatId, listOf(msg)) } diff --git a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/FullRefreshMembersTest.kt b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/FullRefreshMembersTest.kt new file mode 100644 index 0000000000..44a674844b --- /dev/null +++ b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/FullRefreshMembersTest.kt @@ -0,0 +1,147 @@ +package com.flipcash.shared.chat + +import android.content.Context +import androidx.test.core.app.ApplicationProvider +import com.flipcash.app.persistence.FlipcashDatabase +import com.flipcash.app.persistence.sources.ChatMemberDataSource +import com.flipcash.app.persistence.sources.mapper.chat.ChatEntityMapper +import com.flipcash.services.controllers.EventStreamingController +import com.flipcash.services.models.UserProfile +import com.flipcash.services.models.chat.ChatId +import com.flipcash.services.models.chat.ChatMember +import com.flipcash.services.models.chat.ChatMetadata +import com.flipcash.services.models.chat.ChatType +import com.flipcash.services.models.chat.ChatUpdate +import com.flipcash.services.models.chat.MetadataUpdate +import com.flipcash.shared.chat.internal.ChatStateHolder +import com.flipcash.shared.chat.internal.delegates.EventStreamDelegate +import com.getcode.opencode.model.core.ID +import io.mockk.every +import io.mockk.mockk +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancel +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeout +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import kotlin.test.assertEquals +import kotlin.time.Instant + +/** + * A `FullRefresh` carries the chat's metadata, and for a group its `members` are only a page of + * the roster. Replacing the cached members with that page deleted everyone else the device held, + * so a group whose last message came from one of them lost the sender's name in the Chats list + * preview. Runs against the real member table, because the bug is in what the write keeps. + */ +@RunWith(RobolectricTestRunner::class) +class FullRefreshMembersTest { + + private val context = ApplicationProvider.getApplicationContext() + private val chatId = ChatId("aabbccdd") + + private val a: ID = listOf(1) + private val b: ID = listOf(2) + private val c: ID = listOf(3) + + private val memberDataSource = ChatMemberDataSource(ChatEntityMapper()) + private val scope = CoroutineScope(SupervisorJob() + Dispatchers.Default) + + @Before + fun setUp() { + FlipcashDatabase.init(context, "full-refresh-members-test") + runBlocking { memberDataSource.clear() } + } + + @After + fun tearDown() { + scope.cancel() + FlipcashDatabase.closeDb() + } + + private fun member(id: ID, name: String) = ChatMember( + userId = id, + userProfile = UserProfile( + displayName = name, + socialAccounts = emptyList(), + phoneNumber = null, + email = null, + ), + pointers = emptyList(), + ) + + private fun fullRefresh(type: ChatType, members: List) = ChatUpdate( + chatId = chatId, + metadataUpdates = listOf( + MetadataUpdate.FullRefresh( + ChatMetadata( + chatId = chatId, + type = type, + members = members, + lastMessage = null, + lastActivity = Instant.fromEpochSeconds(1_000), + ), + ), + ), + ) + + /** Streams [update] through the delegate, then waits for [renamedA] to land. */ + private suspend fun apply(update: ChatUpdate, renamedA: String) { + val streaming = mockk(relaxed = true) + every { streaming.isConnected } returns true + every { streaming.chatUpdates } returns flowOf(update) + + EventStreamDelegate( + eventStreamingController = streaming, + messagingController = mockk(relaxed = true), + metadataDataSource = mockk(relaxed = true), + messageDataSource = mockk(relaxed = true), + memberDataSource = memberDataSource, + tokenCoordinator = mockk(relaxed = true), + userManager = mockk(relaxed = true), + stateHolder = ChatStateHolder(), + analytics = mockk(relaxed = true), + exchange = mockk(relaxed = true), + ).apply { + initialize(scope) + open() + } + + // The refresh renames A, and both writes commit A with the rest of the change in one + // transaction — so once A's new name is visible, whatever the write did to B and C is too. + withTimeout(5_000) { + memberDataSource.observeMembers(chatId).first { members -> + members.any { it.userId == a && it.userProfile.displayName == renamedA } + } + } + } + + private suspend fun storedNames(): Map = + memberDataSource.getMembersForChat(chatId).associate { it.userId to it.userProfile.displayName } + + @Test + fun `a group refresh keeps the members its page leaves out`() = runBlocking { + memberDataSource.upsert(chatId, listOf(member(a, "A"), member(b, "B"), member(c, "C"))) + + apply(fullRefresh(ChatType.GROUP, listOf(member(a, "A2"))), renamedA = "A2") + + // C is who the Chats list names when C sent the last message; the row reads the members + // it is handed off this table. + assertEquals(mapOf(a to "A2", b to "B", c to "C"), storedNames()) + } + + @Test + fun `a DM refresh still replaces its members`() = runBlocking { + memberDataSource.upsert(chatId, listOf(member(a, "A"), member(b, "B"))) + + apply(fullRefresh(ChatType.CONTACT_DM, listOf(member(a, "A2"))), renamedA = "A2") + + assertEquals(mapOf(a to "A2"), storedNames()) + } +} From 00d38273716c63fa17e24c45a3a71b2b79e1e498 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Mon, 28 Sep 2026 11:35:45 -0400 Subject: [PATCH 2/2] fix(messenger): skip the sender name line when the name is blank The profile proto allows an empty display_name, and the app stores it as-is, so a group member with no name drew an empty textSmall line above their run. The avatar still marks the run. --- .../internal/screens/components/MessageRow.kt | 4 ++- .../internal/GroupTranscriptScreenshotTest.kt | 33 +++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/MessageRow.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/MessageRow.kt index a8ebcefbdd..c745734705 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/MessageRow.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/MessageRow.kt @@ -269,7 +269,9 @@ internal fun MessageRow( // name line. It carries the gutter's width as a leading inset to stay on the // edge it sat on when the column held it. Column(modifier = Modifier.fillMaxWidth()) { - if (sender != null && runStart) { + // A profile can arrive with no display name; drawing it would leave an empty + // line above the run. The avatar below still marks whose run it is. + if (sender != null && runStart && sender.displayName.isNotBlank()) { Text( modifier = Modifier.padding( start = senderNameInset(showsSenderGutter && !item.isFromSelf), diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/GroupTranscriptScreenshotTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/GroupTranscriptScreenshotTest.kt index b532297a90..d675a82540 100644 --- a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/GroupTranscriptScreenshotTest.kt +++ b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/GroupTranscriptScreenshotTest.kt @@ -12,7 +12,12 @@ import androidx.compose.foundation.layout.width import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier import androidx.compose.ui.draw.alpha +import androidx.compose.ui.semantics.SemanticsProperties +import androidx.compose.ui.semantics.getOrNull +import androidx.compose.ui.test.SemanticsMatcher +import androidx.compose.ui.test.assertCountEquals import androidx.compose.ui.test.junit4.createAndroidComposeRule +import androidx.compose.ui.test.onAllNodesWithText import androidx.compose.ui.unit.dp import androidx.paging.PagingData import androidx.paging.compose.LazyPagingItems @@ -105,6 +110,34 @@ class GroupTranscriptScreenshotTest { capture("group_transcript.png") } + /** + * A member whose profile has no display name still gets their face in the gutter, but no name + * line: an empty one would leave a blank gap above the run. + */ + @Test + fun omitsNameLineForBlankDisplayName() { + val nameless = SenderIdentity(userId = listOf(3), displayName = "", picture = null) + val items = listOf( + bubble(3, "Not even close", noah, secondsIn = 20), + bubble(2, "Who's in for Saturday?", nameless, secondsIn = 10), + ChatListItem.DateSeparator(start), + ) + + composeRule.setContent { + FlipcashThemeWrapper().Wrap { + Transcript(flowOf(PagingData.from(items)).collectAsLazyPagingItems()) + } + } + repeat(20) { composeRule.mainClock.advanceTimeByFrame() } + + composeRule.onAllNodesWithText("Noah Parker").assertCountEquals(1) + composeRule.onAllNodes( + SemanticsMatcher("has blank text") { node -> + node.config.getOrNull(SemanticsProperties.Text).orEmpty().any { it.isBlank() } + }, + ).assertCountEquals(0) + } + /** * The unread divider between read and unread messages, first on its own and then sharing its * gap with a day change, where the date sits above it.