From 0705b889710d488002fecca742faa2ae40ee181e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Tue, 19 Nov 2024 09:05:52 +0100 Subject: [PATCH] feat(room_preview): Compute display name for `RoomPreview` when possible --- crates/matrix-sdk/src/room_preview.rs | 10 ++++-- .../tests/integration/room_preview.rs | 36 ++++++++++++++++++- .../src/tests/sliding_sync/room.rs | 26 +++++++++----- 3 files changed, 59 insertions(+), 13 deletions(-) diff --git a/crates/matrix-sdk/src/room_preview.rs b/crates/matrix-sdk/src/room_preview.rs index 18f43120800..9238b9eeb74 100644 --- a/crates/matrix-sdk/src/room_preview.rs +++ b/crates/matrix-sdk/src/room_preview.rs @@ -93,11 +93,12 @@ impl RoomPreview { num_joined_members: u64, num_active_members: Option, state: Option, + computed_display_name: Option, ) -> Self { RoomPreview { room_id: room_info.room_id().to_owned(), canonical_alias: room_info.canonical_alias().map(ToOwned::to_owned), - name: room_info.name().map(ToOwned::to_owned), + name: computed_display_name.or_else(|| room_info.name().map(ToOwned::to_owned)), topic: room_info.topic().map(ToOwned::to_owned), avatar_url: room_info.avatar_url().map(ToOwned::to_owned), room_type: room_info.room_type().cloned(), @@ -128,12 +129,15 @@ impl RoomPreview { pub(crate) async fn from_known(room: &Room) -> Self { let is_direct = room.is_direct().await.ok(); + let display_name = room.compute_display_name().await.ok().map(|name| name.to_string()); + Self::from_room_info( room.clone_info(), is_direct, room.joined_members_count(), Some(room.active_members_count()), Some(room.state()), + display_name, ) } @@ -247,7 +251,7 @@ impl RoomPreview { let num_active_members = cached_room.as_ref().map(|r| r.active_members_count()); - let is_direct = if let Some(cached_room) = cached_room { + let is_direct = if let Some(cached_room) = &cached_room { cached_room.is_direct().await.ok() } else { None @@ -266,7 +270,7 @@ impl RoomPreview { is_world_readable: response.world_readable, state, is_direct, - heroes: None, + heroes: cached_room.map(|r| r.heroes()), }) } diff --git a/crates/matrix-sdk/tests/integration/room_preview.rs b/crates/matrix-sdk/tests/integration/room_preview.rs index 142df0b83ef..1d78439c1c9 100644 --- a/crates/matrix-sdk/tests/integration/room_preview.rs +++ b/crates/matrix-sdk/tests/integration/room_preview.rs @@ -1,9 +1,15 @@ +#[cfg(feature = "experimental-sliding-sync")] +use js_int::uint; use matrix_sdk::{config::SyncSettings, test_utils::logged_in_client_with_server}; +#[cfg(feature = "experimental-sliding-sync")] +use matrix_sdk_base::sliding_sync; use matrix_sdk_base::RoomState; use matrix_sdk_test::{ async_test, InvitedRoomBuilder, JoinedRoomBuilder, KnockedRoomBuilder, SyncResponseBuilder, }; -use ruma::{room_id, space::SpaceRoomJoinRule, RoomId}; +#[cfg(feature = "experimental-sliding-sync")] +use ruma::{api::client::sync::sync_events::v5::response::Hero, assign}; +use ruma::{owned_user_id, room_id, space::SpaceRoomJoinRule, RoomId}; use serde_json::json; use wiremock::{ matchers::{header, method, path_regex}, @@ -93,6 +99,34 @@ async fn test_room_preview_leave_unknown_room_fails() { assert!(client.get_room(room_id).is_none()); } +#[cfg(feature = "experimental-sliding-sync")] +#[async_test] +async fn test_room_preview_computes_name_if_room_is_known() { + let (client, _) = logged_in_client_with_server().await; + let room_id = room_id!("!room:localhost"); + + // Given a room with no name but a hero + let room = assign!(sliding_sync::http::response::Room::new(), { + name: None, + heroes: Some(vec![assign!(Hero::new(owned_user_id!("@alice:matrix.org")), { + name: Some("Alice".to_owned()), + avatar: None, + })]), + joined_count: Some(uint!(1)), + invited_count: Some(uint!(1)), + }); + let mut response = sliding_sync::http::Response::new("0".to_owned()); + response.rooms.insert(room_id.to_owned(), room); + + client.process_sliding_sync_test_helper(&response).await.expect("Failed to process sync"); + + // When we get its preview + let room_preview = client.get_room_preview(room_id.into(), Vec::new()).await.unwrap(); + + // Its name is computed from its heroes + assert_eq!(room_preview.name.unwrap(), "Alice"); +} + async fn mock_leave(room_id: &RoomId, server: &MockServer) { Mock::given(method("POST")) .and(path_regex(r"^/_matrix/client/r0/rooms/.*/leave")) diff --git a/testing/matrix-sdk-integration-testing/src/tests/sliding_sync/room.rs b/testing/matrix-sdk-integration-testing/src/tests/sliding_sync/room.rs index 15a277d6314..aa85dcb0360 100644 --- a/testing/matrix-sdk-integration-testing/src/tests/sliding_sync/room.rs +++ b/testing/matrix-sdk-integration-testing/src/tests/sliding_sync/room.rs @@ -1091,9 +1091,10 @@ async fn test_room_preview() -> Result<()> { // Dummy test for `Client::get_room_preview` which may call one or the other // methods. info!("Alice gets a preview of the public room using any method"); - let preview = alice.get_room_preview(room_id.into(), Vec::new()).await.unwrap(); - assert_room_preview_from_unknown(&preview, &room_alias); + let preview = alice.get_room_preview(room_id.into(), Vec::new()).await?; + assert_room_preview(&preview, &room_alias); assert_eq!(preview.state, Some(RoomState::Joined)); + assert!(preview.heroes.is_some()); } Ok(()) @@ -1131,6 +1132,7 @@ async fn test_room_preview_with_room_directory_search_and_room_alias_only() { .await .expect("room preview couldn't be retrieved"); assert_eq!(preview.room_id, expected_room_id); + assert!(preview.heroes.is_none()); } #[async_test] @@ -1191,9 +1193,10 @@ async fn test_room_preview_with_room_directory_search_and_room_alias_only_in_sev .await .expect("room preview couldn't be retrieved"); assert_eq!(preview.room_id, expected_room_id); + assert!(preview.heroes.is_none()); } -fn assert_room_preview_from_unknown(preview: &RoomPreview, room_alias: &str) { +fn assert_room_preview(preview: &RoomPreview, room_alias: &str) { assert_eq!(preview.canonical_alias.as_ref().unwrap().alias(), room_alias); assert_eq!(preview.name.as_ref().unwrap(), "Alice's Room"); assert_eq!(preview.topic.as_ref().unwrap(), "Discussing Alice's Topic"); @@ -1202,7 +1205,6 @@ fn assert_room_preview_from_unknown(preview: &RoomPreview, room_alias: &str) { assert!(preview.room_type.is_none()); assert_eq!(preview.join_rule, SpaceRoomJoinRule::Invite); assert!(preview.is_world_readable); - assert!(preview.heroes.is_none()); } async fn get_room_preview_with_room_state( @@ -1215,15 +1217,17 @@ async fn get_room_preview_with_room_state( // Alice has joined the room, so they get the full details. info!("Alice gets a preview of the public room from state events"); let preview = RoomPreview::from_state_events(alice, room_id).await.unwrap(); - assert_room_preview_from_unknown(&preview, room_alias); + assert_room_preview(&preview, room_alias); assert_eq!(preview.state, Some(RoomState::Joined)); + assert!(preview.heroes.is_some()); // Bob definitely doesn't know about the room, but they can get a preview of the // room too. info!("Bob gets a preview of the public room from state events"); let preview = RoomPreview::from_state_events(bob, room_id).await.unwrap(); - assert_room_preview_from_unknown(&preview, room_alias); + assert_room_preview(&preview, room_alias); assert!(preview.state.is_none()); + assert!(preview.heroes.is_some()); // Bob can't preview the second room, because its history visibility is neither // world-readable, nor have they joined the room before. @@ -1246,8 +1250,9 @@ async fn get_room_preview_with_room_summary( .await .unwrap(); - assert_room_preview_from_unknown(&preview, room_alias); + assert_room_preview(&preview, room_alias); assert_eq!(preview.state, Some(RoomState::Joined)); + assert!(preview.heroes.is_some()); // The preview also works when using the room alias parameter. info!("Alice gets a preview of the public room from msc3266 using the room alias"); @@ -1261,8 +1266,9 @@ async fn get_room_preview_with_room_summary( .await .unwrap(); - assert_room_preview_from_unknown(&preview, room_alias); + assert_room_preview(&preview, room_alias); assert_eq!(preview.state, Some(RoomState::Joined)); + assert!(preview.heroes.is_some()); // Bob definitely doesn't know about the room, but they can get a preview of the // room too. @@ -1271,8 +1277,9 @@ async fn get_room_preview_with_room_summary( RoomPreview::from_room_summary(bob, room_id.to_owned(), room_id.into(), Vec::new()) .await .unwrap(); - assert_room_preview_from_unknown(&preview, room_alias); + assert_room_preview(&preview, room_alias); assert!(preview.state.is_none()); + assert!(preview.heroes.is_none()); // Bob can preview the second room with the room summary (because its join rule // is set to public, or because Alice is a member of that room). @@ -1287,4 +1294,5 @@ async fn get_room_preview_with_room_summary( .unwrap(); assert_eq!(preview.name.unwrap(), "Alice's Room 2"); assert!(preview.state.is_none()); + assert!(preview.heroes.is_none()); }