status-desktop
status-desktop copied to clipboard
[Epic] General approach for displaying contact details
The Problem
In general, we display model-based contact details using the standard QAbstractItemModel mechanisms. However, in a few In a few places we need to display contact details based on a specific contactId. Here are just a few of them:
- Profile
- Contact Verification
- TransactionAddress
- Activity Center (and there are few more connections for different Actypes and this is done deliberately)
- Contacts for 1-to-1 chats in RootStore
We use the Utils.getContactDetailsAsJson function to retrieve the data, but we also need to know about updates to this contact's and for this we subscribe to the signal from the model in contactsStore. More precisely, to 3 different models because contacts are stored in myContactsModel, receivedContactRequestsModel, sentContactRequestsModel. And we don't receive signals about users who are not in contacts, and we won't see until restart that the user has changed profile pic or alias.
Another problem is that requesting contact data by id is synchronous and can lead to frezzes on UI
TODO:
- [x] #14962
- [x] #14963
- [ ] #14964
- [ ] #14965
- [ ] #14966
- [ ] #14967
I'd like to mention another related problem.
Intro details
When working on https://github.com/status-im/status-desktop/issues/11854 I discovered this property:
https://github.com/status-im/status-desktop/blob/123ed272c20a3c1d49f29c69c6a508fe319dedee/ui/app/AppLayouts/Chat/stores/RootStore.qml#L732
which is updated with many Connections/Binding components below, e.g. this one:
https://github.com/status-im/status-desktop/blob/10a7b385d548a725ff7cddba29a5e775b9b41c09/ui/app/AppLayouts/Chat/stores/RootStore.qml#L785-L790
The problem
I found out that the when condition doesn't seem to work correctly here, we make a lot of getContactDetailsAsJson calls with _d.activeChatType === 6, which is a community channel.
This leads to this an error in logs:
DBG 2023-08-24 19:19:02.087+03:00 id doesn't have expected length topics="contacts-service" tid=34034906 file=service.nim:400 pubkey=0x039fe1dbdd83e6c903f00da15a36ba5279699090aac77f87859bf6da2705f149ee36639205-3654-4bd3-aaad-2ddc4522fc1e parsedChars=76 expectedLength=132
And what?
I don't think that this call is a big deal, but I'm pretty sure that this is only the top of an iceberg. There might be other problems related to this behaviour.
Creating a general approach should fix this as well.
cc @alexjba @MishkaRogachev
I found out that the when condition doesn't seem to work correctly here, we make a lot of getContactDetailsAsJson calls with _d.activeChatType === 6, which is a community channel.
@igor-sirotin Basically, the when probably works correctly. when evaluated to false prevents from assigning value to the target, but not from evaluating value binding itself. Those are two different things.
To prevent unwanted calls we would have to use sth like that:
Binding on oneToOneChatContact {
when: _d.activeChatId && _d.activeChatType === Constants.chatType.oneToOne
value: {
if ( _d.activeChatId && _d.activeChatType === Constants.chatType.oneToOne )
return Utils.getContactDetailsAsJson(_d.activeChatId, false)
else
return {}
}
restoreMode: Binding.RestoreBindingOrValue
}
cc @noeliaSD
Here's my plan for this task:
-
Create a nim model that proxies the service cache. This model will be exposed to qml and could replace the current models (MyMutualContacts, BlockedContacts, IncomingPendingContactRequests, OutgoingPendingContactRequests) in a follow-up task. These models can be replaced with a SFPM where they're needed in the UI. The model will proxy the service cache in as similar way as implemented here https://github.com/status-im/status-desktop/blob/master/src/app/modules/main/wallet_section/assets/balances_model.nim
-
Create a new generic c++ component
SingleModelItemDatathat can provide a live object from a model. This component will receive the keyroleNameand the value and will provide amodelItemvariable that will be a live QObject holding all the model roleNames as properties. I've already started working on this component and it's almost complete. -
Create a qml component
ContactDetailsthat will use the new model and theSingleModelItemData. Its responsibility is to provide default values and handle other cases (requesting contacts whenever the contact is not in the model, requesting extra data if needed)
This is a usage example:
ContactDetails.qml
required property var contactsModel
required property string publicKey
[other optional input args if needed]
readonly property string name: data.modelItem.available ? data.modelItem.name : ""
readonly property OnlineStatus onlineStatus: data.modelItem.available ? data.modelItem.onlineStatus : OnlineStatus.Available
[other readonly properties]
SingleModelItemData {
id: data
sourceModel: root.contactsModel
key: "pubKey"
value: root. publicKey
}
[other optional logic like request extra data if needed]
@jrainville @osmaczko @MishkaRogachev @igor-sirotin I'd appreciate some thoughts on this. Do you see any major blockers?
I'd appreciate some thoughts on this. Do you see any major blockers?
@alexjba I can mostly talk for the Nim part, but it should be pretty straightforward.
Do note that you can reuse user_model as is and just create a new generic model in contacts/view that will contain . That way, you can keep the old models in the meantime and do the refactor step by step.
I like the approach and can't think of any blockers off the top of my head. The only thing I would be careful about is the case when the contact is removed from the underlying model at the moment we interact with it.
I have some ideas on this, will share next week when I'm back from vacation.
The only thing I would be careful about is the case when the contact is removed from the underlying model at the moment we interact with it.
It's a good point! I'll provide a config option in the SingleModelItemData to allow a model item snapshot on removal and add a flag to the item saying that it was removed from the model. We can cache model items containing basic types, models and property-value pairs from QObjects. I think it will cover 99% of use cases.
@alexjba your idea described above is more or less the same thing I wanted to propose 👍
Just one thing to note.
It would be nice for ContactDetails to support fetching contacts from store nodes, e.g. to be used in SendContactRequestModal, which does that itself now:
https://github.com/status-im/status-desktop/blob/16926511841da84f72c1333c6350aef43a1bc1b7/ui/imports/shared/popups/SendContactRequestModal.qml#L29-L33
This includes API:
- a method to start store node request
loadingproperty- some
errorproperty to see if the contact wasn't found, or any other fetch error
This logic is currently kept in SendContactRequestModal (and I believe in some other places), but we should extract and reuse it.
This task is promoted to Epic and split into smaller chunks.
@noeliaSD We'll need to re-plan this. Based on current priorities, we can't deliver it in 2.30
Closed by mistake
We can take some subtasks again on the next stabilization phase! Changed the target to 2.31. However, probably the complete epic cannot be done in just 1 stabilization phase but just some subtasks.