-
Notifications
You must be signed in to change notification settings - Fork 392
Add an Admin API to fetch an event by ID #18963
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
base: develop
Are you sure you want to change the base?
Conversation
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.
Only minor points, looks fine overall though :)
docs/admin_api/fetch_event.md
Outdated
Response: | ||
```json | ||
{ | ||
"event_id": "$hbFTSxFNaPau73B8fqGTFrkSqxOpaFjlnzOFEPw9tMA", |
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.
is this to match some other response format, or is it just redundant? (You already needed to know the event ID to query this API, also I think it may even feature within the event itself)
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.
Not to matching anything, the event id wasn't being returned as part of the event so I changed the serialization of the event to include it and removed from the top of the response.
await assert_requester_is_admin(self._auth, request) | ||
|
||
event = await self._store.get_event( | ||
event_id, EventRedactBehaviour.as_is, allow_none=True, allow_rejected=True |
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.
do we need to clearly document that we will return rejected events here? (I can see people benefitting from this being said, as well as maybe how you know from the response)
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.
probably helpful, and just to double-check - if an event is rejected there should be a rejection_reason
in the unsigned field of the event, correct?
pretty sure test failures are flakes, but I can't kick the ci to be sure |
Adds an endpoint to allow server admins to fetch an event regardless of their membership in the originating room.