From 9fa968a5936d9f945b2d257f0db95746fd6397bf Mon Sep 17 00:00:00 2001 From: spaced4ndy <8711996+spaced4ndy@users.noreply.github.com> Date: Wed, 27 Nov 2024 18:30:39 +0400 Subject: [PATCH] ui: fix marking chat read (don't use range api) (#5257) --- apps/ios/Shared/Model/SimpleXAPI.swift | 23 ++----- apps/ios/Shared/Views/Chat/ChatView.swift | 2 +- apps/ios/SimpleXChat/APITypes.swift | 4 +- .../chat/simplex/common/model/ChatModel.kt | 29 +++----- .../chat/simplex/common/model/SimpleXAPI.kt | 8 +-- .../simplex/common/views/chat/ChatView.kt | 66 +++++++++++-------- .../views/chatlist/ChatListNavLinkView.kt | 4 +- 7 files changed, 63 insertions(+), 73 deletions(-) diff --git a/apps/ios/Shared/Model/SimpleXAPI.swift b/apps/ios/Shared/Model/SimpleXAPI.swift index 13b11568d8..c03483311d 100644 --- a/apps/ios/Shared/Model/SimpleXAPI.swift +++ b/apps/ios/Shared/Model/SimpleXAPI.swift @@ -1061,8 +1061,8 @@ func apiRejectContactRequest(contactReqId: Int64) async throws { throw r } -func apiChatRead(type: ChatType, id: Int64, itemRange: (Int64, Int64)) async throws { - try await sendCommandOkResp(.apiChatRead(type: type, id: id, itemRange: itemRange)) +func apiChatRead(type: ChatType, id: Int64) async throws { + try await sendCommandOkResp(.apiChatRead(type: type, id: id)) } func apiChatItemsRead(type: ChatType, id: Int64, itemIds: [Int64]) async throws { @@ -1368,15 +1368,13 @@ func apiGetNetworkStatuses() throws -> [ConnNetworkStatus] { throw r } -func markChatRead(_ chat: Chat, aboveItem: ChatItem? = nil) async { +func markChatRead(_ chat: Chat) async { do { if chat.chatStats.unreadCount > 0 { - let minItemId = chat.chatStats.minUnreadItemId - let itemRange = (minItemId, aboveItem?.id ?? chat.chatItems.last?.id ?? minItemId) let cInfo = chat.chatInfo - try await apiChatRead(type: cInfo.chatType, id: cInfo.apiId, itemRange: itemRange) + try await apiChatRead(type: cInfo.chatType, id: cInfo.apiId) await MainActor.run { - withAnimation { ChatModel.shared.markChatItemsRead(cInfo, aboveItem: aboveItem) } + withAnimation { ChatModel.shared.markChatItemsRead(cInfo) } } } if chat.chatStats.unreadChat { @@ -1399,17 +1397,6 @@ func markChatUnread(_ chat: Chat, unreadChat: Bool = true) async { } } -func apiMarkChatItemRead(_ cInfo: ChatInfo, _ cItem: ChatItem) async { - do { - try await apiChatRead(type: cInfo.chatType, id: cInfo.apiId, itemRange: (cItem.id, cItem.id)) - DispatchQueue.main.async { - ChatModel.shared.markChatItemsRead(cInfo, [cItem.id]) - } - } catch { - logger.error("apiChatRead error: \(responseError(error))") - } -} - func apiMarkChatItemsRead(_ cInfo: ChatInfo, _ itemIds: [ChatItem.ID]) async { do { try await apiChatItemsRead(type: cInfo.chatType, id: cInfo.apiId, itemIds: itemIds) diff --git a/apps/ios/Shared/Views/Chat/ChatView.swift b/apps/ios/Shared/Views/Chat/ChatView.swift index 1acf08035c..6b287d52a1 100644 --- a/apps/ios/Shared/Views/Chat/ChatView.swift +++ b/apps/ios/Shared/Views/Chat/ChatView.swift @@ -987,7 +987,7 @@ struct ChatView: View { } } else if chatItem.isRcvNew { waitToMarkRead { - await apiMarkChatItemRead(chat.chatInfo, chatItem) + await apiMarkChatItemsRead(chat.chatInfo, [chatItem.id]) } } } diff --git a/apps/ios/SimpleXChat/APITypes.swift b/apps/ios/SimpleXChat/APITypes.swift index f8cc2ac8b7..1df6d07813 100644 --- a/apps/ios/SimpleXChat/APITypes.swift +++ b/apps/ios/SimpleXChat/APITypes.swift @@ -137,7 +137,7 @@ public enum ChatCommand { case apiCallStatus(contact: Contact, callStatus: WebRTCCallStatus) // WebRTC calls / case apiGetNetworkStatuses - case apiChatRead(type: ChatType, id: Int64, itemRange: (Int64, Int64)) + case apiChatRead(type: ChatType, id: Int64) case apiChatItemsRead(type: ChatType, id: Int64, itemIds: [Int64]) case apiChatUnread(type: ChatType, id: Int64, unreadChat: Bool) case receiveFile(fileId: Int64, userApprovedRelays: Bool, encrypted: Bool?, inline: Bool?) @@ -310,7 +310,7 @@ public enum ChatCommand { case .apiGetCallInvitations: return "/_call get" case let .apiCallStatus(contact, callStatus): return "/_call status @\(contact.apiId) \(callStatus.rawValue)" case .apiGetNetworkStatuses: return "/_network_statuses" - case let .apiChatRead(type, id, itemRange: (from, to)): return "/_read chat \(ref(type, id)) from=\(from) to=\(to)" + case let .apiChatRead(type, id): return "/_read chat \(ref(type, id))" case let .apiChatItemsRead(type, id, itemIds): return "/_read chat items \(ref(type, id)) \(joinedIds(itemIds))" case let .apiChatUnread(type, id, unreadChat): return "/_unread chat \(ref(type, id)) \(onOff(unreadChat))" case let .receiveFile(fileId, userApprovedRelays, encrypt, inline): return "/freceive \(fileId)\(onOffParam("approved_relays", userApprovedRelays))\(onOffParam("encrypt", encrypt))\(onOffParam("inline", inline))" diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/ChatModel.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/ChatModel.kt index ca03d0ce72..56e1376ea2 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/ChatModel.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/ChatModel.kt @@ -333,9 +333,8 @@ object ChatModel { chatItems = arrayListOf(newPreviewItem), chatStats = if (cItem.meta.itemStatus is CIStatus.RcvNew) { - val minUnreadId = if(chat.chatStats.minUnreadItemId == 0L) cItem.id else chat.chatStats.minUnreadItemId increaseUnreadCounter(rhId, currentUser.value!!) - chat.chatStats.copy(unreadCount = chat.chatStats.unreadCount + 1, minUnreadItemId = minUnreadId) + chat.chatStats.copy(unreadCount = chat.chatStats.unreadCount + 1) } else chat.chatStats @@ -514,23 +513,19 @@ object ChatModel { } } - fun markChatItemsRead(remoteHostId: Long?, chatInfo: ChatInfo, range: CC.ItemRange? = null, unreadCountAfter: Int? = null) { + fun markChatItemsRead(remoteHostId: Long?, chatInfo: ChatInfo, itemIds: List? = null) { val cInfo = chatInfo - val markedRead = markItemsReadInCurrentChat(chatInfo, range) + val markedRead = markItemsReadInCurrentChat(chatInfo, itemIds) // update preview val chatIdx = getChatIndex(remoteHostId, cInfo.id) if (chatIdx >= 0) { val chat = chats[chatIdx] val lastId = chat.chatItems.lastOrNull()?.id if (lastId != null) { - val unreadCount = unreadCountAfter ?: if (range != null) chat.chatStats.unreadCount - markedRead else 0 + val unreadCount = if (itemIds != null) chat.chatStats.unreadCount - markedRead else 0 decreaseUnreadCounter(remoteHostId, currentUser.value!!, chat.chatStats.unreadCount - unreadCount) chats[chatIdx] = chat.copy( - chatStats = chat.chatStats.copy( - unreadCount = unreadCount, - // Can't use minUnreadItemId currently since chat items can have unread items between read items - //minUnreadItemId = if (range != null) kotlin.math.max(chat.chatStats.minUnreadItemId, range.to + 1) else lastId + 1 - ) + chatStats = chat.chatStats.copy(unreadCount = unreadCount) ) } } @@ -642,21 +637,17 @@ object ChatModel { } } - private fun markItemsReadInCurrentChat(chatInfo: ChatInfo, range: CC.ItemRange? = null): Int { + private fun markItemsReadInCurrentChat(chatInfo: ChatInfo, itemIds: List? = null): Int { val cInfo = chatInfo var markedRead = 0 if (chatId.value == cInfo.id) { val items = chatItems.value var i = items.lastIndex - val itemIdsFromRange = if (range != null) { - (range.from .. range.to).toMutableSet() - } else { - mutableSetOf() - } + val itemIdsFromRange = itemIds?.toMutableSet() ?: mutableSetOf() val markedReadIds = mutableSetOf() while (i >= 0) { val item = items[i] - if (item.meta.itemStatus is CIStatus.RcvNew && (range == null || itemIdsFromRange.contains(item.id))) { + if (item.meta.itemStatus is CIStatus.RcvNew && (itemIds == null || itemIdsFromRange.contains(item.id))) { val newItem = item.withStatus(CIStatus.RcvRead()) items[i] = newItem if (newItem.meta.itemLive != true && newItem.meta.itemTimed?.ttl != null) { @@ -666,7 +657,7 @@ object ChatModel { } markedReadIds.add(item.id) markedRead++ - if (range != null) { + if (itemIds != null) { itemIdsFromRange.remove(item.id) // already set all needed items as read, can finish the loop if (itemIdsFromRange.isEmpty()) break @@ -674,7 +665,7 @@ object ChatModel { } i-- } - chatItemsChangesListener?.read(if (range != null) markedReadIds else null, items) + chatItemsChangesListener?.read(if (itemIds != null) markedReadIds else null, items) } return markedRead } diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/SimpleXAPI.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/SimpleXAPI.kt index 0cab7ce8e9..a18dd0ac14 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/SimpleXAPI.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/SimpleXAPI.kt @@ -1599,8 +1599,8 @@ object ChatController { return null } - suspend fun apiChatRead(rh: Long?, type: ChatType, id: Long, range: CC.ItemRange): Boolean { - val r = sendCmd(rh, CC.ApiChatRead(type, id, range)) + suspend fun apiChatRead(rh: Long?, type: ChatType, id: Long): Boolean { + val r = sendCmd(rh, CC.ApiChatRead(type, id)) if (r is CR.CmdOk) return true Log.e(TAG, "apiChatRead bad response: ${r.responseType} ${r.details}") return false @@ -3172,7 +3172,7 @@ sealed class CC { class ApiGetNetworkStatuses(): CC() class ApiAcceptContact(val incognito: Boolean, val contactReqId: Long): CC() class ApiRejectContact(val contactReqId: Long): CC() - class ApiChatRead(val type: ChatType, val id: Long, val range: ItemRange): CC() + class ApiChatRead(val type: ChatType, val id: Long): CC() class ApiChatItemsRead(val type: ChatType, val id: Long, val itemIds: List): CC() class ApiChatUnread(val type: ChatType, val id: Long, val unreadChat: Boolean): CC() class ReceiveFile(val fileId: Long, val userApprovedRelays: Boolean, val encrypt: Boolean, val inline: Boolean?): CC() @@ -3338,7 +3338,7 @@ sealed class CC { is ApiEndCall -> "/_call end @${contact.apiId}" is ApiCallStatus -> "/_call status @${contact.apiId} ${callStatus.value}" is ApiGetNetworkStatuses -> "/_network_statuses" - is ApiChatRead -> "/_read chat ${chatRef(type, id)} from=${range.from} to=${range.to}" + is ApiChatRead -> "/_read chat ${chatRef(type, id)}" is ApiChatItemsRead -> "/_read chat items ${chatRef(type, id)} ${itemIds.joinToString(",")}" is ApiChatUnread -> "/_unread chat ${chatRef(type, id)} ${onOff(unreadChat)}" is ReceiveFile -> diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ChatView.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ChatView.kt index 875868103e..2a66daf2db 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ChatView.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ChatView.kt @@ -490,19 +490,35 @@ fun ChatView(staleChatId: State, onComposed: suspend (chatId: String) - }, addMembers = { groupInfo -> addGroupMembers(view = view, groupInfo = groupInfo, rhId = chatRh, close = { ModalManager.end.closeModals() }) }, openGroupLink = { groupInfo -> openGroupLink(view = view, groupInfo = groupInfo, rhId = chatRh, close = { ModalManager.end.closeModals() }) }, - markRead = { range, unreadCountAfter -> + markItemsRead = { itemsIds -> withBGApi { withChats { // It's important to call it on Main thread. Otherwise, composable crash occurs from time-to-time without useful stacktrace withContext(Dispatchers.Main) { - markChatItemsRead(chatRh, chatInfo, range, unreadCountAfter) + markChatItemsRead(chatRh, chatInfo, itemsIds) + } + ntfManager.cancelNotificationsForChat(chatInfo.id) + chatModel.controller.apiChatItemsRead( + chatRh, + chatInfo.chatType, + chatInfo.apiId, + itemsIds + ) + } + } + }, + markChatRead = { + withBGApi { + withChats { + // It's important to call it on Main thread. Otherwise, composable crash occurs from time-to-time without useful stacktrace + withContext(Dispatchers.Main) { + markChatItemsRead(chatRh, chatInfo) } ntfManager.cancelNotificationsForChat(chatInfo.id) chatModel.controller.apiChatRead( chatRh, chatInfo.chatType, - chatInfo.apiId, - range + chatInfo.apiId ) } } @@ -602,7 +618,8 @@ fun ChatLayout( showItemDetails: (ChatInfo, ChatItem) -> Unit, addMembers: (GroupInfo) -> Unit, openGroupLink: (GroupInfo) -> Unit, - markRead: (CC.ItemRange, unreadCountAfter: Int?) -> Unit, + markItemsRead: (List) -> Unit, + markChatRead: () -> Unit, changeNtfsState: (Boolean, currentValue: MutableState) -> Unit, onSearchValueChanged: (String) -> Unit, onComposed: suspend (chatId: String) -> Unit, @@ -649,7 +666,7 @@ fun ChatLayout( useLinkPreviews, linkMode, selectedChatItems, showMemberInfo, loadMessages, deleteMessage, deleteMessages, receiveFile, cancelFile, joinGroup, acceptCall, acceptFeature, openDirectChat, forwardItem, updateContactStats, updateMemberStats, syncContactConnection, syncMemberConnection, findModelChat, findModelMember, - setReaction, showItemDetails, markRead, remember { { onComposed(it) } }, developerTools, showViaProxy, + setReaction, showItemDetails, markItemsRead, markChatRead, remember { { onComposed(it) } }, developerTools, showViaProxy, ) } } @@ -937,7 +954,8 @@ fun BoxScope.ChatItemsList( findModelMember: (String) -> GroupMember?, setReaction: (ChatInfo, ChatItem, Boolean, MsgReaction) -> Unit, showItemDetails: (ChatInfo, ChatItem) -> Unit, - markRead: (CC.ItemRange, unreadCountAfter: Int?) -> Unit, + markItemsRead: (List) -> Unit, + markChatRead: () -> Unit, onComposed: suspend (chatId: String) -> Unit, developerTools: Boolean, showViaProxy: Boolean @@ -1284,15 +1302,15 @@ fun BoxScope.ChatItemsList( DateSeparator(last.meta.itemTs) } if (item.isRcvNew) { - val (itemIdStart, itemIdEnd) = when (merged) { - is MergedItem.Single -> merged.item.item.id to merged.item.item.id - is MergedItem.Grouped -> merged.items.last().item.id to merged.items.first().item.id + val itemIds = when (merged) { + is MergedItem.Single -> listOf(merged.item.item.id) + is MergedItem.Grouped -> merged.items.map { it.item.id } } - MarkItemsReadAfterDelay(keyForItem(item), itemIdStart, itemIdEnd, finishedInitialComposition, chatInfo.id, listState, markRead) + MarkItemsReadAfterDelay(keyForItem(item), itemIds, finishedInitialComposition, chatInfo.id, listState, markItemsRead) } } } - FloatingButtons(loadingMoreItems, mergedItems, unreadCount, maxHeight, composeViewHeight, remoteHostId, chatInfo, searchValue, markRead, listState) + FloatingButtons(loadingMoreItems, mergedItems, unreadCount, maxHeight, composeViewHeight, remoteHostId, chatInfo, searchValue, markChatRead, listState) FloatingDate(Modifier.padding(top = 10.dp + topPaddingToContent()).align(Alignment.TopCenter), mergedItems, listState) LaunchedEffect(Unit) { @@ -1385,7 +1403,7 @@ fun BoxScope.FloatingButtons( remoteHostId: Long?, chatInfo: ChatInfo, searchValue: State, - markRead: (CC.ItemRange, unreadCountAfter: Int?) -> Unit, + markChatRead: () -> Unit, listState: State ) { val scope = rememberCoroutineScope() @@ -1453,12 +1471,7 @@ fun BoxScope.FloatingButtons( generalGetString(MR.strings.mark_read), painterResource(MR.images.ic_check), onClick = { - val chat = chatModel.chats.value.firstOrNull { it.remoteHostId == remoteHostId && it.id == chatInfo.id } ?: return@ItemAction - val minUnreadItemId = chat.chatStats.minUnreadItemId - markRead( - CC.ItemRange(minUnreadItemId, chat.chatItems.lastOrNull()?.id ?: return@ItemAction), - 0 - ) + markChatRead() showDropDown.value = false }) } @@ -1779,12 +1792,11 @@ private fun DateSeparator(date: Instant) { @Composable private fun MarkItemsReadAfterDelay( itemKey: String, - itemIdStart: Long, - itemIdEnd: Long, + itemIds: List, finishedInitialComposition: State, chatId: ChatId, listState: State, - markRead: (CC.ItemRange, unreadCountAfter: Int?) -> Unit + markItemsRead: (List) -> Unit ) { // items can be "visible" in terms of LazyColumn but hidden behind compose view/appBar. So don't count such item as visible and not mark read val itemIsPartiallyAboveCompose = remember { derivedStateOf { @@ -1795,11 +1807,11 @@ private fun MarkItemsReadAfterDelay( false } } } - LaunchedEffect(itemIsPartiallyAboveCompose.value, itemIdStart, itemIdEnd, finishedInitialComposition.value, chatId) { + LaunchedEffect(itemIsPartiallyAboveCompose.value, itemIds, finishedInitialComposition.value, chatId) { if (chatId != ChatModel.chatId.value || !itemIsPartiallyAboveCompose.value || !finishedInitialComposition.value) return@LaunchedEffect delay(600L) - markRead(CC.ItemRange(itemIdStart, itemIdEnd), null) + markItemsRead(itemIds) } } @@ -2406,7 +2418,8 @@ fun PreviewChatLayout() { showItemDetails = { _, _ -> }, addMembers = { _ -> }, openGroupLink = {}, - markRead = { _, _ -> }, + markItemsRead = { _ -> }, + markChatRead = {}, changeNtfsState = { _, _ -> }, onSearchValueChanged = {}, onComposed = {}, @@ -2478,7 +2491,8 @@ fun PreviewGroupChatLayout() { showItemDetails = { _, _ -> }, addMembers = { _ -> }, openGroupLink = {}, - markRead = { _, _ -> }, + markItemsRead = { _ -> }, + markChatRead = {}, changeNtfsState = { _, _ -> }, onSearchValueChanged = {}, onComposed = {}, diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chatlist/ChatListNavLinkView.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chatlist/ChatListNavLinkView.kt index d071a9d4fd..226030fcd4 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chatlist/ChatListNavLinkView.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chatlist/ChatListNavLinkView.kt @@ -537,15 +537,13 @@ fun markChatRead(c: Chat, chatModel: ChatModel) { var chat = c withApi { if (chat.chatStats.unreadCount > 0) { - val minUnreadItemId = chat.chatStats.minUnreadItemId withChats { markChatItemsRead(chat.remoteHostId, chat.chatInfo) } chatModel.controller.apiChatRead( chat.remoteHostId, chat.chatInfo.chatType, - chat.chatInfo.apiId, - CC.ItemRange(minUnreadItemId, chat.chatItems.last().id) + chat.chatInfo.apiId ) chat = chatModel.getChat(chat.id) ?: return@withApi }