Merge pull request #2287 from RobotJINI/develop

[IMPROVEMENT] Immediate message loading

app/src/main/java/chat/rocket/android/chatroom/presentation/ChatRoomPresenter.kt

@@ -117,7 +117,8 @@ class ChatRoomPresenter @Inject constructor(
 
     private var chatRoomId: String? = null
     private lateinit var chatRoomType: String
-    private var chatIsBroadcast: Boolean = false
+    private lateinit var chatRoomName: String
+    private var isBroadcast: Boolean = false
     private var chatRoles = emptyList<ChatRoomRole>()
     private val stateChannel = Channel<State>()
     private var typingStatusSubscriptionId: String? = null
@@ -136,56 +137,91 @@ class ChatRoomPresenter @Inject constructor(
         draftKey = "${currentServer}_${LocalRepository.DRAFT_KEY}$roomId"
         chatRoomId = roomId
         chatRoomType = roomType
+        chatRoomName = roomName
+        chatRoles = emptyList()
+        var canModerate = isOwnerOrMod()
+
         GlobalScope.launch(Dispatchers.IO + strategy.jobs) {
-            try {
-                chatRoles = if (roomTypeOf(roomType) !is RoomType.DirectMessage) {
-                    client.chatRoomRoles(roomType = roomTypeOf(roomType), roomName = roomName)
-                } else {
-                    emptyList()
-                }
-            } catch (ex: Exception) {
-                Timber.e(ex)
-                chatRoles = emptyList()
-            } finally {
-                // User has at least an 'owner' or 'moderator' role.
-                val canModerate = isOwnerOrMod()
-                // Can post anyway if has the 'post-readonly' permission on server.
-                val room = dbManager.getRoom(roomId)
-                room?.let {
-                    chatIsBroadcast = it.chatRoom.broadcast ?: false
-                    val roomUiModel = roomMapper.map(it, true)
-                    launchUI(strategy) {
-                        view.onRoomUpdated(roomUiModel = roomUiModel.copy(
-                            broadcast = chatIsBroadcast,
+            // Can post anyway if has the 'post-readonly' permission on server.
+            val room = dbManager.getRoom(roomId)
+            room?.let {
+                isBroadcast = it.chatRoom.broadcast ?: false
+                val roomUiModel = roomMapper.map(it, true)
+                launchUI(strategy) {
+                    view.onRoomUpdated(
+                        roomUiModel = roomUiModel.copy(
+                            broadcast = isBroadcast,
                             canModerate = canModerate,
                             writable = roomUiModel.writable || canModerate
-                        ))
-                    }
+                        )
+                    )
                 }
+            }
 
-                loadMessages(roomId, roomType, clearDataSet = true)
-                chatRoomMessage?.let { messageHelper.messageIdFromPermalink(it) }
-                    ?.let { messageId ->
-                        val name = messageHelper.roomNameFromPermalink(chatRoomMessage)
-                        citeMessage(
-                            name!!,
-                            messageHelper.roomTypeFromPermalink(chatRoomMessage)!!,
-                            messageId,
-                            true
-                        )
-                    }
-                subscribeRoomChanges()
+            loadMessages(roomId, chatRoomType, clearDataSet = true)
+            loadActiveMembers(roomId, chatRoomType, filterSelfOut = true)
+
+            chatRoomMessage?.let { messageHelper.messageIdFromPermalink(it) }
+                ?.let { messageId ->
+                    val name = messageHelper.roomNameFromPermalink(chatRoomMessage)
+                    citeMessage(
+                        name!!,
+                        messageHelper.roomTypeFromPermalink(chatRoomMessage)!!,
+                        messageId,
+                        true
+                    )
+                }
+
+
+            /*FIXME:  Get chat role can cause unresponsive problems especially on slower connections
+                      We are updating the room again after the first step so that initial messages
+                      get loaded in and the system appears more responsive.  Something should be
+                      done to either fix the load in speed of moderator roles or store the
+                      information locally*/
+            if (getChatRole()) {
+                canModerate = isOwnerOrMod()
+                if (canModerate) {
+                    //FIXME: add this in when moderator page is actually created
+                    //view.updateModeration()
+                }
             }
+
+            subscribeRoomChanges()
         }
     }
 
+    private suspend fun getChatRole(): Boolean {
+        try {
+            if (roomTypeOf(chatRoomType) !is RoomType.DirectMessage) {
+                chatRoles = withContext(Dispatchers.IO + strategy.jobs) {
+                    client.chatRoomRoles(
+                        roomType = roomTypeOf(chatRoomType),
+                        roomName = chatRoomName
+                    )
+                }
+                return true
+            } else {
+                chatRoles = emptyList()
+            }
+        } catch (ex: Exception) {
+            Timber.e(ex)
+            chatRoles = emptyList()
+        }
+        return false
+    }
+
     private suspend fun subscribeRoomChanges() {
         withContext(Dispatchers.IO + strategy.jobs) {
             chatRoomId?.let {
                 manager.addRoomChannel(it, roomChangesChannel)
                 for (room in roomChangesChannel) {
                     dbManager.getRoom(room.id)?.let { chatRoom ->
-                        view.onRoomUpdated(roomMapper.map(chatRoom = chatRoom, showLastMessage = true))
+                        view.onRoomUpdated(
+                            roomMapper.map(
+                                chatRoom = chatRoom,
+                                showLastMessage = true
+                            )
+                        )
                     }
                 }
             }
@@ -210,8 +246,8 @@ class ChatRoomPresenter @Inject constructor(
     ) {
         this.chatRoomId = chatRoomId
         this.chatRoomType = chatRoomType
-        launchUI(strategy) {
-            view.showLoading()
+
+        GlobalScope.launch(Dispatchers.IO + strategy.jobs) {
             try {
                 if (offset == 0L) {
                     // FIXME - load just 50 messages from DB to speed up. We will reload from Network after that
@@ -219,10 +255,10 @@ class ChatRoomPresenter @Inject constructor(
                     val localMessages = messagesRepository.getRecentMessages(chatRoomId, 50)
                     val oldMessages = mapper.map(
                         localMessages, RoomUiModel(
-                        roles = chatRoles,
-                        // FIXME: Why are we fixing isRoom attribute to true here?
-                        isBroadcast = chatIsBroadcast, isRoom = true
-                    )
+                            roles = chatRoles,
+                            // FIXME: Why are we fixing isRoom attribute to true here?
+                            isBroadcast = isBroadcast, isRoom = true
+                        )
                     )
                     lastMessageId = localMessages.firstOrNull()?.id
                     val lastSyncDate = messagesRepository.getLastSyncDate(chatRoomId)
@@ -248,8 +284,6 @@ class ChatRoomPresenter @Inject constructor(
                 }.ifNull {
                     view.showGenericErrorMessage()
                 }
-            } finally {
-                view.hideLoading()
             }
 
             subscribeTypingStatus()
@@ -284,7 +318,7 @@ class ChatRoomPresenter @Inject constructor(
         view.showMessages(
             mapper.map(
                 messages,
-                RoomUiModel(roles = chatRoles, isBroadcast = chatIsBroadcast, isRoom = true)
+                RoomUiModel(roles = chatRoles, isBroadcast = isBroadcast, isRoom = true)
             ),
             clearDataSet
         )
@@ -300,7 +334,7 @@ class ChatRoomPresenter @Inject constructor(
                 view.showSearchedMessages(
                     mapper.map(
                         messages,
-                        RoomUiModel(chatRoles, chatIsBroadcast, true)
+                        RoomUiModel(chatRoles, isBroadcast, true)
                     )
                 )
             } catch (ex: Exception) {
@@ -332,7 +366,11 @@ class ChatRoomPresenter @Inject constructor(
                         timestamp = Instant.now().toEpochMilli(),
                         sender = SimpleUser(user?.id, user?.username ?: username, user?.name),
                         attachments = null,
-                        avatar = currentServer.avatarUrl(username!!, token?.userId, token?.authToken),
+                        avatar = currentServer.avatarUrl(
+                            username!!,
+                            token?.userId,
+                            token?.authToken
+                        ),
                         channels = null,
                         editedAt = null,
                         editedBy = null,
@@ -354,7 +392,7 @@ class ChatRoomPresenter @Inject constructor(
                         view.showNewMessage(
                             mapper.map(
                                 newMessage,
-                                RoomUiModel(roles = chatRoles, isBroadcast = chatIsBroadcast)
+                                RoomUiModel(roles = chatRoles, isBroadcast = isBroadcast)
                             ), false
                         )
                         client.sendMessage(id, chatRoomId, text)
@@ -605,16 +643,21 @@ class ChatRoomPresenter @Inject constructor(
                     Timber.d("History: $messages")
 
                     if (messages.result.isNotEmpty()) {
-                        val models = mapper.map(messages.result, RoomUiModel(
-                            roles = chatRoles,
-                            isBroadcast = chatIsBroadcast,
-                            // FIXME: Why are we fixing isRoom attribute to true here?
-                            isRoom = true
-                        ))
+                        val models = mapper.map(
+                            messages.result, RoomUiModel(
+                                roles = chatRoles,
+                                isBroadcast = isBroadcast,
+                                // FIXME: Why are we fixing isRoom attribute to true here?
+                                isRoom = true
+                            )
+                        )
                         messagesRepository.saveAll(messages.result)
                         //if success - saving last synced time
                         //assume that BE returns ordered messages, the first message is the latest one
-                        messagesRepository.saveLastSyncDate(chatRoomId, messages.result.first().timestamp)
+                        messagesRepository.saveLastSyncDate(
+                            chatRoomId,
+                            messages.result.first().timestamp
+                        )
 
                         launchUI(strategy) {
                             view.showNewMessage(models, true)
@@ -691,9 +734,9 @@ class ChatRoomPresenter @Inject constructor(
                     replyMarkdown = "[ ]($currentServer/$chatRoomType/$room?msg=$id) $mention ",
                     quotedMessage = mapper.map(
                         message, RoomUiModel(
-                        roles = chatRoles,
-                        isBroadcast = chatIsBroadcast
-                    )
+                            roles = chatRoles,
+                            isBroadcast = isBroadcast
+                        )
                     ).last().preview?.message ?: ""
                 )
             }
@@ -796,13 +839,15 @@ class ChatRoomPresenter @Inject constructor(
         }
     }
 
-    fun loadActiveMembers(
+    suspend fun loadActiveMembers(
         chatRoomId: String,
         chatRoomType: String,
         offset: Long = 0,
         filterSelfOut: Boolean = false
     ) {
-        launchUI(strategy) {
+        val activeUsers = mutableListOf<PeopleSuggestionUiModel>()
+
+        withContext(Dispatchers.IO + strategy.jobs) {
             try {
                 val members = retryIO("getMembers($chatRoomId, $chatRoomType, $offset)") {
                     client.getMembers(chatRoomId, roomTypeOf(chatRoomType), offset, 50).result
@@ -813,12 +858,12 @@ class ChatRoomPresenter @Inject constructor(
                 // Take at most the 100 most recent messages distinguished by user. Can return less.
                 val recentMessages = messagesRepository.getRecentMessages(chatRoomId, 100)
                     .filterNot { filterSelfOut && it.sender?.username == self }
-                val activeUsers = mutableListOf<PeopleSuggestionUiModel>()
                 recentMessages.forEach {
                     val sender = it.sender
                     val username = sender?.username ?: ""
                     val name = sender?.name ?: ""
-                    val avatarUrl = currentServer.avatarUrl(username, token?.userId, token?.authToken)
+                    val avatarUrl =
+                        currentServer.avatarUrl(username, token?.userId, token?.authToken)
                     val found = members.firstOrNull { member -> member.username == username }
                     val status = if (found != null) found.status else UserStatus.Offline()
                     val searchList = mutableListOf(username, name)
@@ -839,7 +884,8 @@ class ChatRoomPresenter @Inject constructor(
                 activeUsers.addAll(others.map {
                     val username = it.username ?: ""
                     val name = it.name ?: ""
-                    val avatarUrl = currentServer.avatarUrl(username, token?.userId, token?.authToken)
+                    val avatarUrl =
+                        currentServer.avatarUrl(username, token?.userId, token?.authToken)
                     val searchList = mutableListOf(username, name)
                     PeopleSuggestionUiModel(
                         avatarUrl,
@@ -852,11 +898,13 @@ class ChatRoomPresenter @Inject constructor(
                     )
                 })
 
-                view.populatePeopleSuggestions(activeUsers)
             } catch (e: RocketChatException) {
                 Timber.e(e)
             }
         }
+        launchUI(strategy) {
+            view.populatePeopleSuggestions(activeUsers)
+        }
     }
 
     fun spotlight(query: String, @AutoCompleteType type: Int, filterSelfOut: Boolean = false) {
@@ -969,48 +1017,49 @@ class ChatRoomPresenter @Inject constructor(
     }
 
     // TODO: move this to new interactor or FetchChatRoomsInteractor?
-    private suspend fun getChatRoomsAsync(name: String? = null): List<ChatRoom> = withContext(Dispatchers.IO) {
-        retryDB("getAllSync()") {
-            dbManager.chatRoomDao().getAllSync().filter {
-                if (name == null) {
-                    return@filter true
-                }
-                it.chatRoom.name == name || it.chatRoom.fullname == name
-            }.map {
-                with(it.chatRoom) {
-                    ChatRoom(
-                        id = id,
-                        subscriptionId = subscriptionId,
-                        parentId = parentId,
-                        type = roomTypeOf(type),
-                        unread = unread,
-                        broadcast = broadcast ?: false,
-                        alert = alert,
-                        fullName = fullname,
-                        name = name ?: "",
-                        favorite = favorite ?: false,
-                        default = isDefault ?: false,
-                        readonly = readonly,
-                        open = open,
-                        lastMessage = null,
-                        archived = false,
-                        status = null,
-                        user = null,
-                        userMentions = userMentions,
-                        client = client,
-                        announcement = null,
-                        description = null,
-                        groupMentions = groupMentions,
-                        roles = null,
-                        topic = null,
-                        lastSeen = this.lastSeen,
-                        timestamp = timestamp,
-                        updatedAt = updatedAt
-                    )
+    private suspend fun getChatRoomsAsync(name: String? = null): List<ChatRoom> =
+        withContext(Dispatchers.IO) {
+            retryDB("getAllSync()") {
+                dbManager.chatRoomDao().getAllSync().filter {
+                    if (name == null) {
+                        return@filter true
+                    }
+                    it.chatRoom.name == name || it.chatRoom.fullname == name
+                }.map {
+                    with(it.chatRoom) {
+                        ChatRoom(
+                            id = id,
+                            subscriptionId = subscriptionId,
+                            parentId = parentId,
+                            type = roomTypeOf(type),
+                            unread = unread,
+                            broadcast = broadcast ?: false,
+                            alert = alert,
+                            fullName = fullname,
+                            name = name ?: "",
+                            favorite = favorite ?: false,
+                            default = isDefault ?: false,
+                            readonly = readonly,
+                            open = open,
+                            lastMessage = null,
+                            archived = false,
+                            status = null,
+                            user = null,
+                            userMentions = userMentions,
+                            client = client,
+                            announcement = null,
+                            description = null,
+                            groupMentions = groupMentions,
+                            roles = null,
+                            topic = null,
+                            lastSeen = this.lastSeen,
+                            timestamp = timestamp,
+                            updatedAt = updatedAt
+                        )
+                    }
                 }
             }
         }
-    }
 
     fun joinChat(chatRoomId: String) {
         launchUI(strategy) {
@@ -1019,7 +1068,8 @@ class ChatRoomPresenter @Inject constructor(
                 val canPost = permissions.canPostToReadOnlyChannels()
                 dbManager.getRoom(chatRoomId)?.let {
                     val roomUiModel = roomMapper.map(it, true).copy(
-                        writable = canPost)
+                        writable = canPost
+                    )
                     view.onJoined(roomUiModel = roomUiModel)
                     view.onRoomUpdated(roomUiModel = roomUiModel)
                 }
@@ -1280,8 +1330,8 @@ class ChatRoomPresenter @Inject constructor(
         launchUI(strategy) {
             val viewModelStreamedMessage = mapper.map(
                 streamedMessage, RoomUiModel(
-                roles = chatRoles, isBroadcast = chatIsBroadcast, isRoom = true
-            )
+                    roles = chatRoles, isBroadcast = isBroadcast, isRoom = true
+                )
             )
             val roomMessages = messagesRepository.getByRoomId(streamedMessage.roomId)
             val index = roomMessages.indexOfFirst { msg -> msg.id == streamedMessage.id }

app/src/main/java/chat/rocket/android/chatroom/ui/ChatRoomFragment.kt

@@ -429,25 +429,12 @@ class ChatRoomFragment : Fragment(), ChatRoomView, EmojiKeyboardListener, EmojiR
                 }
             }
 
-            if (recycler_view.adapter == null) {
-                recycler_view.adapter = adapter
-                if (dataSet.size >= 30) {
-                    recycler_view.addOnScrollListener(endlessRecyclerViewScrollListener)
-                }
-                recycler_view.addOnLayoutChangeListener(layoutChangeListener)
-                recycler_view.addOnScrollListener(onScrollListener)
-
-                // Load just once, on the first page...
-                presenter.loadActiveMembers(chatRoomId, chatRoomType, filterSelfOut = true)
-            }
-
             val oldMessagesCount = adapter.itemCount
             adapter.appendData(dataSet)
             if (oldMessagesCount == 0 && dataSet.isNotEmpty()) {
                 recycler_view.scrollToPosition(0)
                 verticalScrollOffset.set(0)
             }
-            presenter.loadActiveMembers(chatRoomId, chatRoomType, filterSelfOut = true)
             empty_chat_view.isVisible = adapter.itemCount == 0
             dismissEmojiKeyboard()
         }
@@ -469,10 +456,8 @@ class ChatRoomFragment : Fragment(), ChatRoomView, EmojiKeyboardListener, EmojiR
             setupMessageComposer(roomUiModel)
             isBroadcastChannel = roomUiModel.broadcast
             isFavorite = roomUiModel.favorite.orFalse()
-            if (isBroadcastChannel && !roomUiModel.canModerate) {
-                disableMenu = true
-                activity?.invalidateOptionsMenu()
-            }
+            disableMenu = (roomUiModel.broadcast && !roomUiModel.canModerate)
+            activity?.invalidateOptionsMenu()
         }
     }
 
@@ -807,7 +792,12 @@ class ChatRoomFragment : Fragment(), ChatRoomView, EmojiKeyboardListener, EmojiR
                 presenter.loadMessages(chatRoomId, chatRoomType, page * 30L)
             }
         }
+
+        recycler_view.adapter = adapter
         recycler_view.addOnScrollListener(fabScrollListener)
+        recycler_view.addOnScrollListener(endlessRecyclerViewScrollListener)
+        recycler_view.addOnLayoutChangeListener(layoutChangeListener)
+        recycler_view.addOnScrollListener(onScrollListener)
     }
 
     private fun setupFab() {