From fc08338dfc9d56d6384554dc22a143ba33b0fe2c Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Tue, 15 Oct 2024 15:27:01 +0200 Subject: [PATCH 1/4] timeline: use a `TimelineItemId` to react to a timeline item Changelog: `Timeline::toggle_reaction` now identifies the item that's reacted to with a `TimelineEventItemId`. --- bindings/matrix-sdk-ffi/src/timeline/mod.rs | 16 +++++----- .../src/timeline/controller/mod.rs | 8 ++--- crates/matrix-sdk-ui/src/timeline/mod.rs | 11 +++---- .../matrix-sdk-ui/src/timeline/tests/mod.rs | 23 +++++++------- .../src/timeline/tests/reactions.rs | 28 ++++++++++------- crates/matrix-sdk-ui/src/timeline/util.rs | 30 +++++++++++++------ .../tests/integration/timeline/focus_event.rs | 2 +- .../tests/integration/timeline/reactions.rs | 28 ++++++++--------- .../src/tests/timeline.rs | 25 ++++++++-------- 9 files changed, 96 insertions(+), 75 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/timeline/mod.rs b/bindings/matrix-sdk-ffi/src/timeline/mod.rs index 7d3117c3b8c..66d686fcb49 100644 --- a/bindings/matrix-sdk-ffi/src/timeline/mod.rs +++ b/bindings/matrix-sdk-ffi/src/timeline/mod.rs @@ -495,7 +495,7 @@ impl Timeline { new_content: EditedContent, ) -> Result { self.inner - .edit_by_id(&(event_or_transaction_id.try_into()?), new_content.try_into()?) + .edit_by_id(&event_or_transaction_id.try_into()?, new_content.try_into()?) .await .map_err(Into::into) } @@ -530,19 +530,21 @@ impl Timeline { /// Toggle a reaction on an event. /// - /// The `unique_id` parameter is a string returned by - /// the `TimelineItem::unique_id()` method. As such, this method works both - /// on local echoes and remote items. - /// /// Adds or redacts a reaction based on the state of the reaction at the /// time it is called. /// + /// This method works both on local echoes and remote items. + /// /// When redacting a previous reaction, the redaction reason is not set. /// /// Ensures that only one reaction is sent at a time to avoid race /// conditions and spamming the homeserver with requests. - pub async fn toggle_reaction(&self, unique_id: String, key: String) -> Result<(), ClientError> { - self.inner.toggle_reaction(&unique_id, &key).await?; + pub async fn toggle_reaction( + &self, + item_id: EventOrTransactionId, + key: String, + ) -> Result<(), ClientError> { + self.inner.toggle_reaction(&item_id.try_into()?, &key).await?; Ok(()) } diff --git a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs b/crates/matrix-sdk-ui/src/timeline/controller/mod.rs index ba23ab6e5bb..94ff0944459 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/mod.rs @@ -70,7 +70,7 @@ use crate::{ event_item::EventTimelineItemKind, pinned_events_loader::{PinnedEventsLoader, PinnedEventsLoaderError}, reactions::FullReactionKey, - util::rfind_event_by_uid, + util::rfind_event_by_item_id, TimelineEventFilterFn, }, unable_to_decrypt_hook::UtdHookManager, @@ -484,12 +484,12 @@ impl TimelineController

