Fix LivestreamChannelController not connecting chat after coming from background
#3778
Conversation
WalkthroughAdds foreground-handling to LivestreamChannelController to reload the first page when the app returns to foreground if the client is disconnected; introduces mockable connectionStatus in ChatClient_Mock; adds tests for this conditional reload; updates CHANGELOG with the fix. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant Controller as LivestreamChannelController
participant ChatClient
participant API as APIClient
App->>Controller: applicationDidMoveToForeground()
Controller->>ChatClient: read connectionStatus
alt connectionStatus != .connected
Controller->>Controller: loadFirstPage()
Controller->>API: Endpoint<ChannelPayload>.updateChannel (first page)
API-->>Controller: response
else connectionStatus == .connected
Controller-->>App: no action
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Assessment against linked issues
Out-of-scope changes(No out-of-scope functional changes detected.) Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
…ller-after-coming-from-background
nuno-vieira
changed the title
LivestreamChannelController not resuming chat after coming from backgroundLivestreamChannelController not connecting chat after coming from background
SDK Size
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
CHANGELOG.md (1)
14-14: Wording nit: prefer “resuming chat after returning from background.”To better reflect the behavior and the PR’s title/intent, consider rephrasing “not connecting chat” to “not resuming chat after returning from background”.
Apply this diff:
-- Fix `LivestreamChannelController` not connecting chat after coming from background [#3778](https://github.com/GetStream/stream-chat-swift/pull/3778) +- Fix `LivestreamChannelController` not resuming chat after returning from background [#3778](https://github.com/GetStream/stream-chat-swift/pull/3778)Sources/StreamChat/Controllers/ChannelController/LivestreamChannelController.swift (1)
763-767: Foreground reload when not connected — LGTM; consider scoping to “disconnected” only.The current condition triggers a first-page reload for all non-connected states, which may include transient states like “connecting” or “waiting for network”. Functionally fine, but if you want to avoid redundant calls during transitions, consider scoping the reload to a definitively disconnected state.
Example (optional) refinement:
public func applicationDidMoveToForeground() { // Only reload if we're definitively disconnected if case .disconnected = client.connectionStatus { loadFirstPage() } }This keeps behavior for the reported bug while avoiding extra traffic during short-lived transitions. If other non-connected states should also force a reload, the current implementation is appropriate.
TestTools/StreamChatTestTools/Mocks/StreamChat/ChatClient_Mock.swift (1)
79-87: Nice addition: mockableconnectionStatus; add cleanup reset to avoid cross-test leakage.Overriding
connectionStatusthroughconnectionStatus_mockis exactly what the new tests need. To make the mock safer for reuse, also resetconnectionStatus_mock(and optionallymockedEventNotificationCenter) incleanUp().Follow-up change (outside the selected lines):
// In ChatClient_Mock.cleanUp() func cleanUp() { (apiClient as? APIClient_Spy)?.cleanUp() fetchCurrentUserIdFromDatabase_called = false createBackgroundWorkers_called = false completeConnectionIdWaiters_called = false completeConnectionIdWaiters_connectionId = nil completeTokenWaiters_called = false completeTokenWaiters_token = nil connectionStatus_mock = nil mockedEventNotificationCenter = nil _backgroundWorkers.removeAll() init_completion = nil }Tests/StreamChatTests/Controllers/ChannelController/LivestreamChannelController_Tests.swift (1)
910-937: Good coverage for foreground behavior (disconnected vs connected).Both paths are validated against the expected
updateChannelendpoint and no-call scenario. Consider adding a third test for a transient state (e.g.,.connectingor.waitingForNetwork) to lock-in the intended behavior for those cases and prevent regressions if the foreground logic changes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
CHANGELOG.md(1 hunks)Sources/StreamChat/Controllers/ChannelController/LivestreamChannelController.swift(1 hunks)TestTools/StreamChatTestTools/Mocks/StreamChat/ChatClient_Mock.swift(3 hunks)Tests/StreamChatTests/Controllers/ChannelController/LivestreamChannelController_Tests.swift(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
Sources/StreamChat/Controllers/ChannelController/LivestreamChannelController.swift (4)
Sources/StreamChat/Audio/AppStateObserving.swift (1)
applicationDidMoveToForeground(28-28)Tests/StreamChatTests/Audio/StreamAppStateObserver_Tests.swift (1)
applicationDidMoveToForeground(180-182)Sources/StreamChat/Controllers/MessageController/MessageController.swift (1)
loadFirstPage(575-585)Sources/StreamChat/Controllers/ChannelController/ChannelController.swift (1)
loadFirstPage(654-661)
Tests/StreamChatTests/Controllers/ChannelController/LivestreamChannelController_Tests.swift (2)
Sources/StreamChat/Controllers/ChannelController/LivestreamChannelController.swift (1)
applicationDidMoveToForeground(763-767)Tests/StreamChatTests/APIClient/Endpoints/ChannelEndpoints_Tests.swift (1)
channelQuery(48-52)
🔇 Additional comments (2)
TestTools/StreamChatTestTools/Mocks/StreamChat/ChatClient_Mock.swift (2)
30-34: LGTM on the event center override.Falling back to
super.eventNotificationCenterkeeps behavior consistent while allowing targeted test control viamockedEventNotificationCenter.
66-68: SimplifiedcurrentUserIdoverride — LGTM.Returning
currentUserId_mockkeeps tests deterministic and avoids touching internal state elsewhere.
SDK Performance
|
Generated by 🚫 Danger |
Public Interface public class LivestreamChannelController: DataStoreProvider, EventsControllerDelegate, AppStateObserverDelegate
+ public func applicationDidMoveToForeground() |
SDK Size
|
testableapple
added
🧪 QAing
🟢 QAed
|
nuno-vieira
deleted the
fix/missing-messages-in-livestream-controller-after-coming-from-background
branch
🔗 Issue Links
https://linear.app/stream/issue/IOS-1071/fix-chat-not-resuming-in-livestream-controller-after-coming-from
🎯 Goal
Fix livestream controller not resuming chat after coming from background
🧪 Manual Testing Notes
☑️ Contributor Checklist
docs-contentrepoSummary by CodeRabbit
Bug Fixes
Documentation