refactor(timeline): don't return a bool in Timeline::edit

See previous commit for explanations. This makes for a simpler API
anyways.

Changelog: `Timeline::edit` doesn't return a bool anymore to indicate it
couldn't manage the edit in some cases, but will return errors
indicating what the cause of the error is.

bindings/matrix-sdk-ffi/src/timeline/mod.rs

@@ -500,8 +500,8 @@ impl Timeline {
             .edit(&event_or_transaction_id.clone().try_into()?, new_content.clone().try_into()?)
             .await
         {
-            Ok(true) => Ok(()),
-            Ok(false) | Err(timeline::Error::EventNotInTimeline(_)) => {
+            Ok(()) => Ok(()),
+            Err(timeline::Error::EventNotInTimeline(_)) => {
                 // If we couldn't edit, assume it was an (remote) event that wasn't in the
                 // timeline, and try to edit it via the room itself.
                 let event_id = match event_or_transaction_id {

crates/matrix-sdk-ui/src/timeline/error.rs

@@ -14,7 +14,6 @@
 
 use matrix_sdk::{
     event_cache::{paginator::PaginatorError, EventCacheError},
-    room::edit::EditError,
     send_queue::RoomSendQueueError,
     HttpError,
 };
@@ -79,6 +78,21 @@ pub enum Error {
     RedactError(#[from] RedactError),
 }
 
+#[derive(Error, Debug)]
+pub enum EditError {
+    /// The content types have changed.
+    #[error("the new content type ({new}) doesn't match that of the previous content ({original}")]
+    ContentMismatch { original: String, new: String },
+
+    /// The local echo we tried to edit has been lost.
+    #[error("Invalid state: the local echo we tried to abort has been lost.")]
+    InvalidLocalEchoState,
+
+    /// An error happened at a lower level.
+    #[error(transparent)]
+    RoomError(#[from] matrix_sdk::room::edit::EditError),
+}
+
 #[derive(Error, Debug)]
 pub enum RedactError {
     /// Local event to redact wasn't found for transaction id

crates/matrix-sdk-ui/src/timeline/mod.rs

@@ -449,30 +449,26 @@ impl Timeline {
     ///
     /// Only supports events for which [`EventTimelineItem::is_editable()`]
     /// returns `true`.
-    ///
-    /// # Returns
-    ///
-    /// - Returns `Ok(true)` if the edit was added to the send queue.
-    /// - Returns `Ok(false)` if the edit targets an item that has no local nor
-    ///   matching remote item.
-    /// - Returns an error if there was an issue sending the redaction event, or
-    ///   interacting with the sending queue.
     #[instrument(skip(self, new_content))]
     pub async fn edit(
         &self,
         item_id: &TimelineEventItemId,
         new_content: EditedContent,
-    ) -> Result<bool, Error> {
+    ) -> Result<(), Error> {
         let items = self.items().await;
         let Some((_pos, item)) = rfind_event_by_item_id(&items, item_id) else {
             return Err(Error::EventNotInTimeline(item_id.clone()));
         };
 
         match item.handle() {
             TimelineItemHandle::Remote(event_id) => {
-                let content = self.room().make_edit_event(event_id, new_content).await?;
+                let content = self
+                    .room()
+                    .make_edit_event(event_id, new_content)
+                    .await
+                    .map_err(EditError::RoomError)?;
                 self.send(content).await?;
-                Ok(true)
+                Ok(())
             }
 
             TimelineItemHandle::Local(handle) => {
@@ -482,8 +478,11 @@ impl Timeline {
                         if matches!(item.content, TimelineItemContent::Message(_)) {
                             AnyMessageLikeEventContent::RoomMessage(message.into())
                         } else {
-                            warn!("New content (m.room.message) doesn't match previous event content.");
-                            return Ok(false);
+                            return Err(EditError::ContentMismatch {
+                                original: item.content.debug_string().to_owned(),
+                                new: "a message".to_owned(),
+                            }
+                            .into());
                         }
                     }
                     EditedContent::PollStart { new_content, .. } => {
@@ -494,13 +493,20 @@ impl Timeline {
                                 ),
                             )
                         } else {
-                            warn!("New content (poll start) doesn't match previous event content.");
-                            return Ok(false);
+                            return Err(EditError::ContentMismatch {
+                                original: item.content.debug_string().to_owned(),
+                                new: "a poll".to_owned(),
+                            }
+                            .into());
                         }
                     }
                 };
 
-                Ok(handle.edit(new_content).await.map_err(RoomSendQueueError::StorageError)?)
+                if !handle.edit(new_content).await.map_err(RoomSendQueueError::StorageError)? {
+                    return Err(EditError::InvalidLocalEchoState.into());
+                }
+
+                Ok(())
             }
         }
     }

crates/matrix-sdk-ui/tests/integration/timeline/edit.rs

@@ -30,7 +30,8 @@ use matrix_sdk_test::{
 };
 use matrix_sdk_ui::{
     timeline::{
-        Error, EventSendState, RoomExt, TimelineDetails, TimelineEventItemId, TimelineItemContent,
+        EditError, Error, EventSendState, RoomExt, TimelineDetails, TimelineEventItemId,
+        TimelineItemContent,
     },
     Timeline,
 };
@@ -235,18 +236,15 @@ async fn test_edit_local_echo() {
         .mount(&server)
         .await;
 
-    // Let's edit the local echo.
-    let did_edit = timeline
+    // Editing the local echo works, since it was in the failed state.
+    timeline
         .edit(
             &item.identifier(),
             EditedContent::RoomMessage(RoomMessageEventContent::text_plain("hello, world").into()),
         )
         .await
         .unwrap();
 
-    // We could edit the local echo, since it was in the failed state.
-    assert!(did_edit);
-
     // Observe local echo being replaced.
     assert_let!(Some(VectorDiff::Set { index: 1, value: item }) = timeline_stream.next().await);
 
@@ -410,7 +408,7 @@ async fn test_send_reply_edit() {
         .mount(&server)
         .await;
 
-    let edited = timeline
+    timeline
         .edit(
             &reply_item.identifier(),
             EditedContent::RoomMessage(RoomMessageEventContentWithoutRelation::text_plain(
@@ -419,7 +417,6 @@ async fn test_send_reply_edit() {
         )
         .await
         .unwrap();
-    assert!(edited);
 
     // Let the send queue handle the event.
     yield_now().await;
@@ -521,7 +518,7 @@ async fn test_edit_to_replied_updates_reply() {
         .await;
 
     // If I edit the first message,…
-    let edited = timeline
+    timeline
         .edit(
             &replied_to_item.identifier(),
             EditedContent::RoomMessage(RoomMessageEventContentWithoutRelation::text_plain(
@@ -530,7 +527,6 @@ async fn test_edit_to_replied_updates_reply() {
         )
         .await
         .unwrap();
-    assert!(edited);
 
     yield_now().await; // let the send queue handle the edit.
 
@@ -822,10 +818,10 @@ async fn test_edit_local_echo_with_unsupported_content() {
     };
 
     // Let's edit the local echo (message) with an unsupported type (poll start).
-    let did_edit = timeline.edit(&item.identifier(), poll_start_content).await.unwrap();
+    let edit_err = timeline.edit(&item.identifier(), poll_start_content).await.unwrap_err();
 
     // We couldn't edit the local echo, since their content types didn't match
-    assert!(!did_edit);
+    assert_matches!(edit_err, Error::EditError(EditError::ContentMismatch { .. }));
 
     timeline
         .send(AnyMessageLikeEventContent::UnstablePollStart(UnstablePollStartEventContent::New(
@@ -840,18 +836,18 @@ async fn test_edit_local_echo_with_unsupported_content() {
     assert_matches!(item.send_state(), Some(EventSendState::NotSentYet));
 
     // Let's edit the local echo (poll start) with an unsupported type (message).
-    let did_edit = timeline
+    let edit_err = timeline
         .edit(
             &item.identifier(),
             EditedContent::RoomMessage(RoomMessageEventContentWithoutRelation::text_plain(
                 "edited",
             )),
         )
         .await
-        .unwrap();
+        .unwrap_err();
 
     // We couldn't edit the local echo, since their content types didn't match
-    assert!(!did_edit);
+    assert_matches!(edit_err, Error::EditError(EditError::ContentMismatch { .. }));
 }
 
 struct PendingEditHelper {

testing/matrix-sdk-integration-testing/src/tests/timeline.rs

@@ -289,17 +289,15 @@ async fn test_stale_local_echo_time_abort_edit() {
     }
 
     // Now do a crime: try to edit the local echo.
-    let did_edit = timeline
+    // The edit works on the local echo and applies to the remote echo \o/.
+    timeline
         .edit(
             &local_echo.identifier(),
             EditedContent::RoomMessage(RoomMessageEventContent::text_plain("bonjour").into()),
         )
         .await
         .unwrap();
 
-    // The edit works on the local echo and applies to the remote echo \o/.
-    assert!(did_edit);
-
     let vector_diff = timeout(Duration::from_secs(5), stream.next()).await.unwrap().unwrap();
     let remote_echo = assert_matches!(vector_diff, VectorDiff::Set { index: 0, value } => value);
     assert!(!remote_echo.is_local_echo());