Update subscriptions if visible range rooms change without the range itself changing (#3220)

* Remove room list range deduplication and allow subscribing to rooms when then update to come into the visible range.

* Fixes #3088 - Update subscriptions if visible range rooms change without the range itself changing
- we switched from range based subscribing with duplicate suppression to id based, but ids can change without ranges changing
- the all rooms list fetches at least 1 event per rooms and can cause rooms to come in and out of the visible range without user interaction

ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift

@@ -40,7 +40,7 @@ enum HomeScreenViewAction {
     case startChat
     case confirmRecoveryKey
     case skipRecoveryKeyConfirmation
-    case updateVisibleItemRange(range: Range<Int>, isScrolling: Bool)
+    case updateVisibleItemRange(Range<Int>)
     case globalSearch
     case markRoomAsUnread(roomIdentifier: String)
     case markRoomAsRead(roomIdentifier: String)

ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift

@@ -30,9 +30,6 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol
     
     private var migrationCancellable: AnyCancellable?
     
-    private var visibleItemRangeObservationToken: AnyCancellable?
-    private let visibleItemRangePublisher = CurrentValueSubject<(range: Range<Int>, isScrolling: Bool), Never>((0..<0, false))
-    
     private var actionsSubject: PassthroughSubject<HomeScreenViewModelAction, Never> = .init()
     var actions: AnyPublisher<HomeScreenViewModelAction, Never> {
         actionsSubject.eraseToAnyPublisher()
@@ -151,8 +148,8 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol
             actionsSubject.send(.presentSecureBackupSettings)
         case .skipRecoveryKeyConfirmation:
             state.securityBannerMode = .dismissed
-        case .updateVisibleItemRange(let range, let isScrolling):
+        case .updateVisibleItemRange(let range):
-            visibleItemRangePublisher.send((range, isScrolling))
+            roomSummaryProvider?.updateVisibleRange(range)
         case .startChat:
             actionsSubject.send(.presentStartChatScreen)
         case .globalSearch:
@@ -266,14 +263,9 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol
             .store(in: &cancellables)
         
         roomSummaryProvider.roomListPublisher
-            .dropFirst(1) // We don't care about its initial value
             .receive(on: DispatchQueue.main)
             .sink { [weak self] _ in
                 self?.updateRooms()
-                
-                // Wait for the all rooms view to receive its first update before installing
-                // dynamic timeline modifiers
-                self?.installListRangeModifiers()
             }
             .store(in: &cancellables)
     }
@@ -315,27 +307,6 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol
             }
         }
     }
