fix(sdk): Introduce LatestEventValue::LocalHasBeenSent.

The problem we are trying to solve is the following:

- a local event is being sent,
- the `LatestEventValue` is `LocalIsSending`,
- the local event is finally sent,
- the `LatestEventValue` is still `LocalIsSending` purposely, with the
  hope that an update from the Event Cache will replace it.

But sometimes, this update from the Event Cache comes **before** the
update from the Send Queue. Why is it problem? Because updates from the
Event Cache are ignored until the buffer of local `LatestEventValue`s
aren't empty, which means that if an update from the Event Cache is
received before `RoomSendQueueUpdate::SentEvent`, it is ignored, and the
`LatestEventValue` stays in the `LocalIsSending` state. That's annoying.

The idea is to introduce a new state: `LocalHasBeenSent` which mimics
`Remote`, but for a local event. It clarifies the state of a sent event,
without relying on the Event Cache.

Author: Hywan

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

@@ -31,8 +31,9 @@ use matrix_sdk_common::{
 };
 use matrix_sdk_ui::timeline::{
     self, AttachmentConfig, AttachmentSource, EventItemOrigin,
-    LatestEventValue as UiLatestEventValue, MediaUploadProgress as SdkMediaUploadProgress, Profile,
-    TimelineDetails, TimelineUniqueId as SdkTimelineUniqueId,
+    LatestEventValue as UiLatestEventValue, LatestEventValueLocalState,
+    MediaUploadProgress as SdkMediaUploadProgress, Profile, TimelineDetails,
+    TimelineUniqueId as SdkTimelineUniqueId,
 };
 use mime::Mime;
 use reply::{EmbeddedEventDetails, InReplyToDetails};
@@ -1301,7 +1302,7 @@ impl LazyTimelineItemProvider {
 }
 
 /// Mimic the [`UiLatestEventValue`] type.
-#[derive(Clone, uniffi::Enum)]
+#[derive(uniffi::Enum)]
 pub enum LatestEventValue {
     None,
     Remote {
@@ -1316,7 +1317,7 @@ pub enum LatestEventValue {
         sender: String,
         profile: ProfileDetails,
         content: TimelineItemContent,
-        is_sending: bool,
+        state: LatestEventValueLocalState,
     },
 }
 
@@ -1333,13 +1334,13 @@ impl From<UiLatestEventValue> for LatestEventValue {
                     content: content.into(),
                 }
             }
-            UiLatestEventValue::Local { timestamp, sender, profile, content, is_sending } => {
+            UiLatestEventValue::Local { timestamp, sender, profile, content, state } => {
                 Self::Local {
                     timestamp: timestamp.into(),
                     sender: sender.to_string(),
                     profile: profile.into(),
                     content: content.into(),
-                    is_sending,
+                    state,
                 }
             }
         }

crates/matrix-sdk-base/src/latest_event.rs

