Skip to content
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.

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

Aggregations: redacting an event should also redact all of its edits. #1328

Open
lampholder opened this issue Jul 2, 2019 · 9 comments · Fixed by matrix-org/synapse#5629
Labels
wart A point where the protocol is inconsistent or inelegant

Comments

@lampholder
Copy link
Member

No description provided.

@jryans
Copy link
Contributor

jryans commented Jul 2, 2019

Should fix element-hq/element-web#10111.

@anoadragon453
Copy link
Member

anoadragon453 commented Jul 5, 2019

I assume this should redact any aggregation on the event, like reactions, not just edits correct?

This likely won't be possible without some other changes as you can't redact an event somebody else sent.

Edit: I think it wouldn't be great to do this, thinking of a use case where you have a forum, and a main post gets deleted whereas comments on that post should still stick around.

@anoadragon453
Copy link
Member

Since this will create a few more redaction events, do you need them to be returned in the response to /_matrix/client/r0/rooms/{roomId}/redact/{eventId}/{txnId}?

@anoadragon453
Copy link
Member

After meeting: We're just going to continue to return the original redaction event ID and leave the rest to come down /sync.

@anoadragon453
Copy link
Member

For dealing with calling /relations for a redacted event, we're going to just return an empty chunks array.

@anoadragon453
Copy link
Member

I believe we need to handle this for /aggregations as well.

@anoadragon453
Copy link
Member

So this is currently half-implemented by blocking users from requesting relations/aggregations on a redacted event, but clients aren't told down sync that all edits should be redacted (and thus they have to work this out for themselves).

@lampholder Is the current state sufficient for Riot? Is it something that should be changed still, as in is it a good long-term solution or is it all a bit hacky from the client's side still?

@anoadragon453
Copy link
Member

Reopening as edits are still not redacted.

@ShadowJonathan
Copy link
Contributor

I don't think that synapse should be handling this, as relations can be arbitrary, and a redaction being a cascading event should be explicitly noted.

This sounds more like a spec issue, then, and/or it should be taken into the Relations MSC.

I think it's better for clients to manually find and redact all "appropriate" events, i.e. not reactions to their own events, but if they're removing a message, only all edits. This should be a case-by-case situation.

@richvdh richvdh transferred this issue from matrix-org/synapse Nov 8, 2022
@richvdh richvdh added the wart A point where the protocol is inconsistent or inelegant label Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wart A point where the protocol is inconsistent or inelegant
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants