-
Notifications
You must be signed in to change notification settings - Fork 417
Implement MSC4380: Invite blocking #19203
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
Conversation
In particular, make `InviteRulesConfig` a base class which can have multiple operations. To demonstarate it, use an `AllowAllInviteRulesConfig` class for the common case where the user has no config.
c491633 to
9df4502
Compare
| self.bob_token = self.login("bob", "pass") | ||
|
|
||
| @override_config({"experimental_features": {"msc4380_enabled": True}}) | ||
| def test_misc4380_block_invite_local(self) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feels like we should just verify this behavior with some Complement tests instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't disagree that Complement tests would be nice to have (in addition), but having these tests mean we get a tighter development cycle and it's easier to test different combinations (eg what happens when msc4380 is disabled).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤷 We ran into this with the Sliding Sync tests. We naturally fell into writing Synapse tests but they would have much more benefit as Complement tests so other homeserver implementations can verify behavior.
(eg what happens when msc4380 is disabled).
Out-of-repo Complement tests can also be written for this kind of thing. For example, while we don't have this setup for the Synapse project, we do for the Secure Border Gateway, TI-Messenger Proxy, and Synapse Pro for small hosts. Something for the future here ⏩
Co-authored-by: Eric Eastwood <[email protected]>
0a2ba48 to
277619b
Compare
|
277619b makes some minor changes to the format of the account data, but I'm going to take the spirit of Eric's ✔️ and run with it. |
MSC4380 aims to be a simplified implementation of MSC4155; the hope is that we can get it specced and rolled out rapidly, so that we can resolve the fact that
matrix.orghas enabled MSC4155.The implementation leans heavily on what's already there for MSC4155.
It has its own
experimental_featuresflag. If both MSC4155 and MSC4380 are enabled, and a user has both configurations set, then we prioritise the MSC4380 one.Contributed wearing my 🎩 Spec Core Team hat.
Pull Request Checklist
EventStoretoEventWorkerStore.".code blocks.