From b1ecd80ccc6f7744d679f792eb4928372dbefb36 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Thu, 27 Aug 2026 11:04:43 +0200 Subject: [PATCH] fix(chat): keep unknown system messages from rendering as chat bubbles DUMMY is the marker for "not a system message", so mapping an unrecognised systemMessage identifier to it made getCalculateMessageType() classify the message as REGULAR_TEXT_MESSAGE. The server text then rendered in a chat bubble with avatar and actor name instead of as centered grey text, and every system message identifier added by a newer server regressed the same way. Reserve DUMMY for what the server means by it, an empty systemMessage, and map any other unrecognised identifier to the new UNKNOWN type. Future server-side additions now render correctly without an app change, since their text comes from the server anyway. Add the three identifiers the client was missing: preserve_conversation, preserve_conversation_off and phone_removed. convertToString() had the same defect on the conversation list last-message path, which serialises ChatMessageJson into the conversation entity: CLEARED_CHAT was written as "clear_history" while parsing expects "history_cleared", and THREAD_RENAMED had no branch at all, so both came back as DUMMY. UNKNOWN serialises to a sentinel that parses back to UNKNOWN so it survives that round trip too. getFromString() now takes String?, which is what LoganSquare passes for a null JSON value. The new test asserts every enum member survives a convertToString -> getFromString round trip, which is what would have caught the "clear_history" typo. Assisted-by: Claude Code:claude-opus-5 Signed-off-by: Andy Scherzinger --- .../talk/chat/data/model/ChatMessage.kt | 5 ++ .../EnumSystemMessageTypeConverter.kt | 25 +++++++- .../EnumSystemMessageTypeConverterTest.kt | 60 +++++++++++++++++++ 3 files changed, 87 insertions(+), 3 deletions(-) create mode 100644 app/src/test/java/com/nextcloud/talk/models/json/converters/EnumSystemMessageTypeConverterTest.kt diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/model/ChatMessage.kt b/app/src/main/java/com/nextcloud/talk/chat/data/model/ChatMessage.kt index 7b58159836..57dde7f3a5 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/model/ChatMessage.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/model/ChatMessage.kt @@ -260,7 +260,9 @@ data class ChatMessage( * see https://nextcloud-talk.readthedocs.io/en/latest/chat/#system-messages */ enum class SystemMessageType { + // No system message at all, as opposed to UNKNOWN which is a system message of an unknown type DUMMY, + UNKNOWN, CONVERSATION_CREATED, CONVERSATION_RENAMED, DESCRIPTION_REMOVED, @@ -277,6 +279,8 @@ data class ChatMessage( LISTABLE_NONE, LISTABLE_USERS, LISTABLE_ALL, + PRESERVE_CONVERSATION, + PRESERVE_CONVERSATION_OFF, LOBBY_NONE, LOBBY_NON_MODERATORS, LOBBY_OPEN_TO_EVERYONE, @@ -325,6 +329,7 @@ data class ChatMessage( FEDERATED_USER_ADDED, FEDERATED_USER_REMOVED, PHONE_ADDED, + PHONE_REMOVED, THREAD_CREATED, THREAD_RENAMED, MESSAGE_PINNED, diff --git a/app/src/main/java/com/nextcloud/talk/models/json/converters/EnumSystemMessageTypeConverter.kt b/app/src/main/java/com/nextcloud/talk/models/json/converters/EnumSystemMessageTypeConverter.kt index 984bb53aa8..e7ca6445a2 100644 --- a/app/src/main/java/com/nextcloud/talk/models/json/converters/EnumSystemMessageTypeConverter.kt +++ b/app/src/main/java/com/nextcloud/talk/models/json/converters/EnumSystemMessageTypeConverter.kt @@ -62,8 +62,11 @@ import com.nextcloud.talk.chat.data.model.ChatMessage.SystemMessageType.OBJECT_S import com.nextcloud.talk.chat.data.model.ChatMessage.SystemMessageType.PASSWORD_REMOVED import com.nextcloud.talk.chat.data.model.ChatMessage.SystemMessageType.PASSWORD_SET import com.nextcloud.talk.chat.data.model.ChatMessage.SystemMessageType.PHONE_ADDED +import com.nextcloud.talk.chat.data.model.ChatMessage.SystemMessageType.PHONE_REMOVED import com.nextcloud.talk.chat.data.model.ChatMessage.SystemMessageType.POLL_CLOSED import com.nextcloud.talk.chat.data.model.ChatMessage.SystemMessageType.POLL_VOTED +import com.nextcloud.talk.chat.data.model.ChatMessage.SystemMessageType.PRESERVE_CONVERSATION +import com.nextcloud.talk.chat.data.model.ChatMessage.SystemMessageType.PRESERVE_CONVERSATION_OFF import com.nextcloud.talk.chat.data.model.ChatMessage.SystemMessageType.REACTION import com.nextcloud.talk.chat.data.model.ChatMessage.SystemMessageType.REACTION_DELETED import com.nextcloud.talk.chat.data.model.ChatMessage.SystemMessageType.REACTION_REVOKED @@ -74,6 +77,7 @@ import com.nextcloud.talk.chat.data.model.ChatMessage.SystemMessageType.RECORDIN import com.nextcloud.talk.chat.data.model.ChatMessage.SystemMessageType.RECORDING_STOPPED import com.nextcloud.talk.chat.data.model.ChatMessage.SystemMessageType.THREAD_CREATED import com.nextcloud.talk.chat.data.model.ChatMessage.SystemMessageType.THREAD_RENAMED +import com.nextcloud.talk.chat.data.model.ChatMessage.SystemMessageType.UNKNOWN import com.nextcloud.talk.chat.data.model.ChatMessage.SystemMessageType.USER_ADDED import com.nextcloud.talk.chat.data.model.ChatMessage.SystemMessageType.USER_REMOVED @@ -83,7 +87,7 @@ import com.nextcloud.talk.chat.data.model.ChatMessage.SystemMessageType.USER_REM */ class EnumSystemMessageTypeConverter : StringBasedTypeConverter() { @Suppress("Detekt.LongMethod") - override fun getFromString(string: String): ChatMessage.SystemMessageType = + override fun getFromString(string: String?): ChatMessage.SystemMessageType = when (string) { "conversation_created" -> CONVERSATION_CREATED "conversation_renamed" -> CONVERSATION_RENAMED @@ -101,6 +105,8 @@ class EnumSystemMessageTypeConverter : StringBasedTypeConverter LISTABLE_NONE "listable_users" -> LISTABLE_USERS "listable_all" -> LISTABLE_ALL + "preserve_conversation" -> PRESERVE_CONVERSATION + "preserve_conversation_off" -> PRESERVE_CONVERSATION_OFF "lobby_none" -> LOBBY_NONE "lobby_non_moderators" -> LOBBY_NON_MODERATORS "lobby_timer_reached" -> LOBBY_OPEN_TO_EVERYONE @@ -149,11 +155,15 @@ class EnumSystemMessageTypeConverter : StringBasedTypeConverter FEDERATED_USER_ADDED "federated_user_removed" -> FEDERATED_USER_REMOVED "phone_added" -> PHONE_ADDED + "phone_removed" -> PHONE_REMOVED "thread_created" -> THREAD_CREATED "thread_renamed" -> THREAD_RENAMED "message_pinned" -> MESSAGE_PINNED "message_unpinned" -> MESSAGE_UNPINNED - else -> DUMMY + null, "" -> DUMMY + // An identifier added by a newer server is still a system message. Its text is rendered from + // the server, so it must not fall back to DUMMY, which marks a regular chat message. + else -> UNKNOWN } @Suppress("Detekt.ComplexMethod", "Detekt.LongMethod") @@ -176,6 +186,8 @@ class EnumSystemMessageTypeConverter : StringBasedTypeConverter "listable_none" LISTABLE_USERS -> "listable_users" LISTABLE_ALL -> "listable_all" + PRESERVE_CONVERSATION -> "preserve_conversation" + PRESERVE_CONVERSATION_OFF -> "preserve_conversation_off" LOBBY_NONE -> "lobby_none" LOBBY_NON_MODERATORS -> "lobby_non_moderators" LOBBY_OPEN_TO_EVERYONE -> "lobby_timer_reached" @@ -204,7 +216,7 @@ class EnumSystemMessageTypeConverter : StringBasedTypeConverter "matterbridge_config_removed" MATTERBRIDGE_CONFIG_ENABLED -> "matterbridge_config_enabled" MATTERBRIDGE_CONFIG_DISABLED -> "matterbridge_config_disabled" - CLEARED_CHAT -> "clear_history" + CLEARED_CHAT -> "history_cleared" REACTION -> "reaction" REACTION_DELETED -> "reaction_deleted" REACTION_REVOKED -> "reaction_revoked" @@ -224,9 +236,16 @@ class EnumSystemMessageTypeConverter : StringBasedTypeConverter "federated_user_added" FEDERATED_USER_REMOVED -> "federated_user_removed" PHONE_ADDED -> "phone_added" + PHONE_REMOVED -> "phone_removed" THREAD_CREATED -> "thread_created" + THREAD_RENAMED -> "thread_renamed" MESSAGE_PINNED -> "message_pinned" MESSAGE_UNPINNED -> "message_unpinned" + UNKNOWN -> UNKNOWN_IDENTIFIER else -> "" } + + companion object { + private const val UNKNOWN_IDENTIFIER = "unknown_system_message" + } } diff --git a/app/src/test/java/com/nextcloud/talk/models/json/converters/EnumSystemMessageTypeConverterTest.kt b/app/src/test/java/com/nextcloud/talk/models/json/converters/EnumSystemMessageTypeConverterTest.kt new file mode 100644 index 0000000000..bb3ad8cc4d --- /dev/null +++ b/app/src/test/java/com/nextcloud/talk/models/json/converters/EnumSystemMessageTypeConverterTest.kt @@ -0,0 +1,60 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.models.json.converters + +import com.nextcloud.talk.chat.data.model.ChatMessage +import org.junit.Assert +import org.junit.Test + +class EnumSystemMessageTypeConverterTest { + + private val converter = EnumSystemMessageTypeConverter() + + @Test + fun getFromString_mapsPreserveConversation() { + Assert.assertEquals( + ChatMessage.SystemMessageType.PRESERVE_CONVERSATION, + converter.getFromString("preserve_conversation") + ) + Assert.assertEquals( + ChatMessage.SystemMessageType.PRESERVE_CONVERSATION_OFF, + converter.getFromString("preserve_conversation_off") + ) + } + + @Test + fun getFromString_mapsPhoneRemoved() { + Assert.assertEquals( + ChatMessage.SystemMessageType.PHONE_REMOVED, + converter.getFromString("phone_removed") + ) + } + + @Test + fun getFromString_dummyOnlyForAbsentSystemMessage() { + Assert.assertEquals(ChatMessage.SystemMessageType.DUMMY, converter.getFromString("")) + Assert.assertEquals(ChatMessage.SystemMessageType.DUMMY, converter.getFromString(null)) + } + + @Test + fun getFromString_unknownIdentifierStaysASystemMessage() { + val type = converter.getFromString("some_future_server_message") + Assert.assertEquals(ChatMessage.SystemMessageType.UNKNOWN, type) + Assert.assertTrue(ChatMessage(systemMessageType = type).isSystemMessage) + } + + @Test + fun everyTypeSurvivesARoundTrip() { + ChatMessage.SystemMessageType.entries.forEach { type -> + Assert.assertEquals( + "$type is not restored from its serialized form", + type, + converter.getFromString(converter.convertToString(type)) + ) + } + } +}