fix: address unread DM review feedback

This commit is contained in:
callebtc 2026-07-27 18:23:08 +02:00
parent 8c62e90711
commit 21cfa62cc2
8 changed files with 190 additions and 69 deletions

View File

@ -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<List<UnreadConversationSummary>> = 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<Set<String>> = state.joinedChannels
val currentChannel: StateFlow<String?> = state.currentChannel
val channelMessages: StateFlow<Map<String, List<BitchatMessage>>> = 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<String>()
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) { }
}
}
}

View File

@ -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<String> = emptySet()
excludedIdentityAliases: Set<String> = 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) {

View File

@ -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<String> = emptySet(),
peopleCount: Int = 0,
excludedConversationIDs: Set<String> = emptySet(),
excludedIdentityAliases: Set<String> = 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<String> = sortedPeers +
@ -593,7 +592,6 @@ fun PeopleSection(
@Composable
private fun UnreadDirectMessagesSection(
conversations: List<UnreadConversationSummary>,
connectedPeers: List<String>,
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

View File

@ -153,11 +153,17 @@ class MessageManager(private val state: ChatState) {
state.setPrivateChats(updatedChats)
}
fun clearPrivateUnreadMessages(peerID: String) {
fun clearPrivateUnreadMessages(
peerID: String,
aliases: Set<String> = 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)
}

View File

@ -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<String> = 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)

View File

@ -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<String>,
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<UnreadConversationSummary> { 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<String>,
canonicalConversationID: String,
canonicalize: (String) -> String
): Set<String> {
val normalizedCanonicalID = canonicalConversationID.lowercase()
return unreadConversationIDs
.filterTo(mutableSetOf()) { unreadID ->
canonicalize(unreadID).equals(normalizedCanonicalID, ignoreCase = true)
}
.plus(canonicalConversationID)
.mapTo(mutableSetOf()) { it.lowercase() }
}

View File

@ -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<MeshService>()
state.setUnreadPrivateMessages(
setOf(canonicalID, nostrAlias, meshAlias, unrelatedConversation)
)
manager.startPrivateChat(
peerID = canonicalID,
meshService = meshService,
unreadAliases = setOf(canonicalID, nostrAlias, meshAlias)
)
assertEquals(
setOf(unrelatedConversation),
state.getUnreadPrivateMessagesValue()
)
}
}

View File

@ -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,