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

Add redact method to all event_enum! generated enums #138

Merged
merged 8 commits into from Jul 16, 2020

Conversation

DevinR528
Copy link
Member

@DevinR528 DevinR528 commented Jul 16, 2020

I did some commit manipulation on this I think I'm getting the hang of it!

What do you think of redact returning an Option<RedactedEvent>, the method can validate the EventIds match and whatever else is needed (I think the event ids are it but maybe in the future it could be more complicated)?

@jplatte
Copy link
Member

jplatte commented Jul 16, 2020

What do you think of redact returning an Option<RedactedEvent>, the method can validate the EventIds match and whatever else is needed (I think the event ids are it but maybe in the future it could be more complicated)?

I thought about that when proposing these methods, but I feel that it would actually be worse because any code calling this will already have to look for the correct event to apply the redaction to so would end up .unwrap()ing that Option anyway.

maybe in the future it could be more complicated

I don't see how.

ruma-events/src/room/aliases.rs Outdated Show resolved Hide resolved
ruma-events-macros/src/event_enum.rs Outdated Show resolved Hide resolved
ruma-events/tests/redacted.rs Outdated Show resolved Hide resolved
ruma-events-macros/src/event_enum.rs Outdated Show resolved Hide resolved
pub fn redact_v1(self) -> RedactedAliasesEventContent {
RedactedAliasesEventContent { aliases: Some(self.aliases) }
pub fn redact(self, version: RoomVersionId) -> RedactedAliasesEventContent {
if version.is_version_6() {
Copy link
Member

@jplatte jplatte Jul 16, 2020

Choose a reason for hiding this comment

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

I wonder how we should be handling custom room versions. I thought I had read somewhere that they should generally be considered following the latest room version rules, so this would have to include || version.is_custom(). But I can't find that. Will ask #matrix-spec. But we should still invert this logic so when a new version is introduced, this doesn't revert back to v1 behaviour for that version. Since it's then going to be easier to have custom == v6 logic, let's do that.

ruma-events/src/room/aliases.rs Outdated Show resolved Hide resolved
@DevinR528
Copy link
Member Author

DevinR528 commented Jul 16, 2020

Yes much better! Thanks

@jplatte jplatte merged commit 1db0082 into ruma:master Jul 16, 2020
1 of 3 checks passed
@DevinR528 DevinR528 deleted the redact-method-re branch May 3, 2021
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

2 participants