ui: Fix day divider logic

… for when a remote event is re-received while a local echo is pending.
Also simplify test_togglling_reaction integration test so it still passes.

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

@@ -999,30 +999,23 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
 
                 // Local echoes that are pending should stick to the bottom,
                 // find the latest event that isn't that.
-                let mut latest_event_stream = self
+                let (latest_event_idx, latest_event) = self
                     .items
                     .iter()
                     .enumerate()
                     .rev()
-                    .filter_map(|(idx, item)| Some((idx, item.as_event()?)));
-
-                // Find the latest event, independently of success or failure status.
-                let latest_event = latest_event_stream.clone().next().unzip().1;
-
-                // Find the index of the latest non-failure event.
-                let latest_nonfailed_event_idx = latest_event_stream
-                    .find(|(_, evt)| {
+                    .filter_map(|(idx, item)| Some((idx, item.as_event()?)))
+                    .find(|(_, item)| {
                         !matches!(
-                            evt.send_state(),
+                            item.send_state(),
                             Some(EventSendState::NotSentYet | EventSendState::Sent { .. })
                         )
                     })
-                    .unzip()
-                    .0;
+                    .unzip();
 
-                // Insert the next item after the latest non-failed event item,
-                // or at the start if there is no such item.
-                let mut insert_idx = latest_nonfailed_event_idx.map_or(0, |idx| idx + 1);
+                // Insert the next item after the latest event item that's not a
+                // pending local echo, or at the start if there is no such item.
+                let mut insert_idx = latest_event_idx.map_or(0, |idx| idx + 1);
 
                 // Keep push semantics, if we're inserting at the end.
                 let should_push = insert_idx == self.items.len();

crates/matrix-sdk-ui/src/timeline/tests/echo.rs

@@ -165,3 +165,49 @@ async fn remote_echo_new_position() {
     let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value);
     assert!(!item.as_event().unwrap().is_local_echo());
 }
+
+#[async_test]
+async fn day_divider_duplication() {
+    let timeline = TestTimeline::new();
+
+    // Given two remote events from one day, and a local event from another day…
+    timeline.handle_live_message_event(&BOB, RoomMessageEventContent::text_plain("A")).await;
+    timeline.handle_live_message_event(&BOB, RoomMessageEventContent::text_plain("B")).await;
+    timeline
+        .handle_local_event(AnyMessageLikeEventContent::RoomMessage(
+            RoomMessageEventContent::text_plain("C"),
+        ))
+        .await;
+
+    let items = timeline.inner.items().await;
+    assert_eq!(items.len(), 5);
+    assert!(items[0].is_day_divider());
+    assert!(items[1].is_remote_event());
+    assert!(items[2].is_remote_event());
+    assert!(items[3].is_day_divider());
+    assert!(items[4].is_local_echo());
+
+    // … when the second remote event is re-received (day still the same)
+    let event_id = items[2].as_event().unwrap().event_id().unwrap();
+    timeline
+        .handle_live_custom_event(json!({
+            "content": {
+                "body": "B",
+                "msgtype": "m.text",
+            },
+            "sender": &*BOB,
+            "event_id": event_id,
+            "origin_server_ts": 1,
+            "type": "m.room.message",
+        }))
+        .await;
+
+    // … it should not impact the day dividers.
+    let items = timeline.inner.items().await;
+    assert_eq!(items.len(), 5);
+    assert!(items[0].is_day_divider());
+    assert!(items[1].is_remote_event());
+    assert!(items[2].is_remote_event());
+    assert!(items[3].is_day_divider());
+    assert!(items[4].is_local_echo());
+}

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

@@ -19,7 +19,7 @@ use assert_matches::assert_matches;
 use assign::assign;
 use eyeball_im::VectorDiff;
 use futures_core::Stream;
-use futures_util::{future::join_all, StreamExt};
+use futures_util::{future::join_all, FutureExt, StreamExt};
 use matrix_sdk::{
     config::SyncSettings,
     ruma::{
@@ -50,41 +50,19 @@ async fn test_toggling_reaction() -> Result<()> {
 
     // Send message
     timeline.send(RoomMessageEventContent::text_plain("hi!").into(), None).await;
-    {
-        let _day_divider = stream.next().await;
-        let event =
-            assert_matches!(stream.next().await, Some(VectorDiff::PushBack { value }) => value);
-        let event = event.as_event().unwrap();
-        assert_matches!(event.content().as_message(), Some(_));
-        assert!(event.is_local_echo());
-        assert!(event.reactions().is_empty());
-    }
 
-    // Receive updated local echo with event ID
-    let event_id = {
-        let event = assert_matches!(stream.next().await, Some(VectorDiff::Set { index: 1, value }) => value);
-        let event = event.as_event().unwrap();
-        event.event_id().unwrap().to_owned()
+    // Sync until the remote echo arrives
+    let event_id = loop {
+        sync_room(&alice, room_id).await?;
+        let items = timeline.items().await;
+        let last_item = items.last().unwrap().as_event().unwrap();
+        if !last_item.is_local_echo() {
+            break last_item.event_id().unwrap().to_owned();
+        }
     };
 
-    // Sync remaining remote events
-    sync_room(&alice, room_id).await?;
-    {
-        let _room_create = stream.next().await;
-        let _membership_change = stream.next().await;
-        let _power_levels = stream.next().await;
-        let _join_rules = stream.next().await;
-        let _history_visibility = stream.next().await;
-        let _guest_access = stream.next().await;
-    }
-
-    // Check we have the remote echo
-    {
-        let event = assert_matches!(stream.next().await, Some(VectorDiff::Set { index: 7, value }) => value);
-        let event = event.as_event().unwrap();
-        assert_eq!(event.event_id().unwrap(), &event_id);
-        assert!(!event.is_local_echo());
-    };
+    // Skip all stream updates that have happened so far
+    while stream.next().now_or_never().is_some() {}
 
     let message_position = timeline.items().await.len() - 1;
     let reaction = Annotation::new(event_id.clone(), reaction_key.into());