From 2ca7fef0bd252d3fc629ccbc731c531d506327f1 Mon Sep 17 00:00:00 2001 From: Jonathan Rainville Date: Wed, 30 Oct 2024 14:20:59 -0400 Subject: [PATCH] refactor(contacts): improve updates by not removing and re-adding We used to update contact items by removing them from the models and re-adding them. This is highly inefficient. Instead, the proper way is to update only the values that changed. --- .../main/profile_section/contacts/module.nim | 49 +++++++------ src/app/modules/shared_models/user_model.nim | 70 ++++++++++++++++++- .../Profile/panels/ContactsListPanel.qml | 2 +- 3 files changed, 98 insertions(+), 23 deletions(-) diff --git a/src/app/modules/main/profile_section/contacts/module.nim b/src/app/modules/main/profile_section/contacts/module.nim index 3db72b7ab7..be003db88e 100644 --- a/src/app/modules/main/profile_section/contacts/module.nim +++ b/src/app/modules/main/profile_section/contacts/module.nim @@ -4,7 +4,6 @@ import io_interface, view, controller, json import ../../../shared_models/user_item import ../../../shared_models/user_model import ../io_interface as delegate_interface -import ../../../../global/global_singleton import ../../../../core/eventemitter import app_service/common/types @@ -158,35 +157,45 @@ method removeContact*(self: Module, publicKey: string) = method changeContactNickname*(self: Module, publicKey: string, nickname: string) = self.controller.changeContactNickname(publicKey, nickname) -# TODO rename this function -proc addItemToAppropriateModel(self: Module, item: UserItem) = - if singletonInstance.userProfile.getPubKey() == item.pubKey: - return - - self.view.contactsModel().addItem(item) - -proc removeItemWithPubKeyFromAllModels(self: Module, publicKey: string) = - self.view.contactsModel().removeItemById(publicKey) - -proc removeIfExistsAndAddToAppropriateModel(self: Module, publicKey: string) = - self.removeItemWithPubKeyFromAllModels(publicKey) +proc updateContactItem(self: Module, publicKey: string) = let item = self.createItemFromPublicKey(publicKey) - self.addItemToAppropriateModel(item) + self.view.contactsModel().updateItem( + publicKey, + item.displayName, + item.ensName, + item.isEnsVerified, + item.localNickname, + item.alias, + item.icon, + item.trustStatus, + item.onlineStatus, + item.isContact, + item.isBlocked, + item.contactRequest, + item.lastUpdated, + item.lastUpdatedLocally, + item.bio, + item.thumbnailImage, + item.largeImage, + item.isContactRequestReceived, + item.isContactRequestSent, + item.isRemoved, + ) method contactAdded*(self: Module, publicKey: string) = - self.removeIfExistsAndAddToAppropriateModel(publicKey) + self.view.contactsModel().addItem(self.createItemFromPublicKey(publicKey)) method contactBlocked*(self: Module, publicKey: string) = - self.removeIfExistsAndAddToAppropriateModel(publicKey) + self.updateContactItem(publicKey) method contactUnblocked*(self: Module, publicKey: string) = - self.removeIfExistsAndAddToAppropriateModel(publicKey) + self.updateContactItem(publicKey) method contactRemoved*(self: Module, publicKey: string) = - self.removeIfExistsAndAddToAppropriateModel(publicKey) + self.updateContactItem(publicKey) method contactUpdated*(self: Module, publicKey: string) = - self.removeIfExistsAndAddToAppropriateModel(publicKey) + self.updateContactItem(publicKey) method contactsStatusUpdated*(self: Module, statusUpdates: seq[StatusUpdateDto]) = for s in statusUpdates: @@ -219,7 +228,7 @@ method requestContactInfo*(self: Module, publicKey: string) = method onContactInfoRequestFinished*(self: Module, publicKey: string, ok: bool) = if ok: - self.removeIfExistsAndAddToAppropriateModel(publicKey) + self.updateContactItem(publicKey) self.view.onContactInfoRequestFinished(publicKey, ok) method shareUserUrlWithData*(self: Module, pubkey: string): string = diff --git a/src/app/modules/shared_models/user_model.nim b/src/app/modules/shared_models/user_model.nim index e59b039988..f2cfb19a64 100644 --- a/src/app/modules/shared_models/user_model.nim +++ b/src/app/modules/shared_models/user_model.nim @@ -202,11 +202,11 @@ QtObject: # if we add an item with offline status we add it as the first offline item (after the last online item) var position = -1 for i in 0 ..< self.items.len: - if(self.items[i].onlineStatus == OnlineStatus.Inactive): + if self.items[i].onlineStatus == OnlineStatus.Inactive: position = i break - if(position == -1): + if position == -1: position = self.items.len let parentModelIndex = newQModelIndex() @@ -299,6 +299,18 @@ QtObject: alias: string, icon: string, trustStatus: TrustStatus, + onlineStatus: OnlineStatus, + isContact: bool, + isBlocked: bool, + contactRequest: ContactRequest, + lastUpdated: int64, + lastUpdatedLocally: int64, + bio: string, + thumbnailImage: string, + largeImage: string, + isContactRequestReceived: bool, + isContactRequestSent: bool, + isRemoved: bool, ) = let ind = self.findIndexByPubKey(pubKey) if ind == -1: @@ -318,6 +330,18 @@ QtObject: updateRole(alias, Alias) updateRole(icon, Icon) updateRole(trustStatus, TrustStatus) + updateRole(onlineStatus, OnlineStatus) + updateRole(isContact, IsContact) + updateRole(isBlocked, IsBlocked) + updateRole(contactRequest, ContactRequest) + updateRole(lastUpdated, LastUpdated) + updateRole(lastUpdatedLocally, LastUpdatedLocally) + updateRole(bio, Bio) + updateRole(thumbnailImage, ThumbnailImage) + updateRole(largeImage, LargeImage) + updateRole(isContactRequestReceived, IsContactRequestReceived) + updateRole(isContactRequestSent, IsContactRequestSent) + updateRole(isRemoved, IsRemoved) if preferredDisplayNameChanged: roles.add(ModelRole.PreferredDisplayName.int) @@ -326,6 +350,10 @@ QtObject: roles.add(ModelRole.IsUntrustworthy.int) roles.add(ModelRole.IsVerified.int) + # The image is actually a URL that doesn't change. We need to force refresh it just in case + roles.add(ModelRole.ThumbnailImage.int) + roles.add(ModelRole.LargeImage.int) + if roles.len == 0: return @@ -334,6 +362,44 @@ QtObject: self.dataChanged(index, index, roles) self.itemChanged(pubKey) + proc updateItem*( + self: Model, + pubKey: string, + displayName: string, + ensName: string, + isEnsVerified: bool, + localNickname: string, + alias: string, + icon: string, + trustStatus: TrustStatus, + ) = + let ind = self.findIndexByPubKey(pubKey) + if ind == -1: + return + let item = self.items[ind] + self.updateItem( + pubKey, + displayName, + ensName, + isEnsVerified, + localNickname, + alias, + icon, + trustStatus, + item.onlineStatus, + item.isContact, + item.isBlocked, + item.contactRequest, + item.lastUpdated, + item.lastUpdatedLocally, + item.bio, + item.thumbnailImage, + item.largeImage, + item.isContactRequestReceived, + item.isContactRequestSent, + item.isRemoved, + ) + proc updateTrustStatus*(self: Model, pubKey: string, trustStatus: TrustStatus) = let ind = self.findIndexByPubKey(pubKey) if ind == -1: diff --git a/ui/app/AppLayouts/Profile/panels/ContactsListPanel.qml b/ui/app/AppLayouts/Profile/panels/ContactsListPanel.qml index ee05437b7c..0972906855 100644 --- a/ui/app/AppLayouts/Profile/panels/ContactsListPanel.qml +++ b/ui/app/AppLayouts/Profile/panels/ContactsListPanel.qml @@ -103,7 +103,7 @@ Item { width: ListView.view.width name: model.preferredDisplayName - iconSource: model.icon + iconSource: model.thumbnailImage subTitle: model.ensVerified ? "" : Utils.getElidedCompressedPk(model.pubKey) pubKeyColor: Utils.colorForPubkey(model.pubKey)