Skip to content

feat(timeline): send timeline redactions through the send queue #6428

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Johennes wants to merge 1 commit into
base: main
Choose a base branch
from
+47 −3
Open

feat(timeline): send timeline redactions through the send queue #6428

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions bindings/matrix-sdk-ffi/changelog.d/6428.changed.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[**breaking**] Send redactions issued via `Timeline::redact_event` through the send queue.

@Hywan Hywan Jun 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is it a breaking change?

@Johennes Johennes Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The API shape is technically the same. However, redact.await will no longer mean that the redaction has been sent but instead just that it has been enqueued in the send queue. Callers will have to monitor send queue updates if they're interested in the sending state. That's why I thought it would qualify as a breaking change.

1 change: 1 addition & 0 deletions crates/matrix-sdk-ui/changelog.d/6428.changed.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[**breaking**] Send redactions issued via `Timeline::redact` through the send queue.
4 changes: 4 additions & 0 deletions crates/matrix-sdk-ui/src/timeline/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ pub enum Error {
#[error("Failed sending attachment")]
FailedSendingAttachment,

/// The redaction could not be sent.
#[error("Failed sending redaction")]
FailedSendingRedaction,

/// The reaction could not be toggled.
#[error("Failed toggling reaction")]
FailedToToggleReaction,
Expand Down
10 changes: 7 additions & 3 deletions crates/matrix-sdk-ui/src/timeline/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -628,16 +628,20 @@ impl Timeline {

match event.handle() {
TimelineItemHandle::Remote(event_id) => {
self.room().redact(event_id, reason, None).await.map_err(RedactError::HttpError)?;
self.room()
.send_queue()
.redact(event_id.to_owned(), reason)
.await
.map_err(|_| Error::FailedSendingRedaction)?;
Ok(())
}
TimelineItemHandle::Local(handle) => {
if !handle.abort().await.map_err(RoomSendQueueError::StorageError)? {
return Err(RedactError::InvalidLocalEchoState.into());
}
Ok(())
}
}

Ok(())
}

/// Fetch unavailable details about the event with the given ID.
Expand Down
34 changes: 34 additions & 0 deletions crates/matrix-sdk-ui/tests/integration/timeline/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,20 @@ async fn test_redact_message() {

timeline.redact(&first.as_event().unwrap().identifier(), Some("inapprops")).await.unwrap();

assert_let_timeout!(Some(timeline_updates) = timeline_stream.next());
assert_eq!(timeline_updates.len(), 1);

assert_let!(VectorDiff::Set { index: 1, value: item } = &timeline_updates[0]);
assert!(item.as_event().unwrap().content().is_redacted());

assert_let_timeout!(Some(timeline_updates) = timeline_stream.next());
assert_eq!(timeline_updates.len(), 2);

assert_let!(VectorDiff::Set { index: 1, value: item } = &timeline_updates[0]);
assert!(item.as_event().unwrap().content().is_redacted());
assert_let!(VectorDiff::Set { index: 1, value: item } = &timeline_updates[1]);
assert!(item.as_event().unwrap().content().is_redacted());

Comment on lines +449 to +462

@Johennes Johennes Apr 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm unsure why this is issued three times instead of just twice (local and remote echo of the redaction event). 🤔

@andybalaam andybalaam Apr 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I got curious about this, and found that all three items have the same "unique" ID, 0.

The first one looks like this:

TimelineItem {
    kind: Event(
        EventTimelineItem {
            sender: "@a:b.com",
...
            timestamp: 2026-04-15T09:12:45.439,
            content: MsgLike(
                MsgLikeContent {
                    kind: Redacted,
...                    
            unredacted_item: Some(
                UnredactedEventTimelineItem {
...
                                {"content":{"body":"buy my bitcoins bro","msgtype":"m.text"},"event_id":"$kZVJhqxRxGeHOpPwPTu2a9i00EiKLmYNtr4JgFaFxck","origin_server_ts":1776244365439,"sender":"@a:b.com","type":"m.room.message"},
...
            kind: Remote(
                RemoteEventTimelineItem {
                    event_id: "$kZVJhqxRxGeHOpPwPTu2a9i00EiKLmYNtr4JgFaFxck",
...
    internal_id: TimelineUniqueId(
        "0",
    ),
}

and the second two are identical (at least in terms of debug formatting) and look like:

TimelineItem {
    kind: Event(
        EventTimelineItem {
            sender: "@a:b.com",
...
            timestamp: 2026-04-15T09:12:45.439,
            content: MsgLike(
                MsgLikeContent {
                    kind: Redacted,
...
            unredacted_item: None,
            kind: Remote(
                RemoteEventTimelineItem {
                    event_id: "$kZVJhqxRxGeHOpPwPTu2a9i00EiKLmYNtr4JgFaFxck",
...
    internal_id: TimelineUniqueId(
        "0",
    ),
}

@andybalaam andybalaam Apr 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wonder whether @bnjbvr @Hywan might know whether this is expected?

@bnjbvr bnjbvr Apr 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There should indeed be only two (one for the local echo, one after it's been sent by the send queue it should be saved in the event cache). If there was a fake sync mimicking the remote echo, that would explain the third one, but it's not clear here why there's a duplicate one. Wondering if this is a spurious redaction aggregation applied to the redacted item itself, or something like that 👀 If you're interested in digging deeper, that might help avoiding spurious updates.

@andybalaam andybalaam Apr 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@Johennes ideally we would work this out before merging, but since I suspect it is unrelated to your change, I could be persuaded to merge without if no-one else on the Rust team objects.

@Johennes Johennes Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I spent some time looking into this but so far couldn't figure out where the third update comes from, unfortunately. 😕

@andybalaam andybalaam Jun 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am going to defer to @poljar on whether it is ok to merge this without understanding why this code is not behaving how we think it should.

// Redacting a local event works.
timeline
.send(RoomMessageEventContent::text_plain("i will disappear soon").into())
Expand Down Expand Up @@ -536,6 +550,26 @@ async fn test_redact_local_sent_message() {

// Let's redact the local echo with the remote handle.
timeline.redact(&event.identifier(), None).await.unwrap();

// We receive an update in the timeline from the send queue: the redaction's
// local echo.
assert_let_timeout!(Some(timeline_updates) = timeline_stream.next());
assert_eq!(timeline_updates.len(), 1);

assert_let!(VectorDiff::Set { index: 1, value: item } = &timeline_updates[0]);
let event = item.as_event().unwrap();
assert!(event.content().is_redacted());

// We receive an update in the timeline from the send queue: the redaction's
// remote echo.
assert_let_timeout!(Some(timeline_updates) = timeline_stream.next());
assert_eq!(timeline_updates.len(), 1);

assert_let!(VectorDiff::Set { index: 1, value: item } = &timeline_updates[0]);
let event = item.as_event().unwrap();
assert!(event.content().is_redacted());

assert_pending!(timeline_stream);
}

#[async_test]
Expand Down
Loading