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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Jan 15, 2025
Merged

Add backend support for Manage Users pagination #8370

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3c5d502
Add backend pagination support for manage users
mpbrown Dec 18, 2024
bfb92d5
Add paginated search backend
mpbrown Dec 24, 2024
ee111f6
Add missing override annotation
mpbrown Dec 24, 2024
d3166f1
Update unit tests
mpbrown Dec 26, 2024
89e4c6b
Handle zero users in org
mpbrown Jan 10, 2025
c595f42
Update unit test
mpbrown Jan 10, 2025
a6bfa4e
Fix sonar issues
mpbrown Jan 10, 2025
a8eb38a
Move constants to resolver
mpbrown Jan 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions backend/src/main/java/gov/cdc/usds/simplereport/api/apiuser/UserResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package gov.cdc.usds.simplereport.api.apiuser;

import static gov.cdc.usds.simplereport.service.ApiUserService.DEFAULT_OKTA_USER_PAGE_SIZE;

import gov.cdc.usds.simplereport.api.model.ApiUserWithStatus;
import gov.cdc.usds.simplereport.api.model.User;
import gov.cdc.usds.simplereport.api.model.errors.IllegalGraphqlArgumentException;
Expand All @@ -9,6 +11,7 @@
import java.util.List;
import java.util.UUID;
import org.apache.commons.lang3.StringUtils;
import org.springframework.data.domain.Page;
import org.springframework.graphql.data.method.annotation.Argument;
import org.springframework.graphql.data.method.annotation.QueryMapping;
import org.springframework.stereotype.Controller;
Expand Down Expand Up @@ -38,6 +41,19 @@ public List<ApiUserWithStatus> usersWithStatus() {
return _userService.getUsersAndStatusInCurrentOrg();
}

@QueryMapping
public Page<ApiUserWithStatus> usersWithStatusPage(
@Argument int pageNumber, @Argument String searchQuery) {
if (pageNumber < 0) {
pageNumber = ApiUserService.DEFAULT_OKTA_USER_PAGE_OFFSET;
}
if (!searchQuery.isBlank()) {
return _userService.searchUsersAndStatusInCurrentOrgPaged(
Copy link
Collaborator Author

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.

Copy link
Collaborator

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?

Copy link
Collaborator Author

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

pageNumber, DEFAULT_OKTA_USER_PAGE_SIZE, searchQuery);
}
return _userService.getPagedUsersAndStatusInCurrentOrg(pageNumber, DEFAULT_OKTA_USER_PAGE_SIZE);
}

