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

Add room details admin endpoint #7317

Merged
merged 9 commits into from
May 7, 2020
Merged

Add room details admin endpoint #7317

merged 9 commits into from
May 7, 2020

Conversation

awesome-manuel
Copy link
Contributor

@awesome-manuel awesome-manuel commented Apr 21, 2020 •
edited
Loading

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.
  • Pull request includes a sign off
  • Code style is correct (run the linters)

@awesome-manuel awesome-manuel changed the base branch from master to develop April 21, 2020 17:35
clokep
clokep reviewed Apr 21, 2020
View reviewed changes
synapse/rest/admin/rooms.py Outdated Show resolved Hide resolved
@dklimpel
Copy link
Contributor

Related to #7225

@clokep clokep requested review from a team and clokep and removed request for a team April 22, 2020 13:32
@clokep
Copy link
Member

clokep commented Apr 22, 2020

@awesome-manuel I supposed now that #7225 has been merged, would it make sense to include those additional parameters in this endpoint? I don't think it is imperative too, but curious if you would have a use-case for any of those.

@ThyMYthOS
Copy link

Yes, should be included. We use react-admin for the UI and one recommendation is to have the same elements in list and single queries.

This was referenced Apr 28, 2020
Closed
Closed
@awesome-manuel awesome-manuel requested a review from clokep April 30, 2020 21:01
@clokep
Copy link
Member

clokep commented May 1, 2020

@awesome-manuel I supposed now that #7225 has been merged, would it make sense to include those additional parameters in this endpoint? I don't think it is imperative too, but curious if you would have a use-case for any of those.

@awesome-manuel Any thoughts on this? I didn't see a reply.

@awesome-manuel
Copy link
Contributor Author

@awesome-manuel I supposed now that #7225 has been merged, would it make sense to include those additional parameters in this endpoint? I don't think it is imperative too, but curious if you would have a use-case for any of those.

@awesome-manuel Any thoughts on this? I didn't see a reply.

Ah sorry, I did a force push. All attributes from #7225 should now be included.

@clokep
Copy link
Member

clokep commented May 1, 2020

Great! I'll try to take another look at this soon.

clokep
clokep suggested changes May 1, 2020
View reviewed changes
Copy link
Member

@clokep clokep left a comment

Choose a reason for hiding this comment

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

Sorry for the delay on doing a full review here. I left a few comments, but I think they're fairly trivial!

docs/admin_api/rooms.md Show resolved Hide resolved
synapse/rest/admin/rooms.py Outdated Show resolved Hide resolved
synapse/rest/admin/rooms.py Outdated Show resolved Hide resolved
synapse/rest/admin/rooms.py Outdated Show resolved Hide resolved
synapse/storage/data_stores/main/room.py Outdated Show resolved Hide resolved
@awesome-manuel awesome-manuel requested a review from clokep May 1, 2020 18:12
@clokep
Copy link
Member

clokep commented May 1, 2020

Worked well in my testing! Have a couple more questions, but this is close!

@awesome-manuel awesome-manuel mentioned this pull request May 4, 2020
Closed
@awesome-manuel awesome-manuel requested a review from clokep May 5, 2020 12:00
@clokep
Copy link
Member

clokep commented May 5, 2020

@awesome-manuel I should point out that the packaging failure is unrelated to this PR (merging in master or rebasing would fix it). Looks like there's also a style issue though -- I usually use the ./scripts-dev/lint.sh to fix these easily.

clokep
clokep reviewed May 6, 2020
View reviewed changes
docs/admin_api/rooms.md Outdated Show resolved Hide resolved
@clokep
Copy link
Member

clokep commented May 6, 2020

@awesome-manuel This looks good, I made one suggestion to update a comment. Please merge develop into this so that the CI passes.

@awesome-manuel awesome-manuel requested a review from clokep May 7, 2020 08:15
clokep
clokep reviewed May 7, 2020
View reviewed changes
synapse/rest/admin/rooms.py Outdated Show resolved Hide resolved
@awesome-manuel awesome-manuel requested a review from clokep May 7, 2020 13:29
@clokep
Copy link
Member

clokep commented May 7, 2020

Looks good! Thanks for working with us through the improvements! 👍

@clokep clokep merged commit a4a5ec4 into matrix-org:develop May 7, 2020
anoadragon453 added a commit that referenced this pull request May 11, 2020
@anoadragon453
Merge branch 'develop' of github.com:matrix-org/synapse into hawkowl/…
edb04df 
…cache-config-without-synctl

* 'develop' of github.com:matrix-org/synapse: (43 commits)
  Remove unused store method get_hosts_in_room (#7448)
  Don't UPGRADE database rows
  RST indenting
  Put rollback instructions in upgrade notes
  Fix changelog typo
  Oh yeah, RST
  Absolute URL it is then
  Fix upgrade notes link
  Provide summary of upgrade issues in changelog. Fix )
  Move next version notes from changelog to upgrade notes
  Changelog fixes
  1.13.0rc1
  Documentation on setting up redis (#7446)
  Rework UI Auth session validation for registration (#7455)
  Extend spam checker to allow for multiple modules (#7435)
  Implement OpenID Connect-based login (#7256)
  Add room details admin endpoint (#7317)
  Fix errors from malformed log line (#7454)
  Drop support for redis.dbid (#7450)
  Fixes typo (bellow -> below) (#7449)
  ...
@awesome-manuel awesome-manuel deleted the room_admin branch May 11, 2020 19:14
phil-flex pushed a commit to phil-flex/synapse that referenced this pull request Jun 16, 2020
@awesome-manuel @phil-flex
Add room details admin endpoint (matrix-org#7317)
ef7c638
@tulir tulir mentioned this pull request Sep 11, 2020
Closed
@anoadragon453 anoadragon453 mentioned this pull request Oct 15, 2020
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@anoadragon453 anoadragon453 anoadragon453 left review comments

@clokep clokep clokep 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.
5 participants
@awesome-manuel @dklimpel @clokep @ThyMYthOS @anoadragon453