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

Event changes for room versions 7, 8, and 9 #771

Merged
merged 2 commits into from Nov 26, 2021

Conversation

DevinR528
Copy link
Member

Partial fix for #718

Taken from #738

crates/ruma-events/src/room/member.rs Outdated Show resolved Hide resolved
crates/ruma-events/src/room/member.rs Outdated Show resolved Hide resolved
crates/ruma-events/src/room/member.rs Outdated Show resolved Hide resolved
#[cfg_attr(docsrs, doc(cfg(feature = "unstable-pre-spec")))]
#[serde(skip_serializing_if = "Option::is_none")]
#[ruma_event(skip_redaction)]
pub join_authorised_via_users_server: Option<UserId>,
Copy link
Member

Choose a reason for hiding this comment

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

Oh no, did the spec introduce a British English name once again? If this is really what the spec says, can you add a serde(rename) but use authorized (with z) for the field name, for consistency within Ruma?

Copy link
Member

@iinuwa iinuwa Nov 26, 2021

Choose a reason for hiding this comment

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

I remember reading that British English is part of the Matrix spec style guide, but I can't seem to find the reference now.

Edit: found it. https://github.com/matrix-org/matrix-doc/blob/main/meta/documentation_style.rst#general

Copy link
Member

Choose a reason for hiding this comment

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

There's 25 matches for authoriz (and 11 for authoris) on https://spec.matrix.org/v1.1/client-server-api/, so if there is the policy at all, they are not very good at enforcing it 😅

Comment on lines 158 to 160
// FIXME: Any entries in the list which do not match the expected format are ignored. Thus, if all
// entries are invalid, the list behaves as if empty and all users without an invite are rejected.
// i.e. ser/de needs to be lax
Copy link
Member

Choose a reason for hiding this comment

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

What is this fixme about? How could entries in the list be honored if they don't follow the expected format?

Copy link
Member Author

@DevinR528 DevinR528 Nov 26, 2021

Choose a reason for hiding this comment

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

I think it's more that you can't throw away the event (the join rule event) as invalid but turn it into a restricted join rule event with no entries?

https://github.com/matrix-org/matrix-doc/blob/main/proposals/3083-restricted-rooms.md#proposal Third actual "paragraph" or first one after bullet points.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure that's what that paragraph says. Either way our deserialization is already "lax" because we have AllowRule::_Custom, no?

Copy link
Member Author

@DevinR528 DevinR528 Nov 26, 2021

Choose a reason for hiding this comment

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

Oh are they more talking about the shape of the JSON, like it has to look like

{
    "type": "m.some_supported_type",
    "room_id": "!realroomid"
}

or something they come up with in the future and not that the individual pieces could be invalid.

Well either way I got rid of the FIXME.

crates/ruma-events/src/room/member.rs Outdated Show resolved Hide resolved
crates/ruma-events/src/room/member.rs Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants