Skip to content

Commit

Permalink
feat(room_preview): Compute display name for RoomPreview when possible
Browse files Browse the repository at this point in the history
  • Loading branch information
jmartinesp committed Nov 19, 2024
1 parent c07645e commit 0705b88
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 13 deletions.
10 changes: 7 additions & 3 deletions crates/matrix-sdk/src/room_preview.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,12 @@ impl RoomPreview {
num_joined_members: u64,
num_active_members: Option<u64>,
state: Option<RoomState>,
computed_display_name: Option<String>,
) -> 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(),
Expand Down Expand Up @@ -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,
)
}

Expand Down Expand Up @@ -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
Expand All @@ -266,7 +270,7 @@ impl RoomPreview {
is_world_readable: response.world_readable,
state,
is_direct,
heroes: None,
heroes: cached_room.map(|r| r.heroes()),
})
}

Expand Down
36 changes: 35 additions & 1 deletion crates/matrix-sdk/tests/integration/room_preview.rs
Original file line number Diff line number Diff line change
@@ -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},
Expand Down Expand Up @@ -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"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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");
Expand All @@ -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(
Expand All @@ -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.
Expand All @@ -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");
Expand All @@ -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.
Expand All @@ -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).
Expand All @@ -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());
}

0 comments on commit 0705b88

Please sign in to comment.