timeline: prefer a bundled edit to a pending edit when adding a new message

This commit is contained in:
Benjamin Bouvier
2024-10-02 11:17:40 +02:00
parent efbf9472f2
commit 45968b2a2b
4 changed files with 109 additions and 12 deletions

View File

@@ -947,10 +947,9 @@ impl<P: RoomDataProvider> TimelineController<P> {
// Replace the local-related state (kind) and the content state.
let new_item = TimelineItem::new(
prev_item.with_kind(ti_kind).with_content(
TimelineItemContent::message(content, Default::default(), &txn.items),
None,
),
prev_item
.with_kind(ti_kind)
.with_content(TimelineItemContent::message(content, None, &txn.items), None),
prev_item.internal_id.to_owned(),
);

View File

@@ -51,7 +51,7 @@ use super::{
controller::{TimelineMetadata, TimelineStateTransaction},
day_dividers::DayDividerAdjuster,
event_item::{
AnyOtherFullStateEventContent, EventSendState, EventTimelineItemKind,
extract_edit_content, AnyOtherFullStateEventContent, EventSendState, EventTimelineItemKind,
LocalEventTimelineItem, Profile, ReactionsByKeyBySender, RemoteEventOrigin,
RemoteEventTimelineItem, TimelineEventItemId,
},
@@ -328,7 +328,33 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
AnyMessageLikeEventContent::RoomMessage(c) => {
if should_add {
self.add_item(TimelineItemContent::message(c, relations, self.items));
// Always remove the pending edit, if there's any. The reason is that if
// there's an edit in the relations mapping, we want to prefer it over any
// other pending edit, since it's more likely to be up to date, and we
// don't want to apply another pending edit on top of it.
let pending_edit = if let Flow::Remote { event_id, .. } = &self.ctx.flow {
let edits = &mut self.meta.pending_edits;
edits
.iter()
.position(|(prev_event_id, _)| prev_event_id == event_id)
.into_iter()
.filter_map(|pos| {
Some(
as_variant!(
edits.remove(pos).unwrap().1,
PendingEdit::RoomMessage
)?
.new_content,
)
})
.next()
} else {
None
};
let edit = extract_edit_content(relations).or(pending_edit);
self.add_item(TimelineItemContent::message(c, edit, self.items));
}
}

View File

@@ -37,7 +37,10 @@ use ruma::{
history_visibility::RoomHistoryVisibilityEventContent,
join_rules::RoomJoinRulesEventContent,
member::{Change, RoomMemberEventContent},
message::{Relation, RoomMessageEventContent, SyncRoomMessageEvent},
message::{
Relation, RoomMessageEventContent, RoomMessageEventContentWithoutRelation,
SyncRoomMessageEvent,
},
name::RoomNameEventContent,
pinned_events::RoomPinnedEventsEventContent,
power_levels::RoomPowerLevelsEventContent,
@@ -48,8 +51,8 @@ use ruma::{
},
space::{child::SpaceChildEventContent, parent::SpaceParentEventContent},
sticker::{StickerEventContent, SyncStickerEvent},
AnyFullStateEventContent, AnySyncMessageLikeEvent, AnySyncTimelineEvent,
BundledMessageLikeRelations, FullStateEventContent, MessageLikeEventType, StateEventType,
AnyFullStateEventContent, AnySyncTimelineEvent, FullStateEventContent,
MessageLikeEventType, StateEventType,
},
OwnedDeviceId, OwnedMxcUri, OwnedUserId, RoomVersionId, UserId,
};
@@ -272,10 +275,10 @@ impl TimelineItemContent {
// allow users to call them directly, which should not be supported
pub(crate) fn message(
c: RoomMessageEventContent,
relations: BundledMessageLikeRelations<AnySyncMessageLikeEvent>,
edit: Option<RoomMessageEventContentWithoutRelation>,
timeline_items: &Vector<Arc<TimelineItem>>,
) -> Self {
Self::Message(Message::from_event(c, extract_edit_content(relations), timeline_items))
Self::Message(Message::from_event(c, edit, timeline_items))
}
#[cfg(not(tarpaulin_include))] // debug-logging functionality

View File

@@ -25,7 +25,7 @@ use ruma::{
events::room::message::{MessageType, RedactedRoomMessageEventContent},
server_name, EventId,
};
use stream_assert::assert_next_matches;
use stream_assert::{assert_next_matches, assert_pending};
use super::TestTimeline;
use crate::timeline::TimelineItemContent;
@@ -222,3 +222,72 @@ async fn test_edit_updates_encryption_info() {
assert_let!(MessageType::Text(text) = message.msgtype());
assert_eq!(text.body, "!!edited!! **better** message");
}
#[async_test]
async fn test_relations_edit_overrides_pending_edit() {
let timeline = TestTimeline::new();
let mut stream = timeline.subscribe().await;
let f = &timeline.factory;
let original_event_id = event_id!("$original");
let edit1_event_id = event_id!("$edit1");
let edit2_event_id = event_id!("$edit2");
// Pending edit is stashed, nothing comes from the stream.
timeline
.handle_live_event(
f.text_msg("*edit 1")
.sender(*ALICE)
.edit(original_event_id, MessageType::text_plain("edit 1").into())
.event_id(edit1_event_id),
)
.await;
assert_pending!(stream);
// Now we receive the original event, with a bundled relations group.
let ev = sync_timeline_event!({
"content": {
"body": "original",
"msgtype": "m.text"
},
"event_id": &original_event_id,
"origin_server_ts": timeline.event_builder.next_server_ts(),
"sender": *ALICE,
"type": "m.room.message",
"unsigned": {
"m.relations": {
"m.replace": {
"content": {
"body": "* edit 2",
"m.new_content": {
"body": "edit 2",
"msgtype": "m.text"
},
"m.relates_to": {
"event_id": original_event_id,
"rel_type": "m.replace"
},
"msgtype": "m.text"
},
"event_id": edit2_event_id,
"origin_server_ts": timeline.event_builder.next_server_ts(),
"sender": *ALICE,
"type": "m.room.message",
}
}
}
});
timeline.handle_live_event(ev).await;
let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value);
// We receive the latest edit, not the pending one.
let text = item.as_event().unwrap().content().as_message().unwrap();
assert_eq!(text.body(), "edit 2");
let day_divider = assert_next_matches!(stream, VectorDiff::PushFront { value } => value);
assert!(day_divider.is_day_divider());
assert_pending!(stream);
}