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,19 @@ impl LatestEventValue {
         }
     }
 
+    /// Check whether an event is remote-like, i.e. has been sent to a
+    /// homeserver (by this client, or another), i.e. is **not**
+    /// [`LocalIsSending`] nor [`LocalCannotBeSent`].
+    ///
+    /// [`LocalIsSending`]: LatestEventValue::LocalIsSending
+    /// [`LocalCannotBeSent`]: LatestEventValue::LocalCannotBeSent
+    pub fn is_remote_like(&self) -> bool {
+        match self {
+            Self::LocalIsSending(_) | Self::LocalCannotBeSent(_) => false,
+            Self::LocalHasBeenSent(_) | Self::Remote(_) | Self::None => true,
+        }
+    }
+
     /// 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_remote_like`].
+    pub fn latest_event_is_remote_like(&self) -> bool {
+        self.info.read().latest_event_value.is_remote_like()
     }
 }

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_remote_like`.
+    pub(super) cached_latest_event_is_remote_like: 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_remote_like = self.inner.latest_event_is_remote_like();
         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_remote_like = inner.latest_event_is_remote_like();
         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_remote_like,
             cached_recency_stamp,
             cached_display_name,
             cached_is_space,

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

@@ -12,18 +12,22 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use std::cmp::Ordering;
+use std::{cmp::Ordering, ops::Not};
 
 use super::{RoomListItem, Sorter};
 
-fn cmp<F>(are_latest_events_locals: F, left: &RoomListItem, right: &RoomListItem) -> Ordering
+fn cmp<F>(
+    are_latest_events_not_remote_like: 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.
     // NOTE: This is the same as a.cmp(b).reverse() for booleans.
-    match are_latest_events_locals(left, right) {
+    match are_latest_events_not_remote_like(left, right) {
         // `false` and `false`, i.e.:
         // - `None` == `None`.
         // - `None` == `Remote`.
@@ -61,9 +65,12 @@ pub fn new_sorter() -> impl Sorter {
         // 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)
+        // `Room::latest_event_is_remote_like`, where the value is cached in
+        // `RoomListItem`.
+        (
+            left.cached_latest_event_is_remote_like.not(),
+            right.cached_latest_event_is_remote_like.not(),
+        )
     };
 
     move |left, right| -> Ordering { cmp(latest_events, left, right) }
@@ -119,6 +126,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 +146,10 @@ mod tests {
         })
     }
 
+    fn pair(left: LatestEventValue, right: LatestEventValue) -> (bool, bool) {
+        (left.is_remote_like().not(), right.is_remote_like().not())
+    }
+
     #[async_test]
     async fn test_none_or_remote_and_none_or_remote() {
         let (client, server) = logged_in_client_with_server().await;
@@ -138,34 +159,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 +188,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 +228,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 +268,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
             );
         }