ui/ffi: Add timestamp to reactions

Author: aringenbach

bindings/matrix-sdk-ffi/src/timeline.rs

@@ -324,7 +324,13 @@ impl EventTimelineItem {
             .map(|(k, v)| Reaction {
                 key: k.to_owned(),
                 count: v.len().try_into().unwrap(),
-                senders: v.senders().map(ToString::to_string).collect(),
+                senders: v
+                    .senders()
+                    .map(|v| ReactionSenderData {
+                        sender_id: v.sender_id.to_string(),
+                        timestamp: v.timestamp.0.into(),
+                    })
+                    .collect(),
             })
             .collect()
     }
@@ -1082,13 +1088,13 @@ impl EncryptedMessage {
 pub struct Reaction {
     pub key: String,
     pub count: u64,
-    pub senders: Vec<String>,
+    pub senders: Vec<ReactionSenderData>,
 }
 
-#[derive(Clone)]
-pub struct ReactionDetails {
-    pub id: String,
-    pub sender: String,
+#[derive(Clone, uniffi::Record)]
+pub struct ReactionSenderData {
+    pub sender_id: String,
+    pub timestamp: u64,
 }
 
 /// A [`TimelineItem`](super::TimelineItem) that doesn't correspond to an event.

crates/matrix-sdk-ui/src/timeline/event_handler.rs

@@ -42,16 +42,17 @@ use tracing::{debug, error, field::debug, info, instrument, trace, warn};
 
 use super::{
     event_item::{
-        AnyOtherFullStateEventContent, BundledReactions, EventSendState, EventTimelineItemKind,
-        LocalEventTimelineItem, Profile, RemoteEventOrigin, RemoteEventTimelineItem,
+        AnyOtherFullStateEventContent, BundledReactions, EventItemIdentifier, EventSendState,
+        EventTimelineItemKind, LocalEventTimelineItem, Profile, RemoteEventOrigin,
+        RemoteEventTimelineItem,
     },
     find_read_marker,
     read_receipts::maybe_add_implicit_read_receipt,
     rfind_event_by_id, rfind_event_item, EventTimelineItem, Message, OtherState, ReactionGroup,
     Sticker, TimelineDetails, TimelineInnerState, TimelineItem, TimelineItemContent,
     VirtualTimelineItem, DEFAULT_SANITIZER_MODE,
 };
-use crate::events::SyncTimelineEventWithoutContent;
+use crate::{events::SyncTimelineEventWithoutContent, timeline::event_item::ReactionSenderData};
 
 #[derive(Clone)]
 pub(super) enum Flow {
@@ -208,11 +209,7 @@ pub(super) struct TimelineEventHandler<'a> {
     meta: TimelineEventMetadata,
     flow: Flow,
     items: &'a mut ObservableVector<Arc<TimelineItem>>,
-    #[allow(clippy::type_complexity)]
-    reaction_map: &'a mut HashMap<
-        (Option<OwnedTransactionId>, Option<OwnedEventId>),
-        (OwnedUserId, Annotation),
-    >,
+    reaction_map: &'a mut HashMap<EventItemIdentifier, (ReactionSenderData, Annotation)>,
     pending_reactions: &'a mut HashMap<OwnedEventId, IndexSet<OwnedEventId>>,
     fully_read_event: &'a mut Option<OwnedEventId>,
     event_should_update_fully_read_marker: &'a mut bool,
@@ -433,9 +430,11 @@ impl<'a> TimelineEventHandler<'a> {
     fn handle_reaction(&mut self, c: ReactionEventContent) {
         let event_id: &EventId = &c.relates_to.event_id;
         let (reaction_id, old_txn_id) = match &self.flow {
-            Flow::Local { txn_id, .. } => ((Some(txn_id.clone()), None), None),
+            Flow::Local { txn_id, .. } => {
+                (EventItemIdentifier::TransactionId(txn_id.clone()), None)
+            }
             Flow::Remote { event_id, txn_id, .. } => {
-                ((None, Some(event_id.clone())), txn_id.as_ref())
+                (EventItemIdentifier::EventId(event_id.clone()), txn_id.as_ref())
             }
         };
 
@@ -455,15 +454,22 @@ impl<'a> TimelineEventHandler<'a> {
                 let reaction_group = reactions.entry(c.relates_to.key.clone()).or_default();
 
                 if let Some(txn_id) = old_txn_id {
+                    let id = EventItemIdentifier::TransactionId(txn_id.clone());
                     // Remove the local echo from the related event.
-                    if reaction_group.0.remove(&(Some(txn_id.clone()), None)).is_none() {
+                    if reaction_group.0.remove(&id).is_none() {
                         warn!(
                             "Received reaction with transaction ID, but didn't \
                              find matching reaction in the related event's reactions"
                         );
                     }
                 }
-                reaction_group.0.insert(reaction_id.clone(), self.meta.sender.clone());
+                reaction_group.0.insert(
+                    reaction_id.clone(),
+                    ReactionSenderData {
+                        sender_id: self.meta.sender.clone(),
+                        timestamp: self.meta.timestamp,
+                    },
+                );
 
                 trace!("Adding reaction");
                 self.items.set(
@@ -476,26 +482,31 @@ impl<'a> TimelineEventHandler<'a> {
             }
         } else {
             trace!("Timeline item not found, adding reaction to the pending list");
-            let (None, Some(reaction_event_id)) = &reaction_id else {
+            let EventItemIdentifier::EventId(reaction_event_id) = reaction_id.clone() else {
                 error!("Adding local reaction echo to event absent from the timeline");
                 return;
             };
 
             let pending = self.pending_reactions.entry(event_id.to_owned()).or_default();
 
-            pending.insert(reaction_event_id.clone());
+            pending.insert(reaction_event_id);
         }
 
         if let Flow::Remote { txn_id: Some(txn_id), .. } = &self.flow {
+            let id = EventItemIdentifier::TransactionId(txn_id.clone());
             // Remove the local echo from the reaction map.
-            if self.reaction_map.remove(&(Some(txn_id.clone()), None)).is_none() {
+            if self.reaction_map.remove(&id).is_none() {
                 warn!(
                     "Received reaction with transaction ID, but didn't \
                      find matching reaction in reaction_map"
                 );
             }
         }
-        self.reaction_map.insert(reaction_id, (self.meta.sender.clone(), c.relates_to));
+        let reaction_sender_data = ReactionSenderData {
+            sender_id: self.meta.sender.clone(),
+            timestamp: self.meta.timestamp,
+        };
+        self.reaction_map.insert(reaction_id, (reaction_sender_data, c.relates_to));
     }
 
     #[instrument(skip_all)]
@@ -507,7 +518,8 @@ impl<'a> TimelineEventHandler<'a> {
     // Redacted redactions are no-ops (unfortunately)
     #[instrument(skip_all, fields(redacts_event_id = ?redacts))]
     fn handle_redaction(&mut self, redacts: OwnedEventId, _content: RoomRedactionEventContent) {
-        if let Some((_, rel)) = self.reaction_map.remove(&(None, Some(redacts.clone()))) {
+        let id = EventItemIdentifier::EventId(redacts.clone());
+        if let Some((_, rel)) = self.reaction_map.remove(&id) {
             update_timeline_item!(self, &rel.event_id, "redaction", |event_item| {
                 let Some(remote_event_item) = event_item.as_remote() else {
                     error!("inconsistent state: redaction received on a non-remote event item");
@@ -522,7 +534,7 @@ impl<'a> TimelineEventHandler<'a> {
                     };
                     let group = group_entry.get_mut();
 
-                    if group.0.remove(&(None, Some(redacts.clone()))).is_none() {
+                    if group.0.remove(&id).is_none() {
                         error!(
                             "inconsistent state: reaction from reaction_map not in reaction list \
                              of timeline item"
@@ -543,7 +555,7 @@ impl<'a> TimelineEventHandler<'a> {
 
             if self.result.items_updated == 0 {
                 if let Some(reactions) = self.pending_reactions.get_mut(&rel.event_id) {
-                    if !reactions.remove(&redacts) {
+                    if !reactions.remove(&redacts.clone()) {
                         error!(
                             "inconsistent state: reaction from reaction_map not in reaction list \
                              of pending_reactions"
@@ -589,7 +601,8 @@ impl<'a> TimelineEventHandler<'a> {
         redacts: OwnedTransactionId,
         _content: RoomRedactionEventContent,
     ) {
-        if let Some((_, rel)) = self.reaction_map.remove(&(Some(redacts.clone()), None)) {
+        let id = EventItemIdentifier::TransactionId(redacts);
+        if let Some((_, rel)) = self.reaction_map.remove(&id) {
             update_timeline_item!(self, &rel.event_id, "redaction", |event_item| {
                 let Some(remote_event_item) = event_item.as_remote() else {
                     error!("inconsistent state: redaction received on a non-remote event item");
@@ -602,7 +615,7 @@ impl<'a> TimelineEventHandler<'a> {
                 };
                 let group = group_entry.get_mut();
 
-                if group.0.remove(&(Some(redacts.clone()), None)).is_none() {
+                if group.0.remove(&id).is_none() {
                     error!(
                         "inconsistent state: reaction from reaction_map not in reaction list \
                          of timeline item"
@@ -941,8 +954,10 @@ impl<'a> TimelineEventHandler<'a> {
                 let mut bundled = IndexMap::new();
 
                 for reaction_event_id in reactions {
-                    let reaction_id = (None, Some(reaction_event_id));
-                    let Some((sender, annotation)) = self.reaction_map.get(&reaction_id) else {
+                    let reaction_id = EventItemIdentifier::EventId(reaction_event_id);
+                    let Some((reaction_sender_data, annotation)) =
+                        self.reaction_map.get(&reaction_id)
+                    else {
                         error!(
                             "inconsistent state: reaction from pending_reactions not in reaction_map"
                         );
@@ -951,7 +966,7 @@ impl<'a> TimelineEventHandler<'a> {
 
                     let group: &mut ReactionGroup =
                         bundled.entry(annotation.key.clone()).or_default();
-                    group.0.insert(reaction_id, sender.clone());
+                    group.0.insert(reaction_id, reaction_sender_data.clone());
                 }
 
                 Some(bundled)

crates/matrix-sdk-ui/src/timeline/event_item/content.rs

@@ -60,7 +60,7 @@ use ruma::{
 };
 use tracing::{debug, error, warn};
 
-use super::{EventTimelineItem, Profile, TimelineDetails};
+use super::{EventItemIdentifier, EventTimelineItem, Profile, ReactionSenderData, TimelineDetails};
 use crate::timeline::{
     traits::RoomDataProvider, Error as TimelineError, TimelineItem, DEFAULT_SANITIZER_MODE,
 };
@@ -538,22 +538,17 @@ impl From<RoomEncryptedEventContent> for EncryptedMessage {
 /// Key: The reaction, usually an emoji.\
 /// Value: The group of reactions.
 pub type BundledReactions = IndexMap<String, ReactionGroup>;
-
-// The long type after a long visibility specified trips up rustfmt currently.
-// This works around. Report: https://github.com/rust-lang/rustfmt/issues/5703
-type ReactionGroupInner = IndexMap<(Option<OwnedTransactionId>, Option<OwnedEventId>), OwnedUserId>;
-
 /// A group of reaction events on the same event with the same key.
 ///
 /// This is a map of the event ID or transaction ID of the reactions to the ID
 /// of the sender of the reaction.
 #[derive(Clone, Debug, Default)]
-pub struct ReactionGroup(pub(in crate::timeline) ReactionGroupInner);
+pub struct ReactionGroup(pub(in crate::timeline) IndexMap<EventItemIdentifier, ReactionSenderData>);
 
 impl ReactionGroup {
     /// The (deduplicated) senders of the reactions in this group.
-    pub fn senders(&self) -> impl Iterator<Item = &UserId> {
-        self.values().unique().map(AsRef::as_ref)
+    pub fn senders(&self) -> impl Iterator<Item = &ReactionSenderData> {
+        self.values().unique_by(|v| &v.sender_id)
     }
 
     /// All reactions within this reaction group that were sent by the given
@@ -565,13 +560,17 @@ impl ReactionGroup {
         &'a self,
         user_id: &'a UserId,
     ) -> impl Iterator<Item = (Option<&OwnedTransactionId>, Option<&OwnedEventId>)> + 'a {
-        self.iter()
-            .filter_map(move |(k, v)| (*v == user_id).then_some((k.0.as_ref(), k.1.as_ref())))
+        self.iter().filter_map(move |(k, v)| {
+            (v.sender_id == user_id).then_some(match k {
+                EventItemIdentifier::TransactionId(txn_id) => (Some(txn_id), None),
+                EventItemIdentifier::EventId(event_id) => (None, Some(event_id)),
+            })
+        })
     }
 }
 
 impl Deref for ReactionGroup {
-    type Target = IndexMap<(Option<OwnedTransactionId>, Option<OwnedEventId>), OwnedUserId>;
+    type Target = IndexMap<EventItemIdentifier, ReactionSenderData>;
 
     fn deref(&self) -> &Self::Target {
         &self.0

crates/matrix-sdk-ui/src/timeline/event_item/mod.rs

@@ -21,7 +21,8 @@ use once_cell::sync::Lazy;
 use ruma::{
     events::{receipt::Receipt, room::message::MessageType, AnySyncTimelineEvent},
     serde::Raw,
-    EventId, MilliSecondsSinceUnixEpoch, OwnedMxcUri, OwnedUserId, TransactionId, UserId,
+    EventId, MilliSecondsSinceUnixEpoch, OwnedEventId, OwnedMxcUri, OwnedTransactionId,
+    OwnedUserId, TransactionId, UserId,
 };
 use tracing::warn;
 
@@ -69,6 +70,24 @@ pub(super) enum EventTimelineItemKind {
     Remote(RemoteEventTimelineItem),
 }
 
+/// A wrapper that can contain either a transaction id, or an event id.
+#[derive(Clone, Debug, Eq, Hash, PartialEq)]
+pub enum EventItemIdentifier {
+    TransactionId(OwnedTransactionId),
+    EventId(OwnedEventId),
+}
+
+/// Data associated with a reaction sender. It can be used to display
+/// a details UI component for a reaction with both sender
+/// names and the date at which they sent a reaction.
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub struct ReactionSenderData {
+    /// Sender identifier.
+    pub sender_id: OwnedUserId,
+    /// Date at which the sender reacted.
+    pub timestamp: MilliSecondsSinceUnixEpoch,
+}
+
 impl EventTimelineItem {
     pub(super) fn new(
         sender: OwnedUserId,