{ #[instrument(skip_all)] pub(super) async fn toggle_reaction_local( &self, - unique_id: &str, + item_id: &TimelineEventItemId, key: &str, ) -> Result { let mut state = self.state.write().await; - let Some((item_pos, item)) = rfind_event_by_uid(&state.items, unique_id) else { + let Some((item_pos, item)) = rfind_event_by_item_id(&state.items, item_id) else { warn!("Timeline item not found, can't add reaction"); return Err(Error::FailedToToggleReaction); }; @@ -502,7 +502,7 @@ impl TimelineController

{ .map(|reaction_info| reaction_info.status.clone()); let Some(prev_status) = prev_status else { - match &item.inner.kind { + match &item.kind { EventTimelineItemKind::Local(local) => { if let Some(send_handle) = local.send_handle.clone() { if send_handle diff --git a/crates/matrix-sdk-ui/src/timeline/mod.rs b/crates/matrix-sdk-ui/src/timeline/mod.rs index 0dc57b1e936..bc5cbc60ad6 100644 --- a/crates/matrix-sdk-ui/src/timeline/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/mod.rs @@ -547,9 +547,6 @@ impl Timeline { /// Toggle a reaction on an event. /// - /// The `unique_id` parameter is a string returned by - /// [`TimelineItem::unique_id()`]. - /// /// Adds or redacts a reaction based on the state of the reaction at the /// time it is called. /// @@ -557,8 +554,12 @@ impl Timeline { /// /// Ensures that only one reaction is sent at a time to avoid race /// conditions and spamming the homeserver with requests. - pub async fn toggle_reaction(&self, unique_id: &str, reaction_key: &str) -> Result<(), Error> { - self.controller.toggle_reaction_local(unique_id, reaction_key).await?; + pub async fn toggle_reaction( + &self, + item_id: &TimelineEventItemId, + reaction_key: &str, + ) -> Result<(), Error> { + self.controller.toggle_reaction_local(item_id, reaction_key).await?; Ok(()) } diff --git a/crates/matrix-sdk-ui/src/timeline/tests/mod.rs b/crates/matrix-sdk-ui/src/timeline/tests/mod.rs index 5c12916221f..46ae26f589b 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/mod.rs @@ -60,7 +60,9 @@ use super::{ event_handler::TimelineEventKind, event_item::RemoteEventOrigin, traits::RoomDataProvider, - EventTimelineItem, Profile, TimelineController, TimelineFocus, TimelineItem, + util::rfind_event_by_item_id, + EventTimelineItem, Profile, TimelineController, TimelineEventItemId, TimelineFocus, + TimelineItem, }; use crate::{ timeline::pinned_events_loader::PinnedEventsRoom, unable_to_decrypt_hook::UtdHookManager, @@ -265,17 +267,16 @@ impl TestTimeline { self.controller.handle_read_receipts(ev_content).await; } - async fn toggle_reaction_local(&self, unique_id: &str, key: &str) -> Result<(), super::Error> { - if self.controller.toggle_reaction_local(unique_id, key).await? { + async fn toggle_reaction_local( + &self, + item_id: &TimelineEventItemId, + key: &str, + ) -> Result<(), super::Error> { + if self.controller.toggle_reaction_local(item_id, key).await? { // TODO(bnjbvr): hacky? - if let Some(event_id) = self - .controller - .items() - .await - .iter() - .rfind(|item| item.unique_id() == unique_id) - .and_then(|item| item.as_event()?.as_remote()) - .map(|event_item| event_item.event_id.clone()) + let items = self.controller.items().await; + if let Some(event_id) = rfind_event_by_item_id(&items, item_id) + .and_then(|(_pos, item)| item.event_id().map(ToOwned::to_owned)) { // Fake a local echo, for new reactions. self.handle_local_event( diff --git a/crates/matrix-sdk-ui/src/timeline/tests/reactions.rs b/crates/matrix-sdk-ui/src/timeline/tests/reactions.rs index 2321563c107..bb0a8ab51da 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/reactions.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/reactions.rs @@ -29,7 +29,7 @@ use tokio::time::timeout; use crate::timeline::{ controller::TimelineEnd, event_item::RemoteEventOrigin, tests::TestTimeline, ReactionStatus, - TimelineItem, + TimelineEventItemId, TimelineItem, }; const REACTION_KEY: &str = "👍"; @@ -83,7 +83,11 @@ async fn test_add_reaction_on_non_existent_event() { let timeline = TestTimeline::new(); let mut stream = timeline.subscribe().await; - timeline.toggle_reaction_local("nonexisting_unique_id", REACTION_KEY).await.unwrap_err(); + let event_id = EventId::parse("$nonexisting_unique_id").unwrap(); + timeline + .toggle_reaction_local(&TimelineEventItemId::EventId(event_id), REACTION_KEY) + .await + .unwrap_err(); assert!(stream.next().now_or_never().is_none()); } @@ -92,10 +96,10 @@ async fn test_add_reaction_on_non_existent_event() { async fn test_add_reaction_success() { let timeline = TestTimeline::new(); let mut stream = timeline.subscribe().await; - let (msg_uid, event_id, item_pos) = send_first_message(&timeline, &mut stream).await; + let (item_id, event_id, item_pos) = send_first_message(&timeline, &mut stream).await; // If I toggle a reaction on an event which didn't have any… - timeline.toggle_reaction_local(&msg_uid, REACTION_KEY).await.unwrap(); + timeline.toggle_reaction_local(&item_id, REACTION_KEY).await.unwrap(); // The timeline item is updated, with a local echo for the reaction. assert_reaction_is_updated!(stream, &event_id, item_pos, false); @@ -123,7 +127,7 @@ async fn test_redact_reaction_success() { let f = &timeline.factory; let mut stream = timeline.subscribe().await; - let (msg_uid, event_id, item_pos) = send_first_message(&timeline, &mut stream).await; + let (item_id, event_id, item_pos) = send_first_message(&timeline, &mut stream).await; // A reaction is added by sync. let reaction_id = event_id!("$reaction_id"); @@ -135,7 +139,7 @@ async fn test_redact_reaction_success() { assert_reaction_is_updated!(stream, &event_id, item_pos, true); // Toggling the reaction locally… - timeline.toggle_reaction_local(&msg_uid, REACTION_KEY).await.unwrap(); + timeline.toggle_reaction_local(&item_id, REACTION_KEY).await.unwrap(); // Will immediately redact it on the item. let event = assert_item_update!(stream, &event_id, item_pos); @@ -166,12 +170,12 @@ async fn test_redact_reaction_success() { async fn test_reactions_store_timestamp() { let timeline = TestTimeline::new(); let mut stream = timeline.subscribe().await; - let (msg_uid, event_id, msg_pos) = send_first_message(&timeline, &mut stream).await; + let (item_id, event_id, msg_pos) = send_first_message(&timeline, &mut stream).await; // Creating a reaction adds a valid timestamp. let timestamp_before = MilliSecondsSinceUnixEpoch::now(); - timeline.toggle_reaction_local(&msg_uid, REACTION_KEY).await.unwrap(); + timeline.toggle_reaction_local(&item_id, REACTION_KEY).await.unwrap(); let event = assert_reaction_is_updated!(stream, &event_id, msg_pos, false); let reactions = event.reactions().get(&REACTION_KEY.to_owned()).unwrap(); @@ -216,15 +220,17 @@ async fn test_initial_reaction_timestamp_is_stored() { async fn send_first_message( timeline: &TestTimeline, stream: &mut (impl Stream>> + Unpin), -) -> (String, OwnedEventId, usize) { +) -> (TimelineEventItemId, OwnedEventId, usize) { timeline.handle_live_event(timeline.factory.text_msg("I want you to react").sender(&BOB)).await; let item = assert_next_matches!(*stream, VectorDiff::PushBack { value } => value); - let event_id = item.as_event().unwrap().as_remote().unwrap().event_id.clone(); + let event_item = item.as_event().unwrap(); + let item_id = event_item.identifier(); + let event_id = event_item.event_id().unwrap().to_owned(); let position = timeline.len().await - 1; let day_divider = assert_next_matches!(*stream, VectorDiff::PushFront { value } => value); assert!(day_divider.is_day_divider()); - (item.unique_id().to_owned(), event_id, position) + (item_id, event_id, position) } diff --git a/crates/matrix-sdk-ui/src/timeline/util.rs b/crates/matrix-sdk-ui/src/timeline/util.rs index d3e7d715622..91159f8654c 100644 --- a/crates/matrix-sdk-ui/src/timeline/util.rs +++ b/crates/matrix-sdk-ui/src/timeline/util.rs @@ -21,7 +21,8 @@ use ruma::{EventId, MilliSecondsSinceUnixEpoch}; #[cfg(doc)] use super::controller::TimelineMetadata; use super::{ - event_item::EventTimelineItemKind, EventTimelineItem, ReactionsByKeyBySender, TimelineItem, + event_item::EventTimelineItemKind, EventTimelineItem, ReactionsByKeyBySender, + TimelineEventItemId, TimelineItem, }; pub(super) struct EventTimelineItemWithId<'a> { @@ -80,26 +81,37 @@ pub(super) fn rfind_event_item( rfind_event_item_internal(items, |item_with_id| f(item_with_id.inner)) } -/// Find the timeline item that matches the given internal id, if any. +/// Find the timeline item that matches the given event id, if any. /// /// WARNING: Linear scan of the items, see documentation of /// [`rfind_event_item`]. -pub(super) fn rfind_event_by_uid<'a>( +pub(super) fn rfind_event_by_id<'a>( items: &'a Vector>, - internal_id: &'a str, + event_id: &EventId, ) -> Option<(usize, EventTimelineItemWithId<'a>)> { - rfind_event_item_internal(items, |item_with_id| item_with_id.internal_id == internal_id) + rfind_event_item(items, |it| it.event_id() == Some(event_id)) } -/// Find the timeline item that matches the given event id, if any. +/// Find the timeline item that matches the given item (event or transaction) +/// id, if any. /// /// WARNING: Linear scan of the items, see documentation of /// [`rfind_event_item`]. -pub(super) fn rfind_event_by_id<'a>( +pub(super) fn rfind_event_by_item_id<'a>( items: &'a Vector>, - event_id: &EventId, + item_id: &TimelineEventItemId, ) -> Option<(usize, EventTimelineItemWithId<'a>)> { - rfind_event_item(items, |it| it.event_id() == Some(event_id)) + match item_id { + TimelineEventItemId::TransactionId(txn_id) => { + rfind_event_item(items, |item| match &item.kind { + EventTimelineItemKind::Local(local) => local.transaction_id == *txn_id, + EventTimelineItemKind::Remote(remote) => { + remote.transaction_id.as_deref() == Some(txn_id) + } + }) + } + TimelineEventItemId::EventId(event_id) => rfind_event_by_id(items, event_id), + } } /// Result of comparing events position in the timeline. diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/focus_event.rs b/crates/matrix-sdk-ui/tests/integration/timeline/focus_event.rs index 27a6056fa42..fa47850034e 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/focus_event.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/focus_event.rs @@ -320,7 +320,7 @@ async fn test_focused_timeline_local_echoes() { assert_pending!(timeline_stream); // Add a reaction to the focused event, which will cause a local echo to happen. - timeline.toggle_reaction(items[1].unique_id(), "✨").await.unwrap(); + timeline.toggle_reaction(&event_item.identifier(), "✨").await.unwrap(); // We immediately get the local echo for the reaction. let item = assert_next_matches_with_timeout!(timeline_stream, VectorDiff::Set { index: 1, value: item } => item); diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/reactions.rs b/crates/matrix-sdk-ui/tests/integration/timeline/reactions.rs index 8202af31d40..671750aea23 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/reactions.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/reactions.rs @@ -75,7 +75,7 @@ async fn test_abort_before_being_sent() { assert_let!(Some(VectorDiff::PushBack { value: first }) = stream.next().await); let item = first.as_event().unwrap(); - let unique_id = first.unique_id(); + let item_id = item.identifier(); assert_eq!(item.content().as_message().unwrap().body(), "hello"); assert_let!(Some(VectorDiff::PushFront { value: day_divider }) = stream.next().await); @@ -102,7 +102,7 @@ async fn test_abort_before_being_sent() { mock_redaction(event_id!("$3")).mount(&server).await; // We add the reaction… - timeline.toggle_reaction(unique_id, "👍").await.unwrap(); + timeline.toggle_reaction(&item_id, "👍").await.unwrap(); // First toggle (local echo). { @@ -119,7 +119,7 @@ async fn test_abort_before_being_sent() { } // We toggle another reaction at the same time… - timeline.toggle_reaction(unique_id, "🥰").await.unwrap(); + timeline.toggle_reaction(&item_id, "🥰").await.unwrap(); { assert_let!(Some(VectorDiff::Set { index: 1, value: item }) = stream.next().await); @@ -140,7 +140,7 @@ async fn test_abort_before_being_sent() { // Then we remove the first one; because it was being sent, it should lead to a // redaction event. - timeline.toggle_reaction(unique_id, "👍").await.unwrap(); + timeline.toggle_reaction(&item_id, "👍").await.unwrap(); { assert_let!(Some(VectorDiff::Set { index: 1, value: item }) = stream.next().await); @@ -157,7 +157,7 @@ async fn test_abort_before_being_sent() { // But because the first one was being sent, this one won't and the local echo // could be discarded. - timeline.toggle_reaction(unique_id, "🥰").await.unwrap(); + timeline.toggle_reaction(&item_id, "🥰").await.unwrap(); { assert_let!(Some(VectorDiff::Set { index: 1, value: item }) = stream.next().await); @@ -214,12 +214,11 @@ async fn test_redact_failed() { let _response = client.sync_once(Default::default()).await.unwrap(); server.reset().await; - let unique_id = assert_next_matches_with_timeout!(stream, VectorDiff::PushBack { value: item } => { - let unique_id = item.unique_id().to_owned(); + let item_id = assert_next_matches_with_timeout!(stream, VectorDiff::PushBack { value: item } => { let item = item.as_event().unwrap(); assert_eq!(item.content().as_message().unwrap().body(), "hello"); assert!(item.reactions().is_empty()); - unique_id + item.identifier() }); assert_next_matches_with_timeout!(stream, VectorDiff::Set { index: 0, value: item } => { @@ -242,7 +241,7 @@ async fn test_redact_failed() { .await; // We toggle the reaction, which fails with an error. - timeline.toggle_reaction(&unique_id, "😆").await.unwrap_err(); + timeline.toggle_reaction(&item_id, "😆").await.unwrap_err(); // The local echo is removed (assuming the redaction works)… assert_next_matches_with_timeout!(stream, VectorDiff::Set { index: 1, value: item } => { @@ -310,13 +309,12 @@ async fn test_local_reaction_to_local_echo() { let _ = timeline.send(RoomMessageEventContent::text_plain("lol").into()).await.unwrap(); // Receive a local echo. - let unique_id = assert_next_matches_with_timeout!(stream, VectorDiff::PushBack { value: item } => { - let unique_id = item.unique_id().to_owned(); + let item_id = assert_next_matches_with_timeout!(stream, VectorDiff::PushBack { value: item } => { let item = item.as_event().unwrap(); assert!(item.is_local_echo()); assert_eq!(item.content().as_message().unwrap().body(), "lol"); assert!(item.reactions().is_empty()); - unique_id + item.identifier() }); // Good ol' day divider. @@ -326,7 +324,7 @@ async fn test_local_reaction_to_local_echo() { // Add a reaction before the remote echo comes back. let key1 = "🤣"; - timeline.toggle_reaction(&unique_id, key1).await.unwrap(); + timeline.toggle_reaction(&item_id, key1).await.unwrap(); // The reaction is added to the local echo. assert_next_matches_with_timeout!(stream, VectorDiff::Set { index: 1, value: item } => { @@ -338,7 +336,7 @@ async fn test_local_reaction_to_local_echo() { // Add another reaction. let key2 = "😈"; - timeline.toggle_reaction(&unique_id, key2).await.unwrap(); + timeline.toggle_reaction(&item_id, key2).await.unwrap(); // Also comes as a local echo. assert_next_matches_with_timeout!(stream, VectorDiff::Set { index: 1, value: item } => { @@ -350,7 +348,7 @@ async fn test_local_reaction_to_local_echo() { // Remove second reaction. It's immediately removed, since it was a local echo, // and it wasn't being sent. - timeline.toggle_reaction(&unique_id, key2).await.unwrap(); + timeline.toggle_reaction(&item_id, key2).await.unwrap(); assert_next_matches_with_timeout!(stream, VectorDiff::Set { index: 1, value: item } => { let reactions = item.as_event().unwrap().reactions(); diff --git a/testing/matrix-sdk-integration-testing/src/tests/timeline.rs b/testing/matrix-sdk-integration-testing/src/tests/timeline.rs index bf8c670105f..4097ce04c46 100644 --- a/testing/matrix-sdk-integration-testing/src/tests/timeline.rs +++ b/testing/matrix-sdk-integration-testing/src/tests/timeline.rs @@ -96,17 +96,15 @@ async fn test_toggling_reaction() -> Result<()> { items.iter().find_map(|item| { let event = item.as_event()?; if !event.is_local_echo() && event.content().as_message()?.body().trim() == "hi!" { - event - .event_id() - .map(|event_id| (item.unique_id().to_owned(), event_id.to_owned())) + event.event_id().map(ToOwned::to_owned) } else { None } }) }; - if let Some(pair) = find_event_id(&items) { - return Ok(pair); + if let Some(event_id) = find_event_id(&items) { + return Ok(event_id); } warn!(?items, "Waiting for updates…"); @@ -114,8 +112,8 @@ async fn test_toggling_reaction() -> Result<()> { while let Some(diff) = stream.next().await { warn!(?diff, "received a diff"); diff.apply(&mut items); - if let Some(pair) = find_event_id(&items) { - return Ok(pair); + if let Some(event_id) = find_event_id(&items) { + return Ok(event_id); } } @@ -130,7 +128,7 @@ async fn test_toggling_reaction() -> Result<()> { debug!("Sending initial message…"); timeline.send(RoomMessageEventContent::text_plain("hi!").into()).await.unwrap(); - let (msg_uid, event_id) = timeout(Duration::from_secs(10), event_id_task) + let event_id = timeout(Duration::from_secs(10), event_id_task) .await .expect("timeout") .expect("failed to join tokio task") @@ -150,10 +148,13 @@ async fn test_toggling_reaction() -> Result<()> { diff.apply(&mut items); } - let message_position = items + let (message_position, item_id) = items .iter() .enumerate() - .find_map(|(i, item)| (item.as_event()?.event_id()? == event_id).then_some(i)) + .find_map(|(i, item)| { + let item = item.as_event()?; + (item.event_id()? == event_id).then_some((i, item.identifier())) + }) .expect("couldn't find the final position for the event id"); let reaction_key = "👍".to_owned(); @@ -163,7 +164,7 @@ async fn test_toggling_reaction() -> Result<()> { debug!("Starting the toggle reaction tests…"); // Add the reaction. - timeline.toggle_reaction(&msg_uid, &reaction_key).await.expect("toggling reaction"); + timeline.toggle_reaction(&item_id, &reaction_key).await.expect("toggling reaction"); // Local echo is added. { @@ -192,7 +193,7 @@ async fn test_toggling_reaction() -> Result<()> { // Redact the reaction. timeline - .toggle_reaction(&msg_uid, &reaction_key) + .toggle_reaction(&item_id, &reaction_key) .await .expect("toggling reaction the second time"); From 02e104e095d036fa08688b9259166e167628e896 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Tue, 15 Oct 2024 15:29:50 +0200 Subject: [PATCH 2/4] timeline: get rid of conversions from string to `TimelineEventItemId` I suppose these were useful at the FFI layer at some point, but they aren't anymore, so they could be removed. Changelog: Got rid of `From` for `TimelineEventItemId`. --- .../src/timeline/event_item/mod.rs | 32 +------------------ 1 file changed, 1 insertion(+), 31 deletions(-) diff --git a/crates/matrix-sdk-ui/src/timeline/event_item/mod.rs b/crates/matrix-sdk-ui/src/timeline/event_item/mod.rs index 62984c09d8d..67ee1299029 100644 --- a/crates/matrix-sdk-ui/src/timeline/event_item/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/event_item/mod.rs @@ -103,22 +103,6 @@ pub enum TimelineEventItemId { EventId(OwnedEventId), } -impl From for TimelineEventItemId { - fn from(value: String) -> Self { - value.as_str().into() - } -} - -impl From<&str> for TimelineEventItemId { - fn from(value: &str) -> Self { - if let Ok(event_id) = EventId::parse(value) { - TimelineEventItemId::EventId(event_id) - } else { - TimelineEventItemId::TransactionId(value.into()) - } - } -} - /// An handle that usually allows to perform an action on a timeline event. /// /// If the item represents a remote item, then the event id is usually @@ -749,7 +733,7 @@ mod tests { }; use super::{EventTimelineItem, Profile}; - use crate::timeline::{TimelineDetails, TimelineEventItemId}; + use crate::timeline::TimelineDetails; #[async_test] async fn test_latest_message_event_can_be_wrapped_as_a_timeline_item() { @@ -974,20 +958,6 @@ mod tests { ); } - #[test] - fn test_raw_event_id_into_timeline_event_item_id_gets_event_id() { - let raw_id = "$123:example.com"; - let id: TimelineEventItemId = raw_id.into(); - assert_matches!(id, TimelineEventItemId::EventId(_)); - } - - #[test] - fn test_raw_str_into_timeline_event_item_id_gets_transaction_id() { - let raw_id = "something something"; - let id: TimelineEventItemId = raw_id.into(); - assert_matches!(id, TimelineEventItemId::TransactionId(_)); - } - fn member_event( room_id: &RoomId, user_id: &UserId, From ff2e75d3784476e710fdab660a9efe69b6de5408 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Wed, 16 Oct 2024 12:02:26 +0200 Subject: [PATCH 3/4] feat(multiverse): add support to toggle a reaction on the last message of a room --- labs/multiverse/src/main.rs | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/labs/multiverse/src/main.rs b/labs/multiverse/src/main.rs index d370bc4b928..e844449cec7 100644 --- a/labs/multiverse/src/main.rs +++ b/labs/multiverse/src/main.rs @@ -357,6 +357,41 @@ impl App { } } + async fn toggle_reaction_to_latest_msg(&mut self) { + let selected = self.get_selected_room_id(None); + + if let Some((sdk_timeline, items)) = selected.and_then(|room_id| { + self.timelines + .lock() + .unwrap() + .get(&room_id) + .map(|timeline| (timeline.timeline.clone(), timeline.items.clone())) + }) { + // Look for the latest (most recent) room message. + let item_id = { + let items = items.lock().unwrap(); + items.iter().rev().find_map(|it| { + it.as_event() + .and_then(|ev| ev.content().as_message().is_some().then(|| ev.identifier())) + }) + }; + + // If found, send a reaction. + if let Some(item_id) = item_id { + match sdk_timeline.toggle_reaction(&item_id, "🥰").await { + Ok(_) => { + self.set_status_message("reaction sent!".to_owned()); + } + Err(err) => self.set_status_message(format!("error when reacting: {err}")), + } + } else { + self.set_status_message("no item to react to".to_owned()); + } + } else { + self.set_status_message("missing timeline for room".to_owned()); + }; + } + /// Run a small back-pagination (expect a batch of 20 events, continue until /// we get 10 timeline items or hit the timeline start). fn back_paginate(&mut self) { @@ -484,6 +519,8 @@ impl App { }; } + Char('l') => self.toggle_reaction_to_latest_msg().await, + Char('r') => self.details_mode = DetailsMode::ReadReceipts, Char('t') => self.details_mode = DetailsMode::TimelineItems, Char('e') => self.details_mode = DetailsMode::Events, From 6c7b8168d0f33186749b639bfdcdecb1c1954947 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Wed, 16 Oct 2024 12:37:28 +0200 Subject: [PATCH 4/4] chore(timeline): fix instrumentation of `update_event_send_state` This would not report the `txn_id` field because of the `skip_all`. It's actually interesting to also get the error, so I'm only skipping self from now on. --- crates/matrix-sdk-ui/src/timeline/controller/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs b/crates/matrix-sdk-ui/src/timeline/controller/mod.rs index 94ff0944459..930facfa69b 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/mod.rs @@ -735,7 +735,7 @@ impl TimelineController

{ /// /// If the corresponding local timeline item is missing, a warning is /// raised. - #[instrument(skip_all, fields(txn_id))] + #[instrument(skip(self))] pub(super) async fn update_event_send_state( &self, txn_id: &TransactionId,