feat(ui): latest_event sorter handles LatestEventValue::LocalHasBeenSent.

This patch changes the semantics of the Room List `latest_event`
sorter by changing “is local” to “is remote like”, to include the new
`LatestEventValue::LocalHasBeenSent` variant.

Author: Hywan

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

@@ -65,6 +65,18 @@ impl LatestEventValue {
         }
     }
 
+    /// Check whether the [`LatestEventValue`] represents an unsent event, i.e.
+    /// is [`LocalIsSending`] nor [`LocalCannotBeSent`].
+    ///
+    /// [`LocalIsSending`]: LatestEventValue::LocalIsSending
+    /// [`LocalCannotBeSent`]: LatestEventValue::LocalCannotBeSent
+    pub fn is_unsent(&self) -> bool {
+        match self {
+            Self::LocalIsSending(_) | Self::LocalCannotBeSent(_) => true,
+            Self::LocalHasBeenSent(_) | Self::Remote(_) | Self::None => false,
+        }
+    }
+
     /// Check whether the [`LatestEventValue`] is not set, i.e. [`None`].
     ///
     /// [`None`]: LatestEventValue::None

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

@@ -31,8 +31,8 @@ impl Room {
         self.info.read().latest_event_value.timestamp()
     }
 
-    /// Return the value of [`LatestEventValue::is_local`].
-    pub fn latest_event_is_local(&self) -> bool {
-        self.info.read().latest_event_value.is_local()
+    /// Return the value of [`LatestEventValue::is_unsent`].
+    pub fn latest_event_is_unsent(&self) -> bool {
+        self.info.read().latest_event_value.is_unsent()
     }
 }

crates/matrix-sdk-ui/src/room_list_service/room_list.rs

