Handle member related events and channel truncated event in LivestreamChannelController
#3775
Conversation
WalkthroughAdds member-related and channel truncation event handling to LivestreamChannelController, introduces a datastore-backed channel refresh path, updates demo app navigation title to show members/online counts, extends test tools to override memberCount in dummy payloads, updates changelog, and adds comprehensive tests for the new event flows. Changes
Sequence Diagram(s)sequenceDiagram
participant Event as Incoming Event
participant Controller as LivestreamChannelController
participant Store as DataStore
participant UI as Observers
Event->>Controller: handleChannelEvent(event)
alt Member-related event without channel payload
Controller->>Store: fetch channel by cid
Store-->>Controller: latest channel (if any)
Controller->>UI: notify channel updated
else ChannelTruncatedEvent
Controller->>Controller: update channel from event.channel
Controller->>Controller: replace messages (truncate message or empty)
Controller->>UI: notify channel/messages updated
else Other events
Controller->>Controller: existing handling
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ 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 comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
nuno-vieira
changed the title
LivestreamChannelControllerLivestreamChannelController
SDK Size
|
SDK Performance
|
SDK Size
|
Public Interface🚀 No changes affecting the public interface. |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
DemoApp/Screens/Livestream/DemoLivestreamChatChannelVC.swift (1)
411-422: Consider localization and edge-case formatting for subtitleThe pluralization logic is fine, but strings are hardcoded. Consider:
- Localizing “member(s)” and “online”.
- Optionally hiding “, 0 online” when both counts are zero to reduce noise.
Example minimal tweak (optional):
- subtitleLabel.text = "\(membersText), \(onlineText)" + subtitleLabel.text = memberCount == 0 && watcherCount == 0 + ? membersText + : "\(membersText), \(onlineText)"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
CHANGELOG.md(1 hunks)DemoApp/Screens/Livestream/DemoLivestreamChatChannelVC.swift(4 hunks)Sources/StreamChat/Controllers/ChannelController/LivestreamChannelController.swift(2 hunks)TestTools/StreamChatTestTools/TestData/DummyData/ChannelDetailPayload.swift(2 hunks)Tests/StreamChatTests/Controllers/ChannelController/LivestreamChannelController_Tests.swift(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
Tests/StreamChatTests/Controllers/ChannelController/LivestreamChannelController_Tests.swift (7)
Tests/StreamChatTests/Database/DTOs/ChannelDTO_Tests.swift (1)
channel(1772-1774)Sources/StreamChat/WebSocketClient/Events/EventType.swift (1)
event(182-260)TestTools/StreamChatTestTools/TestData/DummyData/ChannelDetailPayload.swift (1)
dummy(11-56)TestTools/StreamChatTestTools/SpyPattern/Spy/DatabaseContainer_Spy.swift (1)
writeSynchronously(175-182)Sources/StreamChat/Controllers/ChannelController/LivestreamChannelController.swift (2)
synchronize(215-226)eventsController(748-754)Sources/StreamChat/Controllers/ChannelController/ChannelController.swift (3)
synchronize(243-246)synchronize(1816-1840)message(2048-2053)TestTools/StreamChatTestTools/SpyPattern/Spy/APIClient_Spy.swift (1)
test_simulateResponse(108-111)
Sources/StreamChat/Controllers/ChannelController/LivestreamChannelController.swift (3)
DemoApp/Screens/Livestream/DemoLivestreamChatChannelVC.swift (1)
channel(212-214)Tests/StreamChatTests/Database/DTOs/ChannelDTO_Tests.swift (1)
channel(1772-1774)Sources/StreamChat/Controllers/ChannelController/ChannelController.swift (1)
message(2048-2053)
DemoApp/Screens/Livestream/DemoLivestreamChatChannelVC.swift (3)
Sources/StreamChat/Controllers/ChannelController/LivestreamChannelController.swift (5)
livestreamChannelController(11-13)livestreamChannelController(1096-1099)livestreamChannelController(1101-1104)livestreamChannelController(1106-1109)livestreamChannelController(1111-1114)Tests/StreamChatTests/Controllers/ChannelController/LivestreamChannelController_Tests.swift (4)
livestreamChannelController(734-740)livestreamChannelController(742-748)livestreamChannelController(750-756)livestreamChannelController(758-764)Sources/StreamChat/Controllers/ChannelController/LivestreamChannelController+Combine.swift (4)
livestreamChannelController(60-65)livestreamChannelController(67-72)livestreamChannelController(74-79)livestreamChannelController(81-86)
⏰ 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)
- GitHub Check: Test E2E UI (Debug) (0)
- GitHub Check: Test E2E UI (Debug) (1)
🔇 Additional comments (7)
CHANGELOG.md (1)
8-9: Changelog entries look correct and consistentBoth items accurately describe the scope of this PR and link to the right PR. No action needed.
TestTools/StreamChatTestTools/TestData/DummyData/ChannelDetailPayload.swift (1)
29-30: Good addition to support explicit memberCount in testsProviding memberCount with a sensible default to members.count is perfect for precise test setup. No issues spotted.
Also applies to: 52-53
DemoApp/Screens/Livestream/DemoLivestreamChatChannelVC.swift (3)
53-81: Navigation title UI implementation is solidStacked title/subtitle with proper fonts and priorities is clean and predictable for a titleView. Looks good.
156-159: Correctly initializes the custom title viewSetting titleView and calling updateNavigationTitle() during layout ensures an initial value. LGTM.
344-350: Right place to refresh header on channel updatesUpdating the avatar and calling updateNavigationTitle() on didUpdateChannel keeps the header in sync. Good.
Tests/StreamChatTests/Controllers/ChannelController/LivestreamChannelController_Tests.swift (2)
1234-1358: Great coverage for all member-related event variantsIterating across all 8 event types and verifying datastore-driven updates is thorough and effective. Nicely structured.
1360-1453: Truncation behavior tests are clear and robustValidates both with/without truncation message. This matches the controller behavior precisely. LGTM.
Sources/StreamChat/Controllers/ChannelController/LivestreamChannelController.swift
Show resolved
Hide resolved
Sources/StreamChat/Controllers/ChannelController/LivestreamChannelController.swift
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
CHANGELOG.md (1)
8-9: Changelog entries look good; consider clarifying scope.Both entries are clear. If space allows, append a short note that the events are handled via the data store to avoid breaking Channel List updates (as implemented), to make the behavioral nuance discoverable from the changelog.
TestTools/StreamChatTestTools/TestData/DummyData/ChannelDetailPayload.swift (1)
29-29: Good addition; source-compatible and flexible for tests.
- Adding
memberCount: Int? = nilwith a default keeps existing callers intact.- The fallback
memberCount ?? members.countis sensible.Optional: add a brief doc note that
memberCountoverrides the derived count when provided, to avoid confusion in tests.Also applies to: 52-53
DemoApp/Screens/Livestream/DemoLivestreamChatChannelVC.swift (2)
53-81: Custom title view setup is clean; add minor accessibility affordances.Consider:
- Setting
titleStackView.isAccessibilityElement = trueand assigningaccessibilityLabelwith combined title/subtitle for better VoiceOver experience.- Setting
numberOfLines = 1andlineBreakMode = .byTruncatingTailon labels to avoid awkward wrapping in narrow nav bars.I can draft a snippet if you want.
411-422: Localize the subtitle string.The “members, online” text is user-facing. For consistency with the SDK/ demo localizations, move it to a localization key and use pluralized forms. You already handle pluralization logic; just wire it to L10n.
Sources/StreamChat/Controllers/ChannelController/LivestreamChannelController.swift (3)
917-924: Truncation handling is correct; consider resetting pagination state.You replace messages appropriately and update the channel. After truncation, previous/next pagination state may be invalid. Consider resetting
paginationStateHandlerto its initial state to avoid follow-up pagination using stale cursors.
986-994: DB refresh helper is straightforward; guard against event–DB ordering.If middleware writes aren’t guaranteed to complete before this delegate runs,
dataStore.channel(cid:)might briefly return stale data. If that’s a possibility, schedule the read on the next run loop tick:private func updateChannelFromDataStore() { guard let cid = cid else { return } callback { [weak self] in guard let self, let updated = self.dataStore.channel(cid: cid) else { return } self.channel = updated } }This keeps mutations on the main thread and improves the odds the DB has applied the event.
917-924: Apply message limit uniformly for all added messages.Currently,
applyMessageLimit()is only called in theMessageNewEventbranch. To enforce limits also forNewMessagePendingEvent, move the limit application intohandleNewMessage(_:)after insertion (while keeping the pause guard).For example (outside the selected range):
private func handleNewMessage(_ message: ChatMessage) { // ... existing checks ... messages.insert(message, at: 0) if !isPaused { applyMessageLimit() } }Also applies to: 986-994
Tests/StreamChatTests/Controllers/ChannelController/LivestreamChannelController_Tests.swift (1)
934-970: Add a test to assert message limit is enforced for pending events as well.Since
applyMessageLimitcurrently runs only forMessageNewEvent, a regression could let the array grow duringNewMessagePendingEvent. Recommend adding a sibling test that sends 100 messages, then aNewMessagePendingEvent, and asserts the capped size.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
CHANGELOG.md(1 hunks)DemoApp/Screens/Livestream/DemoLivestreamChatChannelVC.swift(4 hunks)Sources/StreamChat/Controllers/ChannelController/LivestreamChannelController.swift(2 hunks)TestTools/StreamChatTestTools/TestData/DummyData/ChannelDetailPayload.swift(2 hunks)Tests/StreamChatTests/Controllers/ChannelController/LivestreamChannelController_Tests.swift(1 hunks)
⏰ 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)
- GitHub Check: Test E2E UI (Debug) (0)
- GitHub Check: Test E2E UI (Debug) (1)
🔇 Additional comments (5)
DemoApp/Screens/Livestream/DemoLivestreamChatChannelVC.swift (2)
156-159: Initializing title view early is fine; ensure updates on first channel load.This calls
updateNavigationTitle()beforesynchronizecompletes, so it will briefly show an empty title/subtitle. That’s acceptable for demo. If you prefer no flash, call it fromdidFinishSynchronizingonly.
349-350: Good: refresh title on channel updates.This keeps the header in sync with member/watcher counts after events.
Tests/StreamChatTests/Controllers/ChannelController/LivestreamChannelController_Tests.swift (3)
1234-1358: Strong coverage for member-related events via data-store-backed updates.Nice table-driven loop verifying all eight event types refresh the channel from the DB. This should prevent regressions in memberCount handling.
1360-1413: Truncation behavior test is solid.Validates both channel update and message replacement as intended.
1415-1453: Good: asserts messages are cleared when truncation message is absent.This pins down the no-message truncation path.
Sources/StreamChat/Controllers/ChannelController/LivestreamChannelController.swift
Show resolved
Hide resolved
🔗 Issue Links
https://linear.app/stream/issue/IOS-1066/livestream-controller-member-and-truncation-events
🎯 Goal
Handle member related events and channel truncated event in
LivestreamChannelController.🛠 Implementation
For now, these events will still go through the DB to make sure it does not break the Channel List updates, but we make sure the data is updated inside the livestream controller as well.
🧪 Manual Testing Notes
Members:
Truncation:
☑️ Contributor Checklist
docs-contentrepoSummary by CodeRabbit
New Features
Improvements
Documentation
Tests