From bb7e6cb5626601cdfa27fb54e643c978b88c3300 Mon Sep 17 00:00:00 2001
From: Benjamin Bouvier <benjamin@bouvier.cc>
Date: Mon, 7 Jul 2025 12:54:35 +0200
Subject: [PATCH] fix(timeline): use the timeline handle to send a new reaction

This is more precise than using the event timeline item kind: indeed,
when the event for which we want to add a reaction is a local echo that
has been sent, it is still marked as a local echo, but the send queue
will not know about it, and thus will not be able to add the reaction
immediately, leading to a silent failure.

The `TimelineItemHandle` was made to help with this kind of situation:
in this case, it will return `TimelineItemHandle::Remote`, even though
the item is a local echo that has been sent; that way we can use the
event id, and correctly send a (remote) reaciton event immediately.

The following commit includes a regression test.
---
 .../src/timeline/controller/mod.rs            | 34 ++++++++-----------
 .../timeline/controller/observable_items.rs   |  4 +--
 2 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs b/crates/matrix-sdk-ui/src/timeline/controller/mod.rs
index e4a471e0e..7da8f7a4f 100644
--- a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs
+++ b/crates/matrix-sdk-ui/src/timeline/controller/mod.rs
@@ -74,7 +74,7 @@ use crate::{
     timeline::{
         algorithms::rfind_event_by_item_id,
         date_dividers::DateDividerAdjuster,
-        event_item::EventTimelineItemKind,
+        event_item::TimelineItemHandle,
         pinned_events_loader::{PinnedEventsLoader, PinnedEventsLoaderError},
         MsgLikeContent, MsgLikeKind, TimelineEventFilterFn,
     },
@@ -608,33 +608,29 @@ impl<P: RoomDataProvider, D: Decryptor> TimelineController<P, D> {
             .and_then(|map| Some(map.get(key)?.get(user_id)?.status.clone()));
 
         let Some(prev_status) = prev_status else {
-            match &item.kind {
-                EventTimelineItemKind::Local(local) => {
-                    if let Some(send_handle) = &local.send_handle {
-                        if send_handle
-                            .react(key.to_owned())
-                            .await
-                            .map_err(|err| Error::SendQueueError(err.into()))?
-                            .is_some()
-                        {
-                            trace!("adding a reaction to a local echo");
-                            return Ok(true);
-                        }
-
-                        warn!("couldn't toggle reaction for local echo");
-                        return Ok(false);
+            // Adding the new reaction.
+            match item.handle() {
+                TimelineItemHandle::Local(send_handle) => {
+                    if send_handle
+                        .react(key.to_owned())
+                        .await
+                        .map_err(|err| Error::SendQueueError(err.into()))?
+                        .is_some()
+                    {
+                        trace!("adding a reaction to a local echo");
+                        return Ok(true);
                     }
 
-                    warn!("missing send handle for local echo; is this a test?");
+                    warn!("couldn't toggle reaction for local echo");
                     return Ok(false);
                 }
 
-                EventTimelineItemKind::Remote(remote) => {
+                TimelineItemHandle::Remote(event_id) => {
                     // Add a reaction through the room data provider.
                     // No need to reflect the effect locally, since the local echo handling will
                     // take care of it.
                     trace!("adding a reaction to a remote echo");
-                    let annotation = Annotation::new(remote.event_id.to_owned(), key.to_owned());
+                    let annotation = Annotation::new(event_id.to_owned(), key.to_owned());
                     self.room_data_provider
                         .send(ReactionEventContent::from(annotation).into())
                         .await?;
diff --git a/crates/matrix-sdk-ui/src/timeline/controller/observable_items.rs b/crates/matrix-sdk-ui/src/timeline/controller/observable_items.rs
index 59add7b61..0ca9a6fe3 100644
--- a/crates/matrix-sdk-ui/src/timeline/controller/observable_items.rs
+++ b/crates/matrix-sdk-ui/src/timeline/controller/observable_items.rs
@@ -726,8 +726,8 @@ mod observable_items_tests {
 
     use super::*;
     use crate::timeline::{
-        controller::{EventTimelineItemKind, RemoteEventOrigin},
-        event_item::{LocalEventTimelineItem, RemoteEventTimelineItem},
+        controller::RemoteEventOrigin,
+        event_item::{EventTimelineItemKind, LocalEventTimelineItem, RemoteEventTimelineItem},
         EventSendState, EventTimelineItem, Message, MsgLikeContent, MsgLikeKind, TimelineDetails,
         TimelineItemContent, TimelineItemKind, TimelineUniqueId, VirtualTimelineItem,
     };