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

Implement MSC2414: Make reason and score parameters optional for reporting content #8551

Closed
anoadragon453 opened this issue Oct 15, 2020 · 3 comments · Fixed by #10077
Closed
Labels
A-Spec-Compliance places where synapse does not conform to the spec good first issue Good for newcomers T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.

Comments

@anoadragon453
Copy link
Member

anoadragon453 commented Oct 15, 2020

MSC2414 made the reason and score parameters for the POST /_matrix/client/r0/rooms/{roomId}/report/{eventId} endpoint.

That endpoint is implemented in Synapse here:

class ReportEventRestServlet(RestServlet):
PATTERNS = client_patterns("/rooms/(?P<room_id>[^/]*)/report/(?P<event_id>[^/]*)$")
def __init__(self, hs):
super().__init__()
self.hs = hs
self.auth = hs.get_auth()
self.clock = hs.get_clock()
self.store = hs.get_datastore()
async def on_POST(self, request, room_id, event_id):
requester = await self.auth.get_user_by_req(request)
user_id = requester.user.to_string()
body = parse_json_object_from_request(request)
assert_params_in_dict(body, ("reason", "score"))

The Event Reports Admin API should also be updated to state that the reason and content fields may be both blank and null, and the Admin API endpoint should be updated to expect None values for these keys as well:

er.reason,
er.content,

(that function may not actually need updating, but it would be good to check that the API still behaves as expected).

Some tests for clients sending reports without reason and score would be great as well.

This will need to be implemented in order to eventually advertise support for the next Client-Server API version.

@anoadragon453 anoadragon453 added Z-Help-Wanted We know exactly how to fix this issue, and would be grateful for any contribution z-p2 (Deprecated Label) A-Spec-Compliance places where synapse does not conform to the spec labels Oct 15, 2020
@clokep
Copy link
Member

clokep commented Oct 15, 2020

Should this be optional for all "versions" of the endpoint or only if accessed via /vNext/rooms/....? Not sure how we usually do that?

@anoadragon453
Copy link
Member Author

@clokep The MSC states that it is a change to the existing endpoint (however clients should check whether CS versions the homeserver advertises before potentially omitting a parameter).

As long as we don't advertise support for $next_cs_version before making this change, all should be fine.

@clokep clokep added good first issue Good for newcomers T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. and removed Z-Help-Wanted We know exactly how to fix this issue, and would be grateful for any contribution z-p2 (Deprecated Label) labels Feb 24, 2021
anoadragon453 pushed a commit that referenced this issue May 27, 2021
@lukasdenk
Copy link
Contributor

@anoadragon453 Is this issue still relevant? Or should it be closed?

@DMRobertson DMRobertson linked a pull request Jan 4, 2022 that will close this issue
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Spec-Compliance places where synapse does not conform to the spec good first issue Good for newcomers T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants