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

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):
+  self.events.on(SIGNAL_CONTACT_LOOKED_UP) do(e: Args):
-    let msgData = MessageSignal(e);
+    var args = ContactArgs(e)
-    if msgData.contacts.len > 0:
+    self.delegate.contactLookedUp(args.contactId)
-      let contacts = self.getContacts(false)
+
-      self.delegate.updateContactList(contacts)
   self.events.on(SIGNAL_CONTACT_ADDED) do(e: Args):
-    var evArgs = ContactArgs(e)
+    var args = ContactAddedArgs(e)
-    self.delegate.contactAdded(evArgs.contact)
+    self.delegate.contactAdded(args.contact)
 
   self.events.on(SIGNAL_CONTACT_BLOCKED) do(e: Args):
-    var evArgs = ContactArgs(e)
+    var args = ContactArgs(e)
-    self.delegate.contactBlocked(evArgs.contact)
+    self.delegate.contactBlocked(args.contactId)
 
   self.events.on(SIGNAL_CONTACT_UNBLOCKED) do(e: Args):
-    var evArgs = ContactArgs(e)
+    var args = ContactArgs(e)
-    self.delegate.contactUnblocked(evArgs.contact)
+    self.delegate.contactUnblocked(args.contactId)
 
   self.events.on(SIGNAL_CONTACT_REMOVED) do(e: Args):
-    var evArgs = ContactArgs(e)
+    var args = ContactArgs(e)
-    self.delegate.contactRemoved(evArgs.contact)
+    self.delegate.contactRemoved(args.contactId)
 
-  self.events.on(SIGNAL_CONTACT_LOOKED_UP) do(e: Args):
+  self.events.on(SIGNAL_CONTACT_NICKNAME_CHANGED) do(e: Args):
-    let args = LookupResolvedArgs(e)
+    var args = ContactNicknameUpdatedArgs(e)
-    self.delegate.contactLookedUp(args.id)
+    self.delegate.contactNicknameChanged(args.contactId, args.nickname)
 
-  self.events.on(SIGNAL_CONTACT_UPDATED) do(e: Args):
+method getContacts*[T](self: Controller[T]): seq[ContactsDto] =
-    # I left this as part it was.
+  return self.contactsService.getContacts()
-    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 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)

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.} =

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

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

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

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) =
+method contactBlocked*[T](self: Module[T], publicKey: string) =
-  self.view.contactBlocked(contact)
+  # 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) =
+method contactUnblocked*[T](self: Module[T], publicKey: string) =
-  self.view.contactUnblocked(contact)
+  # 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) =
+method contactRemoved*[T](self: Module[T], publicKey: string) =
-  self.view.contactRemoved(contact)
+  # 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)

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]) =
+  proc model*(self: View): Model =
-    self.model.setContactList(contacts)
+    return self.model
-
-  proc updateContactList*(self: View, contacts: seq[ContactsDto]) =
-    self.model.updateContactList(contacts)
 
   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)
 

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
+  ContactNicknameUpdatedArgs* = ref object of ContactArgs
-    id*: string
+    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] =
+  proc getContacts*(self: Service): seq[ContactsDto] =
-    if (not useCache):
-      self.fetchContacts()
     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 =
+  proc getContactById*(self: Service, id: string): ContactsDto =
-    result = self.getContactById(id)
+    ## Returns contact details based on passed id (public key)
-    if result.id == "":
+    ## 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)
+    var contact = self.getContactById(publicKey)
-    let updating = contact.added
+    if not contact.added:
-
-    if not updating:
       contact.added = true
-      # discard status_chat.createProfileChat(contact.id)
     else:
       contact.blocked = false
 
     self.saveContact(contact)
-    
+    self.events.emit(SIGNAL_CONTACT_ADDED, ContactAddedArgs(contact: 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]))
 
   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))
+    self.events.emit(SIGNAL_CONTACT_REMOVED, ContactArgs(contactId: contact.id))
-    # status_contacts.rejectContactRequest(publicKey)
 
   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)
+    let data = ContactNicknameUpdatedArgs(contactId: contact.id, nickname: nickname)
-    self.events.emit(SIGNAL_CONTACT_UPDATED, data)
+    self.events.emit(SIGNAL_CONTACT_NICKNAME_CHANGED, data)
-    self.events.emit(SIGNAL_CONTACT_ADDED, ContactArgs(contact: contact))
 
   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))
+    self.events.emit(SIGNAL_CONTACT_REMOVED, ContactArgs(contactId: contact.id))
-  #   let channelId = status_utils.getTimelineChatId(publicKey)
-  #   if status_chat.hasChannel(channelId):
-  #     status_chat.leave(channelId)
 
   proc ensResolved*(self: Service, id: string) {.slot.} =
-    # var contact = self.getContact(id)
+    let data = ContactArgs(contactId: id)
-
-    # if contact == nil or contact.id == "":
-    #   contact = ContactsDto(
-    #     id: id,
-    #     alias: $status_accounts.generateAlias(id),
-    #     ensVerified: false
-    #   )
-
-    let data = LookupResolvedArgs(id: id)
-
     self.events.emit(SIGNAL_CONTACT_LOOKED_UP, data)
 
   proc lookupContact*(self: Service, value: string) =