Skip to content

Commit

Permalink
fix(timeline): avoid adding non-pinned events to the pinned events ti…
Browse files Browse the repository at this point in the history
…meline when back paginating
  • Loading branch information
jmartinesp committed Jan 20, 2025
1 parent 47fc073 commit fe5cdc2
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 27 deletions.
50 changes: 26 additions & 24 deletions crates/matrix-sdk-ui/src/timeline/controller/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -600,31 +600,33 @@ impl TimelineStateTransaction<'_> {
.map_or(RemoteEventOrigin::Unknown, |item| item.origin),
};

match origin {
RemoteEventOrigin::Sync | RemoteEventOrigin::Unknown => {
should_add = match self.timeline_focus {
TimelineFocusKind::PinnedEvents => {
// Only insert timeline items for pinned events, if the event
// came from the sync.
room_data_provider.is_pinned_event(&event_id)
}

TimelineFocusKind::Live => {
// Always add new items to a live timeline receiving items from
// sync.
true
}

TimelineFocusKind::Event => {
// Never add any item to a focused timeline when the item comes
// down from the sync.
false
}
};
// If the event should be added according to the general event filter, use a
// second filter to decide whether it should be added depending on the timeline
// focus and events origin, if needed
match self.timeline_focus {
TimelineFocusKind::PinnedEvents => {
// Only add pinned events for the pinned events timeline
should_add = room_data_provider.is_pinned_event(&event_id);
}

RemoteEventOrigin::Pagination | RemoteEventOrigin::Cache => {
// Forward the previous decision to add it.
TimelineFocusKind::Live => {
if matches!(
origin,
RemoteEventOrigin::Sync | RemoteEventOrigin::Unknown
) {
// Always add new items to a live timeline receiving items from
// sync.
should_add = true;
}
}
TimelineFocusKind::Event => {
if matches!(
origin,
RemoteEventOrigin::Sync | RemoteEventOrigin::Unknown
) {
// Never add any item to a focused timeline when the item comes
// down from the sync.
should_add = false;
}
}
}
}
Expand Down
85 changes: 82 additions & 3 deletions crates/matrix-sdk-ui/tests/integration/timeline/pinned_event.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
use std::time::Duration;
use std::{ops::ControlFlow, time::Duration};

use assert_matches::assert_matches;
use eyeball_im::VectorDiff;
use matrix_sdk::{
assert_next_matches_with_timeout, config::SyncSettings, sync::SyncResponse,
test_utils::logged_in_client_with_server, Client,
assert_next_matches_with_timeout,
config::SyncSettings,
event_cache::{BackPaginationOutcome, TimelineHasBeenResetWhilePaginating},
sync::SyncResponse,
test_utils::logged_in_client_with_server,
Client,
};
use matrix_sdk_base::deserialized_responses::TimelineEvent;
use matrix_sdk_test::{
Expand Down Expand Up @@ -395,6 +399,81 @@ async fn test_pinned_timeline_with_no_pinned_events_and_an_utd_is_just_empty() {
test_helper.server.reset().await;
}

#[async_test]
async fn test_pinned_timeline_with_no_pinned_events_on_pagination_is_just_empty() {
let mut test_helper = TestHelper::new().await;
let room_id = test_helper.room_id.clone();
let event_id = event_id!("$1.localhost");
let sender_id = owned_user_id!("@example:localhost");

// Join the room
let _ = test_helper.setup_initial_sync_response().await;
test_helper.server.reset().await;

// Load initial timeline items: an empty `m.room.pinned_events` event
test_helper.setup_sync_response(Vec::new(), Some(Vec::new())).await.expect("Sync failed");

let room = test_helper.client.get_room(&room_id).unwrap();
let pinned_timeline =
Timeline::builder(&room).with_focus(pinned_events_focus(1)).build().await.unwrap();

// Create a non-pinned event
let not_pinned_event = EventFactory::new()
.room(&room_id)
.sender(&sender_id)
.text_msg("Hey")
.event_id(event_id)
.into_raw_timeline();

mock_event(
&test_helper.server,
&room_id,
event_id,
TimelineEvent::new(not_pinned_event.clone()),
)
.await;

// The timeline couldn't load any events, but it expected none, so it just
// returns an empty list
let (pinned_items, mut pinned_events_stream) = pinned_timeline.subscribe().await;
assert!(pinned_items.is_empty());

// Mock the /messages endpoint with the not pinned event
Mock::given(method("GET"))
.and(path_regex(r"^/_matrix/client/r0/rooms/.*/messages$"))
.and(header("authorization", "Bearer 1234"))
.respond_with(ResponseTemplate::new(200).set_body_json(json!({
"start": "prev1",
"chunk": vec![not_pinned_event],
})))
.expect(1)
.mount(&test_helper.server)
.await;

let (event_cache, _) = room.event_cache().await.expect("Event cache should be accessible");

async fn once(
outcome: BackPaginationOutcome,
_timeline_has_been_reset: TimelineHasBeenResetWhilePaginating,
) -> ControlFlow<BackPaginationOutcome, ()> {
ControlFlow::Break(outcome)
}

// Paginate backwards once using the event cache to load the event
event_cache
.pagination()
.run_backwards(10, once)
.await
.expect("Pagination of events should successful");

// Assert the event is loaded and added to the cache
assert!(event_cache.event(event_id).await.is_some());

// And it won't cause an update in the pinned events timeline since it's not
// pinned
assert_pending!(pinned_events_stream);
}

#[async_test]
async fn test_pinned_timeline_with_pinned_utd_contains_it() {
let test_helper = TestHelper::new().await;
Expand Down

0 comments on commit fe5cdc2

Please sign in to comment.