Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Home always showing all rooms #6666

Merged
merged 6 commits into from
Jul 28, 2022
Merged

Home always showing all rooms #6666

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1ba8d29
fixing rooms from other spaces being included in home due to wrong fi…
ouchadam Jul 28, 2022
e847688
providing a type for the NoFilter to avoid the ambiguity of the nulla…
ouchadam Jul 28, 2022
2d8ef99
adding changelog entry
ouchadam Jul 28, 2022
09d8405
replacing inverted takeIf chain with positive when conditions
ouchadam Jul 28, 2022
8ef0850
removing unused import
ouchadam Jul 28, 2022
e4640f1
adding sdk changelog
ouchadam Jul 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/6665.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixes the room list not taking into account the Show all rooms in Home preference
10 changes: 10 additions & 0 deletions matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/query/SpaceFilter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 2 additions & 2 deletions ...k-android/src/main/java/org/matrix/android/sdk/api/session/room/RoomSummaryQueryParams.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ data class RoomSummaryQueryParams(
/**
* Used to filter room using the current space.
*/
val spaceFilter: SpaceFilter?,
val spaceFilter: SpaceFilter,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe mention this API change in a file 6666.sdk

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

e4640f1

) {

/**
Expand All @@ -101,7 +101,7 @@ data class RoomSummaryQueryParams(
var roomTagQueryFilter: RoomTagQueryFilter? = null
var excludeType: List<String?>? = listOf(RoomType.SPACE)
var includeType: List<String?>? = null
var spaceFilter: SpaceFilter? = null
var spaceFilter: SpaceFilter = SpaceFilter.NoFilter

fun build() = RoomSummaryQueryParams(
displayName = displayName,
Expand Down
2 changes: 1 addition & 1 deletion ...c/main/java/org/matrix/android/sdk/internal/session/room/summary/RoomSummaryDataSource.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions vector/src/main/java/im/vector/app/features/home/HomeDetailViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -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()
}
)

Expand Down
2 changes: 1 addition & 1 deletion vector/src/main/java/im/vector/app/features/home/UnreadMessagesSharedViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ class UnreadMessagesSharedViewModel @AssistedInject constructor(
this.memberships = listOf(Membership.JOIN)
this.spaceFilter = SpaceFilter.OrphanRooms.takeIf {
!vectorPreferences.prefSpacesShowAllRoomInHome()
}
} ?: SpaceFilter.NoFilter
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case I think a if/else would be clearer:

if (vectorPreferences.prefSpacesShowAllRoomInHome()) {
    SpaceFilter.NoFilter
} else {
    SpaceFilter.OrphanRooms
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed, will update 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

09d8405

}
)

Expand Down
9 changes: 5 additions & 4 deletions vector/src/main/java/im/vector/app/features/home/room/list/RoomListSectionBuilder.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -378,7 +379,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 }
}
Expand All @@ -393,7 +394,7 @@ class RoomListSectionBuilder(
)
} else {
filteredPagedRoomSummariesLive.queryParams = roomQueryParams.copy(
spaceFilter = null
spaceFilter = SpaceFilter.NoFilter
)
}
liveQueryParams.update { filteredPagedRoomSummariesLive.queryParams }
Expand Down Expand Up @@ -444,12 +445,12 @@ class RoomListSectionBuilder(
return when (spaceFilter) {
RoomListViewModel.SpaceFilterStrategy.ORPHANS_IF_SPACE_NULL -> {
copy(
spaceFilter = currentSpace?.toActiveSpaceOrOrphanRooms()
spaceFilter = currentSpace.toActiveSpaceOrOrphanRooms()
)
}
RoomListViewModel.SpaceFilterStrategy.ALL_IF_SPACE_NULL -> {
copy(
spaceFilter = currentSpace?.let { SpaceFilter.ActiveSpace(it) }
spaceFilter = currentSpace.toActiveSpaceOrNoFilter()
)
}
RoomListViewModel.SpaceFilterStrategy.NONE -> this
Expand Down
9 changes: 5 additions & 4 deletions vector/src/main/java/im/vector/app/features/home/room/list/home/HomeRoomListViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
selectedSpaceId.toActiveSpaceOrOrphanRooms()
}
RoomListViewModel.SpaceFilterStrategy.ALL_IF_SPACE_NULL -> {
selectedSpaceId?.let { SpaceFilter.ActiveSpace(it) }
selectedSpaceId.toActiveSpaceOrNoFilter()
}
RoomListViewModel.SpaceFilterStrategy.NONE -> null
RoomListViewModel.SpaceFilterStrategy.NONE -> SpaceFilter.NoFilter
}
}

Expand Down
4 changes: 2 additions & 2 deletions vector/src/main/java/im/vector/app/features/spaces/SpaceListViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ class SpaceListViewModel @AssistedInject constructor(
this.memberships = listOf(Membership.JOIN)
this.spaceFilter = SpaceFilter.OrphanRooms.takeIf {
!vectorPreferences.prefSpacesShowAllRoomInHome()
}
} ?: SpaceFilter.NoFilter
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same remark

}, sortOrder = RoomSortOrder.NONE
).asFlow()
.sample(300)
Expand All @@ -117,7 +117,7 @@ class SpaceListViewModel @AssistedInject constructor(
this.memberships = listOf(Membership.JOIN)
this.spaceFilter = SpaceFilter.OrphanRooms.takeIf {
!vectorPreferences.prefSpacesShowAllRoomInHome()
}
} ?: SpaceFilter.NoFilter
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same remark

}
)
val counts = RoomAggregateNotificationCount(
Expand Down