fix: Improve avatar lookup in DMs

- The current user’s avatar was not resolved for loaded messages - When a user’s avatar changed later that avatar wasn’t resolved for loaded messages
prose-im · Sep 5, 2024 · 7dccdea · 7dccdea
1 parent 37376f9
commit 7dccdea
Show file tree

Hide file tree

Showing 9 changed files with 432 additions and 31 deletions.
diff --git a/crates/prose-core-client/src/app/event_handlers/user_info_event_handler.rs b/crates/prose-core-client/src/app/event_handlers/user_info_event_handler.rs
@@ -6,15 +6,26 @@
 use anyhow::Result;
 use async_trait::async_trait;
 
-use prose_proc_macros::InjectDependencies;
-
-use crate::app::deps::DynUserInfoDomainService;
+use crate::app::deps::{
+    DynAppContext, DynClientEventDispatcher, DynConnectedRoomsReadOnlyRepository,
+    DynUserInfoDomainService,
+};
 use crate::app::event_handlers::{
     ServerEvent, ServerEventHandler, UserInfoEvent, UserInfoEventType,
 };
+use crate::domain::user_info::models::Avatar;
+use crate::dtos::ParticipantId;
+use crate::ClientEvent;
+use prose_proc_macros::InjectDependencies;
 
 #[derive(InjectDependencies)]
 pub struct UserInfoEventHandler {
+    #[inject]
+    client_event_dispatcher: DynClientEventDispatcher,
+    #[inject]
+    ctx: DynAppContext,
+    #[inject]
+    connected_rooms_repo: DynConnectedRoomsReadOnlyRepository,
     #[inject]
     user_info_domain_service: DynUserInfoDomainService,
 }
