Fix poll votes list not updating on poll events #3849
Conversation
WalkthroughAdds poll observation to PollVoteListController and surfaces poll updates via delegate, Combine publisher, and SwiftUI observable; wires poll observer and vote linking logic, updates UI handling, mocks, and tests to cover poll and poll-vote update flows. Changes
Sequence Diagram(s)sequenceDiagram
participant WS as WebSocket
participant EC as EventsController
participant PVC as PollVoteListController
participant Repo as Repository
participant PO as PollObserver
participant VO as VoteObserver
participant Delegate as Delegate / UI / Publishers
WS->>EC: PollVoteCasted / PollVoteChanged Event
EC->>PVC: handleEvent(payload)
alt vote matches controller query
PVC->>Repo: link(pollVote, query)
Repo-->>VO: persistence triggers vote observer
VO->>Delegate: controller(_:didChangeVotes:)
else ignored
note right of PVC: No link / ignored
end
WS->>Repo: Poll persisted/updated
Repo-->>PO: DB change for Poll
PO->>PVC: controller(_:didUpdatePoll:)
PVC->>Delegate: controller(_:didUpdatePoll:) -> update UI / pollPublisher / SwiftUI observable
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~40 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 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). (5)
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. Comment |
Generated by 🚫 Danger |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
Sources/StreamChat/Controllers/PollController/PollVoteListController+Combine.swift (1)
34-36: Emit the initial poll value via CurrentValueSubject (keeps API, improves UX).Today pollPublisher emits only after an update. Emitting the current value first makes Combine consistent with statePublisher and typical expectations.
Proposed change:
- /// A backing subject for `pollPublisher`. - let poll: PassthroughSubject<Poll, Never> = .init() + /// A backing subject for `pollPublisher`. + /// Holds the latest poll (if any) so new subscribers get an immediate value. + let poll: CurrentValueSubject<Poll?, Never> @@ init(controller: PollVoteListController) { self.controller = controller - state = .init(controller.state) + state = .init(controller.state) + poll = .init(controller.poll) @@ - public var pollPublisher: AnyPublisher<Poll, Never> { - basePublishers.poll.keepAlive(self) - } + public var pollPublisher: AnyPublisher<Poll, Never> { + basePublishers.poll + .compactMap { $0 } + .keepAlive(self) + } @@ - func controller(_ controller: PollVoteListController, didUpdatePoll poll: Poll) { - self.poll.send(poll) - } + func controller(_ controller: PollVoteListController, didUpdatePoll poll: Poll) { + self.poll.send(poll) + }Note: This keeps the public type as AnyPublisher<Poll, Never> while providing an initial emission when available.
Also applies to: 14-17, 61-63
Sources/StreamChat/Controllers/PollController/PollVoteListController.swift (2)
59-61: Starting observers from computed properties is fine; consider documenting access cost.Accessing votes/poll triggers observer startup on first use. Add a brief note in the property docs to set expectations for potential fetch/start side effects.
Also applies to: 63-68
185-193: Observer start ordering and error path.Order is fine. Minor: if pollObserver throws after list observer succeeds, state becomes .localDataFetchFailed while list keeps observing. Consider logging which observer failed to aid diagnostics.
Tests/StreamChatTests/Controllers/PollsControllers/PollVoteListController+Combine_Tests.swift (1)
82-103: Combine test for pollPublisher is solid.Covers keepAlive semantics and emission on delegate update. Consider adding a case asserting no emission before update, and (if adopting CurrentValueSubject) initial emission when poll exists.
TestTools/StreamChatTestTools/Mocks/StreamChat/Repositories/PollsRepository_Mock.swift (1)
19-21: Mock hook for link(pollVote:to:) enables precise assertions.Nit: consider renaming closure to onLink for clarity and to avoid shadowing the method name.
Also applies to: 72-74
Tests/StreamChatTests/Controllers/PollsControllers/PollVoteListController_Tests.swift (1)
214-508: Improve mock restoration and memory management.The event handling tests have two areas for improvement:
Mock restoration: Each test manually restores
client.mockPollsRepository.linkat the end. If a test fails or throws before reaching the restoration line, the mock remains replaced and could affect subsequent tests.Memory management: The tests create new controller instances but don't verify they can be released, unlike other tests in this file (see line 49-51).
Solution 1: Use
deferfor reliable cleanupApply this pattern to each test:
func test_eventsController_didReceiveEvent_PollVoteCastedEvent_withAnswerVote() { // ... setup code ... // Mock the link method to track calls let originalLink = client.mockPollsRepository.link + defer { client.mockPollsRepository.link = originalLink } client.mockPollsRepository.link = { pollVote, query in linkCallCount += 1 linkedVote = pollVote linkedQuery = query } // Simulate receiving the event answerController.eventsController(EventsController(notificationCenter: client.eventNotificationCenter), didReceiveEvent: event) // Verify the vote was linked XCTAssertEqual(linkCallCount, 1) XCTAssertEqual(linkedVote?.id, answerVote.id) XCTAssertEqual(linkedQuery?.pollId, answerQuery.pollId) XCTAssertEqual(linkedQuery?.optionId, answerQuery.optionId) - - // Restore original method - client.mockPollsRepository.link = originalLink }Solution 2: Add memory management verification
For each test that creates a new controller:
func test_eventsController_didReceiveEvent_PollVoteCastedEvent_withAnswerVote() { - // Create a vote list controller for answers (optionId = nil) + var answerController: PollVoteListController? = PollVoteListController( - let answerQuery = PollVoteListQuery(pollId: pollId, optionId: nil) - let answerController = PollVoteListController( - query: answerQuery, + query: PollVoteListQuery(pollId: pollId, optionId: nil), client: client, environment: env ) // ... test code ... + + AssertAsync.canBeReleased(&answerController) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
Sources/StreamChat/Controllers/PollController/PollVoteListController+Combine.swift(3 hunks)Sources/StreamChat/Controllers/PollController/PollVoteListController+SwiftUI.swift(3 hunks)Sources/StreamChat/Controllers/PollController/PollVoteListController.swift(8 hunks)Sources/StreamChatUI/ChatMessageList/Attachments/Poll/PollResultsVC/PollResultsVoteListVC/PollResultsVoteListVC.swift(1 hunks)TestTools/StreamChatTestTools/Mocks/StreamChat/Controllers/PollVoteListController_Mock.swift(1 hunks)TestTools/StreamChatTestTools/Mocks/StreamChat/Repositories/PollsRepository_Mock.swift(2 hunks)Tests/StreamChatTests/Controllers/PollsControllers/PollVoteListController+Combine_Tests.swift(1 hunks)Tests/StreamChatTests/Controllers/PollsControllers/PollVoteListController+SwiftUI_Tests.swift(1 hunks)Tests/StreamChatTests/Controllers/PollsControllers/PollVoteListController_Tests.swift(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
**/*.swift: Respect .swiftlint.yml rules; do not suppress SwiftLint rules broadly—scope and justify any exceptions
Adhere to the project’s zero warnings policy—fix new warnings and avoid introducing any
Files:
Tests/StreamChatTests/Controllers/PollsControllers/PollVoteListController+Combine_Tests.swiftTestTools/StreamChatTestTools/Mocks/StreamChat/Repositories/PollsRepository_Mock.swiftTestTools/StreamChatTestTools/Mocks/StreamChat/Controllers/PollVoteListController_Mock.swiftTests/StreamChatTests/Controllers/PollsControllers/PollVoteListController+SwiftUI_Tests.swiftSources/StreamChat/Controllers/PollController/PollVoteListController+Combine.swiftSources/StreamChat/Controllers/PollController/PollVoteListController+SwiftUI.swiftTests/StreamChatTests/Controllers/PollsControllers/PollVoteListController_Tests.swiftSources/StreamChat/Controllers/PollController/PollVoteListController.swiftSources/StreamChatUI/ChatMessageList/Attachments/Poll/PollResultsVC/PollResultsVoteListVC/PollResultsVoteListVC.swift
Tests/{StreamChatTests,StreamChatUITests}/**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
Tests/{StreamChatTests,StreamChatUITests}/**/*.swift: Add or extend tests in the matching module’s Tests folder
Prefer using provided test fakes/mocks in tests when possible
Files:
Tests/StreamChatTests/Controllers/PollsControllers/PollVoteListController+Combine_Tests.swiftTests/StreamChatTests/Controllers/PollsControllers/PollVoteListController+SwiftUI_Tests.swiftTests/StreamChatTests/Controllers/PollsControllers/PollVoteListController_Tests.swift
Tests/StreamChatTests/**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
Ensure tests cover core models and API surface of StreamChat
Files:
Tests/StreamChatTests/Controllers/PollsControllers/PollVoteListController+Combine_Tests.swiftTests/StreamChatTests/Controllers/PollsControllers/PollVoteListController+SwiftUI_Tests.swiftTests/StreamChatTests/Controllers/PollsControllers/PollVoteListController_Tests.swift
Sources/{StreamChat,StreamChatUI}/**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
When altering public API, update inline documentation comments in source
Files:
Sources/StreamChat/Controllers/PollController/PollVoteListController+Combine.swiftSources/StreamChat/Controllers/PollController/PollVoteListController+SwiftUI.swiftSources/StreamChat/Controllers/PollController/PollVoteListController.swiftSources/StreamChatUI/ChatMessageList/Attachments/Poll/PollResultsVC/PollResultsVoteListVC/PollResultsVoteListVC.swift
🧠 Learnings (2)
📚 Learning: 2025-09-18T10:00:24.878Z
Learnt from: CR
PR: GetStream/stream-chat-swift#0
File: AGENTS.md:0-0
Timestamp: 2025-09-18T10:00:24.878Z
Learning: Applies to Tests/{StreamChatTests,StreamChatUITests}/**/*.swift : Prefer using provided test fakes/mocks in tests when possible
Applied to files:
TestTools/StreamChatTestTools/Mocks/StreamChat/Controllers/PollVoteListController_Mock.swift
📚 Learning: 2025-09-18T10:00:24.878Z
Learnt from: CR
PR: GetStream/stream-chat-swift#0
File: AGENTS.md:0-0
Timestamp: 2025-09-18T10:00:24.878Z
Learning: Applies to Tests/StreamChatTests/**/*.swift : Ensure tests cover core models and API surface of StreamChat
Applied to files:
Tests/StreamChatTests/Controllers/PollsControllers/PollVoteListController_Tests.swift
🧬 Code graph analysis (8)
Tests/StreamChatTests/Controllers/PollsControllers/PollVoteListController+Combine_Tests.swift (2)
Sources/StreamChat/Controllers/PollController/PollVoteListController+Combine.swift (3)
controller(50-52)controller(54-59)controller(61-63)Sources/StreamChat/Controllers/PollController/PollVoteListController.swift (1)
controller(43-46)
TestTools/StreamChatTestTools/Mocks/StreamChat/Repositories/PollsRepository_Mock.swift (1)
Sources/StreamChat/Repositories/PollsRepository.swift (1)
link(303-311)
Tests/StreamChatTests/Controllers/PollsControllers/PollVoteListController+SwiftUI_Tests.swift (3)
Sources/StreamChat/Controllers/PollController/PollVoteListController+Combine.swift (3)
controller(50-52)controller(54-59)controller(61-63)Sources/StreamChat/Controllers/PollController/PollVoteListController+SwiftUI.swift (3)
controller(41-46)controller(48-50)controller(52-54)Sources/StreamChat/Controllers/PollController/PollVoteListController.swift (1)
controller(43-46)
Sources/StreamChat/Controllers/PollController/PollVoteListController+Combine.swift (4)
Sources/StreamChat/Controllers/PollController/PollVoteListController+SwiftUI.swift (3)
controller(41-46)controller(48-50)controller(52-54)Sources/StreamChat/Controllers/PollController/PollVoteListController.swift (1)
controller(43-46)Tests/StreamChatTests/Controllers/PollsControllers/PollVoteListController_Tests.swift (2)
controller(577-580)controller(582-584)Sources/StreamChat/Controllers/PollController/PollController+Combine.swift (3)
controller(56-58)PollController(8-45)PollController.BasePublishers(47-59)
Sources/StreamChat/Controllers/PollController/PollVoteListController+SwiftUI.swift (4)
Sources/StreamChat/Controllers/PollController/PollVoteListController+Combine.swift (3)
controller(50-52)controller(54-59)controller(61-63)Sources/StreamChat/Controllers/PollController/PollVoteListController.swift (1)
controller(43-46)Tests/StreamChatTests/Controllers/PollsControllers/PollVoteListController_Tests.swift (2)
controller(577-580)controller(582-584)Sources/StreamChat/Controllers/PollController/PollController+SwiftUI.swift (1)
controller(48-50)
Tests/StreamChatTests/Controllers/PollsControllers/PollVoteListController_Tests.swift (6)
TestTools/StreamChatTestTools/Mocks/StreamChat/Database/DatabaseSession_Mock.swift (4)
poll(599-601)pollVote(607-609)user(100-102)savePoll(561-563)TestTools/StreamChatTestTools/Mocks/StreamChat/Repositories/PollsRepository_Mock.swift (1)
link(72-74)Sources/StreamChat/Repositories/PollsRepository.swift (1)
link(303-311)Sources/StreamChat/Controllers/PollController/PollVoteListController.swift (2)
eventsController(263-280)controller(43-46)TestTools/StreamChatTestTools/TestData/DummyData/XCTestCase+Dummy.swift (1)
dummyPollPayload(405-451)TestTools/StreamChatTestTools/SpyPattern/Spy/DatabaseContainer_Spy.swift (1)
writeSynchronously(175-182)
Sources/StreamChat/Controllers/PollController/PollVoteListController.swift (8)
Sources/StreamChatUI/ChatMessageList/Attachments/Poll/PollResultsVC/PollResultsVoteListVC/PollResultsVoteListVC.swift (2)
controller(163-168)controller(170-175)Sources/StreamChat/Controllers/PollController/PollVoteListController+Combine.swift (3)
controller(50-52)controller(54-59)controller(61-63)Sources/StreamChat/Controllers/PollController/PollVoteListController+SwiftUI.swift (3)
controller(41-46)controller(48-50)controller(52-54)Sources/StreamChat/Database/DTOs/PollDTO.swift (3)
poll(246-248)fetchRequest(68-74)asModel(88-136)Sources/StreamChat/Controllers/PollController/PollController.swift (1)
startObserversIfNeeded(257-267)Sources/StreamChat/Controllers/DatabaseObserver/BackgroundEntityDatabaseObserver.swift (1)
onChange(60-64)Sources/StreamChat/StateLayer/DatabaseObserver/StateLayerDatabaseObserver.swift (4)
startObserving(86-88)startObserving(97-113)startObserving(171-175)startObserving(184-194)Sources/StreamChat/Repositories/PollsRepository.swift (1)
link(303-311)
Sources/StreamChatUI/ChatMessageList/Attachments/Poll/PollResultsVC/PollResultsVoteListVC/PollResultsVoteListVC.swift (2)
Sources/StreamChat/Controllers/PollController/PollVoteListController.swift (1)
controller(43-46)Tests/StreamChatTests/Controllers/PollsControllers/PollVoteListController_Tests.swift (2)
controller(577-580)controller(582-584)
⏰ 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). (5)
- GitHub Check: Metrics
- GitHub Check: Test LLC (Debug)
- GitHub Check: Build Test App and Frameworks
- GitHub Check: Metrics
- GitHub Check: Build LLC + UI (Xcode 15)
🔇 Additional comments (13)
Sources/StreamChat/Controllers/PollController/PollVoteListController.swift (6)
29-39: New delegate API looks good.Optional default provided; inline docs added; signature consistent with existing delegate style.
Also applies to: 41-47
80-85: Good: observers start when delegates attach.This prevents missing early updates. No action needed.
168-183: synchronize now starts observers first — correct.Ensures local data is ready before remote fetch completes. LGTM.
246-258: Nice DI point for poll observer.Adds testability and flexibility. LGTM.
110-134: Verify initial poll state is surfaced to delegates after startObserving.The
onChangelistener is only triggered byNSFetchedResultsController'scontrollerDidChangeContentdelegate method, which is not called during the initial fetch—only on subsequent mutations. This meansdidUpdatePollwill not be invoked with the initial poll state when observers start.Confirm that either:
- Initial
didUpdatePollcallback is explicitly triggered afterpollObserver?.startObserving()completes (instartObserversIfNeeded), or- The initial poll is accessible immediately without relying on the change listener (and documentation clarifies this).
Without explicit handling, delegates will not be notified of the poll until the first update occurs, which may cause UI to remain uninitialized.
271-279: The first concern about nil isAnswer is incorrect; however, PollVoteRemovedEvent handling is a valid gap worth clarifying.
isAnswerispublic let isAnswer: Bool(non-optional), not nullable—the code correctly handles bothtrueandfalsecases.- PollVoteRemovedEvent exists but is intentionally not processed in the event handler; no
unlinkmethod exists inPollsRepositoryto reverse linking. Verify whether removal events should trigger unlink or are handled by a different mechanism.TestTools/StreamChatTestTools/Mocks/StreamChat/Controllers/PollVoteListController_Mock.swift (1)
19-22: Mock poll override is correct and minimal.Keeps tests simple; name is clear.
Tests/StreamChatTests/Controllers/PollsControllers/PollVoteListController+SwiftUI_Tests.swift (1)
70-89: LGTM! Poll observation tests follow established patterns.The two new tests correctly verify:
- Initial
pollvalue is nil- Observable object reacts to
didUpdatePolldelegate callbacksThe test structure matches the existing patterns for
votesandstatetests in this file.Sources/StreamChatUI/ChatMessageList/Attachments/Poll/PollResultsVC/PollResultsVoteListVC/PollResultsVoteListVC.swift (1)
163-175: LGTM! Delegate methods correctly update the UI.Both delegate implementations properly handle updates:
didChangeVotescreates a fresh snapshot with all votes and applies it with animationdidUpdatePollupdates the local poll reference and reloads the relevant sectionThe diffable data source usage is correct and follows UIKit best practices.
Sources/StreamChat/Controllers/PollController/PollVoteListController+SwiftUI.swift (1)
21-22: LGTM! Poll property integration follows established patterns.The implementation correctly:
- Declares
@Published pollproperty onObservableObject- Initializes from
controller.pollduring setup- Updates in response to
didUpdatePolldelegate callbacksThis matches the existing pattern for
votesandstateproperties.Also applies to: 35-35, 48-50
Tests/StreamChatTests/Controllers/PollsControllers/PollVoteListController_Tests.swift (3)
214-508: Excellent test coverage for event handling.The event handling tests comprehensively verify:
- Both
PollVoteCastedEventandPollVoteChangedEvent- Both answer votes (optionId nil) and regular votes
- All ignore scenarios (mismatched pollId, optionId, or vote type)
The test logic correctly validates that votes are linked only when:
- Answer votes:
isAnswer == trueANDquery.optionId == nilAND pollIds match- Regular votes:
isAnswer == falseAND optionIds match AND pollIds match
510-568: LGTM! Poll observer tests are comprehensive.The poll observer tests correctly verify:
pollproperty returns the correct poll from the database after synchronizationpollproperty returnsnilwhen no poll exists- Delegate receives
didUpdatePollcallback when poll changes in the databaseThe use of
AssertAsync.willBeTruein the third test properly handles the asynchronous nature of database observer notifications.
573-585: LGTM! Test delegate is well-implemented.The
TestDelegatehelper class:
- Uses
@Atomicfor thread-safe tracking of async delegate calls- Clearly documents that
didChangeVotesis not used in these tests- Provides a clean, focused helper for testing poll update notifications
Sources/StreamChat/Controllers/PollController/PollVoteListController+Combine.swift
Outdated
Show resolved
Hide resolved
SDK Performance
|
Sources/StreamChat/Controllers/PollController/PollVoteListController+Combine.swift
Outdated
Show resolved
Hide resolved
Public Interface+ public extension PollVoteListControllerDelegate
open class PollResultsVoteListVC: _ViewController, ThemeProvider, PollVoteListControllerDelegate, UITableViewDelegate, GroupedSectionListStyling
- public func controller(_ controller: PollVoteListController,didChangeVotes changes: [ListChange<PollVote>])
+ open func loadMoreVotes()
- open func loadMoreVotes()
+ open func didFinishLoadingMoreVotes(with error: Error?)
- open func didFinishLoadingMoreVotes(with error: Error?)
+ public func controller(_ controller: PollVoteListController,didChangeVotes changes: [ListChange<PollVote>])
+ public func controller(_ controller: PollVoteListController,didUpdatePoll poll: Poll)
extension PollVoteListController.ObservableObject: PollVoteListControllerDelegate
- public func controller(_ controller: DataController,didChangeState state: DataController.State)
+ public func controller(_ controller: PollVoteListController,didUpdatePoll poll: Poll)
+ public func controller(_ controller: DataController,didChangeState state: DataController.State)
public class PollVoteListController: DataController, DelegateCallable, DataStoreProvider
- public private var hasLoadedAllVotes: Bool
+ public var poll: Poll?
- public weak var delegate: PollVoteListControllerDelegate?
+ public private var hasLoadedAllVotes: Bool
-
+ public weak var delegate: PollVoteListControllerDelegate?
-
+
- override public func synchronize(_ completion: ((_ error: Error?) -> Void)? = nil)
+
- public func loadMoreVotes(limit: Int? = nil,completion: ((Error?) -> Void)? = nil)
+ override public func synchronize(_ completion: ((_ error: Error?) -> Void)? = nil)
+ public func loadMoreVotes(limit: Int? = nil,completion: ((Error?) -> Void)? = nil) |
SDK Size
|
StreamChat XCSize
|
StreamChatUI XCSize
|
|
nuno-vieira
deleted the
fix/poll-vote-list-controller-not-live-updated
branch
3 participants
🔗 Issue Links
https://linear.app/stream/issue/IOS-1120
🎯 Goal
Fix the poll's vote list not reacting to poll events.
📝 Summary
PollVoteListControllerPollVoteListControllernot updating votes on the vote cast eventPollResultsVoteListVCnot updating the vote countTODO: SwiftUI PR as well
🛠 Implementation
Whenever there was a new vote, the vote was not linked to the query. This is now fixed in the
PollVoteListController. Besides this, in thePollResultsVoteListVC, we were not reloading the header whenever the poll changed, which is responsible for rendering the number of votes.🧪 Manual Testing Notes
☑️ Contributor Checklist
docs-contentrepoSummary by CodeRabbit
New Features
Bug Fixes
Tests