diff --git a/crates/matrix-sdk-ui/src/spaces/room.rs b/crates/matrix-sdk-ui/src/spaces/room.rs index 10799d7f7..6e03f7f80 100644 --- a/crates/matrix-sdk-ui/src/spaces/room.rs +++ b/crates/matrix-sdk-ui/src/spaces/room.rs @@ -12,10 +12,16 @@ // See the License for that specific language governing permissions and // limitations under the License. +use std::cmp::Ordering; + use matrix_sdk::{Room, RoomHero, RoomState}; use ruma::{ - OwnedMxcUri, OwnedRoomAliasId, OwnedRoomId, OwnedServerName, - events::room::{guest_access::GuestAccess, history_visibility::HistoryVisibility}, + MilliSecondsSinceUnixEpoch, OwnedMxcUri, OwnedRoomAliasId, OwnedRoomId, OwnedServerName, + OwnedSpaceChildOrder, + events::{ + room::{guest_access::GuestAccess, history_visibility::HistoryVisibility}, + space::child::HierarchySpaceChildEvent, + }, room::{JoinRuleSummary, RoomSummary, RoomType}, }; @@ -132,4 +138,203 @@ impl SpaceRoom { via: vec![], } } + + /// Sorts space rooms by various criteria as defined in + /// https://spec.matrix.org/latest/client-server-api/#ordering-of-children-within-a-space + pub(crate) fn compare_rooms( + a: &SpaceRoom, + b: &SpaceRoom, + a_state: Option, + b_state: Option, + ) -> Ordering { + match (a_state, b_state) { + (Some(a_state), Some(b_state)) => match (&a_state.order, &b_state.order) { + (Some(a_order), Some(b_order)) => a_order + .cmp(b_order) + .then(a_state.origin_server_ts.cmp(&b_state.origin_server_ts)) + .then(a.room_id.cmp(&b.room_id)), + (Some(_), None) => Ordering::Less, + (None, Some(_)) => Ordering::Greater, + (None, None) => a_state + .origin_server_ts + .cmp(&b_state.origin_server_ts) + .then(a.room_id.to_string().cmp(&b.room_id.to_string())), + }, + _ => a.room_id.to_string().cmp(&b.room_id.to_string()), + } + } +} + +#[derive(Clone, Debug)] +pub(crate) struct SpaceRoomChildState { + pub(crate) order: Option, + pub(crate) origin_server_ts: MilliSecondsSinceUnixEpoch, +} + +impl From<&HierarchySpaceChildEvent> for SpaceRoomChildState { + fn from(event: &HierarchySpaceChildEvent) -> Self { + SpaceRoomChildState { + order: event.content.order.clone(), + origin_server_ts: event.origin_server_ts, + } + } +} + +#[cfg(test)] +mod tests { + use std::cmp::Ordering; + + use matrix_sdk_test::async_test; + use ruma::{MilliSecondsSinceUnixEpoch, OwnedRoomId, SpaceChildOrder, owned_room_id, uint}; + + use crate::spaces::{SpaceRoom, room::SpaceRoomChildState}; + + #[async_test] + async fn test_room_list_sorting() { + // Rooms without a `m.space.child` state event should be sorted by their + // `room_id` + assert_eq!( + SpaceRoom::compare_rooms( + &make_space_room(owned_room_id!("!A:a.b")), + &make_space_room(owned_room_id!("!B:a.b")), + None, + None + ), + Ordering::Less + ); + + assert_eq!( + SpaceRoom::compare_rooms( + &make_space_room(owned_room_id!("!Marțolea:a.b")), + &make_space_room(owned_room_id!("!Luana:a.b")), + None, + None, + ), + Ordering::Greater + ); + + // Rooms without an order provided through the `children_state` should be + // sorted by their `m.space.child` `origin_server_ts` + assert_eq!( + SpaceRoom::compare_rooms( + &make_space_room(owned_room_id!("!Luana:a.b")), + &make_space_room(owned_room_id!("!Marțolea:a.b")), + Some(SpaceRoomChildState { + origin_server_ts: MilliSecondsSinceUnixEpoch(uint!(1)), + order: None + }), + Some(SpaceRoomChildState { + origin_server_ts: MilliSecondsSinceUnixEpoch(uint!(0)), + order: None + }) + ), + Ordering::Greater + ); + + // The `m.space.child` `content.order` field should be used if provided + assert_eq!( + SpaceRoom::compare_rooms( + &make_space_room(owned_room_id!("!Joiana:a.b"),), + &make_space_room(owned_room_id!("!Mioara:a.b"),), + Some(SpaceRoomChildState { + origin_server_ts: MilliSecondsSinceUnixEpoch(uint!(123)), + order: Some(SpaceChildOrder::parse("second").unwrap()) + }), + Some(SpaceRoomChildState { + origin_server_ts: MilliSecondsSinceUnixEpoch(uint!(234)), + order: Some(SpaceChildOrder::parse("first").unwrap()) + }), + ), + Ordering::Greater + ); + + // The timestamp should be used when the `order` is the same + assert_eq!( + SpaceRoom::compare_rooms( + &make_space_room(owned_room_id!("!Joiana:a.b")), + &make_space_room(owned_room_id!("!Mioara:a.b")), + Some(SpaceRoomChildState { + origin_server_ts: MilliSecondsSinceUnixEpoch(uint!(1)), + order: Some(SpaceChildOrder::parse("Same pasture").unwrap()) + }), + Some(SpaceRoomChildState { + origin_server_ts: MilliSecondsSinceUnixEpoch(uint!(0)), + order: Some(SpaceChildOrder::parse("Same pasture").unwrap()) + }), + ), + Ordering::Greater + ); + + // And the `room_id` should be used when both the `order` and the + // `timestamp` are equal + assert_eq!( + SpaceRoom::compare_rooms( + &make_space_room(owned_room_id!("!Joiana:a.b")), + &make_space_room(owned_room_id!("!Mioara:a.b")), + Some(SpaceRoomChildState { + origin_server_ts: MilliSecondsSinceUnixEpoch(uint!(0)), + order: Some(SpaceChildOrder::parse("Same pasture").unwrap()) + }), + Some(SpaceRoomChildState { + origin_server_ts: MilliSecondsSinceUnixEpoch(uint!(0)), + order: Some(SpaceChildOrder::parse("Same pasture").unwrap()) + }), + ), + Ordering::Less + ); + + // When one of the rooms is missing `children_state` data the other one + // should take precedence + assert_eq!( + SpaceRoom::compare_rooms( + &make_space_room(owned_room_id!("!Viola:a.b")), + &make_space_room(owned_room_id!("!Sâmbotina:a.b")), + None, + Some(SpaceRoomChildState { + origin_server_ts: MilliSecondsSinceUnixEpoch(uint!(0)), + order: None + }), + ), + Ordering::Greater + ); + + // If the `order` is missing from one of the rooms but `children_state` + // is present then the other one should come first + assert_eq!( + SpaceRoom::compare_rooms( + &make_space_room(owned_room_id!("!Sâmbotina:a.b")), + &make_space_room(owned_room_id!("!Dumana:a.b")), + Some(SpaceRoomChildState { + origin_server_ts: MilliSecondsSinceUnixEpoch(uint!(1)), + order: None + }), + Some(SpaceRoomChildState { + origin_server_ts: MilliSecondsSinceUnixEpoch(uint!(1)), + order: Some(SpaceChildOrder::parse("Some pasture").unwrap()) + }), + ), + Ordering::Greater + ); + } + + fn make_space_room(room_id: OwnedRoomId) -> SpaceRoom { + SpaceRoom { + room_id, + canonical_alias: None, + name: Some("New room name".to_owned()), + display_name: "Empty room".to_owned(), + topic: None, + avatar_url: None, + room_type: None, + num_joined_members: 0, + join_rule: None, + world_readable: None, + guest_can_join: false, + is_direct: None, + children_count: 0, + state: None, + heroes: None, + via: vec![], + } + } } diff --git a/crates/matrix-sdk-ui/src/spaces/room_list.rs b/crates/matrix-sdk-ui/src/spaces/room_list.rs index 00587cdab..fb8bbc709 100644 --- a/crates/matrix-sdk-ui/src/spaces/room_list.rs +++ b/crates/matrix-sdk-ui/src/spaces/room_list.rs @@ -372,7 +372,7 @@ impl SpaceRoomList { self.pagination_state.set(SpaceRoomListPaginationState::Idle { end_reached: false }); } - /// Sorts spare rooms by various criteria as defined in + /// Sorts space rooms by various criteria as defined in /// https://spec.matrix.org/latest/client-server-api/#ordering-of-children-within-a-space fn compare_rooms( a: &SpaceRoom, @@ -382,23 +382,7 @@ impl SpaceRoomList { let a_state = children_state.get(&a.room_id); let b_state = children_state.get(&b.room_id); - match (a_state, b_state) { - (Some(a_state), Some(b_state)) => { - match (&a_state.content.order, &b_state.content.order) { - (Some(a_order), Some(b_order)) => a_order - .cmp(b_order) - .then(a_state.origin_server_ts.cmp(&b_state.origin_server_ts)) - .then(a.room_id.cmp(&b.room_id)), - (Some(_), None) => Ordering::Less, - (None, Some(_)) => Ordering::Greater, - (None, None) => a_state - .origin_server_ts - .cmp(&b_state.origin_server_ts) - .then(a.room_id.to_string().cmp(&b.room_id.to_string())), - } - } - _ => a.room_id.to_string().cmp(&b.room_id.to_string()), - } + SpaceRoom::compare_rooms(a, b, a_state.map(Into::into), b_state.map(Into::into)) } }