@@ -19,6 +19,10 @@ pub enum LatestEventValue {
     /// The latest event represents a local event that is sending.
     LocalIsSending(LocalLatestEventValue),
 
+    /// The latest event represents a local event that has been sent
+    /// successfully. It should come quickly as a [`Self::Remote`].
+    LocalHasBeenSent(LocalLatestEventValue),
+
     /// The latest event represents a local event that cannot be sent, either
     /// because a previous local event, or this local event cannot be sent.
     LocalCannotBeSent(LocalLatestEventValue),
@@ -28,19 +32,21 @@ impl LatestEventValue {
     /// Get the timestamp of the [`LatestEventValue`].
     ///
     /// If it's [`None`], it returns `None`. If it's [`Remote`], it returns the
-    /// [`TimelineEvent::timestamp`]. If it's [`LocalIsSending`] or
-    /// [`LocalCannotBeSent`], it returns the
+    /// [`TimelineEvent::timestamp`]. If it's [`LocalIsSending`],
+    /// [`LocalHasBeenSent`] or [`LocalCannotBeSent`], it returns the
     /// [`LocalLatestEventValue::timestamp`] value.
     ///
     /// [`None`]: LatestEventValue::None
     /// [`Remote`]: LatestEventValue::Remote
     /// [`LocalIsSending`]: LatestEventValue::LocalIsSending
+    /// [`LocalHasBeenSent`]: LatestEventValue::LocalHasBeenSent
     /// [`LocalCannotBeSent`]: LatestEventValue::LocalCannotBeSent
     pub fn timestamp(&self) -> Option<MilliSecondsSinceUnixEpoch> {
         match self {
             Self::None => None,
             Self::Remote(remote_latest_event_value) => remote_latest_event_value.timestamp(),
             Self::LocalIsSending(LocalLatestEventValue { timestamp, .. })
+            | Self::LocalHasBeenSent(LocalLatestEventValue { timestamp, .. })
             | Self::LocalCannotBeSent(LocalLatestEventValue { timestamp, .. }) => Some(*timestamp),
         }
     }
@@ -52,7 +58,9 @@ impl LatestEventValue {
     /// [`LocalCannotBeSent`]: LatestEventValue::LocalCannotBeSent
     pub fn is_local(&self) -> bool {
         match self {
-            Self::LocalIsSending(_) | Self::LocalCannotBeSent(_) => true,
+            Self::LocalIsSending(_) | Self::LocalHasBeenSent(_) | Self::LocalCannotBeSent(_) => {
+                true
+            }
             Self::None | Self::Remote(_) => false,
         }
     }
@@ -132,6 +140,19 @@ mod tests_latest_event_value {
         assert_eq!(value.timestamp(), Some(MilliSecondsSinceUnixEpoch(uint!(42))));
     }
 
+    #[test]
+    fn test_timestamp_with_local_has_been_sent() {
+        let value = LatestEventValue::LocalHasBeenSent(LocalLatestEventValue {
+            timestamp: MilliSecondsSinceUnixEpoch(uint!(42)),
+            content: SerializableEventContent::new(&AnyMessageLikeEventContent::RoomMessage(
+                RoomMessageEventContent::text_plain("raclette"),
+            ))
+            .unwrap(),
+        });
+
+        assert_eq!(value.timestamp(), Some(MilliSecondsSinceUnixEpoch(uint!(42))));
+    }
+
     #[test]
     fn test_timestamp_with_local_cannot_be_sent() {
         let value = LatestEventValue::LocalCannotBeSent(LocalLatestEventValue {

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

@@ -69,12 +69,19 @@ pub enum LatestEventValue {
         /// The content of the local event.
         content: TimelineItemContent,
 
-        /// Whether the local event is sending if it is set to `true`, otherwise
-        /// it cannot be sent.
-        is_sending: bool,
+        /// Whether the local event is sending, has been sent or cannot be sent.
+        state: LatestEventValueLocalState,
     },
 }
 
+#[derive(Debug)]
+#[cfg_attr(feature = "uniffi", derive(uniffi::Enum))]
+pub enum LatestEventValueLocalState {
+    IsSending,
+    HasBeenSent,
+    CannotBeSent,
+}
+
 impl LatestEventValue {
     pub(crate) async fn from_base_latest_event_value(
         value: BaseLatestEventValue,
@@ -153,14 +160,11 @@ impl LatestEventValue {
                     _ => Self::None,
                 }
             }
-            BaseLatestEventValue::LocalIsSending(LocalLatestEventValue {
-                timestamp,
-                content: ref serialized_content,
-            })
-            | BaseLatestEventValue::LocalCannotBeSent(LocalLatestEventValue {
-                timestamp,
-                content: ref serialized_content,
-            }) => {
+            BaseLatestEventValue::LocalIsSending(ref local_value)
+            | BaseLatestEventValue::LocalHasBeenSent(ref local_value)
+            | BaseLatestEventValue::LocalCannotBeSent(ref local_value) => {
+                let LocalLatestEventValue { timestamp, content: serialized_content } = local_value;
+
                 let Ok(message_like_event_content) = serialized_content.deserialize() else {
                     return Self::None;
                 };
@@ -172,12 +176,28 @@ impl LatestEventValue {
                     .await
                     .map(TimelineDetails::Ready)
                     .unwrap_or(TimelineDetails::Unavailable);
-                let is_sending = matches!(value, BaseLatestEventValue::LocalIsSending(_));
 
                 match TimelineAction::from_content(message_like_event_content, None, None, None) {
-                    TimelineAction::AddItem { content } => {
-                        Self::Local { timestamp, sender, profile, content, is_sending }
-                    }
+                    TimelineAction::AddItem { content } => Self::Local {
+                        timestamp: *timestamp,
+                        sender,
+                        profile,
+                        content,
+                        state: match value {
+                            BaseLatestEventValue::LocalIsSending(_) => {
+                                LatestEventValueLocalState::IsSending
+                            }
+                            BaseLatestEventValue::LocalHasBeenSent(_) => {
+                                LatestEventValueLocalState::HasBeenSent
+                            }
+                            BaseLatestEventValue::LocalCannotBeSent(_) => {
+                                LatestEventValueLocalState::CannotBeSent
+                            }
+                            BaseLatestEventValue::Remote(_) | BaseLatestEventValue::None => {
+                                unreachable!("Only local latest events are supposed to be handled");
+                            }
+                        },
+                    },
 
                     TimelineAction::HandleAggregation { kind, .. } => {
                         // Add some debug logging here to help diagnose issues with the latest
@@ -210,7 +230,7 @@ mod tests {
 
     use super::{
         super::{MsgLikeContent, MsgLikeKind, TimelineItemContent},
-        BaseLatestEventValue, LatestEventValue, TimelineDetails,
+        BaseLatestEventValue, LatestEventValue, LatestEventValueLocalState, TimelineDetails,
     };
 
     #[async_test]
@@ -275,15 +295,43 @@ mod tests {
         let value =
             LatestEventValue::from_base_latest_event_value(base_value, &room, &client).await;
 
-        assert_matches!(value, LatestEventValue::Local { timestamp, sender, profile, content, is_sending } => {
+        assert_matches!(value, LatestEventValue::Local { timestamp, sender, profile, content, state } => {
+            assert_eq!(u64::from(timestamp.get()), 42u64);
+            assert_eq!(sender, "@example:localhost");
+            assert_matches!(profile, TimelineDetails::Unavailable);
+            assert_matches!(
+                content,
+                TimelineItemContent::MsgLike(MsgLikeContent { kind: MsgLikeKind::Message(_), .. })
+            );
+            assert_matches!(state, LatestEventValueLocalState::IsSending);
+        })
+    }
+
+    #[async_test]
+    async fn test_local_has_been_sent() {
+        let server = MatrixMockServer::new().await;
+        let client = server.client_builder().build().await;
+        let room = server.sync_room(&client, JoinedRoomBuilder::new(room_id!("!r0"))).await;
+
+        let base_value = BaseLatestEventValue::LocalHasBeenSent(LocalLatestEventValue {
+            timestamp: MilliSecondsSinceUnixEpoch(uint!(42)),
+            content: SerializableEventContent::new(&AnyMessageLikeEventContent::RoomMessage(
+                RoomMessageEventContent::text_plain("raclette"),
+            ))
+            .unwrap(),
+        });
+        let value =
+            LatestEventValue::from_base_latest_event_value(base_value, &room, &client).await;
+
+        assert_matches!(value, LatestEventValue::Local { timestamp, sender, profile, content, state } => {
             assert_eq!(u64::from(timestamp.get()), 42u64);
             assert_eq!(sender, "@example:localhost");
             assert_matches!(profile, TimelineDetails::Unavailable);
             assert_matches!(
                 content,
                 TimelineItemContent::MsgLike(MsgLikeContent { kind: MsgLikeKind::Message(_), .. })
             );
-            assert!(is_sending);
+            assert_matches!(state, LatestEventValueLocalState::HasBeenSent);
         })
     }
 
@@ -303,15 +351,15 @@ mod tests {
         let value =
             LatestEventValue::from_base_latest_event_value(base_value, &room, &client).await;
 
-        assert_matches!(value, LatestEventValue::Local { timestamp, sender, profile, content, is_sending } => {
+        assert_matches!(value, LatestEventValue::Local { timestamp, sender, profile, content, state } => {
             assert_eq!(u64::from(timestamp.get()), 42u64);
             assert_eq!(sender, "@example:localhost");
             assert_matches!(profile, TimelineDetails::Unavailable);
             assert_matches!(
                 content,
                 TimelineItemContent::MsgLike(MsgLikeContent { kind: MsgLikeKind::Message(_), .. })
             );
-            assert!(is_sending.not());
+            assert_matches!(state, LatestEventValueLocalState::CannotBeSent);
         })
     }
 

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

@@ -101,7 +101,7 @@ pub use self::{
     },
     event_type_filter::TimelineEventTypeFilter,
     item::{TimelineItem, TimelineItemKind, TimelineUniqueId},
-    latest_event::LatestEventValue,
+    latest_event::{LatestEventValue, LatestEventValueLocalState},
     traits::RoomExt,
     virtual_item::VirtualTimelineItem,
 };