Skip to content

Commit 07141d7

Browse files
committed
fix(ui): Sending read receipt in live timeline when latest event is in a thread
Previously, this used the latest event in the thread as the event to mark as read, while this is not right if we're in a context that hides thread events
1 parent 4fbc83a commit 07141d7

File tree

4 files changed

+93
-58
lines changed

4 files changed

+93
-58
lines changed

crates/matrix-sdk-ui/src/timeline/controller/metadata.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -577,10 +577,16 @@ pub(in crate::timeline) struct EventMeta {
577577
/// Note that the #2 timeline item (the day divider) doesn't map to any
578578
/// remote event, but if it moves, it has an impact on this mapping.
579579
pub timeline_item_index: Option<usize>,
580+
581+
pub thread_root_id: Option<OwnedEventId>,
580582
}
581583

582584
impl EventMeta {
583-
pub fn new(event_id: OwnedEventId, visible: bool) -> Self {
584-
Self { event_id, visible, timeline_item_index: None }
585+
pub fn new(
586+
event_id: OwnedEventId,
587+
visible: bool,
588+
thread_root_id: Option<OwnedEventId>,
589+
) -> Self {
590+
Self { event_id, visible, timeline_item_index: None, thread_root_id }
585591
}
586592
}

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

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1761,7 +1761,27 @@ impl TimelineController {
17611761
/// it's folded into another timeline item.
17621762
pub(crate) async fn latest_event_id(&self) -> Option<OwnedEventId> {
17631763
let state = self.state.read().await;
1764-
state.items.all_remote_events().last().map(|event_meta| &event_meta.event_id).cloned()
1764+
let filter_out_thread_events = match self.focus() {
1765+
TimelineFocusKind::Thread { .. } => false,
1766+
TimelineFocusKind::Live { hide_threaded_events } => hide_threaded_events.to_owned(),
1767+
TimelineFocusKind::Event { paginator } => {
1768+
paginator.get().is_some_and(|paginator| paginator.hide_threaded_events())
1769+
}
1770+
_ => true,
1771+
};
1772+
state
1773+
.items
1774+
.all_remote_events()
1775+
.iter()
1776+
.rev()
1777+
.filter_map(|item| {
1778+
if !filter_out_thread_events || item.thread_root_id.is_none() {
1779+
Some(item.event_id.clone())
1780+
} else {
1781+
None
1782+
}
1783+
})
1784+
.next()
17651785
}
17661786

17671787
#[instrument(skip(self), fields(room_id = ?self.room().room_id()))]

crates/matrix-sdk-ui/src/timeline/controller/observable_items.rs

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -796,7 +796,12 @@ mod observable_items_tests {
796796
}
797797

798798
fn event_meta(event_id: &str) -> EventMeta {
799-
EventMeta { event_id: event_id.parse().unwrap(), timeline_item_index: None, visible: false }
799+
EventMeta {
800+
event_id: event_id.parse().unwrap(),
801+
timeline_item_index: None,
802+
visible: false,
803+
thread_root_id: None,
804+
}
800805
}
801806

802807
macro_rules! assert_event_id {
@@ -1922,11 +1927,6 @@ impl AllRemoteEvents {
19221927
Some(event_meta)
19231928
}
19241929

1925-
/// Return a reference to the last remote event if it exists.
1926-
pub fn last(&self) -> Option<&EventMeta> {
1927-
self.0.back()
1928-
}
1929-
19301930
/// Return the index of the last remote event if it exists.
19311931
pub fn last_index(&self) -> Option<usize> {
19321932
self.0.len().checked_sub(1)
@@ -2054,7 +2054,12 @@ mod all_remote_events_tests {
20542054
use super::{AllRemoteEvents, EventMeta};
20552055

20562056
fn event_meta(event_id: &str, timeline_item_index: Option<usize>) -> EventMeta {
2057-
EventMeta { event_id: event_id.parse().unwrap(), timeline_item_index, visible: false }
2057+
EventMeta {
2058+
event_id: event_id.parse().unwrap(),
2059+
timeline_item_index,
2060+
visible: false,
2061+
thread_root_id: None,
2062+
}
20582063
}
20592064

20602065
macro_rules! assert_events {

crates/matrix-sdk-ui/src/timeline/controller/state_transaction.rs

Lines changed: 52 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -476,6 +476,7 @@ impl<'a, P: RoomDataProvider> TimelineStateTransaction<'a, P> {
476476
MilliSecondsSinceUnixEpoch,
477477
Option<OwnedTransactionId>,
478478
Option<TimelineAction>,
479+
Option<OwnedEventId>,
479480
bool,
480481
)> {
481482
let state_key: Option<String> = raw.get_field("state_key").ok().flatten();
@@ -534,6 +535,7 @@ impl<'a, P: RoomDataProvider> TimelineStateTransaction<'a, P> {
534535
origin_server_ts,
535536
transaction_id,
536537
Some(TimelineAction::failed_to_parse(event_type, deserialization_error)),
538+
None,
537539
true,
538540
))
539541
}
@@ -550,7 +552,7 @@ impl<'a, P: RoomDataProvider> TimelineStateTransaction<'a, P> {
550552
// Remember the event before returning prematurely.
551553
// See [`ObservableItems::all_remote_events`].
552554
self.add_or_update_remote_event(
553-
EventMeta::new(event_id, false),
555+
EventMeta::new(event_id, false, None),
554556
sender.as_deref(),
555557
origin_server_ts,
556558
position,
@@ -628,63 +630,65 @@ impl<'a, P: RoomDataProvider> TimelineStateTransaction<'a, P> {
628630
_ => (event.kind.into_raw(), None),
629631
};
630632

631-
let (event_id, sender, timestamp, txn_id, timeline_action, should_add) = match raw
632-
.deserialize()
633-
{
634-
// Classical path: the event is valid, can be deserialized, everything is alright.
635-
Ok(event) => {
636-
let (in_reply_to, thread_root) = self.meta.process_event_relations(
637-
&event,
638-
&raw,
639-
bundled_edit_encryption_info,
640-
&self.items,
641-
self.focus.is_thread(),
642-
);
643-
644-
let should_add = self.should_add_event_item(
645-
room_data_provider,
646-
settings,
647-
&event,
648-
thread_root.as_deref(),
649-
position,
650-
);
651-
652-
(
653-
event.event_id().to_owned(),
654-
event.sender().to_owned(),
655-
event.origin_server_ts(),
656-
event.transaction_id().map(ToOwned::to_owned),
657-
TimelineAction::from_event(
658-
event,
633+
let (event_id, sender, timestamp, txn_id, timeline_action, thread_root, should_add) =
634+
match raw.deserialize() {
635+
// Classical path: the event is valid, can be deserialized, everything is alright.
636+
Ok(event) => {
637+
let (in_reply_to, thread_root) = self.meta.process_event_relations(
638+
&event,
659639
&raw,
640+
bundled_edit_encryption_info,
641+
&self.items,
642+
self.focus.is_thread(),
643+
);
644+
645+
let should_add = self.should_add_event_item(
660646
room_data_provider,
661-
utd_info
662-
.map(|utd_info| (utd_info, self.meta.unable_to_decrypt_hook.as_ref())),
663-
in_reply_to,
647+
settings,
648+
&event,
649+
thread_root.as_deref(),
650+
position,
651+
);
652+
653+
(
654+
event.event_id().to_owned(),
655+
event.sender().to_owned(),
656+
event.origin_server_ts(),
657+
event.transaction_id().map(ToOwned::to_owned),
658+
TimelineAction::from_event(
659+
event,
660+
&raw,
661+
room_data_provider,
662+
utd_info.map(|utd_info| {
663+
(utd_info, self.meta.unable_to_decrypt_hook.as_ref())
664+
}),
665+
in_reply_to,
666+
thread_root.clone(),
667+
thread_summary,
668+
)
669+
.await,
664670
thread_root,
665-
thread_summary,
671+
should_add,
666672
)
667-
.await,
668-
should_add,
669-
)
670-
}
673+
}
671674

672-
// The event seems invalid…
673-
Err(e) => {
674-
if let Some(tuple) =
675-
self.maybe_add_error_item(position, room_data_provider, &raw, e, settings).await
676-
{
677-
tuple
678-
} else {
679-
return false;
675+
// The event seems invalid…
676+
Err(e) => {
677+
if let Some(tuple) = self
678+
.maybe_add_error_item(position, room_data_provider, &raw, e, settings)
679+
.await
680+
{
681+
tuple
682+
} else {
683+
return false;
684+
}
680685
}
681-
}
682-
};
686+
};
683687

684688
// Remember the event.
685689
// See [`ObservableItems::all_remote_events`].
686690
self.add_or_update_remote_event(
687-
EventMeta::new(event_id.clone(), should_add),
691+
EventMeta::new(event_id.clone(), should_add, thread_root),
688692
Some(&sender),
689693
Some(timestamp),
690694
position,

0 commit comments

Comments
 (0)