From d44224ab5dac936275bca7385feebf554ef7464b Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Fri, 16 Oct 2026 23:08:52 -0400 Subject: [PATCH] fix(chat): name the users a join notice is about System messages drew only the server's fallback text, which names no one. SystemContent.user_joined carries the joiner and, when admitted, who let them in; the mapper dropped it. Keep it on MessageContent.System, store it with the message, and word the notice from current profiles: "Alex joined the group", "Sam added Alex", with the viewer as "you". A name not known yet is requested, and the row reads as the fallback text until it lands. The chat list preview names them the same way. --- .../core/src/main/res/values/strings.xml | 5 + .../app/messenger/internal/ChatViewModel.kt | 26 ++++- .../messenger/internal/SystemMessageItems.kt | 8 +- .../internal/SystemMessageItemTest.kt | 7 +- .../shared/chat/models/SystemMessageText.kt | 57 +++++++++-- .../shared/chat/ui/ChatSummaryMapping.kt | 15 ++- .../chat/models/SystemMessageTextTest.kt | 96 +++++++++++++++++++ .../shared/chat/ui/ChatSummaryPreviewTest.kt | 21 ++++ .../converters/ChatTypeConverters.kt | 9 +- .../sources/mapper/chat/ChatEntityMapper.kt | 14 ++- .../network/extensions/LocalToProtobuf.kt | 20 +++- .../network/extensions/ProtobufToLocal.kt | 23 ++++- .../services/models/chat/MessageContent.kt | 17 +++- .../extensions/SystemContentMappingTest.kt | 55 +++++++++++ 14 files changed, 350 insertions(+), 23 deletions(-) create mode 100644 apps/flipcash/shared/chat-ui/src/test/kotlin/com/flipcash/shared/chat/models/SystemMessageTextTest.kt diff --git a/apps/flipcash/core/src/main/res/values/strings.xml b/apps/flipcash/core/src/main/res/values/strings.xml index 257d78da6..1601f1cf5 100644 --- a/apps/flipcash/core/src/main/res/values/strings.xml +++ b/apps/flipcash/core/src/main/res/values/strings.xml @@ -1003,6 +1003,11 @@ This photo can’t be displayed You: %1$s %1$s: %2$s + %1$s joined the group + You joined the group + %1$s added %2$s + %1$s added you + You added %1$s %1$s sent %2$s %1$s tipped %2$s Shared a profile diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt index 64a25d93e..1ddc42422 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt @@ -1379,7 +1379,15 @@ internal class ChatViewModel @AssistedInject constructor( // that and becomes its own kind of row. if (content is MessageContent.System) { return@flatMapIndexed listOfNotNull( - systemMessageItem(message.messageId, index, message.timestamp, content), + systemMessageItem( + messageId = message.messageId, + contentIndex = index, + timestamp = message.timestamp, + content = content, + resources = resources, + selfId = userManager.accountId, + nameOf = { userId -> profiles.systemUserName(userId) }, + ), ) } @@ -1603,6 +1611,22 @@ internal class ChatViewModel @AssistedInject constructor( } } + /** + * A user a system notice names, as [quoteAuthorName] names a quote's author: asked for when the + * roster can't name them, and null until then, so the row reads as its fallback text rather + * than a placeholder. Null for a DM, which has no [senderProfiles]. + */ + private fun Map?.systemUserName(userId: ID): String? { + val profiles = this ?: return null + val profile = profiles[userId.hexEncodedString()] + if (profile == null || profile.displayName.isBlank()) { + chatCoordinator.requestSenderProfile(userId) + } + return profile?.let { + nameOrHandle(it.displayName, it.handle) ?: resources.getString(R.string.title_unnamedUser) + } + } + private fun MessageContent.Media.toPhotoSnippet(message: ChatMessage) = ChatQuoteSnippet.Photo( caption = caption?.text?.takeIf { it.isNotBlank() }, rendition = items.firstOrNull()?.renditions?.firstOrNull(), diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/SystemMessageItems.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/SystemMessageItems.kt index 207d07321..1743cff75 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/SystemMessageItems.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/SystemMessageItems.kt @@ -3,19 +3,25 @@ package com.flipcash.app.messenger.internal import com.flipcash.services.models.chat.MessageContent import com.flipcash.shared.chat.models.ChatListItem import com.flipcash.shared.chat.models.displayText +import com.getcode.opencode.model.core.ID +import com.getcode.util.resources.ResourceHelper import kotlin.time.Instant /** * The transcript row for one System content of a message, or `null` when it has no text to draw. * The null case is a notice stored before the mapper dropped blank ones; it produces no row at all. + * [nameOf] names the users the notice is about; see [displayText]. */ internal fun systemMessageItem( messageId: Long, contentIndex: Int, timestamp: Instant, content: MessageContent.System, + resources: ResourceHelper, + selfId: ID?, + nameOf: (ID) -> String?, ): ChatListItem.SystemMessage? { - val text = content.displayText() ?: return null + val text = content.displayText(resources, selfId, nameOf) ?: return null return ChatListItem.SystemMessage( messageId = messageId, contentIndex = contentIndex, diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/SystemMessageItemTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/SystemMessageItemTest.kt index afc7d52cf..cb194cc55 100644 --- a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/SystemMessageItemTest.kt +++ b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/SystemMessageItemTest.kt @@ -2,6 +2,8 @@ package com.flipcash.app.messenger.internal import com.flipcash.services.models.chat.MessageContent import com.flipcash.services.models.chat.isSystemBlank +import com.getcode.util.resources.ResourceHelper +import io.mockk.mockk import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse @@ -15,8 +17,11 @@ class SystemMessageItemTest { private val at = Instant.fromEpochMilliseconds(1_700_000_000_000) + // A notice with no event never reaches a string resource. + private val NoNames = mockk() + private fun item(text: String, id: Long = 7, index: Int = 0) = - systemMessageItem(id, index, at, MessageContent.System(text)) + systemMessageItem(id, index, at, MessageContent.System(text), NoNames, selfId = null) { null } @Test fun `a notice becomes exactly one row with its own text`() { diff --git a/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/models/SystemMessageText.kt b/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/models/SystemMessageText.kt index 8179de0fb..ab2632f46 100644 --- a/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/models/SystemMessageText.kt +++ b/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/models/SystemMessageText.kt @@ -1,16 +1,61 @@ package com.flipcash.shared.chat.models +import com.flipcash.core.R import com.flipcash.services.models.chat.MessageContent import com.flipcash.services.models.chat.isSystemBlank +import com.getcode.opencode.model.core.ID +import com.getcode.util.resources.ResourceHelper /** * The text a system notice draws and previews as, or `null` when it has none. * - * The one place the notice's wording is chosen. When the server sends a structured event this - * client can localise, it becomes `when (event) { known -> localised; else -> fallbackText }` here, - * with a resource lookup as a parameter, and no caller changes. + * The one place the notice's wording is chosen. A [MessageContent.System.event] this client knows + * is worded here, naming its users through [nameOf] and the viewer ([selfId]) as "you". When there + * is no event, or [nameOf] can't name someone it needs yet, the notice reads as its fallback text. * - * Blank is [isSystemBlank], the definition iOS shares. Text that is not blank is returned as it - * arrived: not trimmed. Rows stored before the mapper dropped blank notices still reach this. + * Blank is [isSystemBlank], the definition iOS shares. Fallback text that is not blank is returned + * as it arrived: not trimmed. Rows stored before the mapper dropped blank notices still reach this. */ -fun MessageContent.System.displayText(): String? = fallbackText.takeUnless { it.isSystemBlank() } +fun MessageContent.System.displayText( + resources: ResourceHelper, + selfId: ID?, + nameOf: (ID) -> String?, +): String? = event?.worded(resources, selfId, nameOf) + ?: fallbackText.takeUnless { it.isSystemBlank() } + +private fun MessageContent.SystemEvent.worded( + resources: ResourceHelper, + selfId: ID?, + nameOf: (ID) -> String?, +): String? = when (this) { + is MessageContent.SystemEvent.UserJoined -> { + val joinedIsSelf = selfId != null && userId == selfId + val admitter = admittedBy?.takeUnless { it == userId } + when { + admitter == null -> if (joinedIsSelf) { + resources.getString(R.string.label_chat_system_joinedSelf) + } else { + nameOf(userId)?.let { resources.getString(R.string.label_chat_system_joined, it) } + } + selfId != null && admitter == selfId -> + nameOf(userId)?.let { resources.getString(R.string.label_chat_system_selfAdded, it) } + joinedIsSelf -> + nameOf(admitter)?.let { resources.getString(R.string.label_chat_system_addedSelf, it) } + else -> { + val admitterName = nameOf(admitter) + val joinedName = nameOf(userId) + if (admitterName != null && joinedName != null) { + resources.getString(R.string.label_chat_system_added, admitterName, joinedName) + } else { + null + } + } + } + } +} + +/** The users [MessageContent.System.event] names, apart from the viewer: the ones [displayText] looks up. */ +fun MessageContent.System.namedUsers(selfId: ID?): List = when (val event = event) { + is MessageContent.SystemEvent.UserJoined -> listOfNotNull(event.userId, event.admittedBy) + null -> emptyList() +}.distinct().filterNot { selfId != null && it == selfId } 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 faab29016..a410db39f 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 @@ -7,6 +7,7 @@ import com.flipcash.services.models.chat.MessageContent import com.flipcash.services.models.chat.WidgetContent import com.flipcash.services.models.handle import com.flipcash.shared.chat.models.displayText +import com.flipcash.shared.chat.models.namedUsers import com.flipcash.shared.chat.ChatSummary import com.flipcash.shared.chat.media.ChatMediaText import com.getcode.opencode.model.core.ID @@ -67,6 +68,11 @@ fun ChatSummary.toConversationReference( */ fun ChatSummary.unnamedGroupSender(selfId: ID?, senderProfiles: Map): ID? { if (metadata.type != ChatType.GROUP) return null + // A notice has no sender; the users it names are what its preview needs. One per call is + // enough: the next is asked for when the list re-emits with the first named. + (metadata.lastMessage?.content?.firstOrNull() as? MessageContent.System)?.let { notice -> + return notice.namedUsers(selfId).firstOrNull { groupSenderName(it, senderProfiles) == null } + } val senderId = metadata.lastMessage?.senderId ?: return null if (senderId == selfId) return null return senderId.takeIf { groupSenderName(senderId, senderProfiles) == null } @@ -100,7 +106,9 @@ private fun ChatSummary.formatPreview( null } return lastMsg.content.firstOrNull() - ?.previewText(sentBySelf, senderName, isGroup, tokensByMint, resources) + ?.previewText(sentBySelf, senderName, isGroup, tokensByMint, resources) { content -> + content.displayText(resources, selfId) { groupSenderName(it, senderProfiles) } + } } /** @@ -116,6 +124,7 @@ private fun MessageContent.previewText( tokensByMint: Map, resources: ResourceHelper, depth: Int = 0, + systemText: (MessageContent.System) -> String?, ): String? = when (this) { is MessageContent.Text -> { val message = text.takeIf { it.isNotEmpty() } @@ -173,7 +182,7 @@ private fun MessageContent.previewText( null } else { content.firstOrNull() - ?.previewText(sentBySelf, senderName, isGroup, tokensByMint, resources, depth + 1) + ?.previewText(sentBySelf, senderName, isGroup, tokensByMint, resources, depth + 1, systemText) } // The feed carries the newest message that still has content, so a tombstone only @@ -193,7 +202,7 @@ private fun MessageContent.previewText( } } // The notice's own words, with no You: prefix and no sender name; it has no sender. - is MessageContent.System -> displayText() + is MessageContent.System -> systemText(this) // A widget this client can't draw previews as nothing, as an undecryptable message does. is MessageContent.Widget -> when (widget) { diff --git a/apps/flipcash/shared/chat-ui/src/test/kotlin/com/flipcash/shared/chat/models/SystemMessageTextTest.kt b/apps/flipcash/shared/chat-ui/src/test/kotlin/com/flipcash/shared/chat/models/SystemMessageTextTest.kt new file mode 100644 index 000000000..62d601266 --- /dev/null +++ b/apps/flipcash/shared/chat-ui/src/test/kotlin/com/flipcash/shared/chat/models/SystemMessageTextTest.kt @@ -0,0 +1,96 @@ +package com.flipcash.shared.chat.models + +import com.flipcash.core.R +import com.flipcash.services.models.chat.MessageContent +import com.flipcash.services.models.chat.MessageContent.SystemEvent.UserJoined +import com.getcode.opencode.model.core.ID +import com.getcode.util.resources.ResourceHelper +import io.mockk.every +import io.mockk.mockk +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull + +/** How a join notice names its users, and when it falls back to the server's text. */ +class SystemMessageTextTest { + + private val self: ID = listOf(1) + private val alex: ID = listOf(2) + private val sam: ID = listOf(3) + private val names = mapOf(alex to "Alex", sam to "Sam") + + private val templates = mapOf( + R.string.label_chat_system_joined to "%1\$s joined the group", + R.string.label_chat_system_joinedSelf to "You joined the group", + R.string.label_chat_system_added to "%1\$s added %2\$s", + R.string.label_chat_system_addedSelf to "%1\$s added you", + R.string.label_chat_system_selfAdded to "You added %1\$s", + ) + + private val resources = mockk { + every { getString(any()) } answers { templates.getValue(firstArg()) } + every { getString(any(), *anyVararg()) } answers { + templates.getValue(firstArg()).format(*(args[1] as Array<*>)) + } + } + + private fun text( + joined: ID, + admittedBy: ID? = null, + nameOf: (ID) -> String? = names::get, + fallback: String = "Someone joined", + ) = MessageContent.System(fallback, UserJoined(joined, admittedBy)) + .displayText(resources, self, nameOf) + + @Test + fun `a direct join names who joined`() { + assertEquals("Alex joined the group", text(alex)) + } + + @Test + fun `the viewer joining reads as you`() { + assertEquals("You joined the group", text(self)) + } + + @Test + fun `an admitted join names both users`() { + assertEquals("Sam added Alex", text(alex, admittedBy = sam)) + } + + @Test + fun `the viewer admitted reads as you`() { + assertEquals("Sam added you", text(self, admittedBy = sam)) + } + + @Test + fun `the viewer admitting reads as you`() { + assertEquals("You added Alex", text(alex, admittedBy = self)) + } + + @Test + fun `admitting yourself is a plain join`() { + assertEquals("Alex joined the group", text(alex, admittedBy = alex)) + assertEquals("You joined the group", text(self, admittedBy = self)) + } + + @Test + fun `a name not known yet falls back to the server's text`() { + assertEquals("Someone joined", text(alex, nameOf = { null })) + assertEquals("Someone joined", text(alex, admittedBy = sam, nameOf = { if (it == sam) "Sam" else null })) + assertEquals("Someone joined", text(alex, admittedBy = sam, nameOf = { if (it == alex) "Alex" else null })) + } + + @Test + fun `a notice with no event reads as its fallback text`() { + assertEquals("Sam left", MessageContent.System("Sam left").displayText(resources, self) { null }) + assertNull(MessageContent.System(" ").displayText(resources, self) { null }) + } + + @Test + fun `the users to look up leave out the viewer`() { + assertEquals(listOf(alex, sam), MessageContent.System("x", UserJoined(alex, sam)).namedUsers(self)) + assertEquals(listOf(sam), MessageContent.System("x", UserJoined(self, sam)).namedUsers(self)) + assertEquals(listOf(alex), MessageContent.System("x", UserJoined(alex, alex)).namedUsers(self)) + assertEquals(emptyList(), MessageContent.System("x").namedUsers(self)) + } +} 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 51dd56713..71e50d50f 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 @@ -395,4 +395,25 @@ class ChatSummaryPreviewTest { fun `a notice with blank text has no preview`() { assertNull(groupReference(listOf(MessageContent.System(" \u0085")), senderId = null).lastMessagePreview) } + + @Test + fun `a join notice names who joined from the roster`() { + every { resources.getString(R.string.label_chat_system_joined, "Alice") } returns "Alice joined the group" + val notice = MessageContent.System("Someone joined", MessageContent.SystemEvent.UserJoined(other, admittedBy = null)) + assertEquals("Alice joined the group", groupReference(listOf(notice), senderId = null).lastMessagePreview) + } + + @Test + fun `a join notice naming someone unknown previews as its fallback and asks for them`() { + val stranger: ID = listOf(5) + val notice = MessageContent.System("Someone joined", MessageContent.SystemEvent.UserJoined(stranger, admittedBy = null)) + assertEquals("Someone joined", groupReference(listOf(notice), senderId = null).lastMessagePreview) + assertEquals(stranger, groupSummary(listOf(notice), senderId = null).unnamedGroupSender(self, emptyMap())) + } + + @Test + fun `a join notice whose users are all named asks for no one`() { + val notice = MessageContent.System("Someone joined", MessageContent.SystemEvent.UserJoined(self, admittedBy = other)) + assertNull(groupSummary(listOf(notice), senderId = null).unnamedGroupSender(self, emptyMap())) + } } diff --git a/apps/flipcash/shared/persistence/db/src/main/kotlin/com/flipcash/app/persistence/converters/ChatTypeConverters.kt b/apps/flipcash/shared/persistence/db/src/main/kotlin/com/flipcash/app/persistence/converters/ChatTypeConverters.kt index ed8b41cfc..f0e80bd2a 100644 --- a/apps/flipcash/shared/persistence/db/src/main/kotlin/com/flipcash/app/persistence/converters/ChatTypeConverters.kt +++ b/apps/flipcash/shared/persistence/db/src/main/kotlin/com/flipcash/app/persistence/converters/ChatTypeConverters.kt @@ -235,9 +235,16 @@ sealed interface MessageContentSerialized { @SerialName("widget_share_profile") data class WidgetShareProfile(val username: String?) : MessageContentSerialized + // Mirrors MessageContent.System. [joinedUser] is set for a UserJoined event, and + // [admittedBy] when someone let them in; both hex-encoded. Rows stored before the event was + // kept have neither and decode as fallback-only. @Serializable @SerialName("system") - data class System(val fallbackText: String) : MessageContentSerialized + data class System( + val fallbackText: String, + val joinedUser: String? = null, + val admittedBy: String? = null, + ) : MessageContentSerialized // Mirrors MessageContent.Unsupported: a content type the build that stored it didn't know. @Serializable diff --git a/apps/flipcash/shared/persistence/sources/src/main/kotlin/com/flipcash/app/persistence/sources/mapper/chat/ChatEntityMapper.kt b/apps/flipcash/shared/persistence/sources/src/main/kotlin/com/flipcash/app/persistence/sources/mapper/chat/ChatEntityMapper.kt index 9dfe768b2..5f7827505 100644 --- a/apps/flipcash/shared/persistence/sources/src/main/kotlin/com/flipcash/app/persistence/sources/mapper/chat/ChatEntityMapper.kt +++ b/apps/flipcash/shared/persistence/sources/src/main/kotlin/com/flipcash/app/persistence/sources/mapper/chat/ChatEntityMapper.kt @@ -411,7 +411,14 @@ private fun MessageContent.toSerialized(): MessageContentSerialized = when (this is MessageContent.Widget -> MessageContentSerialized.WidgetShareProfile( username = (widget as? WidgetContent.ShareProfile)?.username, ) - is MessageContent.System -> MessageContentSerialized.System(fallbackText = fallbackText) + is MessageContent.System -> when (val event = event) { + is MessageContent.SystemEvent.UserJoined -> MessageContentSerialized.System( + fallbackText = fallbackText, + joinedUser = event.userId.hexEncodedString(), + admittedBy = event.admittedBy?.hexEncodedString(), + ) + null -> MessageContentSerialized.System(fallbackText = fallbackText) + } MessageContent.Unsupported -> MessageContentSerialized.Unsupported is MessageContent.Deleted -> MessageContentSerialized.Deleted( deletedAt = deletedTs.epochSeconds, @@ -449,7 +456,10 @@ private fun MessageContentSerialized.toDomain(): MessageContent = when (this) { items = items, caption = caption?.let { MessageContent.Text(it.text) }, ) - is MessageContentSerialized.System -> MessageContent.System(fallbackText = fallbackText) + is MessageContentSerialized.System -> MessageContent.System( + fallbackText = fallbackText, + event = joinedUser?.let { MessageContent.SystemEvent.UserJoined(it.hexToIdExt(), admittedBy?.hexToIdExt()) }, + ) MessageContentSerialized.Unsupported -> MessageContent.Unsupported is MessageContentSerialized.Deleted -> MessageContent.Deleted( deletedTs = Instant.fromEpochSeconds(deletedAt), diff --git a/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/extensions/LocalToProtobuf.kt b/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/extensions/LocalToProtobuf.kt index d03e2af62..121fb8883 100644 --- a/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/extensions/LocalToProtobuf.kt +++ b/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/extensions/LocalToProtobuf.kt @@ -172,9 +172,23 @@ internal fun MessageContent.asContent(): MessagingModel.Content { is MessageContent.Widget -> error("Widget messages are receive-only and are never sent by a client") // Only ever decoded from a type this build doesn't know, so it has no wire form to send. is MessageContent.Unsupported -> error("Unsupported content is decode-only; it has no wire form") - is MessageContent.System -> MessagingModel.Content.newBuilder() - .setSystem(MessagingModel.SystemContent.newBuilder().setFallbackText(fallbackText)) - .build() + is MessageContent.System -> { + val systemBuilder = MessagingModel.SystemContent.newBuilder().setFallbackText(fallbackText) + when (val event = event) { + is MessageContent.SystemEvent.UserJoined -> { + val joined = MessagingModel.SystemContent.UserJoined.newBuilder() + .setUserId(event.userId.asUserId()) + event.admittedBy?.let { + joined.setAdmitted(MessagingModel.SystemContent.UserJoined.Admitted.newBuilder().setAdmittedBy(it.asUserId())) + } ?: joined.setDirect(MessagingModel.SystemContent.UserJoined.Direct.getDefaultInstance()) + systemBuilder.setUserJoined(joined) + } + null -> Unit + } + MessagingModel.Content.newBuilder() + .setSystem(systemBuilder) + .build() + } is MessageContent.Deleted -> { val deletedBuilder = MessagingModel.DeletedContent.newBuilder() .setDeletedTs(deletedTs.asTimestamp()) diff --git a/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/extensions/ProtobufToLocal.kt b/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/extensions/ProtobufToLocal.kt index 2c4f3f3df..246f049f4 100644 --- a/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/extensions/ProtobufToLocal.kt +++ b/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/extensions/ProtobufToLocal.kt @@ -208,7 +208,10 @@ internal fun MessagingModel.Content.toMessageContent(): MessageContent { // An empty caption is no caption. caption = if (media.hasCaption() && media.caption.text.isNotEmpty()) MessageContent.Text(media.caption.text) else null, ) - MessagingModel.Content.TypeCase.SYSTEM -> MessageContent.System(system.fallbackText) + MessagingModel.Content.TypeCase.SYSTEM -> MessageContent.System( + fallbackText = system.fallbackText, + event = system.toSystemEvent(), + ) MessagingModel.Content.TypeCase.DELETED -> MessageContent.Deleted( deletedTs = Instant.fromEpochSeconds(deleted.deletedTs.seconds, deleted.deletedTs.nanos), deletedBy = if (deleted.hasDeletedBy()) deleted.deletedBy.toId() else null, @@ -750,4 +753,20 @@ internal fun com.codeinc.flipcash.gen.blob.v1.Model.RejectionReason.toRejectionR internal fun com.codeinc.flipcash.gen.moderation.v1.Model.FlaggedCategory.toFlaggedCategory(): ModerationResult.FlaggedCategory { return ModerationResult.FlaggedCategory.entries.firstOrNull { it.name == name } ?: ModerationResult.FlaggedCategory.OTHER -} \ No newline at end of file +} + +// A variant this client doesn't know, or one missing the user it names, draws as its fallback text. +private fun MessagingModel.SystemContent.toSystemEvent(): MessageContent.SystemEvent? = when (typeCase) { + MessagingModel.SystemContent.TypeCase.USER_JOINED -> userJoined + .takeIf { it.hasUserId() } + ?.let { joined -> + MessageContent.SystemEvent.UserJoined( + userId = joined.userId.toId(), + // An unset method renders as a plain join, as a direct one does. + admittedBy = joined.admitted + .takeIf { joined.methodCase == MessagingModel.SystemContent.UserJoined.MethodCase.ADMITTED && it.hasAdmittedBy() } + ?.admittedBy?.toId(), + ) + } + else -> null +} diff --git a/services/flipcash/src/main/kotlin/com/flipcash/services/models/chat/MessageContent.kt b/services/flipcash/src/main/kotlin/com/flipcash/services/models/chat/MessageContent.kt index b96c84654..2f992224c 100644 --- a/services/flipcash/src/main/kotlin/com/flipcash/services/models/chat/MessageContent.kt +++ b/services/flipcash/src/main/kotlin/com/flipcash/services/models/chat/MessageContent.kt @@ -41,16 +41,27 @@ sealed interface MessageContent { */ data class Widget(val widget: WidgetContent) : MessageContent /** - * A notice the server wrote about the chat, such as "Alex joined the group". [fallbackText] is - * the only part this client reads; see `displayText` in chat-ui for what a row draws. + * A notice the server wrote about the chat, such as "Alex joined the group". [event] carries the + * users it is about, so a row can name them from their current profiles; [fallbackText] names + * no one and is drawn when [event] is null or a name isn't known yet. See `displayText` in + * chat-ui for what a row draws. */ - data class System(val fallbackText: String) : MessageContent + data class System( + val fallbackText: String, + val event: SystemEvent? = null, + ) : MessageContent /** * A `messaging.v1.Content.type` this client does not recognise. Rendered as an unsupported * message, as an unrecognised [WidgetContent] is. */ data object Unsupported : MessageContent + + /** The `messaging.v1.SystemContent.type` variants this client recognises. */ + sealed interface SystemEvent { + /** [userId] joined the chat; [admittedBy] let them in, or null when they joined on their own. */ + data class UserJoined(val userId: ID, val admittedBy: ID?) : SystemEvent + } data class Deleted( val deletedTs: Instant, val deletedBy: ID?, diff --git a/services/flipcash/src/test/kotlin/com/flipcash/services/internal/network/extensions/SystemContentMappingTest.kt b/services/flipcash/src/test/kotlin/com/flipcash/services/internal/network/extensions/SystemContentMappingTest.kt index 9ef90a9b6..7ab39ea26 100644 --- a/services/flipcash/src/test/kotlin/com/flipcash/services/internal/network/extensions/SystemContentMappingTest.kt +++ b/services/flipcash/src/test/kotlin/com/flipcash/services/internal/network/extensions/SystemContentMappingTest.kt @@ -3,6 +3,8 @@ package com.flipcash.services.internal.network.extensions import com.codeinc.flipcash.gen.messaging.v1.Model as MessagingModel import com.flipcash.services.models.chat.MessageContent import kotlin.test.Test +import com.codeinc.flipcash.gen.common.v1.Common +import com.google.protobuf.ByteString import kotlin.test.assertEquals /** The server's `SystemContent.fallback_text` reaching the app as a notice, and blank ones not. */ @@ -57,4 +59,57 @@ class SystemContentMappingTest { message(system(" "), system("Sam left")).toChatMessage().content, ) } + + private fun userId(byte: Int) = Common.UserId.newBuilder().setValue(ByteString.copyFrom(byteArrayOf(byte.toByte()))).build() + + private fun joined(build: MessagingModel.SystemContent.UserJoined.Builder.() -> Unit) = MessagingModel.Content.newBuilder() + .setSystem( + MessagingModel.SystemContent.newBuilder() + .setFallbackText("Someone joined") + .setUserJoined(MessagingModel.SystemContent.UserJoined.newBuilder().apply(build)), + ) + .build() + + @Test + fun `a direct join carries who joined`() { + assertEquals( + MessageContent.System("Someone joined", MessageContent.SystemEvent.UserJoined(listOf(2), admittedBy = null)), + joined { + setUserId(userId(2)) + setDirect(MessagingModel.SystemContent.UserJoined.Direct.getDefaultInstance()) + }.toMessageContent(), + ) + } + + @Test + fun `an admitted join carries who let them in`() { + assertEquals( + MessageContent.System("Someone joined", MessageContent.SystemEvent.UserJoined(listOf(2), admittedBy = listOf(3))), + joined { + setUserId(userId(2)) + setAdmitted(MessagingModel.SystemContent.UserJoined.Admitted.newBuilder().setAdmittedBy(userId(3))) + }.toMessageContent(), + ) + } + + @Test + fun `a join with no method is a plain join`() { + assertEquals( + MessageContent.System("Someone joined", MessageContent.SystemEvent.UserJoined(listOf(2), admittedBy = null)), + joined { setUserId(userId(2)) }.toMessageContent(), + ) + } + + @Test + fun `a join missing its user is fallback only`() { + assertEquals(MessageContent.System("Someone joined"), joined { }.toMessageContent()) + } + + @Test + fun `a join round-trips to the wire`() { + listOf(null, listOf(3)).forEach { admittedBy -> + val content = MessageContent.System("Someone joined", MessageContent.SystemEvent.UserJoined(listOf(2), admittedBy)) + assertEquals(content, content.asContent().toMessageContent()) + } + } }