@@ -423,8 +423,8 @@ pub struct RoomListItem {
     /// Cache of `Room::latest_event_timestamp`.
     pub(super) cached_latest_event_timestamp: Option<MilliSecondsSinceUnixEpoch>,
 
-    /// Cache of `Room::latest_event_is_local`.
-    pub(super) cached_latest_event_is_local: bool,
+    /// Cache of `Room::latest_event_is_unsent`.
+    pub(super) cached_latest_event_is_unsent: bool,
 
     /// Cache of `Room::recency_stamp`.
     pub(super) cached_recency_stamp: Option<RoomRecencyStamp>,
@@ -448,7 +448,7 @@ impl RoomListItem {
     /// Refresh the cached data.
     pub(super) fn refresh_cached_data(&mut self) {
         self.cached_latest_event_timestamp = self.inner.latest_event_timestamp();
-        self.cached_latest_event_is_local = self.inner.latest_event_is_local();
+        self.cached_latest_event_is_unsent = self.inner.latest_event_is_unsent();
         self.cached_recency_stamp = self.inner.recency_stamp();
         self.cached_display_name = self.inner.cached_display_name().map(|name| name.to_string());
         self.cached_is_space = self.inner.is_space();
@@ -459,7 +459,7 @@ impl RoomListItem {
 impl From<Room> for RoomListItem {
     fn from(inner: Room) -> Self {
         let cached_latest_event_timestamp = inner.latest_event_timestamp();
-        let cached_latest_event_is_local = inner.latest_event_is_local();
+        let cached_latest_event_is_unsent = inner.latest_event_is_unsent();
         let cached_recency_stamp = inner.recency_stamp();
         let cached_display_name = inner.cached_display_name().map(|name| name.to_string());
         let cached_is_space = inner.is_space();
@@ -468,7 +468,7 @@ impl From<Room> for RoomListItem {
         Self {
             inner,
             cached_latest_event_timestamp,
-            cached_latest_event_is_local,
+            cached_latest_event_is_unsent,
             cached_recency_stamp,
             cached_display_name,
             cached_is_space,

crates/matrix-sdk-ui/src/room_list_service/sorters/latest_event.rs

@@ -16,14 +16,15 @@ use std::cmp::Ordering;
 
 use super::{RoomListItem, Sorter};
 
-fn cmp<F>(are_latest_events_locals: F, left: &RoomListItem, right: &RoomListItem) -> Ordering
+fn cmp<F>(are_latest_events_unsent: F, left: &RoomListItem, right: &RoomListItem) -> Ordering
 where
     F: Fn(&RoomListItem, &RoomListItem) -> (bool, bool),
 {
-    // We want local latest event to come first. When there is a remote latest event
-    // or no latest event, we don't want to sort them.
+    // We want latest events representing unsent events to come firsts. When
+    // there is a remote latest event or no latest event, we don't want to sort
+    // them.
     // NOTE: This is the same as a.cmp(b).reverse() for booleans.
-    match are_latest_events_locals(left, right) {
+    match are_latest_events_unsent(left, right) {
         // `false` and `false`, i.e.:
         // - `None` == `None`.
         // - `None` == `Remote`.
@@ -48,22 +49,15 @@ where
 }
 
 /// Create a new sorter that will sort two [`RoomListItem`] by their latest
-/// events' state: latest events representing a local event
-/// ([`LatestEventValue::LocalIsSending`] or
-/// [`LatestEventValue::LocalCannotBeSent`]) come first, and latest event
-/// representing a remote event ([`LatestEventValue::Remote`]) come last.
-///
-/// [`LatestEventValue::LocalIsSending`]: matrix_sdk_base::latest_event::LatestEventValue::LocalIsSending
-/// [`LatestEventValue::LocalCannotBeSent`]: matrix_sdk_base::latest_event::LatestEventValue::LocalCannotBeSent
-/// [`LatestEventValue::Remote`]: matrix_sdk_base::latest_event::LatestEventValue::Remote
+/// events' state: latest events representing unsent events come firsts.
 pub fn new_sorter() -> impl Sorter {
     let latest_events = |left: &RoomListItem, right: &RoomListItem| {
-        // Be careful. This method is called **a lot** in the context of a sorter. Using
-        // `Room::latest_event` would be dramatic as it returns a clone of the
-        // `LatestEventValue`. It's better to use the more specific method
-        // `Room::latest_event_is_local`, where the value is cached in this module's
-        // `Room` type.
-        (left.cached_latest_event_is_local, right.cached_latest_event_is_local)
+        // Be careful. This method is called **a lot** in the context of a
+        // sorter. Using `Room::latest_event` would be dramatic as it returns a
+        // clone of the `LatestEventValue`. It's better to use the more specific
+        // method `Room::latest_event_is_unsent`, where the value is cached
+        // in `RoomListItem`.
+        (left.cached_latest_event_is_unsent, right.cached_latest_event_is_unsent)
     };
 
     move |left, right| -> Ordering { cmp(latest_events, left, right) }
@@ -119,6 +113,16 @@ mod tests {
         })
     }
 
+    fn local_has_been_sent() -> LatestEventValue {
+        LatestEventValue::LocalHasBeenSent(LocalLatestEventValue {
+            timestamp: MilliSecondsSinceUnixEpoch(uint!(42)),
+            content: SerializableEventContent::new(&AnyMessageLikeEventContent::RoomMessage(
+                RoomMessageEventContent::text_plain("raclette"),
+            ))
+            .unwrap(),
+        })
+    }
+
     fn local_cannot_be_sent() -> LatestEventValue {
         LatestEventValue::LocalCannotBeSent(LocalLatestEventValue {
             timestamp: MilliSecondsSinceUnixEpoch(uint!(42)),
@@ -129,6 +133,10 @@ mod tests {
         })
     }
 
+    fn pair(left: LatestEventValue, right: LatestEventValue) -> (bool, bool) {
+        (left.is_unsent(), right.is_unsent())
+    }
+
     #[async_test]
     async fn test_none_or_remote_and_none_or_remote() {
         let (client, server) = logged_in_client_with_server().await;
@@ -138,34 +146,22 @@ mod tests {
 
         // `None` and `None`.
         {
-            assert_eq!(
-                cmp(|_, _| (none().is_local(), none().is_local()), &room_a, &room_b),
-                Ordering::Equal
-            );
+            assert_eq!(cmp(|_, _| pair(none(), none()), &room_a, &room_b), Ordering::Equal);
         }
 
         // `None` and `Remote`.
         {
-            assert_eq!(
-                cmp(|_, _| (none().is_local(), remote().is_local()), &room_a, &room_b),
-                Ordering::Equal
-            );
+            assert_eq!(cmp(|_, _| pair(none(), remote()), &room_a, &room_b), Ordering::Equal);
         }
 
         // `Remote` and `None`.
         {
-            assert_eq!(
-                cmp(|_, _| (remote().is_local(), none().is_local()), &room_a, &room_b),
-                Ordering::Equal
-            );
+            assert_eq!(cmp(|_, _| pair(remote(), none()), &room_a, &room_b), Ordering::Equal);
         }
 
         // `Remote` and `None`.
         {
-            assert_eq!(
-                cmp(|_, _| (remote().is_local(), remote().is_local()), &room_a, &room_b),
-                Ordering::Equal
-            );
+            assert_eq!(cmp(|_, _| pair(remote(), remote()), &room_a, &room_b), Ordering::Equal);
         }
     }
 
@@ -179,31 +175,31 @@ mod tests {
         // `None` and `Local*`.
         {
             assert_eq!(
-                cmp(|_, _| (none().is_local(), local_is_sending().is_local()), &room_a, &room_b),
+                cmp(|_, _| pair(none(), local_is_sending()), &room_a, &room_b),
                 Ordering::Greater
             );
             assert_eq!(
-                cmp(
-                    |_, _| (none().is_local(), local_cannot_be_sent().is_local()),
-                    &room_a,
-                    &room_b
-                ),
+                cmp(|_, _| pair(none(), local_has_been_sent()), &room_a, &room_b),
+                Ordering::Equal
+            );
+            assert_eq!(
+                cmp(|_, _| pair(none(), local_cannot_be_sent()), &room_a, &room_b),
                 Ordering::Greater
             );
         }
 
         // `Remote` and `Local*`.
         {
             assert_eq!(
-                cmp(|_, _| (remote().is_local(), local_is_sending().is_local()), &room_a, &room_b),
+                cmp(|_, _| pair(remote(), local_is_sending()), &room_a, &room_b),
                 Ordering::Greater
             );
             assert_eq!(
-                cmp(
-                    |_, _| (remote().is_local(), local_cannot_be_sent().is_local()),
-                    &room_a,
-                    &room_b
-                ),
+                cmp(|_, _| pair(remote(), local_has_been_sent()), &room_a, &room_b),
+                Ordering::Equal
+            );
+            assert_eq!(
+                cmp(|_, _| pair(remote(), local_cannot_be_sent()), &room_a, &room_b),
                 Ordering::Greater
             );
         }
@@ -219,31 +215,31 @@ mod tests {
         // `Local*` and `None`.
         {
             assert_eq!(
-                cmp(|_, _| (local_is_sending().is_local(), none().is_local()), &room_a, &room_b),
+                cmp(|_, _| pair(local_is_sending(), none()), &room_a, &room_b),
                 Ordering::Less
             );
             assert_eq!(
-                cmp(
-                    |_, _| (local_cannot_be_sent().is_local(), none().is_local()),
-                    &room_a,
-                    &room_b
-                ),
+                cmp(|_, _| pair(local_has_been_sent(), none()), &room_a, &room_b),
+                Ordering::Equal
+            );
+            assert_eq!(
+                cmp(|_, _| pair(local_cannot_be_sent(), none()), &room_a, &room_b),
                 Ordering::Less
             );
         }
 
         // `Local*` and `Remote`.
         {
             assert_eq!(
-                cmp(|_, _| (local_is_sending().is_local(), remote().is_local()), &room_a, &room_b),
+                cmp(|_, _| pair(local_is_sending(), remote()), &room_a, &room_b),
                 Ordering::Less
             );
             assert_eq!(
-                cmp(
-                    |_, _| (local_cannot_be_sent().is_local(), remote().is_local()),
-                    &room_a,
-                    &room_b
-                ),
+                cmp(|_, _| pair(local_has_been_sent(), remote()), &room_a, &room_b),
+                Ordering::Equal
+            );
+            assert_eq!(
+                cmp(|_, _| pair(local_cannot_be_sent(), remote()), &room_a, &room_b),
                 Ordering::Less
             );
         }
@@ -259,35 +255,41 @@ mod tests {
         // `Local*` and `Local*`.
         {
             assert_eq!(
-                cmp(
-                    |_, _| (local_is_sending().is_local(), local_is_sending().is_local()),
-                    &room_a,
-                    &room_b
-                ),
+                cmp(|_, _| pair(local_is_sending(), local_is_sending()), &room_a, &room_b),
                 Ordering::Equal
             );
             assert_eq!(
-                cmp(
-                    |_, _| (local_is_sending().is_local(), local_cannot_be_sent().is_local()),
-                    &room_a,
-                    &room_b
-                ),
+                cmp(|_, _| pair(local_is_sending(), local_has_been_sent()), &room_a, &room_b),
+                Ordering::Less
+            );
+            assert_eq!(
+                cmp(|_, _| pair(local_is_sending(), local_cannot_be_sent()), &room_a, &room_b),
                 Ordering::Equal
             );
+
             assert_eq!(
-                cmp(
-                    |_, _| (local_cannot_be_sent().is_local(), local_is_sending().is_local()),
-                    &room_a,
-                    &room_b
-                ),
+                cmp(|_, _| pair(local_has_been_sent(), local_is_sending()), &room_a, &room_b),
+                Ordering::Greater
+            );
+            assert_eq!(
+                cmp(|_, _| pair(local_has_been_sent(), local_has_been_sent()), &room_a, &room_b),
+                Ordering::Equal
+            );
+            assert_eq!(
+                cmp(|_, _| pair(local_has_been_sent(), local_cannot_be_sent()), &room_a, &room_b),
+                Ordering::Greater
+            );
+
+            assert_eq!(
+                cmp(|_, _| pair(local_cannot_be_sent(), local_is_sending()), &room_a, &room_b),
                 Ordering::Equal
             );
             assert_eq!(
-                cmp(
-                    |_, _| (local_cannot_be_sent().is_local(), local_cannot_be_sent().is_local()),
-                    &room_a,
-                    &room_b
-                ),
+                cmp(|_, _| pair(local_cannot_be_sent(), local_has_been_sent()), &room_a, &room_b),
+                Ordering::Less
+            );
+            assert_eq!(
+                cmp(|_, _| pair(local_cannot_be_sent(), local_cannot_be_sent()), &room_a, &room_b),
                 Ordering::Equal
             );
         }