From 90d6a37b315cdbb41f9c9f23719b5223aa09be89 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Mon, 4 Nov 2024 15:58:20 +0100 Subject: [PATCH] refactor(timeline): factor out in-reply-to updates --- .../src/timeline/event_handler.rs | 96 +++++++------------ .../timeline/event_item/content/message.rs | 10 +- .../src/timeline/tests/redaction.rs | 10 +- 3 files changed, 43 insertions(+), 73 deletions(-) diff --git a/crates/matrix-sdk-ui/src/timeline/event_handler.rs b/crates/matrix-sdk-ui/src/timeline/event_handler.rs index c482a1a43b1..20358cae735 100644 --- a/crates/matrix-sdk-ui/src/timeline/event_handler.rs +++ b/crates/matrix-sdk-ui/src/timeline/event_handler.rs @@ -547,25 +547,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { let internal_id = item.internal_id.to_owned(); // Update all events that replied to this message with the edited content. - self.items.for_each(|mut entry| { - let Some(event_item) = entry.as_event() else { return }; - let Some(message) = event_item.content.as_message() else { return }; - let Some(in_reply_to) = message.in_reply_to() else { return }; - if replacement.event_id == in_reply_to.event_id { - trace!(reply_event_id = ?event_item.identifier(), "Updating response to edited event"); - let in_reply_to = InReplyToDetails { - event_id: in_reply_to.event_id.clone(), - event: TimelineDetails::Ready(Box::new( - RepliedToEvent::from_timeline_item(&new_item), - )), - }; - let new_reply_content = - TimelineItemContent::Message(message.with_in_reply_to(in_reply_to)); - let new_reply_item = - entry.with_kind(event_item.with_content(new_reply_content, None)); - ObservableVectorTransactionEntry::set(&mut entry, new_reply_item); - } - }); + Self::maybe_update_responses(self.items, &replacement.event_id, &new_item); // Update the event itself. self.items.set(item_pos, TimelineItem::new(new_item, internal_id)); @@ -945,7 +927,13 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { debug!("event item is already redacted"); } else { let new_item = item.redact(&self.meta.room_version); - self.items.set(idx, TimelineItem::new(new_item, item.internal_id.to_owned())); + let internal_id = item.internal_id.to_owned(); + + // Look for any timeline event that's a reply to the redacted event, and redact + // the replied-to event there as well. + Self::maybe_update_responses(self.items, &redacted, &new_item); + + self.items.set(idx, TimelineItem::new(new_item, internal_id)); self.result.items_updated += 1; } } else { @@ -954,26 +942,6 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { } else { debug!("Timeline item not found, discarding redaction"); }; - - // Look for any timeline event that's a reply to the redacted event, and redact - // the replied-to event there as well. - self.items.for_each(|mut entry| { - let Some(event_item) = entry.as_event() else { return }; - let Some(message) = event_item.content.as_message() else { return }; - let Some(in_reply_to) = message.in_reply_to() else { return }; - let TimelineDetails::Ready(replied_to_event) = &in_reply_to.event else { return }; - if redacted == in_reply_to.event_id { - let replied_to_event = replied_to_event.redact(&self.meta.room_version); - let in_reply_to = InReplyToDetails { - event_id: in_reply_to.event_id.clone(), - event: TimelineDetails::Ready(Box::new(replied_to_event)), - }; - let content = TimelineItemContent::Message(message.with_in_reply_to(in_reply_to)); - let new_item = entry.with_kind(event_item.with_content(content, None)); - - ObservableVectorTransactionEntry::set(&mut entry, new_item); - } - }); } /// Attempts to redact a reaction, local or remote. @@ -1198,25 +1166,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { trace!("Updating timeline item at position {idx}"); // Update all events that replied to this previously encrypted message. - self.items.for_each(|mut entry| { - let Some(event_item) = entry.as_event() else { return }; - let Some(message) = event_item.content.as_message() else { return }; - let Some(in_reply_to) = message.in_reply_to() else { return }; - if *decrypted_event_id == in_reply_to.event_id { - trace!(reply_event_id = ?event_item.identifier(), "Updating response to edited event"); - let in_reply_to = InReplyToDetails { - event_id: in_reply_to.event_id.clone(), - event: TimelineDetails::Ready(Box::new( - RepliedToEvent::from_timeline_item(&item), - )), - }; - let new_reply_content = - TimelineItemContent::Message(message.with_in_reply_to(in_reply_to)); - let new_reply_item = - entry.with_kind(event_item.with_content(new_reply_content, None)); - ObservableVectorTransactionEntry::set(&mut entry, new_reply_item); - } - }); + Self::maybe_update_responses(self.items, decrypted_event_id, &item); let internal_id = self.items[*idx].internal_id.clone(); self.items.set(*idx, TimelineItem::new(item, internal_id)); @@ -1229,6 +1179,34 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { } } + /// After updating the timeline item `new_item` which id is + /// `target_event_id`, update other items that are responses to this item. + fn maybe_update_responses( + items: &mut ObservableVectorTransaction<'_, Arc>, + target_event_id: &EventId, + new_item: &EventTimelineItem, + ) { + items.for_each(|mut entry| { + let Some(event_item) = entry.as_event() else { return }; + let Some(message) = event_item.content.as_message() else { return }; + let Some(in_reply_to) = message.in_reply_to() else { return }; + if target_event_id == in_reply_to.event_id { + trace!(reply_event_id = ?event_item.identifier(), "Updating response to edited event"); + let in_reply_to = InReplyToDetails { + event_id: in_reply_to.event_id.clone(), + event: TimelineDetails::Ready(Box::new( + RepliedToEvent::from_timeline_item(new_item), + )), + }; + let new_reply_content = + TimelineItemContent::Message(message.with_in_reply_to(in_reply_to)); + let new_reply_item = + entry.with_kind(event_item.with_content(new_reply_content, None)); + ObservableVectorTransactionEntry::set(&mut entry, new_reply_item); + } + }); + } + fn pending_reactions( &mut self, content: &TimelineItemContent, diff --git a/crates/matrix-sdk-ui/src/timeline/event_item/content/message.rs b/crates/matrix-sdk-ui/src/timeline/event_item/content/message.rs index 4e361e3862b..09ee071f923 100644 --- a/crates/matrix-sdk-ui/src/timeline/event_item/content/message.rs +++ b/crates/matrix-sdk-ui/src/timeline/event_item/content/message.rs @@ -35,7 +35,7 @@ use ruma::{ }, html::RemoveReplyFallback, serde::Raw, - OwnedEventId, OwnedUserId, RoomVersionId, UserId, + OwnedEventId, OwnedUserId, UserId, }; use tracing::{error, trace}; @@ -337,14 +337,6 @@ impl RepliedToEvent { } } - pub(in crate::timeline) fn redact(&self, room_version: &RoomVersionId) -> Self { - Self { - content: self.content.redact(room_version), - sender: self.sender.clone(), - sender_profile: self.sender_profile.clone(), - } - } - /// Try to create a `RepliedToEvent` from a `TimelineEvent` by providing the /// room. pub async fn try_from_timeline_event_for_room( diff --git a/crates/matrix-sdk-ui/src/timeline/tests/redaction.rs b/crates/matrix-sdk-ui/src/timeline/tests/redaction.rs index a42b6f3afcf..e592660f074 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/redaction.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/redaction.rs @@ -91,17 +91,17 @@ async fn test_redact_replied_to_event() { timeline.handle_live_event(f.redaction(first_item.event_id().unwrap()).sender(&ALICE)).await; - let first_item_again = - assert_next_matches!(stream, VectorDiff::Set { index: 0, value } => value); - assert_matches!(first_item_again.content(), TimelineItemContent::RedactedMessage); - assert_matches!(first_item_again.original_json(), None); - let second_item_again = assert_next_matches!(stream, VectorDiff::Set { index: 1, value } => value); let message = second_item_again.content().as_message().unwrap(); let in_reply_to = message.in_reply_to().unwrap(); assert_let!(TimelineDetails::Ready(replied_to_event) = &in_reply_to.event); assert_matches!(replied_to_event.content(), TimelineItemContent::RedactedMessage); + + let first_item_again = + assert_next_matches!(stream, VectorDiff::Set { index: 0, value } => value); + assert_matches!(first_item_again.content(), TimelineItemContent::RedactedMessage); + assert_matches!(first_item_again.original_json(), None); } #[async_test]