-    
-    private func installListRangeModifiers() {
-        guard visibleItemRangeObservationToken == nil else {
-            return
-        }
-        
-        visibleItemRangeObservationToken = visibleItemRangePublisher
-            .filter { $0.isScrolling == false }
-            .throttle(for: 0.5, scheduler: DispatchQueue.main, latest: true)
-            .removeDuplicates(by: { $0.isScrolling == $1.isScrolling && $0.range == $1.range })
-            .sink { [weak self] value in
-                guard let self else { return }
-                
-                // Ignore scrolling while filtering rooms
-                guard self.state.bindings.searchQuery.isEmpty else {
-                    return
-                }
-                
-                self.updateVisibleRange(value.range)
-            }
-    }
         
     private func updateRooms() {
         guard let roomSummaryProvider else {
@@ -352,20 +323,7 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol
         
         state.rooms = rooms
     }
-    
-    private func updateVisibleRange(_ range: Range<Int>) {
-        guard !range.isEmpty else {
-            return
-        }
         
-        guard let roomSummaryProvider else {
-            MXLog.error("Visible rooms summary provider unavailable")
-            return
-        }
-        
-        roomSummaryProvider.updateVisibleRange(range)
-    }
-    
     private func markRoomAsFavourite(_ roomID: String, isFavourite: Bool) async {
         guard case let .joined(roomProxy) = await userSession.clientProxy.roomForIdentifier(roomID) else {
             MXLog.error("Failed retrieving room for identifier: \(roomID)")

ElementX/Sources/Screens/HomeScreen/View/HomeScreenContent.swift

@@ -200,6 +200,8 @@ struct HomeScreenContent: View {
     
     private func delayedUpdateVisibleRange() {
         guard let scrollView = scrollViewAdapter.scrollView,
+              scrollViewAdapter.isScrolling.value == false, // Ignore while scrolling
+              context.searchQuery.isEmpty == true, // Ignore while filtering
               context.viewState.visibleRooms.count > 0 else {
             return
         }
@@ -215,6 +217,6 @@ struct HomeScreenContent: View {
         let lastIndex = Int(max(0.0, scrollView.contentOffset.y + scrollView.bounds.height) / cellHeight)
         
         // This will be deduped and throttled on the view model layer
-        context.send(viewAction: .updateVisibleItemRange(range: firstIndex..<lastIndex, isScrolling: scrollViewAdapter.isScrolling.value))
+        context.send(viewAction: .updateVisibleItemRange(firstIndex..<lastIndex))
     }
 }

ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProvider.swift

@@ -30,6 +30,8 @@ class RoomSummaryProvider: RoomSummaryProviderProtocol {
     
     private let serialDispatchQueue: DispatchQueue
     
+    private let visibleItemRangePublisher = CurrentValueSubject<Range<Int>, Never>(0..<0)
+    
     // periphery:ignore - retaining purpose
     private var roomList: RoomListProtocol?
     
@@ -80,6 +82,8 @@ class RoomSummaryProvider: RoomSummaryProviderProtocol {
             .sink { [weak self] in self?.updateRoomsWithDiffs($0) }
             .store(in: &cancellables)
         
+        setupVisibleRangeObservers()
+        
         setupNotificationSettingsSubscription()
     }
     
@@ -114,38 +118,7 @@ class RoomSummaryProvider: RoomSummaryProviderProtocol {
     }
     
     func updateVisibleRange(_ range: Range<Int>) {
-        if range.upperBound >= rooms.count {
+        visibleItemRangePublisher.send(range)
-            listUpdatesSubscriptionResult?.controller().addOnePage()
-        } else if range.lowerBound == 0 {
-            listUpdatesSubscriptionResult?.controller().resetToOnePage()
-        }
-        
-        guard shouldUpdateVisibleRange else {
-            return
-        }
-        
-        // The scroll view content size based visible range calculations might create large ranges
-        // This is just a safety check to not overload the backend
-        var range = range
-        if range.upperBound - range.lowerBound > SlidingSyncConstants.maximumVisibleRangeSize {
-            let upperBound = range.lowerBound + SlidingSyncConstants.maximumVisibleRangeSize
-            range = range.lowerBound..<upperBound
-        }
-
-        MXLog.info("\(name): Requesting subscriptions for visible range: \(range)")
-        
-        let roomIDs = range
-            .filter { $0 < rooms.count }
-            .map { rooms[$0].id }
-        
-        do {
-            try roomListService.subscribeToRooms(roomIds: roomIDs,
-                                                 settings: .init(requiredState: SlidingSyncConstants.defaultRequiredState,
-                                                                 timelineLimit: SlidingSyncConstants.defaultTimelineLimit,
-                                                                 includeHeroes: false))
-        } catch {
-            MXLog.error("Failed subscribing to rooms with error: \(error)")
-        }
     }
     
     func setFilter(_ filter: RoomSummaryProviderFilter) {
@@ -167,7 +140,63 @@ class RoomSummaryProvider: RoomSummaryProviderProtocol {
     }
     
     // MARK: - Private
+    
+    private func setupVisibleRangeObservers() {
+        visibleItemRangePublisher
+            .throttle(for: 0.5, scheduler: DispatchQueue.main, latest: true)
+            .removeDuplicates()
+            .sink { [weak self] range in
+                guard let self else { return }
+                
+                MXLog.info("\(self.name): Updating visible range: \(range)")
+                
+                if range.upperBound >= rooms.count {
+                    listUpdatesSubscriptionResult?.controller().addOnePage()
+                } else if range.lowerBound == 0 {
+                    listUpdatesSubscriptionResult?.controller().resetToOnePage()
+                }
+            }
+            .store(in: &cancellables)
         
+        visibleItemRangePublisher
+            .throttle(for: 0.5, scheduler: DispatchQueue.main, latest: true)
+            .filter { [weak self] range in
+                guard let self else { return false }
+                return !range.isEmpty && shouldUpdateVisibleRange
+            }
+            .compactMap { [weak self] (range: Range) -> [String]? in
+                guard let self else { return nil }
+                
+                // The scroll view content size based visible range calculations might create large ranges
+                // This is just a safety check to not overload the backend
+                var range = range
+                if range.upperBound - range.lowerBound > SlidingSyncConstants.maximumVisibleRangeSize {
+                    let upperBound = range.lowerBound + SlidingSyncConstants.maximumVisibleRangeSize
+                    range = range.lowerBound..<upperBound
+                }
+                
+                return range
+                    .filter { $0 < self.rooms.count }
+                    .map { self.rooms[$0].id }
+            }
+            .removeDuplicates()
+            .sink { [weak self] roomIDs in
+                guard let self else { return }
+                
+                MXLog.debug("\(self.name): Updating subscriptions for visible rooms: \(roomIDs)")
+                
+                do {
+                    try roomListService.subscribeToRooms(roomIds: roomIDs,
+                                                         settings: .init(requiredState: SlidingSyncConstants.defaultRequiredState,
+                                                                         timelineLimit: SlidingSyncConstants.defaultTimelineLimit,
+                                                                         includeHeroes: false))
+                } catch {
+                    MXLog.error("Failed subscribing to rooms with error: \(error)")
+                }
+            }
+            .store(in: &cancellables)
+    }
+    
     fileprivate func updateRoomsWithDiffs(_ diffs: [RoomListEntriesUpdate]) {
         let span = MXLog.createSpan("\(name).process_room_list_diffs")
         span.enter()