Skip to content

Commit

Permalink
refactor(contacts): improve updates by not removing and re-adding
Browse files Browse the repository at this point in the history
Fixes #16549

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.
  • Loading branch information
jrainville committed Oct 30, 2024
1 parent 3b840cb commit ddc2efe
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 23 deletions.
49 changes: 29 additions & 20 deletions src/app/modules/main/profile_section/contacts/module.nim
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -163,35 +162,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:
Expand Down Expand Up @@ -224,7 +233,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 =
Expand Down
70 changes: 68 additions & 2 deletions src/app/modules/shared_models/user_model.nim
Original file line number Diff line number Diff line change
Expand Up @@ -197,11 +197,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()
Expand Down Expand Up @@ -294,6 +294,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:
Expand All @@ -313,6 +325,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)
Expand All @@ -321,6 +345,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

Expand All @@ -329,6 +357,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:
Expand Down
2 changes: 1 addition & 1 deletion ui/app/AppLayouts/Profile/panels/ContactsListPanel.qml
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ Item {
ensVerified: model.isEnsVerified
publicKey: model.pubKey
compressedPk: Utils.getCompressedPk(model.pubKey)
iconSource: model.icon
iconSource: model.thumbnailImage
isContact: model.isContact
isBlocked: model.isBlocked
isVerified: model.isVerified
Expand Down

0 comments on commit ddc2efe

Please sign in to comment.