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

Fix inconsistencies regarding redacted_because #378

Merged
merged 1 commit into from Aug 31, 2016

Conversation

Projects
None yet
3 participants
@Ralith
Contributor

Ralith commented Aug 27, 2016

Different portions of the client-server specification referred to redacted_because in Event and in UnsignedData, and having values of type string and Redaction. In practice, Synapse seems to place values of type Redaction at both locations. These changes attempt to standardize on redacted_because being a field of UnsignedData of type Redaction.

@matrixbot

This comment has been minimized.

Member

matrixbot commented Aug 27, 2016

Can one of the admins verify this patch?

client receives a redaction event it should change the redacted event
in the same way a server does.
An object containing the redaction event under the key ``redacted_because``
should be added to the event under the key ``unsigned``. When a client receives

This comment has been minimized.

@richvdh

richvdh Aug 30, 2016

Member

this sentence is unclear.

How about "The server should add the event causing the redaction to the unsigned property of the redacted event, under the redacted_because key"?

@@ -52,6 +52,10 @@ properties:
This key will only be present for message events sent by the device calling
this API.
type: string
redacted_because:
description: Optional. The event that redacted this event, if any.
title: Redaction

This comment has been minimized.

@richvdh

richvdh Aug 30, 2016

Member

The problem with this is that the spec ends up referring to Redaction without defining what it is. Ideally, we'd reference the m.room.redaction event here, but as you've no doubt already figured out, that causes a loop.

I think the best compromise would be just to put Event here.

description: The reason this event was redacted, if it was redacted
type: string
description: Optional. The event that redacted this event, if any.
title: Redaction

This comment has been minimized.

@richvdh

richvdh Aug 30, 2016

Member

ditto

@richvdh

This comment has been minimized.

Member

richvdh commented Aug 30, 2016

thanks for the corrections, a couple of minor comments. Also the changelog needs merging :(

Fix inconsistencies regarding redacted_because
Signed-off-by: Benjamin Saunders <ben.e.saunders@gmail.com>
@Ralith

This comment has been minimized.

Contributor

Ralith commented Aug 31, 2016

Applied suggested changes and rebased.

re: circularity issue: I actually have no idea how these yaml files work and have just been imitating things as best I can. I got title: Redaction from event-schemas/schema/m.room.redaction.

@richvdh

This comment has been minimized.

Member

richvdh commented Aug 31, 2016

Looks great, tyvm.

@richvdh richvdh merged commit 4833710 into matrix-org:master Aug 31, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment