-
Notifications
You must be signed in to change notification settings - Fork 12.4k
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
API: Add org users with pagination #33788
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good, but there are some stylistic matters and I think mainly the PR should be refactored to not use the bus as we are trying to migrate away from it (it ties us to global state etc, which is bad). I've described what to do instead of going via the bus.
assert.Equal(t, 1000, sentLimit) | ||
assert.Equal(t, 1, sendPage) | ||
|
||
require.Equal(t, http.StatusOK, sc.resp.Code) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit, but it's better to move this to line 66, because the current verifications on line 66 and 67 don't make sense unless the response is non-erroneous.
assert.Equal(t, 10, sentLimit) | ||
assert.Equal(t, 2, sendPage) | ||
|
||
require.Equal(t, http.StatusOK, sc.resp.Code) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned before, it's better to move this to line 100.
Co-authored-by: Arve Knudsen <[email protected]>
Co-authored-by: Arve Knudsen <[email protected]>
Co-authored-by: Arve Knudsen <[email protected]>
Co-authored-by: Arve Knudsen <[email protected]>
Co-authored-by: Arve Knudsen <[email protected]>
pkg/api/org_users.go
Outdated
} | ||
|
||
result, err := hs.searchOrgUsersHelper(&models.SearchOrgUsersQuery{ | ||
OrgId: c.OrgId, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just instead of recreate searchOrgUsersHelper, should be better to reuse getOrgUsersHelper
Co-authored-by: Arve Knudsen <[email protected]>
Co-authored-by: Arve Knudsen <[email protected]>
Co-authored-by: Arve Knudsen <[email protected]>
Co-authored-by: Arve Knudsen <[email protected]>
Co-authored-by: Arve Knudsen <[email protected]>
dc16f82
to
a579069
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks really good now! There are some remaining nits/issues though (mostly missing test cleanup).
Co-authored-by: Arve Knudsen <[email protected]>
Co-authored-by: Arve Knudsen <[email protected]>
Co-authored-by: Arve Knudsen <[email protected]>
Co-authored-by: Arve Knudsen <[email protected]>
Co-authored-by: Arve Knudsen <[email protected]>
Co-authored-by: Arve Knudsen <[email protected]>
Co-authored-by: Arve Knudsen <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for applying my suggestions!
* Add model for search org user and add handler for dispatch * 32540_org_users_with_pagination: Add endpoint for search org users * 32540_org_users_with_pagination: Add test for org user search handler * 32540_org_users_with_pagination: fix indentation * 32540_org_users_with_pagination: Remove newline * 32540_org_users_with_pagination: Remove empty line * 32540_org_users_with_pagination: Fix indentation * Update pkg/api/org_users.go Co-authored-by: Arve Knudsen <[email protected]> * Update pkg/api/org_users.go Co-authored-by: Arve Knudsen <[email protected]> * Update pkg/models/org_user.go Co-authored-by: Arve Knudsen <[email protected]> * Update pkg/api/org_users_test.go Co-authored-by: Arve Knudsen <[email protected]> * Update pkg/api/org_users_test.go Co-authored-by: Arve Knudsen <[email protected]> * 32540_org_users_with_pagination: Use hs.SQLStore.SearchOrgUsers instead of bus * Add model for search org user and add handler for dispatch * 32540_org_users_with_pagination: Add endpoint for search org users * 32540_org_users_with_pagination: Add test for org user search handler * 32540_org_users_with_pagination: fix indentation * 32540_org_users_with_pagination: Remove newline * 32540_org_users_with_pagination: Remove empty line * 32540_org_users_with_pagination: Fix indentation * Update pkg/api/org_users.go Co-authored-by: Arve Knudsen <[email protected]> * Update pkg/api/org_users.go Co-authored-by: Arve Knudsen <[email protected]> * Update pkg/models/org_user.go Co-authored-by: Arve Knudsen <[email protected]> * Update pkg/api/org_users_test.go Co-authored-by: Arve Knudsen <[email protected]> * Update pkg/api/org_users_test.go Co-authored-by: Arve Knudsen <[email protected]> * 32540_org_users_with_pagination: Use hs.SQLStore.SearchOrgUsers instead of bus * 32540_org_users_with_pagination: Add test for the sqlstore * 32540_org_users_with_pagination: Fix sqlstore test * Update pkg/api/org_users.go Co-authored-by: Arve Knudsen <[email protected]> * Update pkg/api/org_users_test.go Co-authored-by: Arve Knudsen <[email protected]> * Update pkg/api/org_users_test.go Co-authored-by: Arve Knudsen <[email protected]> * Update pkg/services/sqlstore/org_users.go Co-authored-by: Arve Knudsen <[email protected]> * Update pkg/services/sqlstore/org_users.go Co-authored-by: Arve Knudsen <[email protected]> * Update pkg/services/sqlstore/org_test.go Co-authored-by: Arve Knudsen <[email protected]> * Update pkg/services/sqlstore/org_test.go Co-authored-by: Arve Knudsen <[email protected]> * 32540: Fix search org users method * 32540: Fix sqlstore test * 32540: Fix go-lint Co-authored-by: Arve Knudsen <[email protected]>
* Add model for search org user and add handler for dispatch * 32540_org_users_with_pagination: Add endpoint for search org users * 32540_org_users_with_pagination: Add test for org user search handler * 32540_org_users_with_pagination: fix indentation * 32540_org_users_with_pagination: Remove newline * 32540_org_users_with_pagination: Remove empty line * 32540_org_users_with_pagination: Fix indentation * Update pkg/api/org_users.go Co-authored-by: Arve Knudsen <[email protected]> * Update pkg/api/org_users.go Co-authored-by: Arve Knudsen <[email protected]> * Update pkg/models/org_user.go Co-authored-by: Arve Knudsen <[email protected]> * Update pkg/api/org_users_test.go Co-authored-by: Arve Knudsen <[email protected]> * Update pkg/api/org_users_test.go Co-authored-by: Arve Knudsen <[email protected]> * 32540_org_users_with_pagination: Use hs.SQLStore.SearchOrgUsers instead of bus * Add model for search org user and add handler for dispatch * 32540_org_users_with_pagination: Add endpoint for search org users * 32540_org_users_with_pagination: Add test for org user search handler * 32540_org_users_with_pagination: fix indentation * 32540_org_users_with_pagination: Remove newline * 32540_org_users_with_pagination: Remove empty line * 32540_org_users_with_pagination: Fix indentation * Update pkg/api/org_users.go Co-authored-by: Arve Knudsen <[email protected]> * Update pkg/api/org_users.go Co-authored-by: Arve Knudsen <[email protected]> * Update pkg/models/org_user.go Co-authored-by: Arve Knudsen <[email protected]> * Update pkg/api/org_users_test.go Co-authored-by: Arve Knudsen <[email protected]> * Update pkg/api/org_users_test.go Co-authored-by: Arve Knudsen <[email protected]> * 32540_org_users_with_pagination: Use hs.SQLStore.SearchOrgUsers instead of bus * 32540_org_users_with_pagination: Add test for the sqlstore * 32540_org_users_with_pagination: Fix sqlstore test * Update pkg/api/org_users.go Co-authored-by: Arve Knudsen <[email protected]> * Update pkg/api/org_users_test.go Co-authored-by: Arve Knudsen <[email protected]> * Update pkg/api/org_users_test.go Co-authored-by: Arve Knudsen <[email protected]> * Update pkg/services/sqlstore/org_users.go Co-authored-by: Arve Knudsen <[email protected]> * Update pkg/services/sqlstore/org_users.go Co-authored-by: Arve Knudsen <[email protected]> * Update pkg/services/sqlstore/org_test.go Co-authored-by: Arve Knudsen <[email protected]> * Update pkg/services/sqlstore/org_test.go Co-authored-by: Arve Knudsen <[email protected]> * 32540: Fix search org users method * 32540: Fix sqlstore test * 32540: Fix go-lint Co-authored-by: Arve Knudsen <[email protected]>
What this PR does / why we need it:
Adds org users with pagination endpoint.
Which issue(s) this PR fixes:
Fixes #32540
Special notes for your reviewer: