refactor(@desktop/members): do not request members list each time cha…

…t/channel changed or member added/removed

src/app/modules/main/chat_section/chat_content/users/controller.nim

@@ -52,21 +52,17 @@ proc handleCommunityOnlyConnections(self: Controller) =
     if (args.communityId == self.sectionId):
       self.delegate.onChatMembersAdded(@[args.pubKey])
 
-  self.events.on(SIGNAL_COMMUNITIES_UPDATE) do(e:Args):
-    let args = CommunitiesArgs(e)
-    for community in args.communities:
-      if (community.id != self.sectionId):
-        continue
-      # If we didn't join the community, all members in the list will have status
-      # joined=false. No need to try add them to the model
-      if community.isMember:
-        let membersPubKeys = community.members.map(x => x.id)
-        self.delegate.onChatMembersAddedOrRemoved(membersPubKeys)
+  self.events.on(SIGNAL_COMMUNITY_MEMBERS_CHANGED) do(e:Args):
+    let args = CommunityMembersArgs(e)
+    if args.communityId != self.sectionId:
+      return
+
+    self.delegate.onMembersChanged(args.members)
 
   self.events.on(SIGNAL_COMMUNITY_MEMBER_REMOVED) do(e: Args):
     let args = CommunityMemberArgs(e)
     if (args.communityId == self.sectionId):
-        self.delegate.onChatMemberRemoved(args.pubKey)
+      self.delegate.onChatMemberRemoved(args.pubKey)
 
 proc init*(self: Controller) =
   # Events that are needed for all chats because of mentions
@@ -121,11 +117,10 @@ proc init*(self: Controller) =
       if (args.chatId == self.chatId):
         self.delegate.onChatMembersAdded(args.ids)
 
-    self.events.on(SIGNAL_CHAT_UPDATE) do(e: Args):
-      var args = ChatUpdateArgs(e)
-      for chat in args.chats:
-        if (chat.id == self.chatId):
-          self.delegate.onChatUpdated(chat)
+    self.events.on(SIGNAL_CHAT_MEMBERS_CHANGED) do(e: Args):
+      var args = ChatMembersChangedArgs(e)
+      if (args.chatId == self.chatId):
+        self.delegate.onMembersChanged(args.members)
 
     self.events.on(SIGNAL_CHAT_MEMBER_REMOVED) do(e: Args):
       let args = ChatMemberRemovedArgs(e)
@@ -145,18 +140,10 @@ proc getChat*(self: Controller): ChatDto =
   return self.chatService.getChatById(self.chatId)
 
 proc getChatMembers*(self: Controller): seq[ChatMember] =
-  var communityId = ""
-  if (self.belongsToCommunity):
-    communityId = self.sectionId
-  return self.chatService.getMembers(communityId, self.chatId)
-
-proc getMembersPublicKeys*(self: Controller): seq[string] =
-  if(self.belongsToCommunity):
-    let communityDto = self.communityService.getCommunityById(self.sectionId)
-    return communityDto.members.map(x => x.id)
-  else:
-    let chatDto = self.getChat()
-    return chatDto.members.map(x => x.id)
+  if self.belongsToCommunity:
+    return self.communityService.getCommunityById(self.sectionId).members
+
+  return self.chatService.getChatById(self.chatId).members
 
 proc getContactNameAndImage*(self: Controller, contactId: string):
     tuple[name: string, image: string, largeImage: string] =

src/app/modules/main/chat_section/chat_content/users/io_interface.nim

@@ -37,10 +37,7 @@ method userProfileUpdated*(self: AccessInterface) {.base.} =
 method loggedInUserImageChanged*(self: AccessInterface) {.base.} =
   raise newException(ValueError, "No implementation available")
 
-method addChatMember*(self: AccessInterface, member: ChatMember) {.base.} =
-  raise newException(ValueError, "No implementation available")
-
-method onChatMembersAddedOrRemoved*(self: AccessInterface, ids: seq[string]) {.base.} =
+method onMembersChanged*(self: AccessInterface, members: seq[ChatMember]) {.base.} =
   raise newException(ValueError, "No implementation available")
 
 method onChatMembersAdded*(self: AccessInterface, ids: seq[string]) {.base.} =
@@ -49,18 +46,12 @@ method onChatMembersAdded*(self: AccessInterface, ids: seq[string]) {.base.} =
 method onChatMemberRemoved*(self: AccessInterface, ids: string) {.base.} =
   raise newException(ValueError, "No implementation available")
 
-method onChatUpdated*(self: AccessInterface, chat: ChatDto) {.base.} =
-  raise newException(ValueError, "No implementation available")
-
 method onChatMemberUpdated*(self: AccessInterface, id: string, admin: bool, joined: bool) {.base.} =
   raise newException(ValueError, "No implementation available")
 
 method viewDidLoad*(self: AccessInterface) {.base.} =
   raise newException(ValueError, "No implementation available")
 
-method getMembersPublicKeys*(self: AccessInterface): string {.base.} =
-  raise newException(ValueError, "No implementation available")
-
 method addGroupMembers*(self: AccessInterface, pubKeys: seq[string]) {.base.} =
   raise newException(ValueError, "No implementation available")
 

src/app/modules/main/chat_section/chat_content/users/module.nim

@@ -22,7 +22,7 @@ type
     moduleLoaded: bool
 
 # Forward declaration
-method addChatMember*(self: Module,  member: ChatMember)
+method addChatMember(self: Module,  member: ChatMember)
 
 proc newModule*(
   events: EventEmitter, sectionId: string, chatId: string,
@@ -96,7 +96,7 @@ method userProfileUpdated*(self: Module) =
 method loggedInUserImageChanged*(self: Module) =
   self.view.model().setIcon(singletonInstance.userProfile.getPubKey(), singletonInstance.userProfile.getIcon())
 
-method addChatMember*(self: Module,  member: ChatMember) =
+method addChatMember(self: Module,  member: ChatMember) =
   if member.id == "":
     return
 
@@ -136,43 +136,21 @@ method addChatMember*(self: Module,  member: ChatMember) =
     isUntrustworthy = contactDetails.details.trustStatus == TrustStatus.Untrustworthy
     ))
 
-method onChatMembersAdded*(self: Module,  ids: seq[string]) =
-  if ids.len() == 0:
-    return
-
-  let members = self.controller.getChatMembers()
-  for id in ids:
-    for member in members:
-      if (member.id == id):
-        self.addChatMember(member)
-
-method onChatUpdated*(self: Module,  chat: ChatDto) =
-  let members = self.controller.getChatMembers()
-  for member in chat.members:
-    for existingMember in members:
-      if existingMember.id == member.id:
-        self.addChatMember(existingMember)
-
-  if chat.members.len > 0:
-    let ids = self.view.model.getItemIds()
-    for id in ids:
-      var found = false
-      for member in chat.members:
-        if (member.id == id):
-          found = true
-          break
-      if (not found):
-        self.view.model().removeItemById(id)
+method onChatMembersAdded*(self: Module, ids: seq[string]) =
+  for memberId in ids:
+    self.addChatMember(ChatMember(id: memberId, admin: false, joined: true, roles: @[]))
 
 method onChatMemberRemoved*(self: Module, id: string) =
   self.view.model().removeItemById(id)
 
-method onChatMembersAddedOrRemoved*(self: Module,  ids: seq[string]) =
+method onMembersChanged*(self: Module,  members: seq[ChatMember]) =
   let modelIDs = self.view.model().getItemIds()
-  let membersAdded = filter(ids, id => not modelIDs.contains(id))
-  let membersRemoved = filter(modelIDs, id => not ids.contains(id))
+  let membersAdded = filter(members, member => not modelIDs.contains(member.id))
+  let membersRemoved = filter(modelIDs, id => not members.any(member => member.id == id))
+
+  for member in membersAdded:
+    self.addChatMember(member)
 
-  self.onChatMembersAdded(membersAdded)
   for id in membersRemoved:
     self.onChatMemberRemoved(id)
 
@@ -193,10 +171,6 @@ method onChatMemberUpdated*(self: Module, publicKey: string, admin: bool, joined
     isUntrustworthy = contactDetails.details.trustStatus == TrustStatus.Untrustworthy,
     )
 
-method getMembersPublicKeys*(self: Module): string =
-  let publicKeys = self.controller.getMembersPublicKeys()
-  return publicKeys.join(" ")
-
 method addGroupMembers*(self: Module, pubKeys: seq[string]) =
   self.controller.addGroupMembers(pubKeys)
 

src/app/modules/main/chat_section/chat_content/users/view.nim

@@ -40,9 +40,6 @@ QtObject:
     read = getModel
     notify = modelChanged
 
-  proc getMembersPublicKeys*(self: View): string {.slot.} =
-    return self.delegate.getMembersPublicKeys()
-
   proc temporaryModelChanged*(self: View) {.signal.}
 
   proc getTemporaryModel(self: View): QVariant {.slot.} =

src/app/modules/main/communities/module.nim

@@ -126,7 +126,7 @@ method getCommunityItem(self: Module, c: CommunityDto): SectionItem =
       c.permissions.access,
       c.permissions.ensOnly,
       c.muted,