@@ -41,9 +52,22 @@ impl UserInfoEventHandler {
     async fn handle_user_info_event(&self, event: UserInfoEvent) -> Result<()> {
         match event.r#type {
             UserInfoEventType::AvatarChanged { metadata } => {
+                let avatar = Avatar::from_metadata(event.user_id.clone(), metadata);
+
                 self.user_info_domain_service
-                    .handle_avatar_changed(&event.user_id, Some(metadata))
+                    .handle_avatar_changed(&event.user_id, Some(avatar.clone()))
                     .await?;
+
+                if let Some(room) = self
+                    .connected_rooms_repo
+                    .get(&self.ctx.connected_account()?, event.user_id.as_ref())
+                {
+                    room.with_participants_mut(|p| {
+                        p.set_avatar(&ParticipantId::User(event.user_id), Some(avatar));
+                    });
+                    self.client_event_dispatcher
+                        .dispatch_event(ClientEvent::SidebarChanged);
+                }
             }
             UserInfoEventType::ProfileChanged { profile } => {
                 self.user_info_domain_service

diff --git a/crates/prose-core-client/src/app/services/account_service.rs b/crates/prose-core-client/src/app/services/account_service.rs
@@ -12,7 +12,7 @@ use prose_xmpp::mods::AvatarData;
 use crate::app::deps::*;
 use crate::domain::account::services::UserProfileFormat;
 use crate::domain::shared::models::{Availability, AvatarId, CachePolicy, ParticipantIdRef};
-use crate::domain::user_info::models::{AvatarMetadata, UserProfile, UserStatus};
+use crate::domain::user_info::models::{Avatar, AvatarMetadata, UserProfile, UserStatus};
 use crate::dtos::{AccountInfo, DeviceId, DeviceInfo, UserProfile as UserProfileDTO};
 use crate::ClientEvent;
 
@@ -164,7 +164,10 @@ impl AccountService {
 
         debug!("Caching avatar metadata");
         self.user_info_domain_service
-            .handle_avatar_changed(&user_id, Some(metadata))
+            .handle_avatar_changed(
+                &user_id,
+                Some(Avatar::from_metadata(user_id.clone(), metadata)),
+            )
             .await?;
 
         Ok(())

diff --git a/crates/prose-core-client/src/app/services/room.rs b/crates/prose-core-client/src/app/services/room.rs
@@ -868,8 +868,6 @@ impl<Kind> Room<Kind> {
             })
             .unwrap_or_else(|| (None, None, None));
 
-        real_id = real_id.or_else(|| id.to_user_id());
-
         if let Some(name) = name {
             return MessageSender {
                 id: id.clone(),
@@ -878,6 +876,8 @@ impl<Kind> Room<Kind> {
             };
         }
 
+        real_id = real_id.or_else(|| id.to_user_id());
+
         let Some(real_id) = real_id else {
             return MessageSender {
                 id: id.clone(),
@@ -886,13 +886,13 @@ impl<Kind> Room<Kind> {
             };
         };
 
-        let name = self
+        let (name, avatar) = self
             .user_info_domain_service
             .get_user_info(&real_id, CachePolicy::ReturnCacheDataDontLoad)
             .await
             .unwrap_or_default()
-            .display_name()
-            .unwrap_or_participant_id(id);
+            .map(|i| (i.display_name().unwrap_or_participant_id(id), i.avatar))
+            .unwrap_or_else(|| (ContactNameBuilder::new().unwrap_or_participant_id(id), None));
 
         MessageSender {
             id: id.clone(),

diff --git a/crates/prose-core-client/src/domain/rooms/models/participant_list.rs b/crates/prose-core-client/src/domain/rooms/models/participant_list.rs
@@ -223,6 +223,15 @@ impl ParticipantList {
             });
     }
 
+    /// Sets the participant's avatar. Does nothing if the participant doesn't exist.
+    pub fn set_avatar(&mut self, id: &ParticipantId, avatar: Option<Avatar>) {
+        self.participants_map
+            .entry(id.clone())
+            .and_modify(|participant| {
+                participant.avatar = avatar;
+            });
+    }
+
     pub fn add_user(
         &mut self,
         real_id: &UserId,

diff --git a/crates/prose-core-client/src/domain/user_info/models/avatar.rs b/crates/prose-core-client/src/domain/user_info/models/avatar.rs
@@ -6,6 +6,7 @@
 use serde::{Deserialize, Serialize};
 
 use crate::domain::shared::models::{AvatarId, ParticipantId, ParticipantIdRef, UserId};
+use crate::domain::user_info::models::AvatarMetadata;
 
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
 pub enum AvatarSource {
@@ -25,6 +26,18 @@ pub struct Avatar {
     pub source: AvatarSource,
 }
 
+impl Avatar {
+    pub fn from_metadata(user_id: UserId, metadata: AvatarMetadata) -> Self {
+        Self {
+            id: metadata.checksum,
+            source: AvatarSource::Pep {
+                owner: user_id,
+                mime_type: metadata.mime_type,
+            },
+        }
+    }
+}
+
 impl Avatar {
     pub fn owner(&self) -> ParticipantIdRef {
         match &self.source {

diff --git a/crates/prose-core-client/src/domain/user_info/services/impls/user_info_domain_service.rs b/crates/prose-core-client/src/domain/user_info/services/impls/user_info_domain_service.rs
@@ -276,19 +276,7 @@ impl UserInfoDomainServiceTrait for UserInfoDomainService {
             .await
     }
 
-    async fn handle_avatar_changed(
-        &self,
-        user_id: &UserId,
-        metadata: Option<AvatarMetadata>,
-    ) -> Result<()> {
-        let avatar = metadata.map(|metadata| Avatar {
-            id: metadata.checksum,
-            source: AvatarSource::Pep {
-                owner: user_id.clone(),
-                mime_type: metadata.mime_type,
-            },
-        });
-
+    async fn handle_avatar_changed(&self, user_id: &UserId, avatar: Option<Avatar>) -> Result<()> {
         self.update_user_info(user_id, |info| info.avatar = avatar)
             .await
     }

diff --git a/crates/prose-core-client/src/domain/user_info/services/user_info_domain_service.rs b/crates/prose-core-client/src/domain/user_info/services/user_info_domain_service.rs
@@ -11,8 +11,7 @@ use prose_wasm_utils::{SendUnlessWasm, SyncUnlessWasm};
 use crate::domain::contacts::models::Contact;
 use crate::domain::shared::models::{CachePolicy, UserId, UserOrResourceId};
 use crate::domain::user_info::models::{
-    Avatar, AvatarMetadata, PlatformImage, Presence, UserInfo, UserMetadata, UserProfile,
-    UserStatus,
+    Avatar, PlatformImage, Presence, UserInfo, UserMetadata, UserProfile, UserStatus,
 };
 
 #[cfg_attr(target_arch = "wasm32", async_trait(? Send))]
@@ -47,11 +46,7 @@ pub trait UserInfoDomainService: SendUnlessWasm + SyncUnlessWasm {
         user_activity: Option<UserStatus>,
     ) -> Result<()>;
 
-    async fn handle_avatar_changed(
-        &self,
-        user_id: &UserId,
-        metadata: Option<AvatarMetadata>,
-    ) -> Result<()>;
+    async fn handle_avatar_changed(&self, user_id: &UserId, avatar: Option<Avatar>) -> Result<()>;
 
     async fn handle_user_profile_changed(
         &self,