mirror of
https://github.com/Novaloop-AG/element-x-ios.git
synced 2026-10-31 16:24:11 +00:00
Update room suggestions and search when space membership changes (#6316)
* Avoid repeated space membership scans in room search Signed-off-by: Ivan Magda <imagda15@gmail.com> * Refresh room candidates when space membership changes Keep known space room IDs in a set updated by the room publisher and by successful additions. Filter both suggestions and search results through that set, retaining exclusions while the space list resets and reloads. Preserve the initial recent-room eligibility filter so existing children do not consume the five suggestion slots. Cover live membership updates and partial-save filtering in both candidate lists. --------- Signed-off-by: Ivan Magda <imagda15@gmail.com>
This commit is contained in:
+21
-16
@@ -17,9 +17,8 @@ class SpaceAddRoomsScreenViewModel: SpaceAddRoomsScreenViewModelType, SpaceAddRo
|
||||
private let userIndicatorController: UserIndicatorControllerProtocol
|
||||
|
||||
private var suggestedRooms: [SpaceAddRoomsScreenRoom] = []
|
||||
/// A place to track rooms that were sucessfully added to the space in order to filter them out from
|
||||
/// the roomsSection when a failure occurs part way through the array.
|
||||
private var addedRoomIDs: Set<String> = []
|
||||
/// Keep known children excluded while the space list resets and reloads after saving.
|
||||
private var existingRoomIDs: Set<String>
|
||||
|
||||
private var actionsSubject: PassthroughSubject<SpaceAddRoomsScreenViewModelAction, Never> = .init()
|
||||
var actions: AnyPublisher<SpaceAddRoomsScreenViewModelAction, Never> {
|
||||
@@ -31,6 +30,7 @@ class SpaceAddRoomsScreenViewModel: SpaceAddRoomsScreenViewModelType, SpaceAddRo
|
||||
roomSummaryProvider: RoomSummaryProviderProtocol,
|
||||
userIndicatorController: UserIndicatorControllerProtocol) {
|
||||
self.spaceRoomListProxy = spaceRoomListProxy
|
||||
existingRoomIDs = Set(spaceRoomListProxy.spaceRoomsPublisher.value.map(\.id))
|
||||
spaceServiceProxy = userSession.clientProxy.spaceService
|
||||
self.roomSummaryProvider = roomSummaryProvider
|
||||
self.userIndicatorController = userIndicatorController
|
||||
@@ -38,6 +38,15 @@ class SpaceAddRoomsScreenViewModel: SpaceAddRoomsScreenViewModelType, SpaceAddRo
|
||||
super.init(initialViewState: SpaceAddRoomsScreenViewState(roomsSection: .init(type: .suggestions, rooms: [])),
|
||||
mediaProvider: userSession.mediaProvider)
|
||||
|
||||
spaceRoomListProxy.spaceRoomsPublisher
|
||||
.receive(on: DispatchQueue.main)
|
||||
.sink { [weak self] spaceRooms in
|
||||
guard let self else { return }
|
||||
existingRoomIDs.formUnion(spaceRooms.map(\.id))
|
||||
updateRooms()
|
||||
}
|
||||
.store(in: &cancellables)
|
||||
|
||||
roomSummaryProvider.roomListPublisher
|
||||
.receive(on: DispatchQueue.main)
|
||||
.sink { [weak self] _ in
|
||||
@@ -45,20 +54,18 @@ class SpaceAddRoomsScreenViewModel: SpaceAddRoomsScreenViewModelType, SpaceAddRo
|
||||
}
|
||||
.store(in: &cancellables)
|
||||
|
||||
Task {
|
||||
let existingRooms = spaceRoomListProxy.spaceRoomsPublisher.value
|
||||
Task { [existingRoomIDs] in
|
||||
// Filter before the recent-room limit so existing children do not displace suggestions.
|
||||
suggestedRooms = await userSession.clientProxy
|
||||
.recentlyVisitedRooms { @MainActor roomProxy in
|
||||
!roomProxy.infoPublisher.value.isDirect
|
||||
&& !roomProxy.infoPublisher.value.isSpace
|
||||
&& roomProxy.infoPublisher.value.membership == .joined
|
||||
&& !existingRooms.contains { $0.id == roomProxy.id }
|
||||
&& !existingRoomIDs.contains(roomProxy.id)
|
||||
}
|
||||
.map { .init(roomProxy: $0) }
|
||||
|
||||
if state.roomsSection.type == .suggestions {
|
||||
state.roomsSection = .init(type: .suggestions, rooms: suggestedRooms)
|
||||
}
|
||||
updateRooms()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -90,16 +97,14 @@ class SpaceAddRoomsScreenViewModel: SpaceAddRoomsScreenViewModelType, SpaceAddRo
|
||||
|
||||
private func updateRooms() {
|
||||
guard !state.bindings.searchQuery.isEmpty else {
|
||||
state.roomsSection = .init(type: .suggestions, rooms: suggestedRooms)
|
||||
state.roomsSection = .init(type: .suggestions, rooms: suggestedRooms.filter { !existingRoomIDs.contains($0.id) })
|
||||
return
|
||||
}
|
||||
|
||||
let existingRooms = spaceRoomListProxy.spaceRoomsPublisher.value
|
||||
|
||||
let rooms = roomSummaryProvider.roomListPublisher.value
|
||||
.lazy
|
||||
.filter { [addedRoomIDs] summary in
|
||||
!summary.isDirect && !existingRooms.contains { $0.id == summary.id } && !addedRoomIDs.contains(summary.id)
|
||||
.filter { [existingRoomIDs] summary in
|
||||
!summary.isDirect && !existingRoomIDs.contains(summary.id)
|
||||
}
|
||||
.map(SpaceAddRoomsScreenRoom.init)
|
||||
|
||||
@@ -142,13 +147,13 @@ class SpaceAddRoomsScreenViewModel: SpaceAddRoomsScreenViewModelType, SpaceAddRo
|
||||
for room in state.selectedRooms {
|
||||
switch await spaceServiceProxy.addChild(room.id, to: spaceRoomListProxy.id) {
|
||||
case .success:
|
||||
addedRoomIDs.insert(room.id)
|
||||
existingRoomIDs.insert(room.id)
|
||||
case .failure(let error):
|
||||
MXLog.error("Failed adding room to space: \(error)")
|
||||
showErrorIndicator()
|
||||
|
||||
// Hide rooms that were successfully added.
|
||||
state.selectedRooms.removeAll { addedRoomIDs.contains($0.id) }
|
||||
state.selectedRooms.removeAll { existingRoomIDs.contains($0.id) }
|
||||
updateRooms()
|
||||
|
||||
return
|
||||
|
||||
@@ -25,7 +25,7 @@ struct SpaceAddRoomsScreenViewModelTests {
|
||||
spaceRoomListProxy = SpaceRoomListProxyMock(.init(spaceServiceRoom: SpaceServiceRoom.mock(isSpace: true)))
|
||||
|
||||
let clientProxy = ClientProxyMock(.init())
|
||||
clientProxy.recentlyVisitedRoomsFilterReturnValue = .init(repeating: JoinedRoomProxyMock(.init()), count: 5)
|
||||
clientProxy.recentlyVisitedRoomsFilterReturnValue = (0..<5).map { JoinedRoomProxyMock(.init(id: "suggestion-\($0)")) }
|
||||
spaceServiceProxy = clientProxy.spaceService as? SpaceServiceProxyMock ?? SpaceServiceProxyMock(.init())
|
||||
|
||||
viewModel = SpaceAddRoomsScreenViewModel(spaceRoomListProxy: spaceRoomListProxy,
|
||||
@@ -63,21 +63,23 @@ struct SpaceAddRoomsScreenViewModelTests {
|
||||
#expect(spaceRoomListProxy.resetCalled, "The room list should be reset to pick up the changes.")
|
||||
}
|
||||
|
||||
@Test
|
||||
func failureWithMultipleRoomsSelected() async throws {
|
||||
// Given a view model with 4 selected rooms.
|
||||
@Test(arguments: [true, false])
|
||||
func failureWithMultipleRoomsSelected(search: Bool) async throws {
|
||||
let initialCount = search ? 4 : 5
|
||||
let sectionType: SpaceAddRoomsScreenViewState.Section.SectionType = search ? .searchResults : .suggestions
|
||||
// Given a view model with several selected rooms.
|
||||
var deferred = deferFulfillment(context.observe(\.viewState.roomsSection),
|
||||
message: "There should be 4 search results.") { section in
|
||||
section.type == .searchResults && section.rooms.count == 4
|
||||
message: "The candidate rooms should be shown.") { section in
|
||||
section.type == sectionType && section.rooms.count == initialCount
|
||||
}
|
||||
context.searchQuery = "f"
|
||||
context.searchQuery = search ? "f" : ""
|
||||
context.send(viewAction: .searchQueryChanged)
|
||||
try await deferred.fulfill()
|
||||
|
||||
for room in context.viewState.roomsSection.rooms {
|
||||
context.send(viewAction: .toggleRoom(room))
|
||||
}
|
||||
#expect(context.viewState.selectedRooms.count == 4, "All of the rooms should be selected.")
|
||||
#expect(context.viewState.selectedRooms.count == initialCount, "All of the rooms should be selected.")
|
||||
|
||||
// When there's a failure half way through saving.
|
||||
let successfulIDs = context.viewState.roomsSection.rooms.map(\.id).prefix(2)
|
||||
@@ -90,8 +92,8 @@ struct SpaceAddRoomsScreenViewModelTests {
|
||||
}
|
||||
|
||||
deferred = deferFulfillment(context.observe(\.viewState.roomsSection),
|
||||
message: "The search results should update.") { section in
|
||||
section.type == .searchResults && section.rooms.count == 2
|
||||
message: "The candidates should update after a partial save.") { section in
|
||||
section.type == sectionType && section.rooms.count == initialCount - 2
|
||||
}
|
||||
context.send(viewAction: .save)
|
||||
try await deferred.fulfill()
|
||||
@@ -103,4 +105,50 @@ struct SpaceAddRoomsScreenViewModelTests {
|
||||
#expect(!context.viewState.roomsSection.rooms.contains { successfulIDs.contains($0.id) },
|
||||
"The added rooms should no longer be listed for selection.")
|
||||
}
|
||||
|
||||
@Test(arguments: [true, false])
|
||||
func spaceMembershipUpdatesCandidates(search: Bool) async throws {
|
||||
let summaries = [RoomSummary.mock(id: "first", name: "First room"),
|
||||
.mock(id: "existing", name: "Existing room"),
|
||||
.mock(id: "second", name: "Second room")]
|
||||
let summariesSubject = CurrentValueSubject<[RoomSummary], Never>(summaries)
|
||||
let summaryProvider = RoomSummaryProviderMock(.init(state: .loaded([])))
|
||||
summaryProvider.roomListPublisher = summariesSubject.asCurrentValuePublisher()
|
||||
let spaceRoomsSubject = CurrentValueSubject<[SpaceServiceRoom], Never>([.mock(id: "existing", isSpace: false)])
|
||||
spaceRoomListProxy.spaceRoomsPublisher = spaceRoomsSubject.asCurrentValuePublisher()
|
||||
let clientProxy = ClientProxyMock(.init())
|
||||
let recentRooms = summaries.map { JoinedRoomProxyMock(.init(id: $0.id)) }
|
||||
clientProxy.recentlyVisitedRoomsFilterClosure = { filter in
|
||||
var rooms: [JoinedRoomProxyProtocol] = []
|
||||
for room in recentRooms where await filter(room) {
|
||||
rooms.append(room)
|
||||
}
|
||||
return rooms
|
||||
}
|
||||
let viewModel = SpaceAddRoomsScreenViewModel(spaceRoomListProxy: spaceRoomListProxy,
|
||||
userSession: UserSessionMock(.init(clientProxy: clientProxy)),
|
||||
roomSummaryProvider: summaryProvider,
|
||||
userIndicatorController: UserIndicatorControllerMock())
|
||||
let context = viewModel.context
|
||||
context.searchQuery = search ? "room" : ""
|
||||
var deferred = deferFulfillment(context.observe(\.viewState.roomsSection)) {
|
||||
$0.rooms.map(\.id) == ["first", "second"]
|
||||
}
|
||||
try await deferred.fulfill()
|
||||
|
||||
deferred = deferFulfillment(context.observe(\.viewState.roomsSection)) {
|
||||
$0.rooms.map(\.id) == ["second"]
|
||||
}
|
||||
spaceRoomsSubject.send([.mock(id: "existing", isSpace: false), .mock(id: "first", isSpace: false)])
|
||||
try await deferred.fulfill()
|
||||
|
||||
// An empty snapshot during reload must not make known children selectable again.
|
||||
deferred = deferFulfillment(context.observe(\.viewState.roomsSection)) {
|
||||
$0.type == .searchResults && $0.rooms.map(\.id) == ["second", "third"]
|
||||
}
|
||||
spaceRoomsSubject.send([])
|
||||
context.searchQuery = "room"
|
||||
summariesSubject.send(summaries + [.mock(id: "third", name: "Third room")])
|
||||
try await deferred.fulfill()
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user