MSC4151: Reporting rooms #1938

Johennes · 2024-08-26T07:33:30Z

As per matrix-org/matrix-spec-proposals#4151.

The MSC proposed that clients should hide reported rooms. However, there isn't any API for ignoring rooms yet. So I left that part out of the spec as otherwise clients would have to invent a system for doing this across devices.

Pull Request Checklist

Pull request includes a changelog file
Pull request includes a sign off
Pull request is classified as 'other changes'

Preview: https://pr1938--matrix-spec-previews.netlify.app

Signed-off-by: Johannes Marbach <n0-0ne+github@mailbox.org>

Johennes · 2024-08-26T07:40:23Z

content/client-server-api/modules/report_content.md

@@ -5,9 +5,6 @@ Users may encounter content which they find inappropriate and should be
 able to report it to the server administrators or room moderators for
 review. This module defines a way for users to report content.

-Content is reported based upon a negative score, where -100 is "most
-offensive" and 0 is "inoffensive".
-


I removed this here because it only applies to event reporting and is repeated in the corresponding endpoint's documentation.

Johennes · 2024-08-26T07:43:38Z

data/api/client-server/report_content.yaml

+              properties:
+                reason:
+                  type: string
+                  description: The reason the room is being reported.


The MSC made this required to align with the event reporting endpoint. However, MSC2414 made both body parameters on that endpoint optional. Therefore, I made reaaonoptional here, too.

Given the event reporting endpoint already has the parameters marked as optional, and MSC2414 was trying to match that endpoint, making them optional here too looks like the right thing to me.

data/api/client-server/report_content.yaml

Johennes · 2024-08-26T07:46:59Z

data/api/client-server/report_content.yaml

@@ -29,7 +92,7 @@ paths:
        will require the homeserver to check whether a user is joined to
        the room. To combat this, homeserver implementations should add
        a random delay when generating a response.
-      operationId: reportContent
+      operationId: reportEvent


I'm unsure what consequences changing this has but the old ID felt unsuitable now that there are two endpoints in the module.

Looks like it's used to generate bindings, and as such we probably shouldn't change it...

matrix-spec/meta/documentation_style.rst

Lines 113 to 114 in 2cbf606

* ``operationId``: a camelCased unique identifier for this endpoint. This will

be used to automatically generate bindings for the endpoint.

@turt2live curious if you happen to know more about this?

I suppose if we want to be safe, keeping operationId: reportContent on the existing endpoint and operationId: reportRoom on the new one would not be the end of the world. So maybe reverting back would be best?

Indeed, operationId usually ends up as the method or class name in generated APIs, and therefore, may (actually will, in the vast majority of cases) break the generated code. However, given that even more fine-grained changes (e.g. adding a field in a request body) that are API-compatible in JSON may be breaking in the generated code (depending on the target language and/or the shape of the API) authors producing auto-generated code have to be careful with any upgrades of the API definitions. In particular, 1.12 has already introduced breaking changes elsewhere from the Quotient perspective. So I wouldn't be too concerned around it, just be cognisant that not everything back-compatible in JSON remains such in a native API.

We concluded in #matrix-docs:matrix.org that changing the operation ID is not ideal but acceptable in this case as otherwise it'll be even harder to fix in future.

Johennes · 2024-08-26T07:48:09Z

data/api/client-server/report_content.yaml

@@ -62,7 +125,7 @@ paths:
                    and 0 is inoffensive.
                reason:
                  type: string
-                  description: The reason the content is being reported. May be blank.
+                  description: The reason the content is being reported.


I have removed this as proposed in MSC2414 and thought it aligns well enough with the rest of this pull request to sneak in. 😇

changelogs/client_server/newsfragments/1938.clarification

changelogs/client_server/newsfragments/1938.new

content/client-server-api/modules/report_content.md

data/api/client-server/report_content.yaml

anoadragon453 · 2024-09-25T17:46:41Z

data/api/client-server/report_content.yaml

@@ -29,7 +92,7 @@ paths:
        will require the homeserver to check whether a user is joined to
        the room. To combat this, homeserver implementations should add
        a random delay when generating a response.
-      operationId: reportContent
+      operationId: reportEvent


