feat(base): improve profile updates for left/banned users (#6097)

This avoids profile updates when a user decides to leave on their own (in which case their profile is kept as is), and when a user is banned (in which case their profile is removed). Signed-off-by: JoFrost <20685007+JoFrost@users.noreply.github.com>
2026-03-30 11:59:43 +02:00
parent c0b26b6f25
commit 4d2f8bb82c
2 changed files with 179 additions and 4 deletions
@@ -32,7 +32,10 @@ pub fn upsert_or_delete(
    // Senders can fake the profile easily so we keep track of profiles that the
    // member set themselves to avoid having confusing profile changes when a
    // member gets kicked/banned.
-    if event.state_key() == event.sender() {
+    // We don't want to update the profile if the member is leaving the room, as the
+    // server may return a dummy empty profile along the leave event. We want to
+    // keep the last known profile in that case.
+    if event.state_key() == event.sender() && *event.membership() != MembershipState::Leave {
        context
            .state_changes
            .profiles
@@ -41,8 +44,8 @@ pub fn upsert_or_delete(
            .insert(event.sender().to_owned(), event.into());
    }

-    if *event.membership() == MembershipState::Invite {
-        // Remove any profile previously stored for the invited user.
+    if matches!(*event.membership(), MembershipState::Invite | MembershipState::Ban) {
+        // Remove any profile previously stored for the invited/banned user.
        //
        // A room member could have joined the room and left it later; in that case, the
        // server may return a dummy, empty profile along the `leave` event. We
@@ -54,5 +57,12 @@ pub fn upsert_or_delete(
            .entry(room_id.to_owned())
            .or_default()
            .push(event.state_key().clone());
+
+        // Address the edge case of "in-flight" profiles. If an earlier event in
+        // this same sync batch inserted a profile (for example user joined then got
+        // banned by an admin of a room), we MUST NOT reinsert it.
+        if let Some(room_profiles) = context.state_changes.profiles.get_mut(room_id) {
+            room_profiles.remove(event.state_key());
+        }
    }
 }
@@ -22,7 +22,8 @@ use matrix_sdk::{
 use matrix_sdk_common::executor::spawn;
 use matrix_sdk_test::{
    ALICE, BOB, CAROL, DEFAULT_TEST_ROOM_ID, JoinedRoomBuilder, SyncResponseBuilder, async_test,
-    event_factory::EventFactory, mocks::mock_encryption_state,
+    event_factory::{EventFactory, PreviousMembership},
+    mocks::mock_encryption_state,
 };
 use matrix_sdk_ui::timeline::{RoomExt, TimelineDetails};
 use ruma::events::room::member::MembershipState;
@@ -34,6 +35,170 @@ use wiremock::{

 use crate::mock_sync;

+#[async_test]
+async fn test_user_profile_after_being_banned() {
+    let (client, server) = logged_in_client_with_server().await;
+    let sync_settings =
+        SyncSettings::new().timeout(Duration::from_millis(3000)).token(SyncToken::NoToken);
+
+    let f = EventFactory::new();
+    let mut sync_builder = SyncResponseBuilder::new();
+    sync_builder.add_joined_room(JoinedRoomBuilder::new(&DEFAULT_TEST_ROOM_ID));
+
+    mock_sync(&server, sync_builder.build_json_sync_response(), None).await;
+    let _response = client.sync_once(sync_settings.clone()).await.unwrap();
+    server.reset().await;
+
+    mock_encryption_state(&server, false).await;
+
+    let room = client.get_room(&DEFAULT_TEST_ROOM_ID).unwrap();
+    let timeline = Arc::new(room.timeline().await.unwrap());
+
+    // Build a simple timeline with Bob joining the room, Alice accepting an invite,
+    // sending some messages, and then getting banned by Bob. Alice's profile should
+    // be unavailable after the ban, while Bob's profile should be unaffected.
+    sync_builder.add_joined_room(
+        JoinedRoomBuilder::new(&DEFAULT_TEST_ROOM_ID)
+            // Bob joins the room
+            .add_timeline_event(
+                f.member(&BOB)
+                    .display_name("Bob")
+                    .avatar_url("mxc://123".into())
+                    .membership(MembershipState::Join),
+            )
+            // Alice accepts an invite into the room
+            .add_timeline_event(
+                f.member(&ALICE)
+                    .display_name("*profanities*")
+                    .avatar_url("mxc://456".into())
+                    .membership(MembershipState::Join)
+                    .previous(MembershipState::Invite),
+            )
+            // Bob sends a text message
+            .add_timeline_event(f.text_msg("hey!").sender(&BOB))
+            // Alice send some insults and gets banned
+            .add_timeline_event(f.text_msg("*insults*").sender(&ALICE))
+            .add_timeline_event(f.text_msg("Nope. You are out.").sender(&BOB))
+            .add_timeline_event(
+                f.member(&ALICE).membership(MembershipState::Ban).sender(&BOB).previous(
+                    PreviousMembership::new(MembershipState::Join)
+                        .display_name("*profanities*")
+                        .avatar_url("mxc://456".into()),
+                ),
+            ),
+    );
+
+    mock_sync(&server, sync_builder.build_json_sync_response(), None).await;
+    let _response = client.sync_once(sync_settings.clone()).await.unwrap();
+
+    let timeline_items = timeline.items().await;
+    // Date divider + 6 events
+    assert_eq!(timeline_items.len(), 7);
+
+    // Alice's profile, and therefore display name, should be empty,
+    // as she has been banned. The SDK **should** return an empty
+    // profile for banned users.
+    // We are only checking Alice's profile here, as we check Bob's profile in a
+    // separate test
+    let alice_event = timeline_items[4].as_event().unwrap();
+    let alice_profile = alice_event.sender_profile();
+    // Was this event sent by Alice?
+    assert_eq!(alice_event.sender(), *ALICE);
+    match alice_profile {
+        TimelineDetails::Ready(profile) => {
+            assert_eq!(profile.display_name, None);
+            assert_eq!(profile.avatar_url, None);
+        }
+        _ => panic!("Expected Alice's profile to be ready"),
+    }
+}
+
+#[async_test]
+async fn test_user_profile_after_leaving() {
+    let (client, server) = logged_in_client_with_server().await;
+    let sync_settings =
+        SyncSettings::new().timeout(Duration::from_millis(3000)).token(SyncToken::NoToken);
+
+    let f = EventFactory::new();
+    let mut sync_builder = SyncResponseBuilder::new();
+    sync_builder.add_joined_room(JoinedRoomBuilder::new(&DEFAULT_TEST_ROOM_ID));
+
+    mock_sync(&server, sync_builder.build_json_sync_response(), None).await;
+    let _response = client.sync_once(sync_settings.clone()).await.unwrap();
+    server.reset().await;
+
+    mock_encryption_state(&server, false).await;
+
+    let room = client.get_room(&DEFAULT_TEST_ROOM_ID).unwrap();
+    let timeline = Arc::new(room.timeline().await.unwrap());
+
+    // Build a simple timeline with Bob joining the room, Alice accepting an invite
+    // and sending a message, Bob sending a message, and Alice leaving
+    sync_builder.add_joined_room(
+        JoinedRoomBuilder::new(&DEFAULT_TEST_ROOM_ID)
+            // Bob joins the room
+            .add_timeline_event(
+                f.member(&BOB)
+                    .display_name("Bob")
+                    .avatar_url("mxc://...".into())
+                    .membership(MembershipState::Join),
+            )
+            // Alice accepts an invite into the room
+            .add_timeline_event(
+                f.member(&ALICE)
+                    .display_name("Alice")
+                    .avatar_url("mxc://...".into())
+                    .membership(MembershipState::Join)
+                    .previous(MembershipState::Invite),
+            )
+            // Bob sends a text message
+            .add_timeline_event(f.text_msg("hey!").sender(&BOB))
+            // Alice send a message and leaves the room
+            .add_timeline_event(f.text_msg("bye now").sender(&ALICE))
+            .add_timeline_event(
+                f.member(&ALICE).membership(MembershipState::Leave).previous(
+                    PreviousMembership::new(MembershipState::Join)
+                        .display_name("Alice")
+                        .avatar_url("mxc://...".into()),
+                ),
+            ),
+    );
+
+    mock_sync(&server, sync_builder.build_json_sync_response(), None).await;
+    let _response = client.sync_once(sync_settings.clone()).await.unwrap();
+
+    let timeline_items = timeline.items().await;
+    // Date divider + 5 events
+    assert_eq!(timeline_items.len(), 6);
+
+    // Alice's profile, and therefore display name, should be still available after
+    // she left the room.
+    let alice_event = timeline_items[4].as_event().unwrap();
+    let alice_profile = alice_event.sender_profile();
+    // Was this event sent by Alice?
+    assert_eq!(alice_event.sender(), *ALICE);
+    match alice_profile {
+        TimelineDetails::Ready(profile) => {
+            assert_eq!(profile.display_name, Some("Alice".to_owned()));
+            assert_eq!(profile.avatar_url, Some("mxc://...".into()));
+        }
+        _ => panic!("Expected Alice's profile to be ready"),
+    }
+
+    // Bob's profile should also be available
+    let bob_event = timeline_items[3].as_event().unwrap();
+    let bob_profile = bob_event.sender_profile();
+    // Was this event sent by Bob?
+    assert_eq!(bob_event.sender(), *BOB);
+    match bob_profile {
+        TimelineDetails::Ready(profile) => {
+            assert_eq!(profile.display_name, Some("Bob".to_owned()));
+            assert_eq!(profile.avatar_url, Some("mxc://...".into()));
+        }
+        _ => panic!("Expected Bob's profile to be ready"),
+    }
+}
+
 #[async_test]
 async fn test_update_sender_profiles() {
    let (client, server) = logged_in_client_with_server().await;