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

Implement endpoint to list available users in UserApiClient. #288

Merged
merged 2 commits into from
Dec 16, 2020

Conversation

DiscoPYF
Copy link
Collaborator

@DiscoPYF DiscoPYF commented Oct 3, 2020

fix #258

Implement remaining endpoint:

  • List available Users GET /_api/user/

https://www.arangodb.com/docs/stable/http/user-management.html#list-available-users

@DiscoPYF DiscoPYF requested a review from a team October 3, 2020 15:30
[Fact]
public async Task GetUsersAsync_ShouldSucceed()
{
GetUsersResponse response = await _userClient.GetUsersAsync();
Copy link
Contributor

@modios modios Dec 3, 2020

Choose a reason for hiding this comment

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

Would make sense to test the case that we don't have admin access? Also that the users are indeed the number that we expect. Additionally, we could delete the users and catch the exception, not so important I assume.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Regarding testing the admin case, the documentation says the following:

Fetches data about all users. You need the Administrate server access level in order to execute this REST call. Otherwise, you will only get information about yourself.

https://www.arangodb.com/docs/stable/http/user-management.html#list-available-users

In any cases, we get information back in an identical format. Here we just want to check that the endpoint and response are working properly rather than testing all cases, so I would say this test case is enough.

For the number of users, we don't know exactly what number of users to expect (e.g. tests are creating/deleting users), hence the check for Count() > 0 which I think is enough.

we could delete the users and catch the exception

Not sure what you mean. We already have tests to cover the delete scenario.

@DiscoPYF
Copy link
Collaborator Author

@modios @rossmills99 Thank you for reviewing. I will merge this and v1 should be good to go.

@DiscoPYF DiscoPYF merged commit 95306f3 into ArangoDB-Community:master Dec 16, 2020
@DiscoPYF DiscoPYF deleted the finalizeUserApi branch December 16, 2020 10:46
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.

Implement missing methods in UserApiClient
3 participants