From b793acd2b155a0fbf36eca4990927f765322c98c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Tue, 8 Oct 2024 15:42:01 +0200 Subject: [PATCH] sdk-ui: allow already sent local events to be redacted using `redact_by_id` Test this use case. --- crates/matrix-sdk-ui/src/timeline/error.rs | 3 - crates/matrix-sdk-ui/src/timeline/mod.rs | 37 +++-- .../tests/integration/timeline/mod.rs | 133 ++++++++++++++++++ 3 files changed, 151 insertions(+), 22 deletions(-) diff --git a/crates/matrix-sdk-ui/src/timeline/error.rs b/crates/matrix-sdk-ui/src/timeline/error.rs index 2afb365e8b8..b1a89956474 100644 --- a/crates/matrix-sdk-ui/src/timeline/error.rs +++ b/crates/matrix-sdk-ui/src/timeline/error.rs @@ -86,9 +86,6 @@ pub enum RedactError { #[error("Local event to redact wasn't found for transaction {0}")] LocalEventNotFound(OwnedTransactionId), - #[error("Local event with transaction id {0} had a remote `TimelineItemHandle`. This should never happen.")] - InvalidTimelineItemHandle(OwnedTransactionId), - /// An error happened while attempting to redact an event. #[error(transparent)] HttpError(#[from] HttpError), diff --git a/crates/matrix-sdk-ui/src/timeline/mod.rs b/crates/matrix-sdk-ui/src/timeline/mod.rs index d4be98e8746..433611c44e4 100644 --- a/crates/matrix-sdk-ui/src/timeline/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/mod.rs @@ -601,32 +601,31 @@ impl Timeline { id: &TimelineEventItemId, reason: Option<&str>, ) -> Result<(), Error> { - match id { + let event_id = match id { TimelineEventItemId::TransactionId(transaction_id) => { - let item = self.item_by_transaction_id(transaction_id).await; + let Some(item) = self.item_by_transaction_id(transaction_id).await else { + return Err(Error::RedactError(RedactError::LocalEventNotFound( + transaction_id.to_owned(), + ))); + }; - match item.as_ref().map(|i| i.handle()) { - Some(TimelineItemHandle::Local(handle)) => { + match item.handle() { + TimelineItemHandle::Local(handle) => { + // If there is a local item that hasn't been sent yet, abort the upload handle.abort().await.map_err(RoomSendQueueError::StorageError)?; - Ok(()) + return Ok(()); } - Some(TimelineItemHandle::Remote(_)) => Err(Error::RedactError( - RedactError::InvalidTimelineItemHandle(transaction_id.to_owned()), - )), - None => Err(Error::RedactError(RedactError::LocalEventNotFound( - transaction_id.to_owned(), - ))), + TimelineItemHandle::Remote(event_id) => event_id.to_owned(), } } - TimelineEventItemId::EventId(event_id) => { - self.room() - .redact(event_id, reason, None) - .await - .map_err(|e| Error::RedactError(RedactError::HttpError(e)))?; + TimelineEventItemId::EventId(event_id) => event_id.to_owned(), + }; + self.room() + .redact(&event_id, reason, None) + .await + .map_err(|e| Error::RedactError(RedactError::HttpError(e)))?; - Ok(()) - } - } + Ok(()) } /// Redact an event. diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/mod.rs b/crates/matrix-sdk-ui/tests/integration/timeline/mod.rs index c05f83498ec..76fb281f9a8 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/mod.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/mod.rs @@ -19,6 +19,7 @@ use assert_matches2::assert_let; use eyeball_im::VectorDiff; use futures_util::StreamExt; use matrix_sdk::{ + assert_let_timeout, config::SyncSettings, test_utils::{events::EventFactory, logged_in_client_with_server}, }; @@ -307,6 +308,72 @@ async fn test_redact_message() { assert_matches!(timeline_stream.next().await, Some(VectorDiff::Remove { index: 2 })); } +#[async_test] +async fn test_redact_local_sent_message() { + let room_id = room_id!("!a98sd12bjh:example.org"); + let (client, server) = logged_in_client_with_server().await; + let sync_settings = SyncSettings::new().timeout(Duration::from_millis(3000)); + + let mut sync_builder = SyncResponseBuilder::new(); + sync_builder.add_joined_room(JoinedRoomBuilder::new(room_id)); + + mock_sync(&server, sync_builder.build_json_sync_response(), None).await; + let _response = client.sync_once(sync_settings.clone()).await.unwrap(); + server.reset().await; + + mock_encryption_state(&server, false).await; + + let room = client.get_room(room_id).unwrap(); + let timeline = room.timeline().await.unwrap(); + let (_, mut timeline_stream) = timeline.subscribe().await; + + // Mock event sending. + Mock::given(method("PUT")) + .and(path_regex(r"^/_matrix/client/r0/rooms/.*/send/.*")) + .and(header("authorization", "Bearer 1234")) + .respond_with( + ResponseTemplate::new(200) + .set_body_json(json!({ "event_id": "$wWgymRfo7ri1uQx0NXO40vLJ" })), + ) + .expect(1) + .mount(&server) + .await; + + // Send the event so it's added to the send queue as a local event. + timeline + .send(RoomMessageEventContent::text_plain("i will disappear soon").into()) + .await + .unwrap(); + + // Assert the local event is in the timeline now and is not sent yet. + assert_let_timeout!(Some(VectorDiff::PushBack { value: item }) = timeline_stream.next()); + let event = item.as_event().unwrap(); + assert!(event.is_local_echo()); + assert_matches!(event.send_state(), Some(EventSendState::NotSentYet)); + + // As well as a day divider. + assert_let_timeout!( + Some(VectorDiff::PushFront { value: day_divider }) = timeline_stream.next() + ); + assert!(day_divider.is_day_divider()); + + // We receive an update in the timeline from the send queue. + assert_let_timeout!(Some(VectorDiff::Set { index, value: item }) = timeline_stream.next()); + assert_eq!(index, 1); + + // Check the event is sent but still considered local. + let event = item.as_event().unwrap(); + assert!(event.is_local_echo()); + assert_matches!(event.send_state(), Some(EventSendState::Sent { .. })); + + // Mock the redaction response for the event we just sent. Ensure it's called + // once. + mock_redaction(event.event_id().unwrap()).expect(1).mount(&server).await; + + // Let's redact the local echo with the remote handle. + timeline.redact(event, None).await.unwrap(); +} + #[async_test] async fn test_redact_by_id_message() { let room_id = room_id!("!a98sd12bjh:example.org"); @@ -381,6 +448,72 @@ async fn test_redact_by_id_message() { assert_matches!(timeline_stream.next().await, Some(VectorDiff::Remove { index: 2 })); } +#[async_test] +async fn test_redact_by_local_sent_message() { + let room_id = room_id!("!a98sd12bjh:example.org"); + let (client, server) = logged_in_client_with_server().await; + let sync_settings = SyncSettings::new().timeout(Duration::from_millis(3000)); + + let mut sync_builder = SyncResponseBuilder::new(); + sync_builder.add_joined_room(JoinedRoomBuilder::new(room_id)); + + mock_sync(&server, sync_builder.build_json_sync_response(), None).await; + let _response = client.sync_once(sync_settings.clone()).await.unwrap(); + server.reset().await; + + mock_encryption_state(&server, false).await; + + let room = client.get_room(room_id).unwrap(); + let timeline = room.timeline().await.unwrap(); + let (_, mut timeline_stream) = timeline.subscribe().await; + + // Mock event sending. + Mock::given(method("PUT")) + .and(path_regex(r"^/_matrix/client/r0/rooms/.*/send/.*")) + .and(header("authorization", "Bearer 1234")) + .respond_with( + ResponseTemplate::new(200) + .set_body_json(json!({ "event_id": "$wWgymRfo7ri1uQx0NXO40vLJ" })), + ) + .expect(1) + .mount(&server) + .await; + + // Send the event so it's added to the send queue as a local event. + timeline + .send(RoomMessageEventContent::text_plain("i will disappear soon").into()) + .await + .unwrap(); + + // Assert the local event is in the timeline now and is not sent yet. + assert_let_timeout!(Some(VectorDiff::PushBack { value: item }) = timeline_stream.next()); + let event = item.as_event().unwrap(); + assert!(event.is_local_echo()); + assert_matches!(event.send_state(), Some(EventSendState::NotSentYet)); + + // As well as a day divider. + assert_let_timeout!( + Some(VectorDiff::PushFront { value: day_divider }) = timeline_stream.next() + ); + assert!(day_divider.is_day_divider()); + + // We receive an update in the timeline from the send queue. + assert_let_timeout!(Some(VectorDiff::Set { index, value: item }) = timeline_stream.next()); + assert_eq!(index, 1); + + // Check the event is sent but still considered local. + let event = item.as_event().unwrap(); + assert!(event.is_local_echo()); + assert_matches!(event.send_state(), Some(EventSendState::Sent { .. })); + + // Mock the redaction response for the event we just sent. Ensure it's called + // once. + mock_redaction(event.event_id().unwrap()).expect(1).mount(&server).await; + + // Let's redact the local echo with the remote handle. + timeline.redact_by_id(&event.identifier(), None).await.unwrap(); +} + #[async_test] async fn test_redact_by_id_message_with_no_remote_message_present() { let room_id = room_id!("!a98sd12bjh:example.org");