From db6af0554ac4d4105369502619361ef9f46dcd88 Mon Sep 17 00:00:00 2001 From: Jonathan Rainville Date: Fri, 21 Apr 2023 15:47:04 -0400 Subject: [PATCH] fix(chat-model): use setData to set the chat model Fixes #10384 The problem was that doing the sort inside the insertRows messed up the model. I removed it and it fixed the issues. However, it created an other issue with ordering. The problem this time is that when populating the model at the start, we do not get the chats in order, so calculating the right position to insert a chat is difficult. Instead, I use a setData function to just put all the chats in the items list, sort it and call it done, using the resetModel function. I also did some clean ups. --- .../modules/main/chat_section/controller.nim | 12 ++--- .../main/chat_section/io_interface.nim | 7 ++- src/app/modules/main/chat_section/model.nim | 14 ++++- src/app/modules/main/chat_section/module.nim | 51 ++++++++++++------- .../tst_communityFlows/test.feature | 2 +- .../Chat/views/ChatMessagesView.qml | 2 +- 6 files changed, 58 insertions(+), 30 deletions(-) diff --git a/src/app/modules/main/chat_section/controller.nim b/src/app/modules/main/chat_section/controller.nim index 4fb6ba2a14..8a2c5fc9fc 100644 --- a/src/app/modules/main/chat_section/controller.nim +++ b/src/app/modules/main/chat_section/controller.nim @@ -178,14 +178,14 @@ proc init*(self: Controller) = var args = ChatUpdateArgs(e) for chat in args.chats: let belongsToCommunity = chat.communityId.len > 0 - self.delegate.addOrUpdateChat(chat, belongsToCommunity, self.events, self.settingsService, self.nodeConfigurationService, + discard self.delegate.addOrUpdateChat(chat, belongsToCommunity, self.events, self.settingsService, self.nodeConfigurationService, self.contactService, self.chatService, self.communityService, self.messageService, self.gifService, self.mailserversService, setChatAsActive = false) self.events.on(SIGNAL_CHAT_CREATED) do(e: Args): var args = CreatedChatArgs(e) let belongsToCommunity = args.chat.communityId.len > 0 - self.delegate.addOrUpdateChat(args.chat, belongsToCommunity, self.events, self.settingsService, self.nodeConfigurationService, + discard self.delegate.addOrUpdateChat(args.chat, belongsToCommunity, self.events, self.settingsService, self.nodeConfigurationService, self.contactService, self.chatService, self.communityService, self.messageService, self.gifService, self.mailserversService, setChatAsActive = true) @@ -200,7 +200,7 @@ proc init*(self: Controller) = self.events.on(SIGNAL_COMMUNITY_CHANNEL_CREATED) do(e:Args): let args = CommunityChatArgs(e) let belongsToCommunity = args.chat.communityId.len > 0 - self.delegate.addOrUpdateChat(args.chat, belongsToCommunity, self.events, self.settingsService, self.nodeConfigurationService, + discard self.delegate.addOrUpdateChat(args.chat, belongsToCommunity, self.events, self.settingsService, self.nodeConfigurationService, self.contactService, self.chatService, self.communityService, self.messageService, self.gifService, self.mailserversService, setChatAsActive = true) @@ -437,7 +437,7 @@ proc getOneToOneChatNameAndImage*(self: Controller, chatId: string): proc createOneToOneChat*(self: Controller, communityID: string, chatId: string, ensName: string) = let response = self.chatService.createOneToOneChat(communityID, chatId, ensName) if(response.success): - self.delegate.addOrUpdateChat(response.chatDto, false, self.events, self.settingsService, self.nodeConfigurationService, + discard self.delegate.addOrUpdateChat(response.chatDto, false, self.events, self.settingsService, self.nodeConfigurationService, self.contactService, self.chatService, self.communityService, self.messageService, self.gifService, self.mailserversService) @@ -507,14 +507,14 @@ proc makeAdmin*(self: Controller, communityID: string, chatId: string, pubKey: s proc createGroupChat*(self: Controller, communityID: string, groupName: string, pubKeys: seq[string]) = let response = self.chatService.createGroupChat(communityID, groupName, pubKeys) if(response.success): - self.delegate.addOrUpdateChat(response.chatDto, false, self.events, self.settingsService, self.nodeConfigurationService, + discard self.delegate.addOrUpdateChat(response.chatDto, false, self.events, self.settingsService, self.nodeConfigurationService, self.contactService, self.chatService, self.communityService, self.messageService, self.gifService, self.mailserversService) proc joinGroupChatFromInvitation*(self: Controller, groupName: string, chatId: string, adminPK: string) = let response = self.chatService.createGroupChatFromInvitation(groupName, chatId, adminPK) if(response.success): - self.delegate.addOrUpdateChat(response.chatDto, false, self.events, self.settingsService, self.nodeConfigurationService, + discard self.delegate.addOrUpdateChat(response.chatDto, false, self.events, self.settingsService, self.nodeConfigurationService, self.contactService, self.chatService, self.communityService, self.messageService, self.gifService, self.mailserversService) diff --git a/src/app/modules/main/chat_section/io_interface.nim b/src/app/modules/main/chat_section/io_interface.nim index 9ea75c0909..1e9be98526 100644 --- a/src/app/modules/main/chat_section/io_interface.nim +++ b/src/app/modules/main/chat_section/io_interface.nim @@ -11,6 +11,7 @@ import ../../../../app_service/service/mailservers/service as mailservers_servic import ../../../../app_service/service/token/service as token_service import model as chats_model +import item as chat_item import ../../../core/eventemitter import ../../../core/unique_event_emitter @@ -77,7 +78,7 @@ method addNewChat*(self: AccessInterface, chatDto: ChatDto, belongsToCommunity: settingsService: settings_service.Service, contactService: contact_service.Service, chatService: chat_service.Service, communityService: community_service.Service, messageService: message_service.Service, gifService: gif_service.Service, - mailserversService: mailservers_service.Service, setChatAsActive: bool = true) {.base.} = + mailserversService: mailservers_service.Service, setChatAsActive: bool = true, insertIntoModel: bool = true): Item {.base.} = raise newException(ValueError, "No implementation available") method doesCatOrChatExist*(self: AccessInterface, chatId: string): bool {.base.} = @@ -98,7 +99,9 @@ method addOrUpdateChat*(self: AccessInterface, messageService: message_service.Service, gifService: gif_service.Service, mailserversService: mailservers_service.Service, - setChatAsActive: bool = true) {.base.} = + setChatAsActive: bool = true, + insertIntoModel: bool = true + ): Item {.base.} = raise newException(ValueError, "No implementation available") method onNewMessagesReceived*(self: AccessInterface, sectionIdMsgBelongsTo: string, chatIdMsgBelongsTo: string, diff --git a/src/app/modules/main/chat_section/model.nim b/src/app/modules/main/chat_section/model.nim index d74953d9a0..e48e6984cd 100644 --- a/src/app/modules/main/chat_section/model.nim +++ b/src/app/modules/main/chat_section/model.nim @@ -190,13 +190,23 @@ QtObject: if result == 0: result = cmp(x.position, y.position) + proc setData*(self: Model, items: seq[Item]) = + self.beginResetModel() + self.items = items + self.items.sort(cmpChatsAndCats) + self.endResetModel() + + self.countChanged() + # IMPORTANT: if you call this function for a chat with a category, make sure the category is appended first proc appendItem*(self: Model, item: Item, ignoreCategory: bool = false) = let parentModelIndex = newQModelIndex() defer: parentModelIndex.delete var indexToInsertTo = item.position - if item.categoryId != "" and not item.isCategory: + if item.isCategory: + indexToInsertTo = item.categoryPosition + elif item.categoryId != "": if ignoreCategory: # We don't care about the category position, just position it at the end indexToInsertTo = self.items.len @@ -209,9 +219,9 @@ QtObject: indexToInsertTo = 0 elif indexToInsertTo >= self.items.len + 1: indexToInsertTo = self.items.len + self.beginInsertRows(parentModelIndex, indexToInsertTo, indexToInsertTo) self.items.insert(item, indexToInsertTo) - self.items.sort(cmpChatsAndCats) self.endInsertRows() self.countChanged() diff --git a/src/app/modules/main/chat_section/module.nim b/src/app/modules/main/chat_section/module.nim index 83384bffb2..219b24b1dc 100644 --- a/src/app/modules/main/chat_section/module.nim +++ b/src/app/modules/main/chat_section/module.nim @@ -81,7 +81,9 @@ proc addOrUpdateChat(self: Module, messageService: message_service.Service, gifService: gif_service.Service, mailserversService: mailservers_service.Service, - setChatAsActive: bool = true) + setChatAsActive: bool = true, + insertIntoModel: bool = true, + ): Item proc buildTokenPermissionItem*(self: Module, tokenPermission: CommunityTokenPermissionDto): TokenPermissionItem @@ -163,9 +165,9 @@ proc removeSubmodule(self: Module, chatId: string) = self.chatContentModules.del(chatId) -proc addCategoryItem(self: Module, category: Category, amIAdmin: bool, communityId: string) = +proc addCategoryItem(self: Module, category: Category, amIAdmin: bool, communityId: string, insertIntoModel: bool = true): Item = let hasUnreadMessages = self.controller.chatsWithCategoryHaveUnreadMessages(communityId, category.id) - let categoryItem = chat_item.initItem( + result = chat_item.initItem( id = category.id, category.name, icon = "", @@ -184,7 +186,8 @@ proc addCategoryItem(self: Module, category: Category, amIAdmin: bool, community category.id, category.position, ) - self.view.chatsModel().appendItem(categoryItem) + if insertIntoModel: + self.view.chatsModel().appendItem(result) proc buildChatSectionUI( self: Module, @@ -201,9 +204,10 @@ proc buildChatSectionUI( var selectedItemId = "" let sectionLastOpenChat = singletonInstance.localAccountSensitiveSettings.getSectionLastOpenChat(self.controller.getMySectionId()) + var items: seq[Item] = @[] for categoryDto in channelGroup.categories: # Add items for the categories. We use a special type to identify categories - self.addCategoryItem(categoryDto, channelGroup.admin, channelGroup.id) + items.add(self.addCategoryItem(categoryDto, channelGroup.admin, channelGroup.id)) for chatDto in channelGroup.chats: var categoryPosition = -1 @@ -222,7 +226,7 @@ proc buildChatSectionUI( categoryPosition = category.position break - self.addOrUpdateChat( + items.add(self.addOrUpdateChat( chatDto, channelGroup, belongsToCommunity = chatDto.communityId.len > 0, @@ -236,8 +240,10 @@ proc buildChatSectionUI( gifService, mailserversService, setChatAsActive = false, - ) + insertIntoModel = false + )) + self.view.chatsModel.setData(items) self.setActiveItem(selectedItemId) proc createItemFromPublicKey(self: Module, publicKey: string): UserItem = @@ -535,7 +541,9 @@ method addNewChat*( messageService: message_service.Service, gifService: gif_service.Service, mailserversService: mailservers_service.Service, - setChatAsActive: bool = true) = + setChatAsActive: bool = true, + insertIntoModel: bool = true, + ): Item = let hasNotification = not chatDto.muted and (chatDto.unviewedMessagesCount > 0 or chatDto.unviewedMentionsCount > 0) let notificationsCount = chatDto.unviewedMentionsCount @@ -578,14 +586,14 @@ method addNewChat*( else: categoryPosition = category.position - let chatItem = chat_item.initItem( + result = chat_item.initItem( chatDto.id, chatName, chatImage, chatDto.color, chatDto.emoji, chatDto.description, - chatDto.chatType.int, + ChatType(chatDto.chatType).int, amIChatAdmin, chatDto.timestamp.int, hasNotification, @@ -616,10 +624,11 @@ method addNewChat*( gifService, mailserversService, ) - self.chatContentModules[chatDto.id].load(chatItem) - self.view.chatsModel().appendItem(chatItem) + self.chatContentModules[chatDto.id].load(result) + if insertIntoModel: + self.view.chatsModel().appendItem(result) if setChatAsActive: - self.setActiveItem(chatItem.id) + self.setActiveItem(result.id) method switchToChannel*(self: Module, channelName: string) = if(not self.controller.isCommunity()): @@ -660,7 +669,7 @@ method onCommunityCategoryCreated*(self: Module, cat: Category, chats: seq[ChatD return # TODO get admin status - self.addCategoryItem(cat, false, communityId) + discard self.addCategoryItem(cat, false, communityId) # Update chat items that now belong to that category self.view.chatsModel().updateItemsWithCategoryDetailsById( chats, @@ -1130,7 +1139,9 @@ proc addOrUpdateChat(self: Module, messageService: message_service.Service, gifService: gif_service.Service, mailserversService: mailservers_service.Service, - setChatAsActive: bool = true) = + setChatAsActive: bool = true, + insertIntoModel: bool = true, + ): Item = let sectionId = self.controller.getMySectionId() if(belongsToCommunity and sectionId != chat.communityId or @@ -1156,7 +1167,7 @@ proc addOrUpdateChat(self: Module, self.onChatRenamed(chat.id, chat.name) return - self.addNewChat( + result = self.addNewChat( chat, channelGroup, belongsToCommunity, @@ -1170,6 +1181,7 @@ proc addOrUpdateChat(self: Module, gifService, mailserversService, setChatAsActive, + insertIntoModel, ) method addOrUpdateChat*(self: Module, @@ -1184,8 +1196,10 @@ method addOrUpdateChat*(self: Module, messageService: message_service.Service, gifService: gif_service.Service, mailserversService: mailservers_service.Service, - setChatAsActive: bool = true) = - self.addOrUpdateChat( + setChatAsActive: bool = true, + insertIntoModel: bool = true, + ): Item = + result = self.addOrUpdateChat( chat, ChannelGroupDto(), belongsToCommunity, @@ -1199,6 +1213,7 @@ method addOrUpdateChat*(self: Module, gifService, mailserversService, setChatAsActive, + insertIntoModel, ) method downloadMessages*(self: Module, chatId: string, filePath: string) = diff --git a/test/ui-test/testSuites/suite_communities/tst_communityFlows/test.feature b/test/ui-test/testSuites/suite_communities/tst_communityFlows/test.feature index 0ae27b1a36..c41aa892f3 100644 --- a/test/ui-test/testSuites/suite_communities/tst_communityFlows/test.feature +++ b/test/ui-test/testSuites/suite_communities/tst_communityFlows/test.feature @@ -33,7 +33,7 @@ Feature: Status Desktop community Examples: | community_channel_name | community_channel_description | method | | test-channel | Community channel description tested 1 | bottom_menu | - # | test-channel2 | Community channel description tested 2 | right_click_menu | + | test-channel2 | Community channel description tested 2 | right_click_menu | Scenario Outline: The admin edits a community channel Given the admin creates a community channel named "", with description "", with the method "bottom_menu" diff --git a/ui/app/AppLayouts/Chat/views/ChatMessagesView.qml b/ui/app/AppLayouts/Chat/views/ChatMessagesView.qml index b959dc7a0f..87ac3704a6 100644 --- a/ui/app/AppLayouts/Chat/views/ChatMessagesView.qml +++ b/ui/app/AppLayouts/Chat/views/ChatMessagesView.qml @@ -49,7 +49,7 @@ Item { readonly property real scrollY: chatLogView.visibleArea.yPosition * chatLogView.contentHeight readonly property bool isMostRecentMessageInViewport: chatLogView.visibleArea.yPosition >= 0.999 - chatLogView.visibleArea.heightRatio - readonly property var chatDetails: chatContentModule.chatDetails || null + readonly property var chatDetails: chatContentModule && chatContentModule.chatDetails || null readonly property var loadMoreMessagesIfScrollBelowThreshold: Backpressure.oneInTimeQueued(root, 100, function() { if(scrollY < 1000) messageStore.loadMoreMessages()