From 21cfa62cc2102764957198ea1c1ecc25382ec242 Mon Sep 17 00:00:00 2001 From: callebtc <93376500+callebtc@users.noreply.github.com> Date: Mon, 27 Jul 2026 18:23:08 +0200 Subject: [PATCH] fix: address unread DM review feedback --- .../com/bitchat/android/ui/ChatViewModel.kt | 120 ++++++++++++++---- .../bitchat/android/ui/GeohashPeopleList.kt | 9 +- .../bitchat/android/ui/MeshPeerListSheet.kt | 44 +++---- .../com/bitchat/android/ui/MessageManager.kt | 12 +- .../bitchat/android/ui/PrivateChatManager.kt | 8 +- .../android/ui/UnreadConversationSummary.kt | 23 +++- .../android/ui/PrivateChatManagerTest.kt | 23 ++++ .../ui/UnreadConversationSummaryTest.kt | 20 +++ 8 files changed, 190 insertions(+), 69 deletions(-) diff --git a/app/src/main/java/com/bitchat/android/ui/ChatViewModel.kt b/app/src/main/java/com/bitchat/android/ui/ChatViewModel.kt index 6ef0ff81..c00504fb 100644 --- a/app/src/main/java/com/bitchat/android/ui/ChatViewModel.kt +++ b/app/src/main/java/com/bitchat/android/ui/ChatViewModel.kt @@ -6,11 +6,13 @@ import androidx.core.app.NotificationManagerCompat import androidx.lifecycle.AndroidViewModel import androidx.lifecycle.viewModelScope import com.bitchat.android.favorites.FavoritesPersistenceService +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.stateIn import com.bitchat.android.mesh.BluetoothMeshDelegate import com.bitchat.android.mesh.BluetoothMeshService @@ -19,10 +21,12 @@ import com.bitchat.android.service.MeshServiceHolder import com.bitchat.android.model.BitchatMessage import com.bitchat.android.model.BitchatMessageType import com.bitchat.android.nostr.NostrIdentityBridge +import com.bitchat.android.nostr.GeohashConversationRegistry import com.bitchat.android.protocol.BitchatPacket import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext import com.bitchat.android.util.NotificationIntervalManager import kotlinx.coroutines.delay import java.util.Date @@ -177,21 +181,54 @@ class ChatViewModel( internal val unreadConversations: StateFlow> = combine( state.unreadPrivateMessages, state.privateChats, - state.nickname - ) { unreadConversationIDs, chats, currentNickname -> + state.nickname, + state.connectedPeers + ) { unreadConversationIDs, chats, currentNickname, connectedPeerIDs -> val seenStore = com.bitchat.android.services.SeenMessageStore.getInstance(getApplication()) + val connectedPeerIDSet = connectedPeerIDs.mapTo(mutableSetOf()) { it.lowercase() } buildUnreadConversationSummaries( unreadConversationIDs = unreadConversationIDs, privateChats = chats, currentUserIdentifiers = setOf(currentNickname, mesh.myPeerID), canonicalize = ContactDirectory::canonicalConversationId, isMessageRead = { message -> seenStore.hasRead(message.id) } + ).map { summary -> + val resolution = ContactDirectory.resolve(summary.conversationID) + val resolvedNostrPubkey = summary.nostrPubkey + ?: resolution.nostrPubkey?.let(ContactIdentityResolver::nostrPubkeyHex) + val aliases = buildSet { + addAll(summary.identityAliases) + add(summary.conversationID) + add(resolution.conversationID) + resolution.meshPeerID?.let(::add) + resolution.noiseKeyHex?.let(::add) + resolvedNostrPubkey + ?.let(ContactIdentityResolver::nostrAliasForPubkey) + ?.let(::add) + }.mapTo(mutableSetOf()) { it.lowercase() } + + summary.copy( + displayName = resolution.displayName + ?.takeUnless { + it.isBlank() || it.equals("Unknown", ignoreCase = true) + } + ?: summary.displayName, + nostrPubkey = resolvedNostrPubkey, + identityAliases = aliases, + isConnected = aliases.any(connectedPeerIDSet::contains), + sourceGeohash = aliases + .asSequence() + .mapNotNull(GeohashConversationRegistry::get) + .firstOrNull() + ) + } + } + .flowOn(Dispatchers.IO) + .stateIn( + scope = viewModelScope, + started = SharingStarted.Eagerly, + initialValue = emptyList() ) - }.stateIn( - scope = viewModelScope, - started = SharingStarted.Eagerly, - initialValue = emptyList() - ) val joinedChannels: StateFlow> = state.joinedChannels val currentChannel: StateFlow = state.currentChannel val channelMessages: StateFlow>> = state.channelMessages @@ -256,18 +293,27 @@ class ChatViewModel( } viewModelScope.launch { try { com.bitchat.android.services.AppStateStore.privateMessages.collect { byPeer -> - val canonicalChats = ContactDirectory.canonicalizePrivateChats(byPeer) + val (canonicalChats, unreadConversationIDs) = withContext(Dispatchers.IO) { + val canonical = ContactDirectory.canonicalizePrivateChats(byPeer) + val unread = try { + val seen = com.bitchat.android.services.SeenMessageStore + .getInstance(getApplication()) + val myNick = state.getNicknameValue().ifBlank { mesh.myPeerID } + canonical + .filterValues { messages -> + messages.any { message -> + message.sender != myNick && !seen.hasRead(message.id) + } + } + .keys + } catch (_: Exception) { + state.getUnreadPrivateMessagesValue() + } + canonical to unread + } state.setPrivateChats(canonicalChats) // Recompute unread set using SeenMessageStore for robustness across Activity recreation - try { - val seen = com.bitchat.android.services.SeenMessageStore.getInstance(getApplication()) - val myNick = state.getNicknameValue() ?: mesh.myPeerID - val unread = mutableSetOf() - canonicalChats.forEach { (peer, list) -> - if (list.any { msg -> msg.sender != myNick && !seen.hasRead(msg.id) }) unread.add(peer) - } - state.setUnreadPrivateMessages(unread) - } catch (_: Exception) { } + state.setUnreadPrivateMessages(unreadConversationIDs) } } catch (_: Exception) { } } viewModelScope.launch { @@ -397,15 +443,26 @@ class ChatViewModel( // MARK: - Private Chat Management (delegated) - fun startPrivateChat(peerID: String) { + suspend fun startPrivateChat(peerID: String) { // For geohash conversation keys, ensure DM subscription is active if (peerID.startsWith("nostr_")) { ensureGeohashDMSubscriptionIfNeeded(peerID) } - - val success = privateChatManager.startPrivateChat(peerID, mesh) + + val (conversationID, unreadAliases) = withContext(Dispatchers.IO) { + val canonicalID = ContactDirectory.canonicalConversationId(peerID) + canonicalID to matchingUnreadAliases( + unreadConversationIDs = state.getUnreadPrivateMessagesValue(), + canonicalConversationID = canonicalID, + canonicalize = ContactDirectory::canonicalConversationId + ) + } + val success = privateChatManager.startPrivateChat( + peerID = conversationID, + meshService = mesh, + unreadAliases = unreadAliases + ) if (success) { - val conversationID = ContactDirectory.canonicalConversationId(peerID) // Notify notification manager about current private chat setCurrentPrivateChatPeer(conversationID) // Clear notifications for this sender since user is now viewing the chat @@ -413,14 +470,21 @@ class ChatViewModel( // Persistently mark all messages in this conversation as read so Nostr fetches // after app restarts won't re-mark them as unread. - try { - val seen = com.bitchat.android.services.SeenMessageStore.getInstance(getApplication()) - val chats = state.getPrivateChatsValue() - val messages = chats[conversationID] ?: emptyList() - messages.forEach { msg -> - try { seen.markRead(msg.id) } catch (_: Exception) { } + withContext(Dispatchers.IO) { + try { + val seen = + com.bitchat.android.services.SeenMessageStore.getInstance(getApplication()) + val chats = state.getPrivateChatsValue() + val messages = chats[conversationID] ?: emptyList() + messages.forEach { msg -> + try { + seen.markRead(msg.id) + } catch (_: Exception) { + } + } + } catch (_: Exception) { } - } catch (_: Exception) { } + } } } diff --git a/app/src/main/java/com/bitchat/android/ui/GeohashPeopleList.kt b/app/src/main/java/com/bitchat/android/ui/GeohashPeopleList.kt index e8f87668..d784610a 100644 --- a/app/src/main/java/com/bitchat/android/ui/GeohashPeopleList.kt +++ b/app/src/main/java/com/bitchat/android/ui/GeohashPeopleList.kt @@ -19,7 +19,6 @@ import androidx.lifecycle.compose.collectAsStateWithLifecycle import com.bitchat.android.ui.theme.BitchatFontFamily import com.bitchat.android.ui.theme.colorForPeer import com.bitchat.android.R -import com.bitchat.android.services.ContactDirectory import com.bitchat.android.ui.theme.LocalBitchatPalette import java.util.* @@ -38,7 +37,7 @@ fun GeohashPeopleList( viewModel: ChatViewModel, onTapPerson: () -> Unit, modifier: Modifier = Modifier, - excludedConversationIDs: Set = emptySet() + excludedIdentityAliases: Set = emptySet() ) { val geohashPeople by viewModel.geohashPeople.collectAsStateWithLifecycle() val selectedLocationChannel by viewModel.selectedLocationChannel.collectAsStateWithLifecycle() @@ -79,10 +78,10 @@ fun GeohashPeopleList( geohashPeople } } - val visiblePeople = remember(peopleIncludingSelf, excludedConversationIDs) { + val visiblePeople = remember(peopleIncludingSelf, excludedIdentityAliases) { peopleIncludingSelf.filterNot { person -> - val alias = "nostr_${person.id.take(16)}" - ContactDirectory.canonicalConversationId(alias) in excludedConversationIDs + val alias = "nostr_${person.id.take(16)}".lowercase() + alias in excludedIdentityAliases } } val sections = remember(visiblePeople, myHex, isTeleported, teleportedGeo) { diff --git a/app/src/main/java/com/bitchat/android/ui/MeshPeerListSheet.kt b/app/src/main/java/com/bitchat/android/ui/MeshPeerListSheet.kt index 7ec2fc0a..a5777ccf 100644 --- a/app/src/main/java/com/bitchat/android/ui/MeshPeerListSheet.kt +++ b/app/src/main/java/com/bitchat/android/ui/MeshPeerListSheet.kt @@ -86,11 +86,12 @@ fun MeshPeerListSheet( val geohashPeopleCount = geohashPeople.size val wifiAwareConnected by com.bitchat.android.wifiaware.WifiAwareController.connectedPeers.collectAsStateWithLifecycle() val wifiAwarePeerIDs = remember(wifiAwareConnected) { wifiAwareConnected.keys.toSet() } - val unreadConversationIDs = remember(unreadConversations) { - unreadConversations.mapTo(mutableSetOf()) { it.conversationID } + val unreadIdentityAliases = remember(unreadConversations) { + unreadConversations + .flatMapTo(mutableSetOf()) { it.identityAliases } } val visibleConnectedPeers = connectedPeers.filterNot { peerID -> - ContactDirectory.canonicalConversationId(peerID) in unreadConversationIDs + peerID.lowercase() in unreadIdentityAliases } // Bottom sheet state @@ -131,7 +132,6 @@ fun MeshPeerListSheet( item(key = "unread_private_messages_section") { UnreadDirectMessagesSection( conversations = unreadConversations, - connectedPeers = connectedPeers, viewModel = viewModel, onPrivateChatStart = { conversationID -> viewModel.showPrivateChatSheet(conversationID) @@ -203,7 +203,7 @@ fun MeshPeerListSheet( GeohashPeopleList( viewModel = viewModel, onTapPerson = onDismiss, - excludedConversationIDs = unreadConversationIDs, + excludedIdentityAliases = unreadIdentityAliases, modifier = Modifier.padding( top = if ( joinedChannels.isNotEmpty() || @@ -229,7 +229,7 @@ fun MeshPeerListSheet( selectedPrivatePeer = selectedPrivatePeer, wifiAwarePeerIDs = wifiAwarePeerIDs, peopleCount = peopleCount, - excludedConversationIDs = unreadConversationIDs, + excludedIdentityAliases = unreadIdentityAliases, viewModel = viewModel, onPrivateChatStart = { peerID -> viewModel.showPrivateChatSheet(peerID) @@ -344,7 +344,7 @@ fun PeopleSection( selectedPrivatePeer: String?, wifiAwarePeerIDs: Set = emptySet(), peopleCount: Int = 0, - excludedConversationIDs: Set = emptySet(), + excludedIdentityAliases: Set = emptySet(), viewModel: ChatViewModel, onPrivateChatStart: (String) -> Unit ) { @@ -461,9 +461,8 @@ fun PeopleSection( val offlineFavorites = FavoritesPersistenceService.shared.getOurFavorites() offlineFavorites.forEach { fav -> val favPeerID = ContactIdentityResolver.noiseKeyHex(fav.peerNoisePublicKey) - val conversationID = ContactDirectory.canonicalConversationId(favPeerID) if ( - conversationID !in excludedConversationIDs && + favPeerID.lowercase() !in excludedIdentityAliases && !isFavoriteMappedToConnected(fav) ) { val dn = peerNicknames[favPeerID] ?: fav.peerNickname @@ -478,10 +477,10 @@ fun PeopleSection( val directMap by viewModel.peerDirect.collectAsStateWithLifecycle() val offlineFavoriteRows = offlineFavorites.filterNot { favorite -> - val favoriteConversationID = ContactDirectory.canonicalConversationId( - ContactIdentityResolver.noiseKeyHex(favorite.peerNoisePublicKey) + val favoriteNoiseKey = ContactIdentityResolver.noiseKeyHex( + favorite.peerNoisePublicKey ) - favoriteConversationID in excludedConversationIDs || + favoriteNoiseKey.lowercase() in excludedIdentityAliases || isFavoriteMappedToConnected(favorite) } val rowKeys: List = sortedPeers + @@ -593,7 +592,6 @@ fun PeopleSection( @Composable private fun UnreadDirectMessagesSection( conversations: List, - connectedPeers: List, viewModel: ChatViewModel, onPrivateChatStart: (String) -> Unit, modifier: Modifier = Modifier @@ -622,31 +620,19 @@ private fun UnreadDirectMessagesSection( Column { if (index > 0) SheetCardDivider() - val resolution = ContactDirectory.resolve(conversation.conversationID) - val connected = resolution.meshPeerID?.let(connectedPeers::contains) == true || - conversation.conversationID in connectedPeers - val aliases = ContactDirectory.aliasesForConversation( - conversation.conversationID - ) - val sourceGeohash = aliases - .asSequence() - .mapNotNull(GeohashConversationRegistry::get) - .firstOrNull() - val displayName = resolution.displayName - ?.takeUnless { it.isBlank() || it.equals("Unknown", ignoreCase = true) } - ?: conversation.displayName val subtitle = when { - sourceGeohash != null -> "#$sourceGeohash" + conversation.sourceGeohash != null -> "#${conversation.sourceGeohash}" conversation.transport == DirectMessageTransport.NOSTR -> stringResource(R.string.cd_reachable_via_nostr) - !connected -> stringResource(R.string.cd_offline_mesh_chat) + !conversation.isConnected -> + stringResource(R.string.cd_offline_mesh_chat) else -> null } val peerIdentity = conversation.nostrPubkey ?.let(viewModel::peerIdentityForNostrPubkey) ?: viewModel.peerIdentityForMeshPeer(conversation.conversationID) val assignedColor = colorForPeer(peerIdentity, palette) - val (baseNameRaw, suffix) = splitSuffix(displayName) + val (baseNameRaw, suffix) = splitSuffix(conversation.displayName) Row( modifier = Modifier diff --git a/app/src/main/java/com/bitchat/android/ui/MessageManager.kt b/app/src/main/java/com/bitchat/android/ui/MessageManager.kt index e93ef39a..0c7b64b2 100644 --- a/app/src/main/java/com/bitchat/android/ui/MessageManager.kt +++ b/app/src/main/java/com/bitchat/android/ui/MessageManager.kt @@ -153,11 +153,17 @@ class MessageManager(private val state: ChatState) { state.setPrivateChats(updatedChats) } - fun clearPrivateUnreadMessages(peerID: String) { + fun clearPrivateUnreadMessages( + peerID: String, + aliases: Set = emptySet() + ) { val conversationID = ContactDirectory.canonicalConversationId(peerID) val updatedUnread = state.getUnreadPrivateMessagesValue().toMutableSet() - updatedUnread.remove(peerID) - updatedUnread.remove(conversationID) + val normalizedAliases = (aliases + peerID + conversationID) + .mapTo(mutableSetOf()) { it.lowercase() } + updatedUnread.removeAll { unreadID -> + unreadID.lowercase() in normalizedAliases + } state.setUnreadPrivateMessages(updatedUnread) } diff --git a/app/src/main/java/com/bitchat/android/ui/PrivateChatManager.kt b/app/src/main/java/com/bitchat/android/ui/PrivateChatManager.kt index 1af48a04..8d7c5e1d 100644 --- a/app/src/main/java/com/bitchat/android/ui/PrivateChatManager.kt +++ b/app/src/main/java/com/bitchat/android/ui/PrivateChatManager.kt @@ -47,7 +47,11 @@ class PrivateChatManager( // MARK: - Private Chat Lifecycle - fun startPrivateChat(peerID: String, meshService: MeshService): Boolean { + fun startPrivateChat( + peerID: String, + meshService: MeshService, + unreadAliases: Set = emptySet() + ): Boolean { val conversationID = ContactDirectory.canonicalConversationId(peerID) val route = ContactDirectory.resolve(conversationID) val meshPeerID = route.meshPeerID ?: peerID.takeIf { ContactIdentityResolver.isMeshPeerId(it) } @@ -75,7 +79,7 @@ class PrivateChatManager( state.setSelectedPrivateChatPeer(conversationID) // Clear unread - messageManager.clearPrivateUnreadMessages(conversationID) + messageManager.clearPrivateUnreadMessages(conversationID, unreadAliases) // Initialize chat if needed messageManager.initializePrivateChat(conversationID) diff --git a/app/src/main/java/com/bitchat/android/ui/UnreadConversationSummary.kt b/app/src/main/java/com/bitchat/android/ui/UnreadConversationSummary.kt index bad2c43c..5d0ad81a 100644 --- a/app/src/main/java/com/bitchat/android/ui/UnreadConversationSummary.kt +++ b/app/src/main/java/com/bitchat/android/ui/UnreadConversationSummary.kt @@ -19,7 +19,10 @@ internal data class UnreadConversationSummary( val unreadCount: Int, val latestMessageAt: Long, val transport: DirectMessageTransport, - val nostrPubkey: String? + val nostrPubkey: String?, + val identityAliases: Set, + val isConnected: Boolean = false, + val sourceGeohash: String? = null ) internal fun buildUnreadConversationSummaries( @@ -73,7 +76,9 @@ internal fun buildUnreadConversationSummaries( } else { DirectMessageTransport.MESH }, - nostrPubkey = nostrPubkey + nostrPubkey = nostrPubkey, + identityAliases = (aliases + conversationID) + .mapTo(mutableSetOf()) { it.lowercase() } ) }.sortedWith( compareByDescending { it.latestMessageAt } @@ -84,3 +89,17 @@ internal fun buildUnreadConversationSummaries( private fun isNostrConversationID(value: String): Boolean = value.startsWith("nostr_") || value.startsWith("nostr:") + +internal fun matchingUnreadAliases( + unreadConversationIDs: Set, + canonicalConversationID: String, + canonicalize: (String) -> String +): Set { + val normalizedCanonicalID = canonicalConversationID.lowercase() + return unreadConversationIDs + .filterTo(mutableSetOf()) { unreadID -> + canonicalize(unreadID).equals(normalizedCanonicalID, ignoreCase = true) + } + .plus(canonicalConversationID) + .mapTo(mutableSetOf()) { it.lowercase() } +} diff --git a/app/src/test/kotlin/com/bitchat/android/ui/PrivateChatManagerTest.kt b/app/src/test/kotlin/com/bitchat/android/ui/PrivateChatManagerTest.kt index 852411e8..8434cab1 100644 --- a/app/src/test/kotlin/com/bitchat/android/ui/PrivateChatManagerTest.kt +++ b/app/src/test/kotlin/com/bitchat/android/ui/PrivateChatManagerTest.kt @@ -104,4 +104,27 @@ class PrivateChatManagerTest { verify(meshService).sendReadReceipt(message.id, meshPeerID, "bob") } + + @Test + fun `opening canonical unread conversation clears all source aliases`() { + val canonicalID = "contact_alice" + val nostrAlias = "nostr_0123456789abcdef" + val meshAlias = "0123456789abcdef" + val unrelatedConversation = "other-contact" + val meshService = mock() + state.setUnreadPrivateMessages( + setOf(canonicalID, nostrAlias, meshAlias, unrelatedConversation) + ) + + manager.startPrivateChat( + peerID = canonicalID, + meshService = meshService, + unreadAliases = setOf(canonicalID, nostrAlias, meshAlias) + ) + + assertEquals( + setOf(unrelatedConversation), + state.getUnreadPrivateMessagesValue() + ) + } } diff --git a/app/src/test/kotlin/com/bitchat/android/ui/UnreadConversationSummaryTest.kt b/app/src/test/kotlin/com/bitchat/android/ui/UnreadConversationSummaryTest.kt index e054d819..2b5f4613 100644 --- a/app/src/test/kotlin/com/bitchat/android/ui/UnreadConversationSummaryTest.kt +++ b/app/src/test/kotlin/com/bitchat/android/ui/UnreadConversationSummaryTest.kt @@ -57,6 +57,10 @@ class UnreadConversationSummaryTest { assertEquals(1, rows.size) assertEquals("contact_alice", rows.single().conversationID) assertEquals(DirectMessageTransport.NOSTR, rows.single().transport) + assertEquals( + setOf("mesh-alias", "nostr_alias", "contact_alice"), + rows.single().identityAliases + ) } @Test @@ -104,6 +108,22 @@ class UnreadConversationSummaryTest { assertTrue(row.displayName.isNotBlank()) } + @Test + fun `canonical unread lookup returns every matching source alias`() { + val aliases = matchingUnreadAliases( + unreadConversationIDs = setOf("mesh-alias", "nostr_alias", "other-contact"), + canonicalConversationID = "contact_alice", + canonicalize = { unreadID -> + if (unreadID == "other-contact") unreadID else "contact_alice" + } + ) + + assertEquals( + setOf("mesh-alias", "nostr_alias", "contact_alice"), + aliases + ) + } + private fun incoming( id: String, sender: String,