From f8e65f53cdaa18a3a0e7776cdc91c3d2a6003edf Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Wed, 2 Oct 2024 18:30:22 +0200 Subject: [PATCH] timeline: provide the edit JSON for edits either pending or bundled --- .../src/timeline/controller/mod.rs | 4 +- .../src/timeline/controller/state.rs | 24 ++-- .../src/timeline/event_handler.rs | 122 +++++++++++------- .../matrix-sdk-ui/src/timeline/tests/edit.rs | 13 +- .../tests/integration/timeline/edit.rs | 7 +- 5 files changed, 111 insertions(+), 59 deletions(-) diff --git a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs b/crates/matrix-sdk-ui/src/timeline/controller/mod.rs index b884d8af1..9095a9493 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/mod.rs @@ -52,8 +52,8 @@ use tracing::{ }; pub(super) use self::state::{ - EventMeta, FullEventMeta, PendingEdit, TimelineEnd, TimelineMetadata, TimelineState, - TimelineStateTransaction, + EventMeta, FullEventMeta, PendingEdit, PendingEditKind, TimelineEnd, TimelineMetadata, + TimelineState, TimelineStateTransaction, }; use super::{ event_handler::TimelineEventKind, diff --git a/crates/matrix-sdk-ui/src/timeline/controller/state.rs b/crates/matrix-sdk-ui/src/timeline/controller/state.rs index 4cc3af19a..b4f8b60f7 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/state.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/state.rs @@ -35,7 +35,7 @@ use ruma::{ }, relation::Replacement, room::message::RoomMessageEventContentWithoutRelation, - AnySyncEphemeralRoomEvent, + AnySyncEphemeralRoomEvent, AnySyncTimelineEvent, }, push::Action, serde::Raw, @@ -811,16 +811,22 @@ impl PendingPollEvents { } #[derive(Clone)] -pub(in crate::timeline) enum PendingEdit { +pub(in crate::timeline) enum PendingEditKind { RoomMessage(Replacement), Poll(Replacement), } +#[derive(Clone)] +pub(in crate::timeline) struct PendingEdit { + pub kind: PendingEditKind, + pub event_json: Raw, +} + impl PendingEdit { pub fn edited_event(&self) -> &EventId { - match self { - PendingEdit::RoomMessage(Replacement { event_id, .. }) - | PendingEdit::Poll(Replacement { event_id, .. }) => event_id, + match &self.kind { + PendingEditKind::RoomMessage(Replacement { event_id, .. }) + | PendingEditKind::Poll(Replacement { event_id, .. }) => event_id, } } } @@ -828,9 +834,11 @@ impl PendingEdit { #[cfg(not(tarpaulin_include))] impl std::fmt::Debug for PendingEdit { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::RoomMessage(_) => f.debug_struct("RoomMessage").finish_non_exhaustive(), - Self::Poll(_) => f.debug_struct("Poll").finish_non_exhaustive(), + match &self.kind { + PendingEditKind::RoomMessage(_) => { + f.debug_struct("RoomMessage").finish_non_exhaustive() + } + PendingEditKind::Poll(_) => f.debug_struct("Poll").finish_non_exhaustive(), } } } diff --git a/crates/matrix-sdk-ui/src/timeline/event_handler.rs b/crates/matrix-sdk-ui/src/timeline/event_handler.rs index 4cc55834d..50b7dd90b 100644 --- a/crates/matrix-sdk-ui/src/timeline/event_handler.rs +++ b/crates/matrix-sdk-ui/src/timeline/event_handler.rs @@ -49,7 +49,7 @@ use ruma::{ use tracing::{debug, error, field::debug, info, instrument, trace, warn}; use super::{ - controller::{TimelineMetadata, TimelineStateTransaction}, + controller::{PendingEditKind, TimelineMetadata, TimelineStateTransaction}, day_dividers::DayDividerAdjuster, event_item::{ extract_edit_content, AnyOtherFullStateEventContent, EventSendState, EventTimelineItemKind, @@ -339,20 +339,40 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { event_id, ) }) - .and_then(|edit| { - Some(as_variant!(edit, PendingEdit::RoomMessage)?.new_content) + .and_then(|edit| match edit.kind { + PendingEditKind::RoomMessage(replacement) => { + Some((Some(edit.event_json), replacement.new_content)) + } + _ => None, }); - let edit = extract_edit_content(relations).or(pending_edit); + let (edit_json, edit_content) = extract_edit_content(relations) + .map(|content| { + let edit_json = as_variant!(&self.ctx.flow, Flow::Remote { raw_event, ..} => raw_event).and_then(|raw| { + // Kids, don't do this at home. We're extracting the edit event + // from the `unsigned`.m.relations`.`m.replace` path. + let raw_unsigned: Raw = raw.get_field("unsigned").ok()??; + let raw_relations: Raw = raw_unsigned.get_field("m.relations").ok()??; + raw_relations.get_field::>("m.replace").ok()? + }); - self.add_item(TimelineItemContent::message(c, edit, self.items)); + (edit_json, content) + } + ).or(pending_edit).unzip(); + + let edit_json = edit_json.flatten(); + + self.add_item( + TimelineItemContent::message(c, edit_content, self.items), + edit_json, + ); } } AnyMessageLikeEventContent::RoomEncrypted(c) => { // TODO: Handle replacements if the replaced event is also UTD let cause = UtdCause::determine(raw_event); - self.add_item(TimelineItemContent::unable_to_decrypt(c, cause)); + self.add_item(TimelineItemContent::unable_to_decrypt(c, cause), None); // Let the hook know that we ran into an unable-to-decrypt that is added to the // timeline. @@ -365,7 +385,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { AnyMessageLikeEventContent::Sticker(content) => { if should_add { - self.add_item(TimelineItemContent::Sticker(Sticker { content })); + self.add_item(TimelineItemContent::Sticker(Sticker { content }), None); } } @@ -383,13 +403,13 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { AnyMessageLikeEventContent::CallInvite(_) => { if should_add { - self.add_item(TimelineItemContent::CallInvite); + self.add_item(TimelineItemContent::CallInvite, None); } } AnyMessageLikeEventContent::CallNotify(_) => { if should_add { - self.add_item(TimelineItemContent::CallNotify) + self.add_item(TimelineItemContent::CallNotify, None) } } @@ -404,7 +424,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { TimelineEventKind::RedactedMessage { event_type } => { if event_type != MessageLikeEventType::Reaction && should_add { - self.add_item(TimelineItemContent::RedactedMessage); + self.add_item(TimelineItemContent::RedactedMessage, None); } } @@ -414,7 +434,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { TimelineEventKind::RoomMember { user_id, content, sender } => { if should_add { - self.add_item(TimelineItemContent::room_member(user_id, content, sender)); + self.add_item(TimelineItemContent::room_member(user_id, content, sender), None); } } @@ -422,29 +442,28 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { // Update room encryption if a `m.room.encryption` event is found in the // timeline if should_add { - self.add_item(TimelineItemContent::OtherState(OtherState { - state_key, - content, - })); + self.add_item( + TimelineItemContent::OtherState(OtherState { state_key, content }), + None, + ); } } TimelineEventKind::FailedToParseMessageLike { event_type, error } => { if should_add { - self.add_item(TimelineItemContent::FailedToParseMessageLike { - event_type, - error, - }); + self.add_item( + TimelineItemContent::FailedToParseMessageLike { event_type, error }, + None, + ); } } TimelineEventKind::FailedToParseState { event_type, state_key, error } => { if should_add { - self.add_item(TimelineItemContent::FailedToParseState { - event_type, - state_key, - error, - }); + self.add_item( + TimelineItemContent::FailedToParseState { event_type, state_key, error }, + None, + ); } } } @@ -474,14 +493,19 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { replacement: Replacement, ) { if let Some((item_pos, item)) = rfind_event_by_id(self.items, &replacement.event_id) { - if let Some(new_item) = self.apply_msg_edit(&item, replacement) { + let edit_json = + as_variant!(&self.ctx.flow, Flow::Remote { raw_event, .. } => raw_event).cloned(); + if let Some(new_item) = self.apply_msg_edit(&item, replacement, edit_json) { trace!("Applied edit"); self.items.set(item_pos, TimelineItem::new(new_item, item.internal_id.to_owned())); self.result.items_updated += 1; } - } else if let Flow::Remote { position, .. } = &self.ctx.flow { + } else if let Flow::Remote { position, raw_event, .. } = &self.ctx.flow { let replaced_event_id = replacement.event_id.clone(); - let replacement = PendingEdit::RoomMessage(replacement); + let replacement = PendingEdit { + kind: PendingEditKind::RoomMessage(replacement), + event_json: raw_event.clone(), + }; self.stash_pending_edit(*position, replaced_event_id, replacement); } else { debug!("Local message edit for a timeline item not found, discarding"); @@ -553,14 +577,16 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { TimelineItemContent::Message(..) => { let pending = Self::find_and_remove_pending(&mut self.meta.pending_edits, event_id)?; - let edit = as_variant!(pending, PendingEdit::RoomMessage)?; - self.apply_msg_edit(item, edit) + let edit = as_variant!(pending.kind, PendingEditKind::RoomMessage)?; + //let json = as_variant!(&self.ctx.flow, Flow::Remote { raw_event, .. } => + // raw_event.clone()); self.apply_msg_edit(item, edit, json) + self.apply_msg_edit(item, edit, Some(pending.event_json)) } TimelineItemContent::Poll(..) => { let pending = Self::find_and_remove_pending(&mut self.meta.pending_edits, event_id)?; - let edit = as_variant!(pending, PendingEdit::Poll)?; - self.apply_poll_edit(item, edit) + let edit = as_variant!(pending.kind, PendingEditKind::Poll)?; + self.apply_poll_edit(item, edit, Some(pending.event_json)) } _ => None, } @@ -574,6 +600,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { &self, item: &EventTimelineItem, replacement: Replacement, + edit_json: Option>, ) -> Option { if self.ctx.sender != item.sender() { info!( @@ -591,11 +618,6 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { return None; }; - let edit_json = match &self.ctx.flow { - Flow::Local { .. } => None, - Flow::Remote { raw_event, .. } => Some(raw_event.clone()), - }; - let mut new_msg = msg.clone(); new_msg.apply_edit(replacement.new_content); @@ -697,9 +719,12 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { replacement: Replacement, ) { let Some((item_pos, item)) = rfind_event_by_id(self.items, &replacement.event_id) else { - if let Flow::Remote { position, .. } = &self.ctx.flow { + if let Flow::Remote { position, raw_event, .. } = &self.ctx.flow { let replaced_event_id = replacement.event_id.clone(); - let replacement = PendingEdit::Poll(replacement); + let replacement = PendingEdit { + kind: PendingEditKind::Poll(replacement), + event_json: raw_event.clone(), + }; self.stash_pending_edit(*position, replaced_event_id, replacement); } else { debug!("Local poll edit for a timeline item not found, discarding"); @@ -707,7 +732,10 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { return; }; - let Some(new_item) = self.apply_poll_edit(item.inner, replacement) else { + let edit_json = + as_variant!(&self.ctx.flow, Flow::Remote { raw_event, .. } => raw_event.clone()); + + let Some(new_item) = self.apply_poll_edit(item.inner, replacement, edit_json) else { return; }; @@ -720,6 +748,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { &self, item: &EventTimelineItem, replacement: Replacement, + edit_json: Option>, ) -> Option { if self.ctx.sender != item.sender() { info!( @@ -742,11 +771,6 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { } }; - let edit_json = match &self.ctx.flow { - Flow::Local { .. } => None, - Flow::Remote { raw_event, .. } => Some(raw_event.clone()), - }; - Some(item.with_content(new_content, edit_json)) } @@ -761,7 +785,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { } if should_add { - self.add_item(TimelineItemContent::Poll(poll_state)); + self.add_item(TimelineItemContent::Poll(poll_state), None); } } @@ -916,7 +940,11 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { } /// Add a new event item in the timeline. - fn add_item(&mut self, content: TimelineItemContent) { + fn add_item( + &mut self, + content: TimelineItemContent, + edit_json: Option>, + ) { self.result.item_added = true; let sender = self.ctx.sender.to_owned(); @@ -955,7 +983,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { is_highlighted: self.ctx.is_highlighted, encryption_info: encryption_info.clone(), original_json: Some(raw_event.clone()), - latest_edit_json: None, + latest_edit_json: edit_json, origin, } .into() diff --git a/crates/matrix-sdk-ui/src/timeline/tests/edit.rs b/crates/matrix-sdk-ui/src/timeline/tests/edit.rs index ddfcdead8..2e6329d72 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/edit.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/edit.rs @@ -259,7 +259,18 @@ async fn test_relations_edit_overrides_pending_edit() { let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); // We receive the latest edit, not the pending one. - let text = item.as_event().unwrap().content().as_message().unwrap(); + let event = item.as_event().unwrap(); + assert_eq!( + event + .latest_edit_json() + .expect("we should have an edit json") + .deserialize() + .unwrap() + .event_id(), + edit2_event_id + ); + + let text = event.content().as_message().unwrap(); assert_eq!(text.body(), "edit 2"); let day_divider = assert_next_matches!(stream, VectorDiff::PushFront { value } => value); diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/edit.rs b/crates/matrix-sdk-ui/tests/integration/timeline/edit.rs index 512ea98b8..0ad3b7353 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/edit.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/edit.rs @@ -851,7 +851,12 @@ async fn test_pending_edit() { // Then I get the edited content immediately. assert_let!(Some(VectorDiff::PushBack { value }) = timeline_stream.next().await); - let msg = value.as_event().unwrap().content().as_message().unwrap(); + + let event = value.as_event().unwrap(); + let latest_edit_json = event.latest_edit_json().expect("we should have an edit json"); + assert_eq!(latest_edit_json.deserialize().unwrap().event_id(), edit_event_id); + + let msg = event.content().as_message().unwrap(); assert!(msg.is_edited()); assert_eq!(msg.body(), "[edit]");