Skip to content

Commit

Permalink
fix(ui): Fix latest_edit_json for live messages
Browse files Browse the repository at this point in the history
This was a regression introduced in
f0d9860.

`latest_edit_json` was first set by the call to
`EventTimelineItem::with_content()`.
It was overwritten in the next section because of the
`if let EventTimelineItemKind::Remote(remote_event)`
that uses `item` instead of `new_item`.
It means that the updated `RemoteEventTimelineItem`
inside`new_item` was replaced by the outdated one inside `item`,
so `latest_edit_json` goes back to its previous value.

I believe that part of why that went unnoticed is that the code looks
more
complicated due to the need to set an inner field, so I decided to
change the API and
move `with_encryption_info` to `EventTimelineItem`, which makes the code
look cleaner.

Signed-off-by: Kévin Commaille <[email protected]>
  • Loading branch information
zecakeh committed Jan 18, 2025
1 parent 47fc073 commit a23393b
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 15 deletions.
2 changes: 2 additions & 0 deletions crates/matrix-sdk-ui/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ All notable changes to this project will be documented in this file.
introduced due to the introduction of the banned state for rooms, and the
non-left room filter did not take the new room stat into account.
([#4448](https://github.com/matrix-org/matrix-rust-sdk/pull/4448))
- Fix `EventTimelineItem::latest_edit_json()` when it is populated by a live
edit. ([#4552](https://github.com/matrix-org/matrix-rust-sdk/pull/4552))

### Features

Expand Down
8 changes: 2 additions & 6 deletions crates/matrix-sdk-ui/src/timeline/event_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -687,12 +687,8 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {

let mut new_item = item.with_content(TimelineItemContent::Message(new_msg), edit_json);

if let EventTimelineItemKind::Remote(remote_event) = &item.kind {
if let Flow::Remote { encryption_info, .. } = &self.ctx.flow {
new_item = new_item.with_kind(EventTimelineItemKind::Remote(
remote_event.with_encryption_info(encryption_info.clone()),
));
}
if let Flow::Remote { encryption_info, .. } = &self.ctx.flow {
new_item = new_item.with_encryption_info(encryption_info.clone());
}

Some(new_item)
Expand Down
10 changes: 10 additions & 0 deletions crates/matrix-sdk-ui/src/timeline/event_item/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,16 @@ impl EventTimelineItem {
Self { sender_profile, ..self.clone() }
}

/// Clone the current event item, and update its `encryption_info`.
pub(super) fn with_encryption_info(&self, encryption_info: Option<EncryptionInfo>) -> Self {
let mut new = self.clone();
if let EventTimelineItemKind::Remote(r) = &mut new.kind {
r.encryption_info = encryption_info;
}

new
}

/// Create a clone of the current item, with content that's been redacted.
pub(super) fn redact(&self, room_version: &RoomVersionId) -> Self {
let content = self.content.redact(room_version);
Expand Down
5 changes: 0 additions & 5 deletions crates/matrix-sdk-ui/src/timeline/event_item/remote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,6 @@ pub(in crate::timeline) struct RemoteEventTimelineItem {
}

impl RemoteEventTimelineItem {
/// Clone the current event item, and update its `encryption_info`.
pub fn with_encryption_info(&self, encryption_info: Option<EncryptionInfo>) -> Self {
Self { encryption_info, ..self.clone() }
}

/// Clone the current event item, and redacts its fields.
pub fn redact(&self) -> Self {
Self { original_json: None, latest_edit_json: None, ..self.clone() }
Expand Down
8 changes: 4 additions & 4 deletions crates/matrix-sdk-ui/src/timeline/tests/encryption.rs
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,7 @@ async fn test_retry_edit_decryption() {
let item = items[1].as_event().unwrap();

assert_matches!(item.encryption_info(), Some(_));
assert_matches!(item.latest_edit_json(), Some(_));
assert_let!(TimelineItemContent::Message(msg) = item.content());
assert!(msg.is_edited());
assert_eq!(msg.body(), "This is Error");
Expand Down Expand Up @@ -452,10 +453,9 @@ async fn test_retry_edit_and_more() {
let timeline_items = timeline.controller.items().await;
assert_eq!(timeline_items.len(), 3);
assert!(timeline_items[0].is_date_divider());
assert_eq!(
timeline_items[1].as_event().unwrap().content().as_message().unwrap().body(),
"edited"
);
let timeline_event = timeline_items[1].as_event().unwrap();
assert!(timeline_event.latest_edit_json().is_some());
assert_eq!(timeline_event.content().as_message().unwrap().body(), "edited");
assert_eq!(
timeline_items[2].as_event().unwrap().content().as_message().unwrap().body(),
"Another message"
Expand Down
4 changes: 4 additions & 0 deletions crates/matrix-sdk-ui/tests/integration/timeline/edit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ async fn test_edit() {
assert_let!(Some(VectorDiff::PushBack { value: first }) = timeline_stream.next().await);
let item = first.as_event().unwrap();
assert_eq!(item.read_receipts().len(), 1, "implicit read receipt");
assert_matches!(item.latest_edit_json(), None);
assert_let!(TimelineItemContent::Message(msg) = item.content());
assert_matches!(msg.msgtype(), MessageType::Text(_));
assert_matches!(msg.in_reply_to(), None);
Expand Down Expand Up @@ -131,6 +132,7 @@ async fn test_edit() {
assert_eq!(item.read_receipts().len(), 1, "implicit read receipt");

assert_let!(TimelineItemContent::Message(msg) = item.content());
assert_matches!(item.latest_edit_json(), None);
assert_let!(MessageType::Text(TextMessageEventContent { body, .. }) = msg.msgtype());
assert_eq!(body, "Test");
assert_matches!(msg.in_reply_to(), None);
Expand All @@ -140,6 +142,7 @@ async fn test_edit() {
// something after the second event.
assert_let!(Some(VectorDiff::Set { index: 1, value: item }) = timeline_stream.next().await);
let item = item.as_event().unwrap();
assert_matches!(item.latest_edit_json(), None);
assert_let!(TimelineItemContent::Message(msg) = item.content());
assert_let!(MessageType::Text(text) = msg.msgtype());
assert_eq!(text.body, "hello");
Expand All @@ -158,6 +161,7 @@ async fn test_edit() {
// The text changes in Alice's message.
assert_let!(Some(VectorDiff::Set { index: 1, value: edit }) = timeline_stream.next().await);
let item = edit.as_event().unwrap();
assert_matches!(item.latest_edit_json(), Some(_));
assert_let!(TimelineItemContent::Message(edited) = item.content());
assert_let!(MessageType::Text(text) = edited.msgtype());
assert_eq!(text.body, "hi");
Expand Down
2 changes: 2 additions & 0 deletions crates/matrix-sdk-ui/tests/integration/timeline/subscribe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ async fn test_event_filter() {
let first_event = first.as_event().unwrap();
assert_eq!(first_event.event_id(), Some(first_event_id));
assert_eq!(first_event.read_receipts().len(), 1, "implicit read receipt");
assert_matches!(first_event.latest_edit_json(), None);
assert_let!(TimelineItemContent::Message(msg) = first_event.content());
assert_matches!(msg.msgtype(), MessageType::Text(_));
assert!(!msg.is_edited());
Expand Down Expand Up @@ -190,6 +191,7 @@ async fn test_event_filter() {
assert_let!(Some(VectorDiff::Set { index: 1, value: first }) = timeline_stream.next().await);
let first_event = first.as_event().unwrap();
assert!(first_event.read_receipts().is_empty());
assert_matches!(first_event.latest_edit_json(), Some(_));
assert_let!(TimelineItemContent::Message(msg) = first_event.content());
assert_let!(MessageType::Text(text) = msg.msgtype());
assert_eq!(text.body, "hi");
Expand Down

0 comments on commit a23393b

Please sign in to comment.