-
Notifications
You must be signed in to change notification settings - Fork 54
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
Add backend support for Manage Users pagination #8370
base: main
Are you sure you want to change the base?
Conversation
backend/src/main/java/gov/cdc/usds/simplereport/idp/repository/DemoOktaRepository.java
Fixed
Show fixed
Hide fixed
backend/src/main/java/gov/cdc/usds/simplereport/idp/repository/DemoOktaRepository.java
Fixed
Show fixed
Hide fixed
backend/src/main/java/gov/cdc/usds/simplereport/idp/repository/LiveOktaRepository.java
Fixed
Show fixed
Hide fixed
backend/src/main/java/gov/cdc/usds/simplereport/idp/repository/LiveOktaRepository.java
Fixed
Show fixed
Hide fixed
c3ecb82
to
3a6b61e
Compare
pageNumber = ApiUserService.DEFAULT_OKTA_USER_PAGE_OFFSET; | ||
} | ||
if (!searchQuery.isBlank()) { | ||
return _userService.searchUsersAndStatusInCurrentOrgPaged( |
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.
Search is being handled by a different service method here only as an intermediate step between now and the rest of the Okta migration. Currently, we have to fetch the entire organization's user list from Okta in order to do this search. Once we switch to getting users from our database, we can have the other method below use the search filter easily.
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.
just to confirm, we aren't using this search functionality on the frontend just yet, right? That's being introduced with your frontend PR?
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.
Correct, so this new endpoint is not currently being used anywhere in the app until the frontend PR
|
||
List<Group> facilityAccessGroup = | ||
groupApi.listGroups(facilityAccessGroupName, null, null, 1, "stats", null, null, null); | ||
private Integer getUsersCountInOktaGroup(String groupName) { |
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.
Pagination needed the total number of users in an organization, so I split this logic out to be shared for facilities and organizations.
|
||
final Map<String, UserStatus> emailsToStatus = | ||
_oktaRepo.getPagedUsersWithStatusForOrganization(org, pageNumber, pageSize); | ||
List<ApiUser> users = _apiUserRepo.findAllByLoginEmailInOrderByName(emailsToStatus.keySet()); |
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.
This part of the method will be updated in this future ticket to fetch users from the DB instead of having to get them from Okta first.
|
||
public Page<ApiUserWithStatus> searchUsersAndStatusInCurrentOrgPaged( | ||
int pageNumber, int pageSize, String searchQuery) { | ||
List<ApiUserWithStatus> allUsers = getUsersAndStatusInCurrentOrg(); |
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.
See earlier comment
3a6b61e
to
d3166f1
Compare
Quality Gate passedIssues Measures |
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.
Left a couple questions! Thanks for all your work on this so far, MIke!
List<ApiUserWithStatus> filteredUsersList = | ||
allUsers.stream() | ||
.filter( | ||
u -> { |
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.
could we do u.getNameInfo().toString().toLowerCase()
to get the full name here instead or are we purposefully trying to avoid including a user's suffix in their name? 🤔
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.
I think we are purposefully excluding suffix based on the existing frontend search in UsersSideNav
that filters based on first, middle, and last names
pageNumber = ApiUserService.DEFAULT_OKTA_USER_PAGE_OFFSET; | ||
} | ||
if (!searchQuery.isBlank()) { | ||
return _userService.searchUsersAndStatusInCurrentOrgPaged( |
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.
just to confirm, we aren't using this search functionality on the frontend just yet, right? That's being introduced with your frontend PR?
BACKEND PULL REQUEST
Related Issue
Changes Proposed
Additional Information
Testing
main