@QueryMapping
public User user(@Argument UUID id, @Argument String email) {
if (!StringUtils.isBlank(email)) {
Expand Down
23 changes: 22 additions & 1 deletion backend/src/main/java/gov/cdc/usds/simplereport/idp/repository/DemoOktaRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,21 @@ public Map<String, UserStatus> getAllUsersWithStatusForOrganization(Organization
.collect(Collectors.toMap(u -> u, u -> getUserStatus(u)));
}

@Override
public Map<String, UserStatus> getPagedUsersWithStatusForOrganization(
Fixed Show fixed Hide fixed
Organization org, int pageNumber, int pageSize) {
if (!orgUsernamesMap.containsKey(org.getExternalId())) {
throw new IllegalGraphqlArgumentException(
"Cannot get Okta users from nonexistent organization.");
}
List<String> allOrgUsernamesList =
orgUsernamesMap.get(org.getExternalId()).stream().sorted().collect(Collectors.toList());
int startIndex = pageNumber * pageSize;
int endIndex = Math.min((startIndex + pageSize), allOrgUsernamesList.size());
List<String> pageContent = allOrgUsernamesList.subList(startIndex, endIndex);
return pageContent.stream().collect(Collectors.toMap(u -> u, this::getUserStatus));
}

// this method doesn't mean much in a demo env
public void createOrganization(Organization org) {
String externalId = org.getExternalId();
Expand Down Expand Up @@ -399,7 +414,8 @@ public void reset() {
allUsernames.clear();
}

public Integer getUsersInSingleFacility(Facility facility) {
@Override
public Integer getUsersCountInSingleFacility(Facility facility) {
Integer accessCount = 0;

for (OrganizationRoleClaims existingClaims : usernameOrgRolesMap.values()) {
Expand All @@ -417,6 +433,11 @@ public Integer getUsersInSingleFacility(Facility facility) {
return accessCount;
}

@Override
public Integer getUsersCountInOrganization(Organization org) {
return orgUsernamesMap.get(org.getExternalId()).size();
}

@Override
public String getApplicationStatusForHealthCheck() {
return ACTIVE_LITERAL;
Expand Down
43 changes: 34 additions & 9 deletions backend/src/main/java/gov/cdc/usds/simplereport/idp/repository/LiveOktaRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,19 @@ public Map<String, UserStatus> getAllUsersWithStatusForOrganization(Organization
.collect(Collectors.toMap(u -> u.getProfile().getLogin(), User::getStatus));
}

@Override
public Map<String, UserStatus> getPagedUsersWithStatusForOrganization(
Organization org, int pageNumber, int pageSize) {
Group orgDefaultOktaGroup = getDefaultOktaGroup(org);
int afterIndex = pageNumber * pageSize;
List<User> groupUsers =
groupApi.listGroupUsers(orgDefaultOktaGroup.getId(), String.valueOf(afterIndex), pageSize);
return groupUsers.stream()
.collect(
Collectors.toMap(
u -> Objects.requireNonNull(u.getProfile()).getLogin(), User::getStatus));
}

private List<User> getAllUsersForOrg(Organization org) {
PagedList<User> pagedUserList = new PagedList<>();
List<User> allUsers = new ArrayList<>();
Expand Down Expand Up @@ -658,27 +671,39 @@ public Optional<OrganizationRoleClaims> getOrganizationRoleClaimsForUser(String
getUserOrThrowError(username, "Cannot get org external ID for nonexistent user"));
}

public Integer getUsersInSingleFacility(Facility facility) {
String facilityAccessGroupName =
generateFacilityGroupName(
facility.getOrganization().getExternalId(), facility.getInternalId());

List<Group> facilityAccessGroup =
groupApi.listGroups(facilityAccessGroupName, null, null, 1, "stats", null, null, null);
private Integer getUsersCountInOktaGroup(String groupName) {
Copy link
Collaborator Author

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.

List<Group> groupList =
groupApi.listGroups(groupName, null, null, 1, "stats", null, null, null);

if (facilityAccessGroup.isEmpty()) {
if (groupList.isEmpty()) {
return 0;
}

try {
LinkedHashMap<String, Object> stats =
(LinkedHashMap) facilityAccessGroup.get(0).getEmbedded().get("stats");
(LinkedHashMap) groupList.get(0).getEmbedded().get("stats");
return ((Integer) stats.get("usersCount"));
} catch (NullPointerException e) {
throw new BadRequestException("Unable to retrieve okta group stats", e);
}
}

@Override
public Integer getUsersCountInSingleFacility(Facility facility) {
Fixed Show fixed Hide fixed
String facilityAccessGroupName =
generateFacilityGroupName(
facility.getOrganization().getExternalId(), facility.getInternalId());

return getUsersCountInOktaGroup(facilityAccessGroupName);
}

@Override
public Integer getUsersCountInOrganization(Organization org) {
String orgDefaultGroupName =
generateRoleGroupName(org.getExternalId(), OrganizationRole.getDefault());
return getUsersCountInOktaGroup(orgDefaultGroupName);
}

public PartialOktaUser findUser(String username) {
User user =
getUserOrThrowError(
Expand Down
13 changes: 12 additions & 1 deletion backend/src/main/java/gov/cdc/usds/simplereport/idp/repository/OktaRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,15 @@ List<String> updateUserPrivilegesAndGroupAccess(

Map<String, UserStatus> getAllUsersWithStatusForOrganization(Organization org);

/**
* @param org Organization being queried
* @param pageNumber Starts at page number 0
* @param pageSize Number of results per page
* @return Map of usernames to the user status in Okta
*/
Map<String, UserStatus> getPagedUsersWithStatusForOrganization(
Organization org, int pageNumber, int pageSize);

void createOrganization(Organization org);

void activateOrganization(Organization org);
Expand All @@ -83,7 +92,9 @@ List<String> updateUserPrivilegesAndGroupAccess(

Optional<OrganizationRoleClaims> getOrganizationRoleClaimsForUser(String username);

Integer getUsersInSingleFacility(Facility facility);
Integer getUsersCountInSingleFacility(Facility facility);

Integer getUsersCountInOrganization(Organization org);

PartialOktaUser findUser(String username);

Expand Down
50 changes: 50 additions & 0 deletions backend/src/main/java/gov/cdc/usds/simplereport/service/ApiUserService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@
import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.support.ScopeNotActiveException;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.PageImpl;
import org.springframework.data.domain.PageRequest;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Propagation;
import org.springframework.transaction.annotation.Transactional;
Expand Down Expand Up @@ -81,6 +84,8 @@ public class ApiUserService {
@Autowired private DbOrgRoleClaimsService _dbOrgRoleClaimsService;

@Autowired private FeatureFlagsConfig _featureFlagsConfig;
public static final int DEFAULT_OKTA_USER_PAGE_SIZE = 10;
public static final int DEFAULT_OKTA_USER_PAGE_OFFSET = 0;
mpbrown marked this conversation as resolved.
Show resolved Hide resolved

private void createUserUpdatedAuditLog(Object authorId, Object updatedUserId) {
log.info("User with id={} updated by user with id={}", authorId, updatedUserId);
Expand Down Expand Up @@ -615,6 +620,51 @@ public List<ApiUser> getUsersInCurrentOrg() {
return usersInOrg;
}

@AuthorizationConfiguration.RequirePermissionManageUsers
public Page<ApiUserWithStatus> getPagedUsersAndStatusInCurrentOrg(int pageNumber, int pageSize) {
Organization org = _orgService.getCurrentOrganization();

final Map<String, UserStatus> emailsToStatus =
_oktaRepo.getPagedUsersWithStatusForOrganization(org, pageNumber, pageSize);
List<ApiUser> users = _apiUserRepo.findAllByLoginEmailInOrderByName(emailsToStatus.keySet());
Copy link
Collaborator Author

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.

List<ApiUserWithStatus> userWithStatusList =
users.stream()
.map(u -> new ApiUserWithStatus(u, emailsToStatus.get(u.getLoginEmail())))
.toList();

Integer userCountInOrg = _oktaRepo.getUsersCountInOrganization(org);
PageRequest pageRequest = PageRequest.of(pageNumber, pageSize);

return new PageImpl<>(userWithStatusList, pageRequest, userCountInOrg);
}

public Page<ApiUserWithStatus> searchUsersAndStatusInCurrentOrgPaged(
int pageNumber, int pageSize, String searchQuery) {
List<ApiUserWithStatus> allUsers = getUsersAndStatusInCurrentOrg();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

See earlier comment


List<ApiUserWithStatus> filteredUsersList =
allUsers.stream()
.filter(
u -> {
Copy link
Collaborator

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? 🤔

Copy link
Collaborator Author

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

String firstName =
u.getFirstName() == null ? "" : String.format("%s ", u.getFirstName());
String middleName =
u.getMiddleName() == null ? "" : String.format("%s ", u.getMiddleName());
String fullName = firstName + middleName + u.getLastName();
return fullName.toLowerCase().contains(searchQuery.toLowerCase());
})
.toList();

int totalResults = filteredUsersList.size();
int startIndex = pageNumber * pageSize;
int endIndex = Math.min((startIndex + pageSize), filteredUsersList.size());

List<ApiUserWithStatus> pageContent = filteredUsersList.subList(startIndex, endIndex);
PageRequest pageRequest = PageRequest.of(pageNumber, pageSize);

return new PageImpl<>(pageContent, pageRequest, totalResults);
}

// To be addressed in #8108
@AuthorizationConfiguration.RequirePermissionManageUsers
public List<ApiUserWithStatus> getUsersAndStatusInCurrentOrg() {
Expand Down
2 changes: 1 addition & 1 deletion backend/src/main/java/gov/cdc/usds/simplereport/service/OrganizationService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@ public FacilityStats getFacilityStats(@Argument UUID facilityId) {
usersWithSingleFacilityAccess =
dbAuthorizationService.getUsersWithSingleFacilityAccessCount(facility);
} else {
usersWithSingleFacilityAccess = this.oktaRepository.getUsersInSingleFacility(facility);
usersWithSingleFacilityAccess = this.oktaRepository.getUsersCountInSingleFacility(facility);
}
return FacilityStats.builder()
.usersSingleAccessCount(usersWithSingleFacilityAccess)
Expand Down
Loading
Loading