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

Fixed set a user as an admin with the new API #6928

Merged
merged 20 commits into from
Feb 28, 2020
Merged

Fixed set a user as an admin with the new API #6928

merged 20 commits into from
Feb 28, 2020

Conversation

dklimpel
Copy link
Contributor

@dklimpel dklimpel commented Feb 15, 2020 •
edited
Loading

Try to fix #6910

I have added tests and clear cache of user who was changed.

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)

Signed-off-by: Dirk Klimpel dirk@klimpel.org

richvdh
richvdh suggested changes Feb 21, 2020
View reviewed changes
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

this looks great. bonus points for new test cases! I have a couple of requests though...

synapse/storage/data_stores/main/registration.py Outdated Show resolved Hide resolved
changelog.d/6910.bugfix Outdated Show resolved Hide resolved
tests/rest/admin/test_user.py Outdated Show resolved Hide resolved
tests/rest/admin/test_user.py Show resolved Hide resolved
@richvdh richvdh self-assigned this Feb 21, 2020
@richvdh richvdh mentioned this pull request Feb 21, 2020
Closed
@anoadragon453 anoadragon453 removed their request for review February 26, 2020 12:58
richvdh
richvdh reviewed Feb 28, 2020
View reviewed changes
tests/rest/admin/test_user.py Outdated Show resolved Hide resolved
tests/rest/admin/test_user.py Outdated Show resolved Hide resolved
tests/rest/admin/test_user.py Outdated Show resolved Hide resolved
tests/rest/admin/test_user.py Outdated Show resolved Hide resolved
tests/rest/admin/test_user.py Outdated Show resolved Hide resolved
tests/rest/admin/test_user.py Outdated Show resolved Hide resolved
tests/rest/admin/test_user.py Outdated Show resolved Hide resolved
richvdh
richvdh approved these changes Feb 28, 2020
View reviewed changes
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

great, thank you! I've tweaked the wording for the testcase descriptions slightly.

@richvdh richvdh merged commit 9b06d8f into matrix-org:develop Feb 28, 2020
@dklimpel dklimpel deleted the fix_admin_api branch February 28, 2020 10:03
richvdh pushed a commit that referenced this pull request Mar 2, 2020
@dklimpel @richvdh
Fixed set a user as an admin with the new API (#6928)
bbeee33 
Fix #6910
phil-flex pushed a commit to phil-flex/synapse that referenced this pull request Apr 15, 2020
@dklimpel @phil-flex
Fixed set a user as an admin with the new API (matrix-org#6928)
9140a06 
Fix matrix-org#6910
phil-flex pushed a commit to phil-flex/synapse that referenced this pull request May 15, 2020
@dklimpel @phil-flex
Fixed set a user as an admin with the new API (matrix-org#6928)
37f7baa 
Fix matrix-org#6910
phil-flex pushed a commit to phil-flex/synapse that referenced this pull request Jun 16, 2020
@dklimpel @phil-flex
Fixed set a user as an admin with the new API (matrix-org#6928)
25c28e6 
Fix matrix-org#6910
babolivier pushed a commit that referenced this pull request Sep 1, 2021
@anoadragon453
Fixed set a user as an admin with the new API (#6928)
4c75c20 
* commit '9b06d8f8a':
  Fixed set a user as an admin with the new API (#6928)
babolivier pushed a commit that referenced this pull request Sep 1, 2021
@anoadragon453
Fixed set a user as an admin with the new API (#6928)
8682867 
* commit 'bbeee33d6':
  Fixed set a user as an admin with the new API (#6928)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@richvdh richvdh richvdh approved these changes

Assignees

@richvdh richvdh

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dklimpel @richvdh