From 4b3e5c6e36f41c382314d459cee6e762c7a5d43e Mon Sep 17 00:00:00 2001 From: Sale Djenic Date: Thu, 18 Nov 2021 15:41:28 +0100 Subject: [PATCH] refactor(contacts-service): general improvement - Signal's arguments updated - Sent payload optimized - Local nickname added to profile section contacts model - Rest updated accordingly to above changes --- .../profile_section/contacts/controller.nim | 62 +++++-------- .../contacts/controller_interface.nim | 2 +- .../profile_section/contacts/io_interface.nim | 25 ++--- .../main/profile_section/contacts/model.nim | 6 ++ .../contacts/models/contact_list.nim | 21 ++++- .../main/profile_section/contacts/module.nim | 31 ++++--- .../main/profile_section/contacts/view.nim | 19 +--- src/app_service/service/contacts/service.nim | 91 +++++++------------ 8 files changed, 117 insertions(+), 140 deletions(-) diff --git a/src/app/modules/main/profile_section/contacts/controller.nim b/src/app/modules/main/profile_section/contacts/controller.nim index fd533b2381..02c27f67c7 100644 --- a/src/app/modules/main/profile_section/contacts/controller.nim +++ b/src/app/modules/main/profile_section/contacts/controller.nim @@ -17,9 +17,6 @@ type contactsService: contacts_service.Service accountsService: accounts_service.ServiceInterface -# forward declaration: -method getContacts*[T](self: Controller[T], useCache: bool = true): seq[ContactsDto] - proc newController*[T](delegate: io_interface.AccessInterface, events: EventEmitter, contactsService: contacts_service.Service, @@ -34,45 +31,32 @@ method delete*[T](self: Controller[T]) = discard method init*[T](self: Controller[T]) = - self.events.on(SignalType.Message.event) do(e: Args): - let msgData = MessageSignal(e); - if msgData.contacts.len > 0: - let contacts = self.getContacts(false) - self.delegate.updateContactList(contacts) + self.events.on(SIGNAL_CONTACT_LOOKED_UP) do(e: Args): + var args = ContactArgs(e) + self.delegate.contactLookedUp(args.contactId) + self.events.on(SIGNAL_CONTACT_ADDED) do(e: Args): - var evArgs = ContactArgs(e) - self.delegate.contactAdded(evArgs.contact) + var args = ContactAddedArgs(e) + self.delegate.contactAdded(args.contact) self.events.on(SIGNAL_CONTACT_BLOCKED) do(e: Args): - var evArgs = ContactArgs(e) - self.delegate.contactBlocked(evArgs.contact) + var args = ContactArgs(e) + self.delegate.contactBlocked(args.contactId) self.events.on(SIGNAL_CONTACT_UNBLOCKED) do(e: Args): - var evArgs = ContactArgs(e) - self.delegate.contactUnblocked(evArgs.contact) + var args = ContactArgs(e) + self.delegate.contactUnblocked(args.contactId) self.events.on(SIGNAL_CONTACT_REMOVED) do(e: Args): - var evArgs = ContactArgs(e) - self.delegate.contactRemoved(evArgs.contact) + var args = ContactArgs(e) + self.delegate.contactRemoved(args.contactId) - self.events.on(SIGNAL_CONTACT_LOOKED_UP) do(e: Args): - let args = LookupResolvedArgs(e) - self.delegate.contactLookedUp(args.id) + self.events.on(SIGNAL_CONTACT_NICKNAME_CHANGED) do(e: Args): + var args = ContactNicknameUpdatedArgs(e) + self.delegate.contactNicknameChanged(args.contactId, args.nickname) - self.events.on(SIGNAL_CONTACT_UPDATED) do(e: Args): - # I left this as part it was. - let contacts = self.getContacts() - self.delegate.setContactList(contacts) - - # Since we have the exact contact which has been updated, then we need to improve the way of updating the view - # and instead setting the whole list for every change we should update only the appropriate item in the view. - # Example: - # let args = ContactUpdatedArgs(e) - # let contactDto = self.contactsService.getContactById(args.id) - # self.delegate.onContactUpdated(contactDto) - -method getContacts*[T](self: Controller[T], useCache: bool = true): seq[ContactsDto] = - return self.contactsService.getContacts(useCache) +method getContacts*[T](self: Controller[T]): seq[ContactsDto] = + return self.contactsService.getContacts() method getContact*[T](self: Controller[T], id: string): ContactsDto = return self.contactsService.getContactById(id) @@ -80,23 +64,23 @@ method getContact*[T](self: Controller[T], id: string): ContactsDto = method generateAlias*[T](self: Controller[T], publicKey: string): string = return self.accountsService.generateAlias(publicKey) -method addContact*[T](self: Controller[T], publicKey: string): void = +method addContact*[T](self: Controller[T], publicKey: string) = self.contactsService.addContact(publicKey) -method rejectContactRequest*[T](self: Controller[T], publicKey: string): void = +method rejectContactRequest*[T](self: Controller[T], publicKey: string) = self.contactsService.rejectContactRequest(publicKey) -method unblockContact*[T](self: Controller[T], publicKey: string): void = +method unblockContact*[T](self: Controller[T], publicKey: string) = self.contactsService.unblockContact(publicKey) -method blockContact*[T](self: Controller[T], publicKey: string): void = +method blockContact*[T](self: Controller[T], publicKey: string) = self.contactsService.blockContact(publicKey) -method removeContact*[T](self: Controller[T], publicKey: string): void = +method removeContact*[T](self: Controller[T], publicKey: string) = self.contactsService.removeContact(publicKey) method changeContactNickname*[T](self: Controller[T], publicKey: string, nickname: string) = self.contactsService.changeContactNickname(publicKey, nickname) -method lookupContact*[T](self: Controller[T], value: string): void = +method lookupContact*[T](self: Controller[T], value: string) = self.contactsService.lookupContact(value) \ No newline at end of file diff --git a/src/app/modules/main/profile_section/contacts/controller_interface.nim b/src/app/modules/main/profile_section/contacts/controller_interface.nim index ed321b70ab..93c833d45a 100644 --- a/src/app/modules/main/profile_section/contacts/controller_interface.nim +++ b/src/app/modules/main/profile_section/contacts/controller_interface.nim @@ -12,7 +12,7 @@ method delete*(self: AccessInterface) {.base.} = method init*(self: AccessInterface) {.base.} = raise newException(ValueError, "No implementation available") -method getContacts*(self: AccessInterface, useCache: bool = true): seq[ContactDto.ContactsDto] {.base.} = +method getContacts*(self: AccessInterface): seq[ContactDto.ContactsDto] {.base.} = raise newException(ValueError, "No implementation available") method getContact*(self: AccessInterface, id: string): ContactDto.ContactsDto {.base.} = diff --git a/src/app/modules/main/profile_section/contacts/io_interface.nim b/src/app/modules/main/profile_section/contacts/io_interface.nim index bc8700838d..73d3b130dd 100644 --- a/src/app/modules/main/profile_section/contacts/io_interface.nim +++ b/src/app/modules/main/profile_section/contacts/io_interface.nim @@ -25,40 +25,43 @@ method getContact*(self: AccessInterface, id: string): ContactsDto {.base.} = method generateAlias*(self: AccessInterface, publicKey: string): string {.base.} = raise newException(ValueError, "No implementation available") -method addContact*(self: AccessInterface, publicKey: string): void {.base.} = +method addContact*(self: AccessInterface, publicKey: string) {.base.} = raise newException(ValueError, "No implementation available") -method contactAdded*(self: AccessInterface, contact: ContactsDto): void {.base.} = +method contactAdded*(self: AccessInterface, contact: ContactsDto) {.base.} = raise newException(ValueError, "No implementation available") -method contactBlocked*(self: AccessInterface, contact: ContactsDto): void {.base.} = +method contactBlocked*(self: AccessInterface, publicKey: string) {.base.} = raise newException(ValueError, "No implementation available") -method contactUnblocked*(self: AccessInterface, contact: ContactsDto): void {.base.} = +method contactUnblocked*(self: AccessInterface, publicKey: string) {.base.} = raise newException(ValueError, "No implementation available") -method contactRemoved*(self: AccessInterface, contact: ContactsDto): void {.base.} = +method contactRemoved*(self: AccessInterface, publicKey: string) {.base.} = raise newException(ValueError, "No implementation available") -method rejectContactRequest*(self: AccessInterface, publicKey: string): void {.base.} = +method contactNicknameChanged*(self: AccessInterface, publicKey: string, nickname: string) {.base.} = raise newException(ValueError, "No implementation available") -method unblockContact*(self: AccessInterface, publicKey: string): void {.base.} = +method rejectContactRequest*(self: AccessInterface, publicKey: string) {.base.} = raise newException(ValueError, "No implementation available") -method blockContact*(self: AccessInterface, publicKey: string): void {.base.} = +method unblockContact*(self: AccessInterface, publicKey: string) {.base.} = raise newException(ValueError, "No implementation available") -method removeContact*(self: AccessInterface, publicKey: string): void {.base.} = +method blockContact*(self: AccessInterface, publicKey: string) {.base.} = + raise newException(ValueError, "No implementation available") + +method removeContact*(self: AccessInterface, publicKey: string) {.base.} = raise newException(ValueError, "No implementation available") method changeContactNickname*(self: AccessInterface, publicKey: string, nickname: string) {.base.} = raise newException(ValueError, "No implementation available") -method lookupContact*(self: AccessInterface, value: string): void {.base.} = +method lookupContact*(self: AccessInterface, value: string) {.base.} = raise newException(ValueError, "No implementation available") -method contactLookedUp*(self: AccessInterface, id: string): void {.base.} = +method contactLookedUp*(self: AccessInterface, id: string) {.base.} = raise newException(ValueError, "No implementation available") type diff --git a/src/app/modules/main/profile_section/contacts/model.nim b/src/app/modules/main/profile_section/contacts/model.nim index 98f7709ce5..141e34e945 100644 --- a/src/app/modules/main/profile_section/contacts/model.nim +++ b/src/app/modules/main/profile_section/contacts/model.nim @@ -57,6 +57,12 @@ QtObject: self.addedContacts.removeContactFromList(contact.id) self.contactRequests.removeContactFromList(contact.id) + proc changeNicknameForContactWithId*(self: Model, id: string, nickname: string) = + self.contactList.changeNicknameForContactWithId(id, nickname) + self.addedContacts.changeNicknameForContactWithId(id, nickname) + self.blockedContacts.changeNicknameForContactWithId(id, nickname) + self.contactRequests.changeNicknameForContactWithId(id, nickname) + proc updateContactList*(self: Model, contacts: seq[ContactsDto]) = for contact in contacts: var requestAlreadyAdded = false diff --git a/src/app/modules/main/profile_section/contacts/models/contact_list.nim b/src/app/modules/main/profile_section/contacts/models/contact_list.nim index a9da4254f9..3275d1c239 100644 --- a/src/app/modules/main/profile_section/contacts/models/contact_list.nim +++ b/src/app/modules/main/profile_section/contacts/models/contact_list.nim @@ -73,8 +73,7 @@ QtObject: of "isBlocked": result = $contact.isBlocked() of "alias": result = contact.alias of "ensVerified": result = $contact.ensVerified - # TODO check if localNickname exists in the contact ContactsDto - of "localNickname": result = ""#$contact.localNickname + of "localNickname": result = $contact.localNickname of "thumbnailImage": result = $contact.image.thumbnail of "largeImage": result = $contact.image.large of "requestReceived": result = $contact.requestReceived() @@ -94,7 +93,7 @@ QtObject: of ContactRoles.IsBlocked: result = newQVariant(contact.isBlocked()) of ContactRoles.Alias: result = newQVariant(contact.alias) of ContactRoles.EnsVerified: result = newQVariant(contact.ensVerified) - of ContactRoles.LocalNickname: result = newQVariant("")#newQVariant(contact.localNickname) + of ContactRoles.LocalNickname: result = newQVariant(contact.localNickname) of ContactRoles.ThumbnailImage: result = newQVariant(contact.image.thumbnail) of ContactRoles.LargeImage: result = newQVariant(contact.image.large) of ContactRoles.RequestReceived: result = newQVariant(contact.requestReceived()) @@ -138,6 +137,9 @@ QtObject: if(c.isContact()): return true return false + # There are much better ways of notifying qml about this change than sending this signal. + # It also may has an impact to the app performances since it's handled on multiple places on the qml side. + # Would be good to get rid of it durign refactor phase. proc contactChanged*(self: ContactList, pubkey: string) {.signal.} proc updateContact*(self: ContactList, contact: ContactsDto) = @@ -163,3 +165,16 @@ QtObject: self.contacts = contactList self.endResetModel() self.countChanged() + + proc changeNicknameForContactWithId*(self: ContactList, id: string, nickname: string) = + for i in 0 ..< self.contacts.len: + if(self.contacts[i].id != id): + continue + + let index = self.createIndex(i, 0, nil) + self.contacts[i].localNickname = nickname + self.dataChanged(index, index, @[ContactRoles.LocalNickname.int]) + + # Wrote about it where this signal is defined, it's emitted from here just because of the qml part. + self.contactChanged(self.contacts[i].id) + return \ No newline at end of file diff --git a/src/app/modules/main/profile_section/contacts/module.nim b/src/app/modules/main/profile_section/contacts/module.nim index f091d476f3..3f249166b7 100644 --- a/src/app/modules/main/profile_section/contacts/module.nim +++ b/src/app/modules/main/profile_section/contacts/module.nim @@ -1,6 +1,6 @@ import NimQml, Tables -import ./io_interface, ./view, ./controller +import io_interface, view, controller, model import ../../../../global/global_singleton import ../../../../../app_service/service/contacts/service as contacts_service @@ -37,10 +37,10 @@ method delete*[T](self: Module[T]) = self.view.delete method setContactList*[T](self: Module[T], contacts: seq[ContactsDto]) = - self.view.setContactList(contacts) + self.view.model().setContactList(contacts) method updateContactList*[T](self: Module[T], contacts: seq[ContactsDto]) = - self.view.updateContactList(contacts) + self.view.model().updateContactList(contacts) method load*[T](self: Module[T]) = self.controller.init() @@ -61,16 +61,25 @@ method addContact*[T](self: Module[T], publicKey: string) = self.controller.addContact(publicKey) method contactAdded*[T](self: Module[T], contact: ContactsDto) = - self.view.contactAdded(contact) + self.view.model().contactAdded(contact) -method contactBlocked*[T](self: Module[T], contact: ContactsDto) = - self.view.contactBlocked(contact) +method contactBlocked*[T](self: Module[T], publicKey: string) = + # once we refactore a model, we should pass only pk from here (like we have for nickname change) + let contact = self.controller.getContact(publicKey) + self.view.model().contactBlocked(contact) -method contactUnblocked*[T](self: Module[T], contact: ContactsDto) = - self.view.contactUnblocked(contact) +method contactUnblocked*[T](self: Module[T], publicKey: string) = + # once we refactore a model, we should pass only pk from here (like we have for nickname change) + let contact = self.controller.getContact(publicKey) + self.view.model().contactUnblocked(contact) -method contactRemoved*[T](self: Module[T], contact: ContactsDto) = - self.view.contactRemoved(contact) +method contactRemoved*[T](self: Module[T], publicKey: string) = + # once we refactore a model, we should pass only pk from here (like we have for nickname change) + let contact = self.controller.getContact(publicKey) + self.view.model().contactRemoved(contact) + +method contactNicknameChanged*[T](self: Module[T], publicKey: string, nickname: string) = + self.view.model().changeNicknameForContactWithId(publicKey, nickname) method rejectContactRequest*[T](self: Module[T], publicKey: string) = self.controller.rejectContactRequest(publicKey) @@ -90,5 +99,5 @@ method changeContactNickname*[T](self: Module[T], publicKey: string, nickname: s method lookupContact*[T](self: Module[T], value: string) = self.controller.lookupContact(value) -method contactLookedUp*[T](self: Module[T], id: string): void = +method contactLookedUp*[T](self: Module[T], id: string) = self.view.contactLookedUp(id) \ No newline at end of file diff --git a/src/app/modules/main/profile_section/contacts/view.nim b/src/app/modules/main/profile_section/contacts/view.nim index 35eb088fc6..58ba64168d 100644 --- a/src/app/modules/main/profile_section/contacts/view.nim +++ b/src/app/modules/main/profile_section/contacts/view.nim @@ -33,11 +33,8 @@ QtObject: result.modelVariant = newQVariant(result.model) result.contactToAdd = ContactsDto() - proc setContactList*(self: View, contacts: seq[ContactsDto]) = - self.model.setContactList(contacts) - - proc updateContactList*(self: View, contacts: seq[ContactsDto]) = - self.model.updateContactList(contacts) + proc model*(self: View): Model = + return self.model proc modelChanged*(self: View) {.signal.} @@ -101,18 +98,6 @@ QtObject: proc addContact*(self: View, publicKey: string) {.slot.} = self.delegate.addContact(publicKey) - proc contactAdded*(self: View, contact: ContactsDto) = - self.model.contactAdded(contact) - - proc contactBlocked*(self: View, contact: ContactsDto) = - self.model.contactBlocked(contact) - - proc contactUnblocked*(self: View, contact: ContactsDto) = - self.model.contactUnblocked(contact) - - proc contactRemoved*(self: View, contact: ContactsDto) = - self.model.contactRemoved(contact) - proc rejectContactRequest*(self: View, publicKey: string) {.slot.} = self.delegate.rejectContactRequest(publicKey) diff --git a/src/app_service/service/contacts/service.nim b/src/app_service/service/contacts/service.nim index 988349f2c9..fa8019efbe 100644 --- a/src/app_service/service/contacts/service.nim +++ b/src/app_service/service/contacts/service.nim @@ -8,7 +8,6 @@ import status/statusgo_backend_new/contacts as status_contacts import status/statusgo_backend_new/accounts as status_accounts import status/statusgo_backend_new/chat as status_chat import status/statusgo_backend_new/utils as status_utils -import status/contacts as old_status_contacts export contacts_dto @@ -17,16 +16,15 @@ include async_tasks logScope: topics = "contacts-service" -type - LookupResolvedArgs* = ref object of Args - id*: string - type ContactArgs* = ref object of Args - contact*: ContactsDto + contactId*: string - ContactUpdatedArgs* = ref object of Args - id*: string + ContactNicknameUpdatedArgs* = ref object of ContactArgs + nickname*: string + + ContactAddedArgs* = ref object of Args + contact*: ContactsDto # Signals which may be emitted by this service: const SIGNAL_CONTACT_LOOKED_UP* = "SIGNAL_CONTACT_LOOKED_UP" @@ -35,7 +33,7 @@ const SIGNAL_CONTACT_ADDED* = "new-contactAdded" const SIGNAL_CONTACT_BLOCKED* = "new-contactBlocked" const SIGNAL_CONTACT_UNBLOCKED* = "new-contactUnblocked" const SIGNAL_CONTACT_REMOVED* = "new-contactRemoved" -const SIGNAL_CONTACT_UPDATED* = "new-contactUpdated" #Once we are done with refactoring we should remove "new-" from all signals +const SIGNAL_CONTACT_NICKNAME_CHANGED* = "new-contactNicknameChanged" QtObject: type Service* = ref object of QObject @@ -70,9 +68,7 @@ QtObject: proc init*(self: Service) = self.fetchContacts() - proc getContacts*(self: Service, useCache: bool = true): seq[ContactsDto] = - if (not useCache): - self.fetchContacts() + proc getContacts*(self: Service): seq[ContactsDto] = return toSeq(self.contacts.values) proc fetchContact(self: Service, id: string): ContactsDto = @@ -87,21 +83,20 @@ QtObject: error "error: ", errDesription return - proc getContactById*(self: Service, id: string): ContactsDto = - if(not self.contacts.hasKey(id)): - return self.fetchContact(id) - - return self.contacts[id] - proc generateAlias*(self: Service, publicKey: string): string = return status_accounts.generateAlias(publicKey).result.getStr - method generateIdenticon*(self: Service, publicKey: string): string = + proc generateIdenticon*(self: Service, publicKey: string): string = return status_accounts.generateIdenticon(publicKey).result.getStr - proc getOrCreateContact*(self: Service, id: string): ContactsDto = - result = self.getContactById(id) - if result.id == "": + proc getContactById*(self: Service, id: string): ContactsDto = + ## Returns contact details based on passed id (public key) + ## If we don't have stored contact localy or in the db then we create it based on public key. + if(self.contacts.hasKey(id)): + return self.contacts[id] + + result = self.fetchContact(id) + if result.id.len == 0: let alias = self.generateAlias(id) let identicon = self.generateIdenticon(id) result = ContactsDto( @@ -117,78 +112,58 @@ QtObject: proc saveContact(self: Service, contact: ContactsDto) = status_contacts.saveContact(contact.id, contact.ensVerified, contact.name, contact.alias, contact.identicon, contact.image.thumbnail, contact.image.large, contact.added, contact.blocked, contact.hasAddedUs, contact.localNickname) + # we must keep local contacts updated + self.contacts[contact.id] = contact proc addContact*(self: Service, publicKey: string) = - var contact = self.getOrCreateContact(publicKey) - let updating = contact.added - - if not updating: + var contact = self.getContactById(publicKey) + if not contact.added: contact.added = true - # discard status_chat.createProfileChat(contact.id) else: contact.blocked = false self.saveContact(contact) - - self.events.emit(SIGNAL_CONTACT_ADDED, ContactArgs(contact: contact)) - # sendContactUpdate(contact.id, accountKeyUID) - # if updating: - # TODO fix this to use ContactsDto - # self.events.emit("contactUpdate", ContactUpdateArgs(contacts: @[profile])) + self.events.emit(SIGNAL_CONTACT_ADDED, ContactAddedArgs(contact: contact)) proc rejectContactRequest*(self: Service, publicKey: string) = var contact = self.getContactById(publicKey) contact.hasAddedUs = false self.saveContact(contact) - self.events.emit(SIGNAL_CONTACT_REMOVED, ContactArgs(contact: contact)) - # status_contacts.rejectContactRequest(publicKey) + self.events.emit(SIGNAL_CONTACT_REMOVED, ContactArgs(contactId: contact.id)) proc changeContactNickname*(self: Service, publicKey: string, nickname: string) = - var contact = self.getOrCreateContact(publicKey) + var contact = self.getContactById(publicKey) contact.localNickname = nickname + self.saveContact(contact) - let data = ContactUpdatedArgs(id: contact.id) - self.events.emit(SIGNAL_CONTACT_UPDATED, data) - self.events.emit(SIGNAL_CONTACT_ADDED, ContactArgs(contact: contact)) + let data = ContactNicknameUpdatedArgs(contactId: contact.id, nickname: nickname) + self.events.emit(SIGNAL_CONTACT_NICKNAME_CHANGED, data) proc unblockContact*(self: Service, publicKey: string) = - # status_contacts.unblockContact(publicKey) var contact = self.getContactById(publicKey) contact.blocked = false + self.saveContact(contact) - self.events.emit(SIGNAL_CONTACT_UNBLOCKED, ContactArgs(contact: contact)) + self.events.emit(SIGNAL_CONTACT_UNBLOCKED, ContactArgs(contactId: contact.id)) proc blockContact*(self: Service, publicKey: string) = var contact = self.getContactById(publicKey) contact.blocked = true + self.saveContact(contact) - self.events.emit(SIGNAL_CONTACT_BLOCKED, ContactArgs(contact: contact)) + self.events.emit(SIGNAL_CONTACT_BLOCKED, ContactArgs(contactId: contact.id)) proc removeContact*(self: Service, publicKey: string) = - # status_contacts.removeContact(publicKey) var contact = self.getContactById(publicKey) contact.added = false contact.hasAddedUs = false self.saveContact(contact) - self.events.emit(SIGNAL_CONTACT_REMOVED, ContactArgs(contact: contact)) - # let channelId = status_utils.getTimelineChatId(publicKey) - # if status_chat.hasChannel(channelId): - # status_chat.leave(channelId) + self.events.emit(SIGNAL_CONTACT_REMOVED, ContactArgs(contactId: contact.id)) proc ensResolved*(self: Service, id: string) {.slot.} = - # var contact = self.getContact(id) - - # if contact == nil or contact.id == "": - # contact = ContactsDto( - # id: id, - # alias: $status_accounts.generateAlias(id), - # ensVerified: false - # ) - - let data = LookupResolvedArgs(id: id) - + let data = ContactArgs(contactId: id) self.events.emit(SIGNAL_CONTACT_LOOKED_UP, data) proc lookupContact*(self: Service, value: string) =