pinned events(refactor): simplify handling of AddTimelineEvents

The timeline already listens to changes to the pinned events list (via a
stream), so there's no need to fully reload all the pinned events every
time we receive a new event that's pinned. Technically it may avoid one
or a few lookups, but this is cheap and a subsequent commit/PR will
merge the pinned event cache into the event cache.

Author: bnjbvr

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

@@ -22,7 +22,7 @@ use matrix_sdk::{
 };
 use ruma::{events::AnySyncTimelineEvent, RoomVersionId};
 use tokio::sync::broadcast::error::RecvError;
-use tracing::{error, info, info_span, trace, warn, Instrument, Span};
+use tracing::{info, info_span, trace, warn, Instrument, Span};
 
 #[cfg(feature = "e2e-encryption")]
 use super::to_device::{handle_forwarded_room_key_event, handle_room_key_event};
@@ -207,8 +207,6 @@ impl TimelineBuilder {
                 info_span!(parent: Span::none(), "room_update_handler", room_id = ?room.room_id());
             span.follows_from(Span::current());
 
-            let focus = Arc::new(focus);
-
             async move {
                 trace!("Spawned the event subscriber task.");
 
@@ -265,22 +263,9 @@ impl TimelineBuilder {
                         RoomEventCacheUpdate::AddTimelineEvents { events, origin } => {
                             trace!("Received new timeline events.");
 
-                            // Special case for pinned events: when we receive new events what
-                            // we'll do is, instead of adding the events, update the pinned events
-                            // cache with them, reload the list of pinned event ids and reload the
-                            // list of pinned events with this info.
-
-                            // TODO(bnjbvr): why is this done? the timeline already listens to
-                            // changes to the pinned events list, so we shouldn't have to trigger a
-                            // manual update here too.
-                            if let TimelineFocus::PinnedEvents { .. } = &*focus.clone() {
-                                if let Some(ret) = inner.update_pinned_events_if_needed(events, &pinned_event_cache).await {
-                                    match ret {
-                                        Ok(events) => inner.replace_with_initial_remote_events(events, RemoteEventOrigin::Sync).await,
-                                        Err(err) => error!("Couldn't update pinned events with incoming timeline events: {err}"),
-                                    }
-                                }
-                            } else {
+                            // Note: we deliberately choose to not handle
+                            // updates/reactions/redactions for pinned events.
+                            if !is_pinned_events {
                                 inner.add_events_at(
                                     events,
                                     TimelineEnd::Back,

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

@@ -354,20 +354,6 @@ impl<P: RoomDataProvider> TimelineInner<P> {
         }
     }
 
-    pub(crate) async fn update_pinned_events_if_needed(
-        &self,
-        events: Vec<SyncTimelineEvent>,
-        pinned_event_cache: &PinnedEventCache,
-    ) -> Option<Result<Vec<SyncTimelineEvent>, PinnedEventsLoaderError>> {
-        let focus_guard = self.focus.read().await;
-
-        if let TimelineFocusData::PinnedEvents { loader } = &*focus_guard {
-            loader.update_if_needed(events, pinned_event_cache).await
-        } else {
-            Some(Err(PinnedEventsLoaderError::TimelineFocusNotPinnedEvents))
-        }
-    }
-
     /// Run a backward pagination (in focused mode) and append the results to
     /// the timeline.
     ///

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

@@ -118,35 +118,6 @@ impl PinnedEventsLoader {
 
         Ok(loaded_events)
     }
-
-    /// Updates the cache with the provided events if they're associated with a
-    /// pinned event id, then reloads the list of pinned events if there
-    /// were any changes.
-    ///
-    /// Returns an `Option` with either a new list of events if any of them
-    /// changed in this update or `None` if they didn't.
-    pub async fn update_if_needed(
-        &self,
-        events: Vec<impl Into<SyncTimelineEvent>>,
-        cache: &PinnedEventCache,
-    ) -> Option<Result<Vec<SyncTimelineEvent>, PinnedEventsLoaderError>> {
-        let mut to_update = Vec::new();
-        for ev in events {
-            let ev = ev.into();
-            if let Some(ev_id) = ev.event_id() {
-                if self.room.is_pinned_event(&ev_id) {
-                    to_update.push(ev);
-                }
-            }
-        }
-
-        if !to_update.is_empty() {
-            cache.set_bulk(to_update).await;
-            Some(self.load_events(cache).await)
-        } else {
-            None
-        }
-    }
 }
 
 #[cfg_attr(target_arch = "wasm32", async_trait::async_trait(?Send))]