From 1ba8d29333bce727dd6e54846ce949459914c4a3 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 28 Jul 2022 10:10:01 +0100 Subject: [PATCH 1/6] fixing rooms from other spaces being included in home due to wrong filter param - we were passing null which meant no filter was being applied --- .../app/features/home/room/list/RoomListSectionBuilder.kt | 4 ++-- .../app/features/home/room/list/home/HomeRoomListViewModel.kt | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/home/room/list/RoomListSectionBuilder.kt b/vector/src/main/java/im/vector/app/features/home/room/list/RoomListSectionBuilder.kt index f929366a5e..46591a0ca6 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/list/RoomListSectionBuilder.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/list/RoomListSectionBuilder.kt @@ -378,7 +378,7 @@ class RoomListSectionBuilder( activeSpaceUpdaters.add(object : RoomListViewModel.ActiveSpaceQueryUpdater { override fun updateForSpaceId(roomId: String?) { filteredPagedRoomSummariesLive.queryParams = roomQueryParams.copy( - spaceFilter = roomId?.toActiveSpaceOrOrphanRooms() + spaceFilter = roomId.toActiveSpaceOrOrphanRooms() ) liveQueryParams.update { filteredPagedRoomSummariesLive.queryParams } } @@ -444,7 +444,7 @@ class RoomListSectionBuilder( return when (spaceFilter) { RoomListViewModel.SpaceFilterStrategy.ORPHANS_IF_SPACE_NULL -> { copy( - spaceFilter = currentSpace?.toActiveSpaceOrOrphanRooms() + spaceFilter = currentSpace.toActiveSpaceOrOrphanRooms() ) } RoomListViewModel.SpaceFilterStrategy.ALL_IF_SPACE_NULL -> { diff --git a/vector/src/main/java/im/vector/app/features/home/room/list/home/HomeRoomListViewModel.kt b/vector/src/main/java/im/vector/app/features/home/room/list/home/HomeRoomListViewModel.kt index 3226ed24f2..55d955cb37 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/list/home/HomeRoomListViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/list/home/HomeRoomListViewModel.kt @@ -124,7 +124,7 @@ class HomeRoomListViewModel @AssistedInject constructor( private fun getSpaceFilter(selectedSpaceId: String?, strategy: RoomListViewModel.SpaceFilterStrategy): SpaceFilter? { return when (strategy) { RoomListViewModel.SpaceFilterStrategy.ORPHANS_IF_SPACE_NULL -> { - selectedSpaceId?.toActiveSpaceOrOrphanRooms() + selectedSpaceId.toActiveSpaceOrOrphanRooms() } RoomListViewModel.SpaceFilterStrategy.ALL_IF_SPACE_NULL -> { selectedSpaceId?.let { SpaceFilter.ActiveSpace(it) } From e8476882fca97d8c470767025f91502272d5fb9a Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 28 Jul 2022 10:22:11 +0100 Subject: [PATCH 2/6] providing a type for the NoFilter to avoid the ambiguity of the nullable param --- .../org/matrix/android/sdk/api/query/SpaceFilter.kt | 10 ++++++++++ .../sdk/api/session/room/RoomSummaryQueryParams.kt | 4 ++-- .../session/room/summary/RoomSummaryDataSource.kt | 2 +- .../im/vector/app/features/home/HomeDetailViewModel.kt | 5 +++-- .../app/features/home/UnreadMessagesSharedViewModel.kt | 2 +- .../features/home/room/list/RoomListSectionBuilder.kt | 5 +++-- .../home/room/list/home/HomeRoomListViewModel.kt | 7 ++++--- .../vector/app/features/spaces/SpaceListViewModel.kt | 4 ++-- 8 files changed, 26 insertions(+), 13 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/query/SpaceFilter.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/query/SpaceFilter.kt index 6383412ffb..ccefd5855f 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/query/SpaceFilter.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/query/SpaceFilter.kt @@ -35,9 +35,19 @@ sealed interface SpaceFilter { * Used to get all the rooms that do not have the provided space in their parent hierarchy. */ data class ExcludeSpace(val spaceId: String) : SpaceFilter + + /** + * Used to apply no filtering to the space. + */ + object NoFilter : SpaceFilter } /** * Return a [SpaceFilter.ActiveSpace] if the String is not null, or [SpaceFilter.OrphanRooms]. */ fun String?.toActiveSpaceOrOrphanRooms(): SpaceFilter = this?.let { SpaceFilter.ActiveSpace(it) } ?: SpaceFilter.OrphanRooms + +/** + * Return a [SpaceFilter.ActiveSpace] if the String is not null, or [SpaceFilter.NoFilter]. + */ +fun String?.toActiveSpaceOrNoFilter(): SpaceFilter = this?.let { SpaceFilter.ActiveSpace(it) } ?: SpaceFilter.NoFilter diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/RoomSummaryQueryParams.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/RoomSummaryQueryParams.kt index 00c6da00b7..60963ef25a 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/RoomSummaryQueryParams.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/RoomSummaryQueryParams.kt @@ -86,7 +86,7 @@ data class RoomSummaryQueryParams( /** * Used to filter room using the current space. */ - val spaceFilter: SpaceFilter?, + val spaceFilter: SpaceFilter, ) { /** @@ -101,7 +101,7 @@ data class RoomSummaryQueryParams( var roomTagQueryFilter: RoomTagQueryFilter? = null var excludeType: List? = listOf(RoomType.SPACE) var includeType: List? = null - var spaceFilter: SpaceFilter? = null + var spaceFilter: SpaceFilter = SpaceFilter.NoFilter fun build() = RoomSummaryQueryParams( displayName = displayName, diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/summary/RoomSummaryDataSource.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/summary/RoomSummaryDataSource.kt index 9d14ebffdd..82fc94df7c 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/summary/RoomSummaryDataSource.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/summary/RoomSummaryDataSource.kt @@ -317,7 +317,7 @@ internal class RoomSummaryDataSource @Inject constructor( is SpaceFilter.ExcludeSpace -> { query.not().contains(RoomSummaryEntityFields.FLATTEN_PARENT_IDS, queryParams.spaceFilter.spaceId) } - null -> Unit // nop + SpaceFilter.NoFilter -> Unit // nop } return query diff --git a/vector/src/main/java/im/vector/app/features/home/HomeDetailViewModel.kt b/vector/src/main/java/im/vector/app/features/home/HomeDetailViewModel.kt index 787458993f..6471eb5141 100644 --- a/vector/src/main/java/im/vector/app/features/home/HomeDetailViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/HomeDetailViewModel.kt @@ -46,6 +46,7 @@ import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.launch import org.matrix.android.sdk.api.query.RoomCategoryFilter import org.matrix.android.sdk.api.query.SpaceFilter +import org.matrix.android.sdk.api.query.toActiveSpaceOrNoFilter import org.matrix.android.sdk.api.query.toActiveSpaceOrOrphanRooms import org.matrix.android.sdk.api.session.Session import org.matrix.android.sdk.api.session.crypto.NewSessionListener @@ -235,7 +236,7 @@ class HomeDetailViewModel @AssistedInject constructor( roomSummaryQueryParams { memberships = listOf(Membership.INVITE) roomCategoryFilter = RoomCategoryFilter.ONLY_DM - spaceFilter = activeSpaceRoomId?.let { SpaceFilter.ActiveSpace(it) } + spaceFilter = activeSpaceRoomId.toActiveSpaceOrNoFilter() } ).size @@ -252,7 +253,7 @@ class HomeDetailViewModel @AssistedInject constructor( roomSummaryQueryParams { memberships = listOf(Membership.JOIN) roomCategoryFilter = RoomCategoryFilter.ONLY_DM - spaceFilter = activeSpaceRoomId?.let { SpaceFilter.ActiveSpace(it) } + spaceFilter = activeSpaceRoomId.toActiveSpaceOrNoFilter() } ) diff --git a/vector/src/main/java/im/vector/app/features/home/UnreadMessagesSharedViewModel.kt b/vector/src/main/java/im/vector/app/features/home/UnreadMessagesSharedViewModel.kt index f2e6b7e064..3ce425ba6f 100644 --- a/vector/src/main/java/im/vector/app/features/home/UnreadMessagesSharedViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/UnreadMessagesSharedViewModel.kt @@ -144,7 +144,7 @@ class UnreadMessagesSharedViewModel @AssistedInject constructor( this.memberships = listOf(Membership.JOIN) this.spaceFilter = SpaceFilter.OrphanRooms.takeIf { !vectorPreferences.prefSpacesShowAllRoomInHome() - } + } ?: SpaceFilter.NoFilter } ) diff --git a/vector/src/main/java/im/vector/app/features/home/room/list/RoomListSectionBuilder.kt b/vector/src/main/java/im/vector/app/features/home/room/list/RoomListSectionBuilder.kt index 46591a0ca6..b2187fb365 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/list/RoomListSectionBuilder.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/list/RoomListSectionBuilder.kt @@ -45,6 +45,7 @@ import org.matrix.android.sdk.api.extensions.tryOrNull import org.matrix.android.sdk.api.query.RoomCategoryFilter import org.matrix.android.sdk.api.query.RoomTagQueryFilter import org.matrix.android.sdk.api.query.SpaceFilter +import org.matrix.android.sdk.api.query.toActiveSpaceOrNoFilter import org.matrix.android.sdk.api.query.toActiveSpaceOrOrphanRooms import org.matrix.android.sdk.api.session.Session import org.matrix.android.sdk.api.session.getRoomSummary @@ -393,7 +394,7 @@ class RoomListSectionBuilder( ) } else { filteredPagedRoomSummariesLive.queryParams = roomQueryParams.copy( - spaceFilter = null + spaceFilter = SpaceFilter.NoFilter ) } liveQueryParams.update { filteredPagedRoomSummariesLive.queryParams } @@ -449,7 +450,7 @@ class RoomListSectionBuilder( } RoomListViewModel.SpaceFilterStrategy.ALL_IF_SPACE_NULL -> { copy( - spaceFilter = currentSpace?.let { SpaceFilter.ActiveSpace(it) } + spaceFilter = currentSpace.toActiveSpaceOrNoFilter() ) } RoomListViewModel.SpaceFilterStrategy.NONE -> this diff --git a/vector/src/main/java/im/vector/app/features/home/room/list/home/HomeRoomListViewModel.kt b/vector/src/main/java/im/vector/app/features/home/room/list/home/HomeRoomListViewModel.kt index 55d955cb37..d74d235dc2 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/list/home/HomeRoomListViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/list/home/HomeRoomListViewModel.kt @@ -39,6 +39,7 @@ import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.launch import org.matrix.android.sdk.api.extensions.orFalse import org.matrix.android.sdk.api.query.SpaceFilter +import org.matrix.android.sdk.api.query.toActiveSpaceOrNoFilter import org.matrix.android.sdk.api.query.toActiveSpaceOrOrphanRooms import org.matrix.android.sdk.api.session.Session import org.matrix.android.sdk.api.session.getRoom @@ -121,15 +122,15 @@ class HomeRoomListViewModel @AssistedInject constructor( ) } - private fun getSpaceFilter(selectedSpaceId: String?, strategy: RoomListViewModel.SpaceFilterStrategy): SpaceFilter? { + private fun getSpaceFilter(selectedSpaceId: String?, strategy: RoomListViewModel.SpaceFilterStrategy): SpaceFilter { return when (strategy) { RoomListViewModel.SpaceFilterStrategy.ORPHANS_IF_SPACE_NULL -> { selectedSpaceId.toActiveSpaceOrOrphanRooms() } RoomListViewModel.SpaceFilterStrategy.ALL_IF_SPACE_NULL -> { - selectedSpaceId?.let { SpaceFilter.ActiveSpace(it) } + selectedSpaceId.toActiveSpaceOrNoFilter() } - RoomListViewModel.SpaceFilterStrategy.NONE -> null + RoomListViewModel.SpaceFilterStrategy.NONE -> SpaceFilter.NoFilter } } diff --git a/vector/src/main/java/im/vector/app/features/spaces/SpaceListViewModel.kt b/vector/src/main/java/im/vector/app/features/spaces/SpaceListViewModel.kt index 3b2fb31b74..ca6bd7a993 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/SpaceListViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/SpaceListViewModel.kt @@ -100,7 +100,7 @@ class SpaceListViewModel @AssistedInject constructor( this.memberships = listOf(Membership.JOIN) this.spaceFilter = SpaceFilter.OrphanRooms.takeIf { !vectorPreferences.prefSpacesShowAllRoomInHome() - } + } ?: SpaceFilter.NoFilter }, sortOrder = RoomSortOrder.NONE ).asFlow() .sample(300) @@ -117,7 +117,7 @@ class SpaceListViewModel @AssistedInject constructor( this.memberships = listOf(Membership.JOIN) this.spaceFilter = SpaceFilter.OrphanRooms.takeIf { !vectorPreferences.prefSpacesShowAllRoomInHome() - } + } ?: SpaceFilter.NoFilter } ) val counts = RoomAggregateNotificationCount( From 2d8ef9925d29a2c6cffd05fdc226493dd3a93175 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 28 Jul 2022 10:23:14 +0100 Subject: [PATCH 3/6] adding changelog entry --- changelog.d/6665.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/6665.bugfix diff --git a/changelog.d/6665.bugfix b/changelog.d/6665.bugfix new file mode 100644 index 0000000000..678b995fcf --- /dev/null +++ b/changelog.d/6665.bugfix @@ -0,0 +1 @@ +Fixes the room list not taking into account the Show all rooms in Home preference From 09d840506e8b3e8e074608b7c443778b90120c77 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 28 Jul 2022 11:04:35 +0100 Subject: [PATCH 4/6] replacing inverted takeIf chain with positive when conditions --- .../home/UnreadMessagesSharedViewModel.kt | 7 ++++--- .../room/list/home/HomeRoomListViewModel.kt | 21 +++++-------------- .../app/features/spaces/SpaceListViewModel.kt | 13 ++++++------ 3 files changed, 16 insertions(+), 25 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/home/UnreadMessagesSharedViewModel.kt b/vector/src/main/java/im/vector/app/features/home/UnreadMessagesSharedViewModel.kt index 3ce425ba6f..5f4782123c 100644 --- a/vector/src/main/java/im/vector/app/features/home/UnreadMessagesSharedViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/UnreadMessagesSharedViewModel.kt @@ -142,9 +142,10 @@ class UnreadMessagesSharedViewModel @AssistedInject constructor( val totalCount = roomService.getNotificationCountForRooms( roomSummaryQueryParams { this.memberships = listOf(Membership.JOIN) - this.spaceFilter = SpaceFilter.OrphanRooms.takeIf { - !vectorPreferences.prefSpacesShowAllRoomInHome() - } ?: SpaceFilter.NoFilter + this.spaceFilter = when { + vectorPreferences.prefSpacesShowAllRoomInHome() -> SpaceFilter.NoFilter + else -> SpaceFilter.OrphanRooms + } } ) diff --git a/vector/src/main/java/im/vector/app/features/home/room/list/home/HomeRoomListViewModel.kt b/vector/src/main/java/im/vector/app/features/home/room/list/home/HomeRoomListViewModel.kt index d74d235dc2..7b257b1d1c 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/list/home/HomeRoomListViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/list/home/HomeRoomListViewModel.kt @@ -27,7 +27,6 @@ import im.vector.app.core.di.MavericksAssistedViewModelFactory import im.vector.app.core.di.hiltMavericksViewModelFactory import im.vector.app.core.platform.StateView import im.vector.app.core.platform.VectorViewModel -import im.vector.app.features.home.room.list.RoomListViewModel import im.vector.app.features.settings.VectorPreferences import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.MutableSharedFlow @@ -107,13 +106,8 @@ class HomeRoomListViewModel @AssistedInject constructor( } .onEach { selectedSpaceOption -> val selectedSpace = selectedSpaceOption.orNull() - val strategy = if (!vectorPreferences.prefSpacesShowAllRoomInHome()) { - RoomListViewModel.SpaceFilterStrategy.ALL_IF_SPACE_NULL - } else { - RoomListViewModel.SpaceFilterStrategy.ORPHANS_IF_SPACE_NULL - } filteredPagedRoomSummariesLive.queryParams = filteredPagedRoomSummariesLive.queryParams.copy( - spaceFilter = getSpaceFilter(selectedSpaceId = selectedSpace?.roomId, strategy) + spaceFilter = getSpaceFilter(selectedSpaceId = selectedSpace?.roomId) ) }.launchIn(viewModelScope) @@ -122,15 +116,10 @@ class HomeRoomListViewModel @AssistedInject constructor( ) } - private fun getSpaceFilter(selectedSpaceId: String?, strategy: RoomListViewModel.SpaceFilterStrategy): SpaceFilter { - return when (strategy) { - RoomListViewModel.SpaceFilterStrategy.ORPHANS_IF_SPACE_NULL -> { - selectedSpaceId.toActiveSpaceOrOrphanRooms() - } - RoomListViewModel.SpaceFilterStrategy.ALL_IF_SPACE_NULL -> { - selectedSpaceId.toActiveSpaceOrNoFilter() - } - RoomListViewModel.SpaceFilterStrategy.NONE -> SpaceFilter.NoFilter + private fun getSpaceFilter(selectedSpaceId: String?): SpaceFilter { + return when { + vectorPreferences.prefSpacesShowAllRoomInHome() -> selectedSpaceId.toActiveSpaceOrNoFilter() + else -> selectedSpaceId.toActiveSpaceOrOrphanRooms() } } diff --git a/vector/src/main/java/im/vector/app/features/spaces/SpaceListViewModel.kt b/vector/src/main/java/im/vector/app/features/spaces/SpaceListViewModel.kt index ca6bd7a993..a104e9b170 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/SpaceListViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/SpaceListViewModel.kt @@ -98,9 +98,7 @@ class SpaceListViewModel @AssistedInject constructor( session.roomService().getPagedRoomSummariesLive( roomSummaryQueryParams { this.memberships = listOf(Membership.JOIN) - this.spaceFilter = SpaceFilter.OrphanRooms.takeIf { - !vectorPreferences.prefSpacesShowAllRoomInHome() - } ?: SpaceFilter.NoFilter + this.spaceFilter = roomsInSpaceFilter() }, sortOrder = RoomSortOrder.NONE ).asFlow() .sample(300) @@ -115,9 +113,7 @@ class SpaceListViewModel @AssistedInject constructor( val totalCount = session.roomService().getNotificationCountForRooms( roomSummaryQueryParams { this.memberships = listOf(Membership.JOIN) - this.spaceFilter = SpaceFilter.OrphanRooms.takeIf { - !vectorPreferences.prefSpacesShowAllRoomInHome() - } ?: SpaceFilter.NoFilter + this.spaceFilter = roomsInSpaceFilter() } ) val counts = RoomAggregateNotificationCount( @@ -134,6 +130,11 @@ class SpaceListViewModel @AssistedInject constructor( .launchIn(viewModelScope) } + private fun roomsInSpaceFilter() = when { + vectorPreferences.prefSpacesShowAllRoomInHome() -> SpaceFilter.NoFilter + else -> SpaceFilter.OrphanRooms + } + override fun handle(action: SpaceListAction) { when (action) { is SpaceListAction.SelectSpace -> handleSelectSpace(action) From 8ef08507523ff6eead67bd10a8a4ed1fa38b6389 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 28 Jul 2022 11:05:28 +0100 Subject: [PATCH 5/6] removing unused import --- .../main/java/im/vector/app/features/home/HomeDetailViewModel.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/vector/src/main/java/im/vector/app/features/home/HomeDetailViewModel.kt b/vector/src/main/java/im/vector/app/features/home/HomeDetailViewModel.kt index 6471eb5141..4be30f753b 100644 --- a/vector/src/main/java/im/vector/app/features/home/HomeDetailViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/HomeDetailViewModel.kt @@ -45,7 +45,6 @@ import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.launch import org.matrix.android.sdk.api.query.RoomCategoryFilter -import org.matrix.android.sdk.api.query.SpaceFilter import org.matrix.android.sdk.api.query.toActiveSpaceOrNoFilter import org.matrix.android.sdk.api.query.toActiveSpaceOrOrphanRooms import org.matrix.android.sdk.api.session.Session From e4640f14d24529ba0efe1e025d09d37714f5a712 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 28 Jul 2022 11:06:30 +0100 Subject: [PATCH 6/6] adding sdk changelog --- changelog.d/6666.sdk | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/6666.sdk diff --git a/changelog.d/6666.sdk b/changelog.d/6666.sdk new file mode 100644 index 0000000000..ad1cea8dc9 --- /dev/null +++ b/changelog.d/6666.sdk @@ -0,0 +1 @@ +SDK - The SpaceFilter is query parameter is no longer nullable, use SpaceFilter.NoFilter instead