Skip to content
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

Fix GET request on /_synapse/admin/v2/users endpoint #6563

Merged
merged 2 commits into from Jan 8, 2020

Conversation

@awesome-manuel
Copy link
Contributor

awesome-manuel commented Dec 18, 2019

Fixes #6552.

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file
  • Pull request includes a sign off
  • Code style is correct (run the linters)
Signed-off-by: Manuel Stahl <manuel.stahl@awesome-technologies.de>
@awesome-manuel awesome-manuel force-pushed the Awesome-Technologies:user_admin branch from d5b10c9 to 30b7a3f Dec 18, 2019
@anoadragon453 anoadragon453 requested a review from matrix-org/synapse-core Dec 19, 2019
@anoadragon453 anoadragon453 added this to In progress in Homeserver Task Board via automation Dec 19, 2019
@anoadragon453 anoadragon453 self-assigned this Dec 19, 2019
@anoadragon453 anoadragon453 moved this from In progress to Community PRs in Homeserver Task Board Dec 19, 2019
Copy link
Member

anoadragon453 left a comment

LGTM otherwise!

tests/rest/admin/test_admin.py Outdated Show resolved Hide resolved
Signed-off-by: Manuel Stahl <manuel.stahl@awesome-technologies.de>
@awesome-manuel awesome-manuel force-pushed the Awesome-Technologies:user_admin branch from 30b7a3f to 137e54b Dec 19, 2019
@awesome-manuel awesome-manuel requested a review from anoadragon453 Dec 19, 2019
Copy link
Member

richvdh left a comment

thanks!

@wellic

This comment has been minimized.

Copy link

wellic commented Dec 21, 2019

@awesome-manuel

I think you forgot to change in code: parse_boolean to parse_integer for guests and deactivated:

guests = parse_integer(request, "guests", default=1)
deactivated = parse_integer(request, "deactivated", default=0)

@awesome-manuel

This comment has been minimized.

Copy link
Contributor Author

awesome-manuel commented Dec 23, 2019

@wellic you're right, the REST interface should accept both, '0', '1', 'false' and 'true', but parse_booleanand parse_integer only accept the one or the other. Internally we could either use booleans or ints. Database uses ints already but I'd prefer booleans for internal interfaces.
@richvdh Your suggestion was to use '0' and '1' on the REST interface, right?

@richvdh

This comment has been minimized.

Copy link
Member

richvdh commented Jan 2, 2020

@richvdh Your suggestion was to use '0' and '1' on the REST interface, right?

uhhh I don't think so... true and false seem better for boolean values, even if they are represented as ints in the database.

@awesome-manuel

This comment has been minimized.

Copy link
Contributor Author

awesome-manuel commented Jan 3, 2020

@richvdh Your suggestion was to use '0' and '1' on the REST interface, right?

uhhh I don't think so... true and false seem better for boolean values, even if they are represented as ints in the database.

@wellic @anoadragon453 then I think the code is already correct.
The API still returns '0' and '1' in the JSON, but this would be another change.

@richvdh richvdh requested review from richvdh and removed request for richvdh Jan 8, 2020
@richvdh richvdh changed the base branch from develop to release-v1.8.0 Jan 8, 2020
@richvdh richvdh merged commit 7caaa29 into matrix-org:release-v1.8.0 Jan 8, 2020
22 checks passed
22 checks passed
buildkite/synapse Build #6228 passed (26 minutes, 30 seconds)
Details
buildkite/synapse/check-sample-config Passed (1 minute, 46 seconds)
Details
buildkite/synapse/check-style Passed (1 minute, 56 seconds)
Details
buildkite/synapse/isort Passed (17 seconds)
Details
buildkite/synapse/mypy Passed (56 seconds)
Details
buildkite/synapse/newspaper-newsfile Passed (13 seconds)
Details
buildkite/synapse/packaging Passed (21 seconds)
Details
buildkite/synapse/pipeline Passed (3 seconds)
Details
buildkite/synapse/python-3-dot-5-slash-postgres-9-dot-5 Passed (9 minutes, 57 seconds)
Details
buildkite/synapse/python-3-dot-5-slash-sqlite Passed (6 minutes, 6 seconds)
Details
buildkite/synapse/python-3-dot-5-slash-sqlite-slash-old-deps Passed (8 minutes, 11 seconds)
Details
buildkite/synapse/python-3-dot-6-slash-sqlite Passed (7 minutes)
Details
buildkite/synapse/python-3-dot-7-slash-postgres-11 Passed (9 minutes, 35 seconds)
Details
buildkite/synapse/python-3-dot-7-slash-postgres-9-dot-5 Passed (10 minutes, 43 seconds)
Details
buildkite/synapse/python-3-dot-7-slash-sqlite Passed (6 minutes, 24 seconds)
Details
buildkite/synapse/synapse-port-db-slash-python-3-dot-5-slash-postgres-9-dot-5 Passed (1 minute, 37 seconds)
Details
buildkite/synapse/synapse-port-db-slash-python-3-dot-7-slash-postgres-11 Passed (1 minute, 40 seconds)
Details
buildkite/synapse/sytest-python-3-dot-5-slash-postgres-9-dot-6-slash-monolith Passed (10 minutes, 33 seconds)
Details
buildkite/synapse/sytest-python-3-dot-5-slash-postgres-9-dot-6-slash-workers Passed (12 minutes, 7 seconds)
Details
buildkite/synapse/sytest-python-3-dot-5-slash-sqlite-slash-monolith Passed (9 minutes, 42 seconds)
Details
buildkite/synapse/sytest-python-3-dot-7-slash-postgres-11-slash-monolith Passed (9 minutes, 57 seconds)
Details
buildkite/synapse/sytest-python-3-dot-7-slash-postgres-11-slash-workers Passed (10 minutes, 6 seconds)
Details
Homeserver Task Board automation moved this from Community PRs to Done Jan 8, 2020
@awesome-manuel awesome-manuel deleted the Awesome-Technologies:user_admin branch Jan 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
4 participants
You can’t perform that action at this time.