From 906744d32a61effafb67d32fefe8c9d3b02050e8 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 10 Jul 2026 22:34:13 +0000 Subject: [PATCH 1/2] fix: treat DM rooms whose newest message is my own as read (#1286, #1287) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sending a reply — from this device or from another one via the self-addressed gift wrap — now counts as having read the conversation: - Account.broadcastPrivately / sendNip04PrivateMessage advance the room's local read marker to the sent message, so the Messages tab dot and the room's new-items bubble clear immediately on send. - unreadPrivateChatRoute and the room/channel/marmot rows in ChatroomHeaderCompose additionally ignore newest messages authored by the logged-in user, covering own messages that arrive from other devices before any local marker exists. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01RqN8cfqNdo1MAvLN4C9krm --- .../vitorpamplona/amethyst/model/Account.kt | 17 ++++ .../ui/screen/loggedIn/AccountViewModel.kt | 25 +++-- .../chats/rooms/ChatroomHeaderCompose.kt | 9 +- .../loggedIn/UnreadPrivateChatRouteTest.kt | 91 +++++++++++++++++++ 4 files changed, 132 insertions(+), 10 deletions(-) create mode 100644 amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/UnreadPrivateChatRouteTest.kt diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt index b8f6eb22d3..4a843d91ea 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt @@ -187,6 +187,7 @@ import com.vitorpamplona.quartz.nip10Notes.threadRootIdOrSelf import com.vitorpamplona.quartz.nip17Dm.NIP17Factory import com.vitorpamplona.quartz.nip17Dm.base.BaseDMGroupEvent import com.vitorpamplona.quartz.nip17Dm.base.ChatroomKey +import com.vitorpamplona.quartz.nip17Dm.base.ChatroomKeyable import com.vitorpamplona.quartz.nip17Dm.base.NIP17Group import com.vitorpamplona.quartz.nip17Dm.files.ChatMessageEncryptedFileHeaderEvent import com.vitorpamplona.quartz.nip17Dm.messages.ChatMessageEvent @@ -2521,6 +2522,8 @@ class Account( cache.justConsumeMyOwnEvent(newEvent) client.publish(newEvent, outboxRelays.flow.value + destinationRelays) + + markDmRoomAsRead(newEvent) } override suspend fun sendNip17EncryptedFile(template: EventTemplate) { @@ -2573,6 +2576,20 @@ class Account( val relayList = computeRelayListToBroadcast(wrap) client.publish(wrap, relayList) } + + markDmRoomAsRead(signedEvents.msg) + } + + /** + * Sending a message into a DM room means the user has caught up with it: advance the + * room's local read marker to the sent message so the unread indicators clear without + * requiring the conversation to be reopened (#1286, #1287). No-op for private events + * that don't belong to a room (private notes, reactions, deletions). + */ + private fun markDmRoomAsRead(event: Event) { + if (event is ChatroomKeyable) { + markAsRead("Room/${event.chatroomKey(signer.pubKey).hashCode()}", event.createdAt) + } } // --- Marmot Group Messaging --- diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt index 9eb163a241..3ead5b6143 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt @@ -1902,12 +1902,7 @@ class AccountViewModel( } } - private fun unreadPrivateChatRoute(chat: Note): Pair? { - val noteEvent = chat.event ?: return null - val room = (noteEvent as? ChatroomKeyable)?.chatroomKey(account.signer.pubKey) ?: return null - if (account.isAllHidden(room.users)) return null - return privateChatRoute(room) to noteEvent.createdAt - } + private fun unreadPrivateChatRoute(chat: Note): Pair? = unreadPrivateChatRoute(chat.event, account.signer.pubKey, account::isAllHidden) private fun markHiddenChatroomsAsRead() { account.chatroomList.rooms.forEach { roomKey, chatroom -> @@ -2592,6 +2587,24 @@ class AccountViewModel( val nip19: Nip19Parser.ParseReturn, ) +/** + * Read-marker route + timestamp for the newest message of a private chat room, or null when + * the room cannot be unread: no event, not a chat message, every participant hidden, or the + * newest message authored by the logged-in user — replying (from this device, or from another + * one via the self-addressed gift wrap) counts as having read the conversation (#1286, #1287). + */ +internal fun unreadPrivateChatRoute( + newestMessage: Event?, + loggedInUser: HexKey, + isAllHidden: (Set) -> Boolean, +): Pair? { + val noteEvent = newestMessage ?: return null + val room = (noteEvent as? ChatroomKeyable)?.chatroomKey(loggedInUser) ?: return null + if (isAllHidden(room.users)) return null + if (noteEvent.pubKey == loggedInUser) return null + return "Room/${room.hashCode()}" to noteEvent.createdAt +} + var mockedCache: AccountViewModel? = null @SuppressLint("ViewModelConstructorInComposable") diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/rooms/ChatroomHeaderCompose.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/rooms/ChatroomHeaderCompose.kt index e54a7e3732..c09ccd0585 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/rooms/ChatroomHeaderCompose.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/rooms/ChatroomHeaderCompose.kt @@ -265,7 +265,7 @@ private fun ChannelRoomCompose( channelTitle = { modifier -> ChannelTitleWithLabelInfo(channelName, R.string.public_chat, modifier) }, channelLastTime = lastMessage.createdAt(), channelLastContent = "$authorName: $description", - hasNewMessages = (noteEvent?.createdAt ?: Long.MIN_VALUE) > lastReadTime, + hasNewMessages = !accountViewModel.isLoggedUser(lastMessage.author) && (noteEvent?.createdAt ?: Long.MIN_VALUE) > lastReadTime, loadProfilePicture = accountViewModel.settings.showProfilePictures(), loadRobohash = accountViewModel.settings.isNotPerformanceMode(), autoPlayGif = @@ -301,7 +301,7 @@ private fun ChannelRoomCompose( channelTitle = { modifier -> ChannelTitleWithLabelInfo(channel.toBestDisplayName(), R.string.ephemeral_relay_chat, modifier) }, channelLastTime = lastMessage.createdAt(), channelLastContent = "$authorName: $description", - hasNewMessages = (noteEvent?.createdAt ?: Long.MIN_VALUE) > lastReadTime, + hasNewMessages = !accountViewModel.isLoggedUser(lastMessage.author) && (noteEvent?.createdAt ?: Long.MIN_VALUE) > lastReadTime, loadProfilePicture = accountViewModel.settings.showProfilePictures(), loadRobohash = accountViewModel.settings.isNotPerformanceMode(), autoPlayGif = @@ -341,7 +341,7 @@ private fun MarmotGroupRoomCompose( channelTitle = { modifier -> ChannelTitleWithLabelInfo(groupName, R.string.marmot_group, modifier) }, channelLastTime = lastMessage.createdAt(), channelLastContent = lastContent, - hasNewMessages = (lastMessage.createdAt() ?: Long.MIN_VALUE) > lastReadTime, + hasNewMessages = !accountViewModel.isLoggedUser(author) && (lastMessage.createdAt() ?: Long.MIN_VALUE) > lastReadTime, loadProfilePicture = accountViewModel.settings.showProfilePictures(), loadRobohash = accountViewModel.settings.isNotPerformanceMode(), autoPlayGif = @@ -572,8 +572,9 @@ private fun UserRoomCompose( } } + // A message I authored (sent here or from another device) counts as read (#1286, #1287). val lastReadTime by accountViewModel.account.loadLastReadFlow("Room/${room.hashCode()}").collectAsStateWithLifecycle() - if ((lastMessage.createdAt() ?: Long.MIN_VALUE) > lastReadTime) { + if (!accountViewModel.isLoggedUser(lastMessage.author) && (lastMessage.createdAt() ?: Long.MIN_VALUE) > lastReadTime) { Spacer(modifier = Height4dpModifier) NewItemsBubble() } diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/UnreadPrivateChatRouteTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/UnreadPrivateChatRouteTest.kt new file mode 100644 index 0000000000..5b72bea7a2 --- /dev/null +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/UnreadPrivateChatRouteTest.kt @@ -0,0 +1,91 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.ui.screen.loggedIn + +import com.vitorpamplona.quartz.nip01Core.core.HexKey +import com.vitorpamplona.quartz.nip17Dm.base.ChatroomKey +import com.vitorpamplona.quartz.nip17Dm.messages.ChatMessageEvent +import com.vitorpamplona.quartz.nip25Reactions.ReactionEvent +import kotlinx.collections.immutable.persistentSetOf +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +/** + * The unread predicate behind the Messages tab dot: a room whose newest message was + * authored by the logged-in user counts as read (#1286, #1287). + */ +class UnreadPrivateChatRouteTest { + private val me: HexKey = "a".repeat(64) + private val peer: HexKey = "b".repeat(64) + + private val roomWithPeer = "Room/${ChatroomKey(persistentSetOf(peer)).hashCode()}" + + private fun message( + from: HexKey, + to: HexKey, + createdAt: Long, + ) = ChatMessageEvent( + id = "0".repeat(64), + pubKey = from, + createdAt = createdAt, + tags = arrayOf(arrayOf("p", to)), + content = "hello", + sig = "", + ) + + @Test + fun newestMessageFromPeerReturnsTheRoomRoute() { + val route = unreadPrivateChatRoute(message(from = peer, to = me, createdAt = 100), me, isAllHidden = { false }) + + assertEquals(roomWithPeer to 100L, route) + } + + @Test + fun newestMessageAuthoredByMeCountsAsRead() { + assertNull(unreadPrivateChatRoute(message(from = me, to = peer, createdAt = 100), me, isAllHidden = { false })) + } + + @Test + fun hiddenRoomsAreNeverUnread() { + assertNull(unreadPrivateChatRoute(message(from = peer, to = me, createdAt = 100), me, isAllHidden = { true })) + } + + @Test + fun missingEventIsNotUnread() { + assertNull(unreadPrivateChatRoute(null, me, isAllHidden = { false })) + } + + @Test + fun nonChatEventsAreNotUnread() { + val reaction = + ReactionEvent( + id = "0".repeat(64), + pubKey = peer, + createdAt = 100, + tags = arrayOf(arrayOf("p", me)), + content = "+", + sig = "", + ) + + assertNull(unreadPrivateChatRoute(reaction, me, isAllHidden = { false })) + } +} From 507de3257521d4b71055d39ed8e64a668cc66e25 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 10 Jul 2026 23:51:55 +0000 Subject: [PATCH 2/2] fix: harden own-message-counts-as-read against audit findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the issues found reviewing the #1286/#1287 fix: - Centralize the room read-marker route in privateChatLastReadRoute() and the authorship rule in chatMessageMarksRoomAsRead(); all writers and readers now share one implementation. - Exempt notes-to-self rooms from the authorship rule — there the user's own messages are the content still to be seen. - Mark own chat messages as read at the ingestion choke point (EventProcessor), covering every send path (including the play-flavor App Functions assistant) and own messages arriving from other devices at the persisted-marker level, not just visually. - Advance the send-path marker to the newest known room message, so a quick-reply to a skew-ahead peer also clears the indicators. - Revert the authorship guard on public-chat/ephemeral/marmot rows: in multi-party channels 'I posted last' does not imply the earlier backlog was seen, and no marker advances there to back the guard. - Restore the new-items bubble for rooms whose newest item is an unsent draft — a draft still needs the user's attention. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01RqN8cfqNdo1MAvLN4C9krm --- .../vitorpamplona/amethyst/model/Account.kt | 18 ++++-- .../model/PrivateChatroomReadState.kt | 63 +++++++++++++++++++ .../ui/screen/loggedIn/AccountViewModel.kt | 28 ++------- .../loggedIn/DecryptAndIndexProcessor.kt | 24 ++++++- .../loggedIn/chats/privateDM/ChatroomView.kt | 3 +- .../chats/rooms/ChatroomHeaderCompose.kt | 26 +++++--- .../PrivateChatroomReadStateTest.kt} | 36 +++++++++-- 7 files changed, 154 insertions(+), 44 deletions(-) create mode 100644 amethyst/src/main/java/com/vitorpamplona/amethyst/model/PrivateChatroomReadState.kt rename amethyst/src/test/java/com/vitorpamplona/amethyst/{ui/screen/loggedIn/UnreadPrivateChatRouteTest.kt => model/PrivateChatroomReadStateTest.kt} (68%) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt index 4a843d91ea..0943f0e37c 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt @@ -2581,14 +2581,22 @@ class Account( } /** - * Sending a message into a DM room means the user has caught up with it: advance the - * room's local read marker to the sent message so the unread indicators clear without - * requiring the conversation to be reopened (#1286, #1287). No-op for private events - * that don't belong to a room (private notes, reactions, deletions). + * Sending a message into a DM room means the user has caught up with what the room + * showed when they replied: advance the local read marker to the newest known message — + * not just the sent one, whose local clock may lag behind a skew-ahead peer's — so the + * unread indicators clear without requiring the conversation to be reopened + * (#1286, #1287). No-op for private events that don't belong to a room (private notes, + * reactions, deletions). */ private fun markDmRoomAsRead(event: Event) { if (event is ChatroomKeyable) { - markAsRead("Room/${event.chatroomKey(signer.pubKey).hashCode()}", event.createdAt) + val room = event.chatroomKey(signer.pubKey) + val newestInRoom = + chatroomList.rooms + .get(room) + ?.newestMessage + ?.createdAt() ?: 0L + markAsRead(privateChatLastReadRoute(room), maxOf(event.createdAt, newestInRoom)) } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/PrivateChatroomReadState.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/PrivateChatroomReadState.kt new file mode 100644 index 0000000000..24e2d00d86 --- /dev/null +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/PrivateChatroomReadState.kt @@ -0,0 +1,63 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.model + +import com.vitorpamplona.quartz.nip01Core.core.Event +import com.vitorpamplona.quartz.nip01Core.core.HexKey +import com.vitorpamplona.quartz.nip17Dm.base.ChatroomKey +import com.vitorpamplona.quartz.nip17Dm.base.ChatroomKeyable + +/** + * Route key under which a private chat room's last-read time is stored in AccountSettings. + * Every marker writer (send paths, ingestion, room view, hidden-room sweep) and reader + * (Messages-tab dot, room-row bubble) must build the key through this function: a format + * drift between a writer and a reader silently splits read state (#1286). + */ +fun privateChatLastReadRoute(room: ChatroomKey) = "Room/${room.hashCode()}" + +/** + * True when [message] marks [room] as read up to its timestamp: the logged-in user authored + * it, so sending it — from this device, or from another one arriving via the self-addressed + * gift wrap — means they had caught up with the conversation (#1286, #1287). Notes-to-self + * rooms are exempt: there the user's own messages ARE the content still to be seen. + */ +fun chatMessageMarksRoomAsRead( + message: Event, + room: ChatroomKey, + loggedInUser: HexKey, +): Boolean = message.pubKey == loggedInUser && room.users.singleOrNull() != loggedInUser + +/** + * Read-marker route + timestamp for the newest message of a private chat room, or null when + * the room cannot be unread: no chat event, a newest message that counts as read (see + * [chatMessageMarksRoomAsRead]), or every participant hidden. + */ +fun unreadPrivateChatRoute( + newestMessage: Event?, + loggedInUser: HexKey, + isAllHidden: (Set) -> Boolean, +): Pair? { + if (newestMessage !is ChatroomKeyable) return null + val room = newestMessage.chatroomKey(loggedInUser) + if (chatMessageMarksRoomAsRead(newestMessage, room, loggedInUser)) return null + if (isAllHidden(room.users)) return null + return privateChatLastReadRoute(room) to newestMessage.createdAt +} diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt index 3ead5b6143..96df4d0538 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt @@ -68,6 +68,8 @@ import com.vitorpamplona.amethyst.model.User import com.vitorpamplona.amethyst.model.privacyOptions.EmptyRoleBasedHttpClientBuilder import com.vitorpamplona.amethyst.model.privacyOptions.IRoleBasedHttpClientBuilder import com.vitorpamplona.amethyst.model.privacyOptions.RoleBasedHttpClientBuilder +import com.vitorpamplona.amethyst.model.privateChatLastReadRoute +import com.vitorpamplona.amethyst.model.unreadPrivateChatRoute import com.vitorpamplona.amethyst.service.ClinkDebitPayer import com.vitorpamplona.amethyst.service.OnlineChecker import com.vitorpamplona.amethyst.service.V4VPaymentHandler @@ -1884,7 +1886,7 @@ class AccountViewModel( } noteEvent is ChatroomKeyable -> { - account.markAsRead("Room/${noteEvent.chatroomKey(account.signer.pubKey).hashCode()}", noteEvent.createdAt) + account.markAsRead(privateChatLastReadRoute(noteEvent.chatroomKey(account.signer.pubKey)), noteEvent.createdAt) } noteEvent is DraftWrapEvent -> { @@ -1892,7 +1894,7 @@ class AccountViewModel( if (innerEvent is IsInPublicChatChannel) { account.markAsRead("Channel/${innerEvent.channelId()}", noteEvent.createdAt) } else if (innerEvent is ChatroomKeyable) { - account.markAsRead("Room/${innerEvent.chatroomKey(account.signer.pubKey).hashCode()}", noteEvent.createdAt) + account.markAsRead(privateChatLastReadRoute(innerEvent.chatroomKey(account.signer.pubKey)), noteEvent.createdAt) } } } @@ -1908,14 +1910,12 @@ class AccountViewModel( account.chatroomList.rooms.forEach { roomKey, chatroom -> if (account.isAllHidden(roomKey.users)) { chatroom.newestMessage?.createdAt()?.let { - account.markAsRead(privateChatRoute(roomKey), it) + account.markAsRead(privateChatLastReadRoute(roomKey), it) } } } } - private fun privateChatRoute(room: ChatroomKey) = "Room/${room.hashCode()}" - class Factory( val account: Account, val settings: UiSettingsState, @@ -2587,24 +2587,6 @@ class AccountViewModel( val nip19: Nip19Parser.ParseReturn, ) -/** - * Read-marker route + timestamp for the newest message of a private chat room, or null when - * the room cannot be unread: no event, not a chat message, every participant hidden, or the - * newest message authored by the logged-in user — replying (from this device, or from another - * one via the self-addressed gift wrap) counts as having read the conversation (#1286, #1287). - */ -internal fun unreadPrivateChatRoute( - newestMessage: Event?, - loggedInUser: HexKey, - isAllHidden: (Set) -> Boolean, -): Pair? { - val noteEvent = newestMessage ?: return null - val room = (noteEvent as? ChatroomKeyable)?.chatroomKey(loggedInUser) ?: return null - if (isAllHidden(room.users)) return null - if (noteEvent.pubKey == loggedInUser) return null - return "Room/${room.hashCode()}" to noteEvent.createdAt -} - var mockedCache: AccountViewModel? = null @SuppressLint("ViewModelConstructorInComposable") diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/DecryptAndIndexProcessor.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/DecryptAndIndexProcessor.kt index e05a9627a1..b2a2c7938b 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/DecryptAndIndexProcessor.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/DecryptAndIndexProcessor.kt @@ -26,6 +26,8 @@ import com.vitorpamplona.amethyst.commons.nipACWebRtcCalls.CallManager import com.vitorpamplona.amethyst.model.Account import com.vitorpamplona.amethyst.model.LocalCache import com.vitorpamplona.amethyst.model.Note +import com.vitorpamplona.amethyst.model.chatMessageMarksRoomAsRead +import com.vitorpamplona.amethyst.model.privateChatLastReadRoute import com.vitorpamplona.quartz.experimental.ephemChat.chat.EphemeralChatEvent import com.vitorpamplona.quartz.marmot.GroupEventResult import com.vitorpamplona.quartz.marmot.MarmotInboundProcessor @@ -96,7 +98,10 @@ class EventProcessor( is CallRenegotiateEvent, -> callManager?.onSignalingEvent(event) - is ChatroomKeyable -> chatHandler.add(event, eventNote, publicNote) + is ChatroomKeyable -> { + chatHandler.add(event, eventNote, publicNote) + markOwnChatMessageAsRead(event) + } is DraftWrapEvent -> draftHandler.add(event, eventNote, publicNote) @@ -110,6 +115,23 @@ class EventProcessor( } } + /** + * A chat message authored by this account — sent from this device through any code + * path, or arriving via the self-addressed gift wrap from another device — means the + * user was caught up with the room when they sent it, so advance the room's read + * marker here at the single ingestion choke point rather than at each send site + * (#1286, #1287). markAsRead is monotonic, so out-of-order history sync cannot move + * the marker backwards. Unsent drafts never reach this branch (DraftEventHandler + * indexes their rumors directly into the chatroom). + */ + private fun markOwnChatMessageAsRead(event: T) where T : Event, T : ChatroomKeyable { + val me = account.signer.pubKey + val room = event.chatroomKey(me) + if (chatMessageMarksRoomAsRead(event, room, me)) { + account.markAsRead(privateChatLastReadRoute(room), event.createdAt) + } + } + suspend fun delete(note: Note) { note.event?.let { event -> try { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/privateDM/ChatroomView.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/privateDM/ChatroomView.kt index 4632448358..efdfd2c396 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/privateDM/ChatroomView.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/privateDM/ChatroomView.kt @@ -48,6 +48,7 @@ import com.vitorpamplona.amethyst.commons.ui.feeds.RelayReachDetailDialog import com.vitorpamplona.amethyst.commons.ui.feeds.RelayReachMarkers import com.vitorpamplona.amethyst.commons.ui.feeds.RelayReachSentinels import com.vitorpamplona.amethyst.commons.ui.feeds.RelayReachState +import com.vitorpamplona.amethyst.model.privateChatLastReadRoute import com.vitorpamplona.amethyst.service.relayClient.reqCommand.event.EventFinderFilterAssemblerSubscription import com.vitorpamplona.amethyst.ui.actions.uploads.resolveSharedMedia import com.vitorpamplona.amethyst.ui.feeds.WatchLifecycleAndUpdateModel @@ -268,7 +269,7 @@ fun ChatroomViewUI( feedContentState = feedViewModel.feedState, accountViewModel = accountViewModel, nav = nav, - routeForLastRead = "Room/${room.hashCode()}", + routeForLastRead = privateChatLastReadRoute(room), avoidDraft = newPostModel.draftTag, onWantsToReply = newPostModel::reply, onWantsToEditDraft = newPostModel::editFromDraft, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/rooms/ChatroomHeaderCompose.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/rooms/ChatroomHeaderCompose.kt index c09ccd0585..442e49cf2d 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/rooms/ChatroomHeaderCompose.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/rooms/ChatroomHeaderCompose.kt @@ -61,7 +61,9 @@ import com.vitorpamplona.amethyst.commons.model.nip29RelayGroups.RelayGroupChann import com.vitorpamplona.amethyst.model.LocalCache import com.vitorpamplona.amethyst.model.Note import com.vitorpamplona.amethyst.model.User +import com.vitorpamplona.amethyst.model.chatMessageMarksRoomAsRead import com.vitorpamplona.amethyst.model.nip11RelayInfo.loadRelayInfo +import com.vitorpamplona.amethyst.model.privateChatLastReadRoute import com.vitorpamplona.amethyst.service.relayClient.reqCommand.channel.observeChannel import com.vitorpamplona.amethyst.service.relayClient.reqCommand.event.observeNoteHasEvent import com.vitorpamplona.amethyst.service.relayClient.reqCommand.user.UserFinderByParentFilterAssemblerSubscription @@ -143,7 +145,7 @@ fun ChatroomComposeChannelOrUser( val baseNoteEvent = baseNote.event if (baseNoteEvent is DraftWrapEvent) { ObserveDraftEvent(baseNote, accountViewModel) { innerNote -> - ChatroomEntry(innerNote, accountViewModel, nav) + ChatroomEntry(innerNote, accountViewModel, nav, isDraft = true) } } else { ChatroomEntry(baseNote, accountViewModel, nav) @@ -155,6 +157,7 @@ private fun ChatroomEntry( lastMessage: Note, accountViewModel: AccountViewModel, nav: INav, + isDraft: Boolean = false, ) { if (lastMessage is RelayGroupServerRoomNote) { RelayGroupServerRoomCompose(lastMessage, accountViewModel, nav) @@ -213,7 +216,7 @@ private fun ChatroomEntry( is ChatroomKeyable -> { val room = baseNoteEvent.chatroomKey(accountViewModel.userProfile().pubkeyHex) - UserRoomCompose(room, lastMessage, accountViewModel, nav) + UserRoomCompose(room, lastMessage, isDraft, accountViewModel, nav) } is EphemeralChatEvent -> { @@ -265,7 +268,7 @@ private fun ChannelRoomCompose( channelTitle = { modifier -> ChannelTitleWithLabelInfo(channelName, R.string.public_chat, modifier) }, channelLastTime = lastMessage.createdAt(), channelLastContent = "$authorName: $description", - hasNewMessages = !accountViewModel.isLoggedUser(lastMessage.author) && (noteEvent?.createdAt ?: Long.MIN_VALUE) > lastReadTime, + hasNewMessages = (noteEvent?.createdAt ?: Long.MIN_VALUE) > lastReadTime, loadProfilePicture = accountViewModel.settings.showProfilePictures(), loadRobohash = accountViewModel.settings.isNotPerformanceMode(), autoPlayGif = @@ -301,7 +304,7 @@ private fun ChannelRoomCompose( channelTitle = { modifier -> ChannelTitleWithLabelInfo(channel.toBestDisplayName(), R.string.ephemeral_relay_chat, modifier) }, channelLastTime = lastMessage.createdAt(), channelLastContent = "$authorName: $description", - hasNewMessages = !accountViewModel.isLoggedUser(lastMessage.author) && (noteEvent?.createdAt ?: Long.MIN_VALUE) > lastReadTime, + hasNewMessages = (noteEvent?.createdAt ?: Long.MIN_VALUE) > lastReadTime, loadProfilePicture = accountViewModel.settings.showProfilePictures(), loadRobohash = accountViewModel.settings.isNotPerformanceMode(), autoPlayGif = @@ -341,7 +344,7 @@ private fun MarmotGroupRoomCompose( channelTitle = { modifier -> ChannelTitleWithLabelInfo(groupName, R.string.marmot_group, modifier) }, channelLastTime = lastMessage.createdAt(), channelLastContent = lastContent, - hasNewMessages = !accountViewModel.isLoggedUser(author) && (lastMessage.createdAt() ?: Long.MIN_VALUE) > lastReadTime, + hasNewMessages = (lastMessage.createdAt() ?: Long.MIN_VALUE) > lastReadTime, loadProfilePicture = accountViewModel.settings.showProfilePictures(), loadRobohash = accountViewModel.settings.isNotPerformanceMode(), autoPlayGif = @@ -521,6 +524,7 @@ private fun ChannelTitleWithLabelInfo( private fun UserRoomCompose( room: ChatroomKey, lastMessage: Note, + isDraft: Boolean, accountViewModel: AccountViewModel, nav: INav, ) { @@ -572,9 +576,15 @@ private fun UserRoomCompose( } } - // A message I authored (sent here or from another device) counts as read (#1286, #1287). - val lastReadTime by accountViewModel.account.loadLastReadFlow("Room/${room.hashCode()}").collectAsStateWithLifecycle() - if (!accountViewModel.isLoggedUser(lastMessage.author) && (lastMessage.createdAt() ?: Long.MIN_VALUE) > lastReadTime) { + // A sent message I authored counts as read (#1286, #1287); an unsent draft still needs my attention. + val newestEvent = lastMessage.event + val countsAsRead = + !isDraft && + newestEvent != null && + chatMessageMarksRoomAsRead(newestEvent, room, accountViewModel.account.signer.pubKey) + + val lastReadTime by accountViewModel.account.loadLastReadFlow(privateChatLastReadRoute(room)).collectAsStateWithLifecycle() + if (!countsAsRead && (lastMessage.createdAt() ?: Long.MIN_VALUE) > lastReadTime) { Spacer(modifier = Height4dpModifier) NewItemsBubble() } diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/UnreadPrivateChatRouteTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/model/PrivateChatroomReadStateTest.kt similarity index 68% rename from amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/UnreadPrivateChatRouteTest.kt rename to amethyst/src/test/java/com/vitorpamplona/amethyst/model/PrivateChatroomReadStateTest.kt index 5b72bea7a2..2227e72a70 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/UnreadPrivateChatRouteTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/model/PrivateChatroomReadStateTest.kt @@ -18,7 +18,7 @@ * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ -package com.vitorpamplona.amethyst.ui.screen.loggedIn +package com.vitorpamplona.amethyst.model import com.vitorpamplona.quartz.nip01Core.core.HexKey import com.vitorpamplona.quartz.nip17Dm.base.ChatroomKey @@ -30,14 +30,16 @@ import org.junit.Assert.assertNull import org.junit.Test /** - * The unread predicate behind the Messages tab dot: a room whose newest message was - * authored by the logged-in user counts as read (#1286, #1287). + * The unread predicate behind the Messages tab dot and the room-row bubble: a room whose + * newest message was authored by the logged-in user counts as read (#1286, #1287), except + * notes-to-self rooms, where the user's own messages are the content still to be seen. */ -class UnreadPrivateChatRouteTest { +class PrivateChatroomReadStateTest { private val me: HexKey = "a".repeat(64) private val peer: HexKey = "b".repeat(64) - private val roomWithPeer = "Room/${ChatroomKey(persistentSetOf(peer)).hashCode()}" + private val roomWithPeer = ChatroomKey(persistentSetOf(peer)) + private val selfRoom = ChatroomKey(persistentSetOf(me)) private fun message( from: HexKey, @@ -56,7 +58,7 @@ class UnreadPrivateChatRouteTest { fun newestMessageFromPeerReturnsTheRoomRoute() { val route = unreadPrivateChatRoute(message(from = peer, to = me, createdAt = 100), me, isAllHidden = { false }) - assertEquals(roomWithPeer to 100L, route) + assertEquals(privateChatLastReadRoute(roomWithPeer) to 100L, route) } @Test @@ -64,6 +66,13 @@ class UnreadPrivateChatRouteTest { assertNull(unreadPrivateChatRoute(message(from = me, to = peer, createdAt = 100), me, isAllHidden = { false })) } + @Test + fun notesToSelfRoomsCanStillBeUnread() { + val route = unreadPrivateChatRoute(message(from = me, to = me, createdAt = 100), me, isAllHidden = { false }) + + assertEquals(privateChatLastReadRoute(selfRoom) to 100L, route) + } + @Test fun hiddenRoomsAreNeverUnread() { assertNull(unreadPrivateChatRoute(message(from = peer, to = me, createdAt = 100), me, isAllHidden = { true })) @@ -88,4 +97,19 @@ class UnreadPrivateChatRouteTest { assertNull(unreadPrivateChatRoute(reaction, me, isAllHidden = { false })) } + + @Test + fun myMessageMarksAPeerRoomAsRead() { + assertEquals(true, chatMessageMarksRoomAsRead(message(from = me, to = peer, createdAt = 100), roomWithPeer, me)) + } + + @Test + fun aPeerMessageNeverMarksTheRoomAsRead() { + assertEquals(false, chatMessageMarksRoomAsRead(message(from = peer, to = me, createdAt = 100), roomWithPeer, me)) + } + + @Test + fun myMessageDoesNotMarkTheSelfRoomAsRead() { + assertEquals(false, chatMessageMarksRoomAsRead(message(from = me, to = me, createdAt = 100), selfRoom, me)) + } }