-      c.members.map(proc(member: Member): MemberItem =
+      c.members.map(proc(member: ChatMember): MemberItem =
         result = self.createMemberItem(member.id, "")),
       historyArchiveSupportEnabled = c.settings.historyArchiveSupportEnabled,
       bannedMembers = c.bannedMembersIds.map(proc(bannedMemberId: string): MemberItem =

src/app_service/service/chat/dto/chat.nim

@@ -172,27 +172,37 @@ proc toCategory*(jsonObj: JsonNode): Category =
   discard jsonObj.getProp("name", result.name)
   discard jsonObj.getProp("position", result.position)
 
-proc toChatMember*(jsonObj: JsonNode): ChatMember =
+proc toChatMember*(jsonObj: JsonNode, memberId: string): ChatMember =
+  # Parse status-go "Member" type
+  # Mapping this DTO is not strightforward since only keys are used for id
   result = ChatMember()
-  discard jsonObj.getProp("id", result.id)
+  result.id = memberId
   discard jsonObj.getProp("admin", result.admin)
   discard jsonObj.getProp("joined", result.joined)
-
   var rolesObj: JsonNode
   if(jsonObj.getProp("roles", rolesObj) and rolesObj.kind == JArray):
     for role in rolesObj:
       result.roles.add(role.getInt)
 
-  # channel group members json contains only roles
-  # fill admin info by checking roles
-  if (not jsonObj.contains("admin")):
-    result.admin = isMemberAdmin(result.roles)
+proc toGroupChatMember*(jsonObj: JsonNode): ChatMember =
+  # parse status-go "ChatMember" type
+  result = ChatMember()
+  discard jsonObj.getProp("id", result.id)
+  discard jsonObj.getProp("admin", result.admin)
+  result.joined = true
 
-proc toChatMember(jsonObj: JsonNode, memberId: string): ChatMember =
+proc toChannelMember*(jsonObj: JsonNode, memberId: string, joined: bool): ChatMember =
+  # Parse status-go "CommunityMember" type
   # Mapping this DTO is not strightforward since only keys are used for id. We
   # handle it a bit different.
-  result = jsonObj.toChatMember()
+  result = ChatMember()
   result.id = memberId
+  var rolesObj: JsonNode
+  if(jsonObj.getProp("roles", rolesObj)):
+    for roleObj in rolesObj:
+      result.roles.add(roleObj.getInt)
+  result.joined = joined
+  result.admin = isMemberAdmin(result.roles)
 
 proc toChatDto*(jsonObj: JsonNode): ChatDto =
   result = ChatDto()
@@ -242,9 +252,11 @@ proc toChatDto*(jsonObj: JsonNode): ChatDto =
   var membersObj: JsonNode
   if(jsonObj.getProp("members", membersObj)):
     if(membersObj.kind == JArray):
+      # during group chat updates
       for memberObj in membersObj:
-        result.members.add(toChatMember(memberObj))
+        result.members.add(toGroupChatMember(memberObj))
     elif(membersObj.kind == JObject):
+      # on a startup, chat/channel creation
       for memberId, memberObj in membersObj:
         result.members.add(toChatMember(memberObj, memberId))
 
@@ -292,7 +304,7 @@ proc toChannelGroupDto*(jsonObj: JsonNode): ChannelGroupDto =
   var membersObj: JsonNode
   if(jsonObj.getProp("members", membersObj) and membersObj.kind == JObject):
     for memberId, memberObj in membersObj:
-      result.members.add(toChatMember(memberObj, memberId))
+      result.members.add(toChannelMember(memberObj, memberId, joined = true))
 
   var bannedMembersIdsObj: JsonNode
   if(jsonObj.getProp("banList", bannedMembersIdsObj) and bannedMembersIdsObj.kind == JArray):

src/app_service/service/chat/service.nim

@@ -73,6 +73,10 @@ type
     chatId*: string
     id*: string
 
+  ChatMembersChangedArgs* = ref object of Args
+    chatId*: string
+    members*: seq[ChatMember]
+
   ChatMemberUpdatedArgs* = ref object of Args
     chatId*: string
     id*: string
@@ -92,6 +96,7 @@ const SIGNAL_CHAT_HISTORY_CLEARED* = "chatHistoryCleared"
 const SIGNAL_CHAT_RENAMED* = "chatRenamed"
 const SIGNAL_GROUP_CHAT_DETAILS_UPDATED* = "groupChatDetailsUpdated"
 const SIGNAL_CHAT_MEMBERS_ADDED* = "chatMemberAdded"
+const SIGNAL_CHAT_MEMBERS_CHANGED* = "chatMembersChanged"
 const SIGNAL_CHAT_MEMBER_REMOVED* = "chatMemberRemoved"
 const SIGNAL_CHAT_MEMBER_UPDATED* = "chatMemberUpdated"
 const SIGNAL_CHAT_SWITCH_TO_OR_CREATE_1_1_CHAT* = "switchToOrCreateOneToOneChat"
@@ -128,7 +133,13 @@ QtObject:
         for chatDto in receivedData.chats:
           if (chatDto.active):
             chats.add(chatDto)
+
+            # Handling members update
+            if self.chats.hasKey(chatDto.id) and self.chats[chatDto.id].members != chatDto.members:
+              self.events.emit(SIGNAL_CHAT_MEMBERS_CHANGED, ChatMembersChangedArgs(chatId: chatDto.id, members: chatDto.members))
+
             self.updateOrAddChat(chatDto)
+
         self.events.emit(SIGNAL_CHAT_UPDATE, ChatUpdateArgs(messages: receivedData.messages, chats: chats))
 
       if (receivedData.clearedHistories.len > 0):
@@ -613,8 +624,7 @@ QtObject:
       let myPubkey = singletonInstance.userProfile.getPubKey()
       result = @[]
       for (id, memberObj) in response.result.pairs:
-        var member = toChatMember(memberObj)
-        member.id = id
+        var member = toChatMember(memberObj, id)
         # Make yourself as the first result
         if (id == myPubkey):
           result.insert(member)

src/app_service/service/community/dto/community.nim

@@ -13,20 +13,6 @@ type RequestToJoinType* {.pure.}= enum
   Accepted = 3,
   Canceled = 4
 
-type Member* = object
-  id*: string
-  roles*: seq[int]
-
-proc toMember*(jsonObj: JsonNode, memberId: string): Member =
-  # Mapping this DTO is not strightforward since only keys are used for id. We
-  # handle it a bit different.
-  result = Member()
-  result.id = memberId
-  var rolesObj: JsonNode
-  if(jsonObj.getProp("roles", rolesObj)):
-    for roleObj in rolesObj:
-      result.roles.add(roleObj.getInt)
-
 type CommunityMembershipRequestDto* = object
   id*: string
   publicKey*: string
@@ -58,7 +44,7 @@ type CommunityDto* = object
   categories*: seq[Category]
   images*: Images
   permissions*: Permission
-  members*: seq[Member]
+  members*: seq[ChatMember]
   canRequestAccess*: bool
   canManageUsers*: bool
   canJoin*: bool
@@ -159,6 +145,7 @@ proc toCommunityDto*(jsonObj: JsonNode): CommunityDto =
   discard jsonObj.getProp("introMessage", result.introMessage)
   discard jsonObj.getProp("outroMessage", result.outroMessage)
   discard jsonObj.getProp("encrypted", result.encrypted)
+  discard jsonObj.getProp("isMember", result.isMember)
 
   var chatsObj: JsonNode
   if(jsonObj.getProp("chats", chatsObj)):
@@ -185,7 +172,8 @@ proc toCommunityDto*(jsonObj: JsonNode): CommunityDto =
   var membersObj: JsonNode
   if(jsonObj.getProp("members", membersObj) and membersObj.kind == JObject):
     for memberId, memberObj in membersObj:
-      result.members.add(toMember(memberObj, memberId))
+      # Do not display members list until the user became a community member
+      result.members.add(toChannelMember(memberObj, memberId, joined = result.isMember))
 
   var tagsObj: JsonNode
   if(jsonObj.getProp("tags", tagsObj)):
@@ -204,7 +192,6 @@ proc toCommunityDto*(jsonObj: JsonNode): CommunityDto =
   discard jsonObj.getProp("color", result.color)
 
   discard jsonObj.getProp("requestedToJoinAt", result.requestedToJoinAt)
-  discard jsonObj.getProp("isMember", result.isMember)
   discard jsonObj.getProp("muted", result.muted)
 
 proc toCommunityMembershipRequestDto*(jsonObj: JsonNode): CommunityMembershipRequestDto =
@@ -267,7 +254,8 @@ proc toChannelGroupDto*(communityDto: CommunityDto): ChannelGroupDto =
     members: communityDto.members.map(m => ChatMember(
         id: m.id,
         joined: true,
-        admin: isMemberAdmin(m.roles)
+        admin: isMemberAdmin(m.roles),
+        roles: m.roles
       )),
     canManageUsers: communityDto.canManageUsers,
     muted: communityDto.muted,