Looks like it's used to generate bindings, and as such we probably shouldn't change it...

matrix-spec/meta/documentation_style.rst

Lines 113 to 114 in 2cbf606

* ``operationId``: a camelCased unique identifier for this endpoint. This will

be used to automatically generate bindings for the endpoint.

@turt2live curious if you happen to know more about this?

anoadragon453 · 2024-09-25T17:48:05Z

data/api/client-server/report_content.yaml

+              properties:
+                reason:
+                  type: string
+                  description: The reason the room is being reported.


Given the event reporting endpoint already has the parameters marked as optional, and MSC2414 was trying to match that endpoint, making them optional here too looks like the right thing to me.

data/api/client-server/report_content.yaml

Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>

Johennes · 2024-10-10T09:09:53Z

@anoadragon453 is there anything else you'd like me to add or change to help this land?

anoadragon453

Ah, sorry. Note that if I'm not requested for review and there's no comments happening I won't get notified about a PR or see it in my review queue, so I'll forget about it 🙃

Thanks for the poke. This LGTM now!

MSC4151: Reporting rooms

fb71935

Signed-off-by: Johannes Marbach <n0-0ne+github@mailbox.org>

Johennes mentioned this pull request Aug 26, 2024

MSC4151: Reporting rooms (Client-Server API) matrix-org/matrix-spec-proposals#4151

Merged

Add changelog

ab88044

Johennes commented Aug 26, 2024

View reviewed changes

data/api/client-server/report_content.yaml Show resolved Hide resolved

Johennes commented Aug 26, 2024

View reviewed changes

Johennes marked this pull request as ready for review August 26, 2024 08:54

Johennes requested a review from a team as a code owner August 26, 2024 08:54

Johennes commented Aug 27, 2024

View reviewed changes

changelogs/client_server/newsfragments/1938.clarification Outdated Show resolved Hide resolved

Update changelogs/client_server/newsfragments/1938.clarification

9d63f31

Johennes commented Aug 27, 2024

View reviewed changes

changelogs/client_server/newsfragments/1938.new Outdated Show resolved Hide resolved

Johennes added 3 commits August 27, 2024 19:36

Update changelogs/client_server/newsfragments/1938.new

1cf5104

Add x-addedInMatrixVersion property

3aad778

Add added-in shortcode

414001a

anoadragon453 reviewed Sep 25, 2024

View reviewed changes

Johennes and others added 4 commits September 25, 2024 20:18

Update content/client-server-api/modules/report_content.md

8c8c37d

Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>

Update content/client-server-api/modules/report_content.md

00d4e6d

Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>

Update data/api/client-server/report_content.yaml

883e9e3

Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>

Merge branch 'main' into johannes/reporting-rooms

c2b90d5

anoadragon453 approved these changes Oct 10, 2024

View reviewed changes

anoadragon453 merged commit c74105d into matrix-org:main Oct 10, 2024
12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MSC4151: Reporting rooms #1938

MSC4151: Reporting rooms #1938

Johennes commented Aug 26, 2024 •

edited by github-actions bot

Loading

Johennes Aug 26, 2024

Johennes Aug 26, 2024

anoadragon453 Sep 25, 2024

Johennes Aug 26, 2024

anoadragon453 Sep 25, 2024

Johennes Sep 26, 2024 •

edited

Loading

KitsuneRal Sep 26, 2024

Johennes Sep 27, 2024

Johennes Aug 26, 2024

anoadragon453 Sep 25, 2024

anoadragon453 Sep 25, 2024

Johennes commented Oct 10, 2024

anoadragon453 left a comment

	* ``operationId``: a camelCased unique identifier for this endpoint. This will
	be used to automatically generate bindings for the endpoint.

MSC4151: Reporting rooms #1938

MSC4151: Reporting rooms #1938

Conversation

Johennes commented Aug 26, 2024 • edited by github-actions bot Loading

Pull Request Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Johennes Sep 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Johennes commented Oct 10, 2024

anoadragon453 left a comment

Choose a reason for hiding this comment

Johennes commented Aug 26, 2024 •

edited by github-actions bot

Loading

Johennes Sep 26, 2024 •

edited

Loading