Skip to content

Add report user API from MSC4260 #18120

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

Merged
merged 32 commits into from
Jun 20, 2025
Merged

Add report user API from MSC4260 #18120

merged 32 commits into from
Jun 20, 2025

Conversation

turt2live
Copy link
Member

@turt2live turt2live commented Jan 30, 2025

matrix-org/matrix-spec-proposals#4260

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
    • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
  • Code style is correct
    (run the linters)

@turt2live turt2live marked this pull request as ready for review January 30, 2025 23:19
@turt2live turt2live requested a review from a team as a code owner January 30, 2025 23:19
@turt2live
Copy link
Member Author

test failures appear unrelated

Copy link
Member

@sandhose sandhose left a comment

Choose a reason for hiding this comment

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

Overall looks good

@turt2live turt2live requested a review from sandhose March 18, 2025 21:10
Copy link
Member

@anoadragon453 anoadragon453 left a comment

Choose a reason for hiding this comment

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

Some further notes and ideas below.

@github-actions github-actions bot deployed to PR Documentation Preview May 2, 2025 17:48 Active
@github-actions github-actions bot deployed to PR Documentation Preview May 2, 2025 17:50 Active
@github-actions github-actions bot deployed to PR Documentation Preview May 2, 2025 17:55 Active
@turt2live turt2live requested a review from anoadragon453 May 2, 2025 17:56
Copy link
Member

@sandhose sandhose left a comment

Choose a reason for hiding this comment

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

Very non-blocking concerns, happy to merge as-is, just let me know

@anoadragon453 anoadragon453 removed their request for review May 29, 2025 11:56
@turt2live turt2live requested review from anoadragon453, sandhose and a team June 19, 2025 16:34
@github-actions github-actions bot deployed to PR Documentation Preview June 19, 2025 16:35 Active
Copy link
Member

@anoadragon453 anoadragon453 left a comment

Choose a reason for hiding this comment

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

Seems we probably should have defined a way to delete these reports during the MSC process... Do we have a plan to clear reports out once they're handled?

Otherwise the below is the only blocking comment.

@turt2live
Copy link
Member Author

Seems we probably should have defined a way to delete these reports during the MSC process... Do we have a plan to clear reports out once they're handled?

Like any other report: no. Implementations are left to their own decisions in this regard.

We'll add applicable admin APIs later - I'd like to avoid overloading this PR.

@anoadragon453
Copy link
Member

Seems we probably should have defined a way to delete these reports during the MSC process... Do we have a plan to clear reports out once they're handled?

Like any other report: no. Implementations are left to their own decisions in this regard.

We'll add applicable admin APIs later - I'd like to avoid overloading this PR.

Sorry, I wasn't suggesting adding any more APIs to this PR. I was more wondering if there were plans to create tooling to remove reports. It sounds like there are, but from a different angle that the C-S API.

Happy to discuss that more in a separate issue/MSC.

@turt2live
Copy link
Member Author

honestly, the hope is to bypass the Synapse database entirely one day, but that's very long term thoughts. In the meantime, we'll slowly add admin APIs as we (T&S) need them, if others don't contribute them.

@github-actions github-actions bot deployed to PR Documentation Preview June 19, 2025 17:55 Active
@turt2live turt2live requested a review from anoadragon453 June 19, 2025 17:59
Copy link
Member

@anoadragon453 anoadragon453 left a comment

Choose a reason for hiding this comment

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

This LGTM. Thanks for working on this and other T&S features ❤️

Comment on lines +80 to +81
if not self._hs.is_mine_id(target_user_id):
return # hide that they're not ours/that we can't do anything about them
Copy link
Member

Choose a reason for hiding this comment

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

Non-blocking, but would it not be useful for the reporter to get an error here telling them that the homeserver operator likely don't do much about remote users? And tell them to contact the remote homeserver's operator instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

Possibly/probably, though IIRC this is in the MSC.

One day we do also hope to support federated reports.

@anoadragon453 anoadragon453 merged commit 74ca7ae into develop Jun 20, 2025
47 checks passed
@anoadragon453 anoadragon453 deleted the travis/report-user branch June 20, 2025 12:02
@anoadragon453 anoadragon453 removed the request for review from sandhose June 20, 2025 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants