Set the channel loading state until the first channel data arrives

stream-chat-android-client/src/main/java/io/getstream/chat/android/client/channel/state/ChannelState.kt

@@ -96,7 +96,7 @@ public interface ChannelState {
     /** If the channel is currently muted. */
     public val muted: StateFlow<Boolean>
 
-    /** If we are currently loading. */
+    /** If the channel is loading for the first time, before any data from the local database or the API arrived. */
     public val loading: StateFlow<Boolean>
 
     /** If we are currently loading older messages. */

stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicImpl.kt

@@ -36,7 +36,6 @@ import io.getstream.chat.android.state.model.querychannels.pagination.internal.Q
 import io.getstream.chat.android.state.model.querychannels.pagination.internal.toAnyChannelPaginationRequest
 import io.getstream.chat.android.state.plugin.state.channel.internal.ChannelMutableState
 import io.getstream.log.taggedLogger
-import io.getstream.result.Error
 import io.getstream.result.Result
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.launch
@@ -101,15 +100,6 @@ internal class ChannelLogicImpl(
 
     override suspend fun watch(limit: Int, userPresence: Boolean): Result<Channel> {
         logger.i { "[watch] messagesLimit: $limit, userPresence: $userPresence" }
-        // Otherwise it's too easy for devs to create UI bugs which DDOS our API
-        if (mutableState.loading.value) {
-            logger.i { "Another request to watch this channel is in progress. Ignoring this request." }
-            return Result.Failure(
-                Error.GenericError(
-                    "Another request to watch this channel is in progress. Ignoring this request.",
-                ),
-            )
-        }
         channelStateLogic.loadingNewestMessages()
         return runChannelQuery(
             "watch",

stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogic.kt

@@ -664,6 +664,7 @@ internal class ChannelStateLogic(
 
         mutableState.setChannelConfig(channel.config)
 
+        mutableState.setLoading(false)
         mutableState.setLoadingOlderMessages(false)
         mutableState.setLoadingNewerMessages(false)
     }
@@ -796,6 +797,7 @@ internal class ChannelStateLogic(
             }
             mutableState.recoveryNeeded = true
         }
+        mutableState.setLoading(false)
         mutableState.setLoadingOlderMessages(false)
         mutableState.setLoadingNewerMessages(false)
     }

stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/channel/internal/ChannelMutableState.kt

@@ -152,7 +152,9 @@ internal class ChannelMutableState(
         messagesTransformation(pinnedMessagesList) { it.isPinned(now) }
 
     override val messagesState: StateFlow<MessagesState> =
-        combineStates(loading, sortedVisibleMessages) { loading: Boolean, messages: List<Message> ->
+        combineStates(loading, sortedVisibleMessages) { loading: Boolean, collected: List<Message> ->
+            // A collected copy of the messages can lag behind loading, so re-read before reporting no results.
+            val messages = if (!loading && collected.isEmpty()) sortedVisibleMessages.value else collected
             when {
                 loading -> MessagesState.Loading
                 messages.isEmpty() -> MessagesState.OfflineNoResults
@@ -338,6 +340,14 @@ internal class ChannelMutableState(
         _loading?.value = isLoading
     }
 
+    /**
+     * Marks the first load of the channel as in progress. Does nothing once the channel has data, so refreshing a
+     * loaded channel never replaces its content with a loading state.
+     */
+    fun setLoadingIfEmpty() {
+        if (_channelData?.value == null) setLoading(true)
+    }
+
     /**
      * Sets hidden.
      *

stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/internal/ChatClientStateCalls.kt

@@ -99,10 +99,12 @@ internal class ChatClientStateCalls(
         request: QueryChannelRequest,
     ): ChannelState {
         logger.v { "[queryChannel] cid: $channelType:$channelId, request: $request" }
-        chatClient.queryChannel(channelType, channelId, request).launch(scope)
-        return deferredState
+        val state = deferredState
             .await()
-            .channel(channelType, channelId)
+            .mutableChannel(channelType, channelId)
+            .apply { setLoadingIfEmpty() }
+        chatClient.queryChannel(channelType, channelId, request).launch(scope)
+        return state
     }
 
     /** Reference request of the watch channel query. */

stream-chat-android-state/src/test/java/io/getstream/chat/android/state/extensions/ChatClientExtensionTests.kt

@@ -32,6 +32,7 @@ import io.getstream.chat.android.state.plugin.config.StatePluginConfig
 import io.getstream.chat.android.state.plugin.factory.StreamStatePluginFactory
 import io.getstream.chat.android.state.plugin.internal.StatePlugin
 import io.getstream.chat.android.state.plugin.state.StateRegistry
+import io.getstream.chat.android.state.plugin.state.channel.internal.ChannelMutableState
 import io.getstream.chat.android.state.plugin.state.global.GlobalState
 import io.getstream.chat.android.test.TestCoroutineExtension
 import io.getstream.chat.android.test.asCall
@@ -79,13 +80,13 @@ internal class ChatClientExtensionTests {
         val statePluginConfig = StatePluginConfig()
         val globalState: GlobalState = mock() {
         }
-        val channelState: ChannelState = mock {
+        val channelState: ChannelMutableState = mock {
             on(it.cid) doReturn channel.cid
             on(it.channelType) doReturn channel.type
             on(it.channelId) doReturn channel.id
         }
         val stateRegistry: StateRegistry = mock { stateRegistry ->
-            on(stateRegistry.channel(channel.type, channel.id)) doReturn channelState
+            on(stateRegistry.mutableChannel(channel.type, channel.id)) doReturn channelState
         }
 
         val request = QueryChannelPaginationRequest(MESSAGE_LIMIT)

stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogicTest.kt

@@ -66,6 +66,7 @@ import org.mockito.kotlin.argumentCaptor
 import org.mockito.kotlin.doAnswer
 import org.mockito.kotlin.doReturn
 import org.mockito.kotlin.eq
+import org.mockito.kotlin.inOrder
 import org.mockito.kotlin.mock
 import org.mockito.kotlin.never
 import org.mockito.kotlin.spy
@@ -247,6 +248,26 @@ internal class ChannelStateLogicTest {
         verify(mutableState, times(0)).upsertMessages(any())
     }
 
+    @Test
+    fun `given channel data arrives, loading should end once the data is in the state`() {
+        val channel = randomChannel()
+
+        channelStateLogic.updateDataForChannel(channel, messageLimit = 30)
+
+        inOrder(mutableState) {
+            verify(mutableState).setChannelData(any())
+            verify(mutableState).upsertMembers(channel.members)
+            verify(mutableState).setLoading(false)
+        }
+    }
+
+    @Test
+    fun `given the query fails, loading should end`() {
+        channelStateLogic.propagateQueryError(Error.GenericError(randomString()))
+
+        verify(mutableState).setLoading(false)
+    }
+
     @Test
     fun `given inside search should not upsert messages when messages are not coming from scroll update`() {
         _insideSearch.value = true

stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/state/channel/internal/ChannelMutableStateTests.kt

@@ -16,7 +16,9 @@
 
 package io.getstream.chat.android.state.plugin.state.channel.internal
 
+import io.getstream.chat.android.models.ChannelData
 import io.getstream.chat.android.models.Message
+import io.getstream.chat.android.models.MessagesState
 import io.getstream.chat.android.models.User
 import io.getstream.chat.android.randomChannelUserRead
 import io.getstream.chat.android.randomConfig
@@ -26,7 +28,10 @@ import io.getstream.chat.android.randomUser
 import io.getstream.chat.android.test.TestCoroutineExtension
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.flow.MutableStateFlow
+import kotlinx.coroutines.launch
+import kotlinx.coroutines.test.StandardTestDispatcher
 import kotlinx.coroutines.test.advanceTimeBy
+import kotlinx.coroutines.test.runCurrent
 import kotlinx.coroutines.test.runTest
 import org.amshove.kluent.`should be equal to`
 import org.junit.jupiter.api.Assertions.assertEquals
@@ -744,6 +749,41 @@ internal class ChannelMutableStateTests {
         assertEquals(delivered, userRead)
     }
 
+    @Test
+    fun `setLoadingIfEmpty on a channel without data should show the loading state`() = runTest {
+        channelState.setLoadingIfEmpty()
+
+        channelState.loading.value `should be equal to` true
+        channelState.messagesState.value `should be equal to` MessagesState.Loading
+    }
+
+    @Test
+    fun `setLoadingIfEmpty on a channel with data should not show the loading state`() = runTest {
+        channelState.setChannelData(ChannelData(type = CHANNEL_TYPE, id = CHANNEL_ID))
+
+        channelState.setLoadingIfEmpty()
+
+        channelState.loading.value `should be equal to` false
+        channelState.messagesState.value `should be equal to` MessagesState.OfflineNoResults
+    }
+
+    @Test
+    fun `messagesState should not report no results while the loaded messages are still propagating`() =
+        runTest(StandardTestDispatcher(testCoroutines.dispatcher.scheduler)) {
+            val emissions = mutableListOf<MessagesState>()
+            backgroundScope.launch { channelState.messagesState.collect { emissions += it } }
+            channelState.setLoadingIfEmpty()
+            runCurrent()
+
+            channelState.setMessages(createMessages(3))
+            channelState.setLoading(false)
+            runCurrent()
+
+            val afterLoading = emissions.dropWhile { it != MessagesState.Loading }
+            afterLoading.contains(MessagesState.OfflineNoResults) `should be equal to` false
+            (afterLoading.last() is MessagesState.Result) `should be equal to` true
+        }
+
     private fun ChannelMutableState.assertPinnedMessagesSizeEqualsTo(size: Int) {
         require(pinnedMessages.value.size == size) {
             "pinnedMessages should have $size items, but was ${pinnedMessages.value.size}"

stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/state/internal/ChatClientStateCallsTest.kt

@@ -21,13 +21,16 @@ import io.getstream.chat.android.client.internal.state.plugin.QueryChannelsIdent
 import io.getstream.chat.android.client.setup.state.ClientState
 import io.getstream.chat.android.models.InitializationState
 import io.getstream.chat.android.models.User
+import io.getstream.chat.android.randomChannel
 import io.getstream.chat.android.state.event.handler.chat.factory.ChatEventHandlerFactory
 import io.getstream.chat.android.state.plugin.internal.StatePlugin
 import io.getstream.chat.android.state.plugin.logic.internal.LogicRegistry
 import io.getstream.chat.android.state.plugin.logic.querychannels.internal.QueryChannelsLogic
 import io.getstream.chat.android.state.plugin.state.StateRegistry
+import io.getstream.chat.android.state.plugin.state.channel.internal.ChannelMutableState
 import io.getstream.chat.android.state.plugin.state.querychannels.QueryChannelsState
 import io.getstream.chat.android.test.TestCoroutineRule
+import io.getstream.chat.android.test.asCall
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.launch
@@ -41,9 +44,11 @@ import org.junit.jupiter.api.Test
 import org.mockito.kotlin.any
 import org.mockito.kotlin.doReturn
 import org.mockito.kotlin.eq
+import org.mockito.kotlin.inOrder
 import org.mockito.kotlin.mock
 import org.mockito.kotlin.never
 import org.mockito.kotlin.verify
+import org.mockito.kotlin.whenever
 
 @OptIn(ExperimentalCoroutinesApi::class)
 internal class ChatClientStateCallsTest {
@@ -142,4 +147,23 @@ internal class ChatClientStateCallsTest {
         // Then - stateRegistry.queryChannels should be called with the identifier
         verify(stateRegistry).queryChannels(identifier)
     }
+
+    @Test
+    fun `watchChannel marks the channel as loading before querying it`() = runTest {
+        // Given
+        userFlow.value = User(id = "test-user")
+        val channelState: ChannelMutableState = mock()
+        whenever(stateRegistry.mutableChannel("messaging", "123")) doReturn channelState
+        whenever(chatClient.queryChannel(any(), any(), any(), any())) doReturn randomChannel().asCall()
+
+        // When
+        val result = chatClientStateCalls.watchChannel("messaging:123", messageLimit = 30, userPresence = true)
+
+        // Then
+        assertEquals(channelState, result)
+        inOrder(channelState, chatClient) {
+            verify(channelState).setLoadingIfEmpty()
+            verify(chatClient).queryChannel(eq("messaging"), eq("123"), any(), any())
+        }
+    }
 }