Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Stop sending events when creating or deleting aliases #6904

Merged
merged 6 commits into from
Feb 18, 2020
Merged

Stop sending events when creating or deleting aliases #6904

merged 6 commits into from
Feb 18, 2020

Conversation

clokep
Copy link
Member

@clokep clokep commented Feb 12, 2020

This implements the first task in #6898, new alias semantics:

  • Update {PUT, DELETE} /_matrix/client/r0/directory/room/{roomAlias} to stop sending m.room.aliases events.
  • Update DELETE /_matrix/client/r0/directory/room/{roomAlias} to check the alt_aliases in m.room.canonical_alias.

@clokep clokep force-pushed the clokep/stop-sending-aliases branch 4 times, most recently from fe29053 to ecc0a5a Compare February 13, 2020 19:54
@clokep clokep mentioned this pull request Feb 13, 2020
Merged
@clokep clokep requested a review from a team February 13, 2020 19:59
@clokep clokep marked this pull request as ready for review February 13, 2020 20:00
@clokep
Copy link
Member Author

clokep commented Feb 13, 2020

I believe this is ready for review. I'm not 100% thrilled about the tests (I don't think they get great coverage of the modified code) -- if anyone has advice of how to capture the result of event_creation_handler.create_and_send_nonmember_event in a unit test?

This needs corresponding changes from matrix-org/sytest#802.

richvdh
richvdh reviewed Feb 13, 2020
View reviewed changes
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

generally looks good, but I agree the tests are a bit weak. You should be able to check that the state of the room has been updated, possibly by calling StateHandler.get_current_state ?

There's an alternative option of testing via the REST API: have a look at something like tests/rest/client/v1/test_rooms.py for examples of some fairly complex test cases in that area. Personally, I think we're covering that base with sytest, but sometimes it's easier to go that way.

synapse/handlers/directory.py Outdated Show resolved Hide resolved
synapse/handlers/directory.py Outdated Show resolved Hide resolved
tests/handlers/test_directory.py Outdated Show resolved Hide resolved
@clokep
Copy link
Member Author

clokep commented Feb 14, 2020

Added some better unit tests (which found some real bugs) and now sytests are failing! Going to look at those again.

@clokep clokep requested a review from richvdh February 14, 2020 18:17
@clokep
Copy link
Member Author

clokep commented Feb 14, 2020

I believe this is ready for another review!

richvdh
richvdh approved these changes Feb 17, 2020
View reviewed changes
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

looks great. Nice thorough tests, thanks!

@clokep clokep merged commit fe3941f into develop Feb 18, 2020
@clokep clokep deleted the clokep/stop-sending-aliases branch February 18, 2020 12:29
This was referenced Feb 19, 2020
Merged
Merged
babolivier pushed a commit that referenced this pull request Sep 1, 2021
@anoadragon453
Stop sending events when creating or deleting aliases (#6904)
688b054 
* commit 'fe3941f6e':
  Stop sending events when creating or deleting aliases (#6904)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@richvdh richvdh richvdh approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@clokep @richvdh