Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -515,8 +515,9 @@ impl<P: RoomDataProvider> TimelineStateTransaction<'_, P> {
}

for (user_id, receipt) in receipts {
if own_receipt_thread == ReceiptThread::Unthreaded {
// If the own receipt thread is unthreaded, we maintain maximal
if matches!(own_receipt_thread, ReceiptThread::Unthreaded | ReceiptThread::Main)
{
// If the own receipt thread is unthreaded or main, we maintain maximal
// compatibility with clients using either unthreaded or main-thread read
// receipts by allowing both here.
if !matches!(
Expand Down
71 changes: 71 additions & 0 deletions crates/matrix-sdk-ui/src/timeline/tests/read_receipts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -826,3 +826,74 @@ async fn test_threaded_latest_user_read_receipt() {
assert_eq!(receipt_event_id, event_id!("$3"));
assert_eq!(receipt.thread, receipt_thread);
}

#[async_test]
async fn test_unthreaded_client_updates_threaded_read_receipts() {
let timeline = TestTimelineBuilder::new()
.settings(TimelineSettings { track_read_receipts: true, ..Default::default() })
.focus(TimelineFocus::Live { hide_threaded_events: true })
.build();
let mut stream = timeline.subscribe().await;

let event_b = event_id!("$event_b");

// Alice sends 2 messages
let f = &timeline.factory;
timeline.handle_live_event(f.text_msg("A").sender(*ALICE)).await;
timeline.handle_live_event(f.text_msg("B").sender(*ALICE).event_id(event_b)).await;
Comment on lines +840 to +843
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious to propose a small change in this test, that I think wouldn't require a change in the rest of the code, in theory :)

Let's add a third event with id event_c, that's sent by Alice in a thread.

This event should be filtered out, because the timeline is set to hide_threaded_events.

Now, make Bob's read receipt point to event_c.

I think the outcome should remain the same, and the read receipt on event_b should be set to Bob's.

(This enhanced version proves that, even if the unthreaded read receipt is on an in-thread event, we can put it "at the right position" in the no-threaded-events timeline.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea! Added it in 875ec25, fits right in 👍


assert_next_matches!(stream, VectorDiff::PushBack { .. });
assert_next_matches!(stream, VectorDiff::PushFront { .. });
assert_next_matches!(stream, VectorDiff::PushBack { .. });
assert_pending!(stream);

// Bob reads the last one from an unthreaded client
timeline
.handle_read_receipts([(
event_b.to_owned(),
ReceiptType::Read,
BOB.to_owned(),
ReceiptThread::Unthreaded,
)])
.await;

// Alice's timeline gets updated
let item_b = assert_next_matches!(stream, VectorDiff::Set { index: 2, value } => value);
let event_b = item_b.as_event().unwrap();
assert_eq!(event_b.read_receipts().len(), 1);
assert!(event_b.read_receipts().get(*BOB).is_some());
assert_pending!(stream);

// Then Alice sends a message in a thread
let event_c = event_id!("$event_c");
let thread_event_id = event_id!("$thread");

timeline
.handle_live_event(
f.text_msg("C")
.sender(*ALICE)
.event_id(event_c)
.in_thread(thread_event_id, event_id!("$latest")),
)
.await;

// Alice is using a threaded client so the main timeline shouldn't change
assert_pending!(stream);

// Bob reads the threaded message
timeline
.handle_read_receipts([(
event_c.to_owned(),
ReceiptType::Read,
BOB.to_owned(),
ReceiptThread::Thread(thread_event_id.to_owned()),
)])
.await;

// The main timeline read receipts are still correct
let event_b = timeline.controller.items().await[2].as_event().unwrap().to_owned();
assert_eq!(event_b.read_receipts().len(), 1);
assert!(event_b.read_receipts().get(*BOB).is_some());

assert_pending!(stream);
}
Loading