Fix errors for manage users Okta pagination (#8432)

* Fix bug searching while on later pages * Paginate without Okta pagination cursor * Update main.graphql for backend testing * GQL codegen
CDCgov · Jan 28, 2025 · d5a8678 · d5a8678
1 parent ba81669
commit d5a8678
Show file tree

Hide file tree

Showing 6 changed files with 70 additions and 61 deletions.
diff --git a/backend/src/main/java/gov/cdc/usds/simplereport/api/apiuser/UserResolver.java b/backend/src/main/java/gov/cdc/usds/simplereport/api/apiuser/UserResolver.java
@@ -46,11 +46,9 @@ public ManageUsersPageWrapper usersWithStatusPage(
     if (pageNumber < 0) {
       pageNumber = DEFAULT_OKTA_USER_PAGE_OFFSET;
     }
-    if (!searchQuery.isBlank()) {
-      return _userService.searchUsersAndStatusInCurrentOrgPaged(
-          pageNumber, DEFAULT_OKTA_USER_PAGE_SIZE, searchQuery);
-    }
-    return _userService.getPagedUsersAndStatusInCurrentOrg(pageNumber, DEFAULT_OKTA_USER_PAGE_SIZE);
+
+    return _userService.getPagedUsersAndStatusInCurrentOrg(
+        pageNumber, DEFAULT_OKTA_USER_PAGE_SIZE, searchQuery);
   }
 
   @QueryMapping

diff --git a/backend/src/main/java/gov/cdc/usds/simplereport/idp/repository/LiveOktaRepository.java b/backend/src/main/java/gov/cdc/usds/simplereport/idp/repository/LiveOktaRepository.java
@@ -229,11 +229,11 @@ public Map<String, UserStatus> getAllUsersWithStatusForOrganization(Organization
   @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()
+    List<User> allUsers = getAllUsersForOrg(org);
+    int startIndex = pageNumber * pageSize;
+    int endIndex = Math.min((startIndex + pageSize), allUsers.size());
+    List<User> userSublist = allUsers.subList(startIndex, endIndex);
+    return userSublist.stream()
         .collect(
             Collectors.toMap(
                 u -> Objects.requireNonNull(u.getProfile()).getLogin(), User::getStatus));

diff --git a/backend/src/main/java/gov/cdc/usds/simplereport/service/ApiUserService.java b/backend/src/main/java/gov/cdc/usds/simplereport/service/ApiUserService.java
@@ -620,56 +620,37 @@ public List<ApiUser> getUsersInCurrentOrg() {
   }
 
   @AuthorizationConfiguration.RequirePermissionManageUsers
-  public ManageUsersPageWrapper 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());
-    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);
-    Page<ApiUserWithStatus> pageContent =
-        new PageImpl<>(userWithStatusList, pageRequest, userCountInOrg);
-
-    return new ManageUsersPageWrapper(pageContent, userCountInOrg);
-  }
-
-  @AuthorizationConfiguration.RequirePermissionManageUsers
-  public ManageUsersPageWrapper searchUsersAndStatusInCurrentOrgPaged(
+  public ManageUsersPageWrapper getPagedUsersAndStatusInCurrentOrg(
       int pageNumber, int pageSize, String searchQuery) {
     List<ApiUserWithStatus> allUsers = getUsersAndStatusInCurrentOrg();
 
-    List<ApiUserWithStatus> totalFilteredUsersList =
-        allUsers.stream()
-            .filter(
-                u -> {
-                  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 totalSearchResults = totalFilteredUsersList.size();
-    int startIndex = pageNumber * pageSize;
-    int endIndex = Math.min((startIndex + pageSize), totalFilteredUsersList.size());
+    List<ApiUserWithStatus> filteredUsers = allUsers;
+
+    if (!searchQuery.isBlank()) {
+      filteredUsers =
+          allUsers.stream()
+              .filter(
+                  u -> {
+                    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();
+    }
 
-    Organization org = _orgService.getCurrentOrganization();
-    Integer userCountInOrg = _oktaRepo.getUsersCountInOrganization(org);
+    int totalSearchResults = filteredUsers.size();
+    int startIndex = totalSearchResults > pageSize ? pageNumber * pageSize : 0;
+    int endIndex = Math.min((startIndex + pageSize), filteredUsers.size());
+
+    List<ApiUserWithStatus> filteredSublist = filteredUsers.subList(startIndex, endIndex);
 
-    List<ApiUserWithStatus> filteredSublist = totalFilteredUsersList.subList(startIndex, endIndex);
-    PageRequest pageRequest = PageRequest.of(pageNumber, pageSize);
     Page<ApiUserWithStatus> pageContent =
-        new PageImpl<>(filteredSublist, pageRequest, totalSearchResults);
+        new PageImpl<>(filteredSublist, PageRequest.of(pageNumber, pageSize), totalSearchResults);
 
-    return new ManageUsersPageWrapper(pageContent, userCountInOrg);
+    return new ManageUsersPageWrapper(pageContent, allUsers.size());
   }
 
   // To be addressed in #8108

diff --git a/backend/src/main/resources/graphql/main.graphqls b/backend/src/main/resources/graphql/main.graphqls
@@ -204,6 +204,16 @@ type ApiUserWithStatus {
   status: String
 }
 
+type ApiUserWithStatusPage {
+  totalElements: Int!
+  content: [ApiUserWithStatus!]
+}
+
+type ApiUserWithStatusAndCountPage {
+  pageContent: ApiUserWithStatusPage!
+  totalUsersInOrg: Int!
+}
+
 # Patient and test types
 
 type Patient
@@ -545,6 +555,7 @@ type Query {
   users: [ApiUser] @requiredPermissions(allOf: ["MANAGE_USERS"])
   usersWithStatus: [ApiUserWithStatus!]
     @requiredPermissions(allOf: ["MANAGE_USERS"])
+  usersWithStatusPage(pageNumber: Int, searchQuery: String): ApiUserWithStatusAndCountPage! @requiredPermissions(allOf: ["MANAGE_USERS"])
   user(id: ID, email: String): User
     @requiredPermissions(allOf: ["MANAGE_USERS"])
   whoami: User!

diff --git a/backend/src/test/java/gov/cdc/usds/simplereport/service/ApiUserServiceTest.java b/backend/src/test/java/gov/cdc/usds/simplereport/service/ApiUserServiceTest.java
@@ -155,7 +155,7 @@ void getUsersAndStatusInCurrentOrg_success() {
   @WithSimpleReportOrgAdminUser
   void getPagedUsersAndStatusInCurrentOrg_success() {
     initSampleData();
-    ManageUsersPageWrapper usersPageWrapper = _service.getPagedUsersAndStatusInCurrentOrg(0, 3);
+    ManageUsersPageWrapper usersPageWrapper = _service.getPagedUsersAndStatusInCurrentOrg(0, 3, "");
     assertEquals(6, usersPageWrapper.getTotalUsersInOrg());
 
     Page<ApiUserWithStatus> usersPage = usersPageWrapper.getPageContent();
@@ -164,28 +164,29 @@ void getPagedUsersAndStatusInCurrentOrg_success() {
 
     checkApiUserWithStatus(users.get(0), "admin@example.com", "Andrews", UserStatus.ACTIVE);
     checkApiUserWithStatus(users.get(1), "bobbity@example.com", "Bobberoo", UserStatus.ACTIVE);
-    checkApiUserWithStatus(
-        users.get(2), "allfacilities@example.com", "Williams", UserStatus.ACTIVE);
+    checkApiUserWithStatus(users.get(2), "invalid@example.com", "Irwin", UserStatus.ACTIVE);
 
-    ManageUsersPageWrapper users2ndPageWrapper = _service.getPagedUsersAndStatusInCurrentOrg(1, 3);
+    ManageUsersPageWrapper users2ndPageWrapper =
+        _service.getPagedUsersAndStatusInCurrentOrg(1, 3, "");
     assertEquals(6, users2ndPageWrapper.getTotalUsersInOrg());
 
     Page<ApiUserWithStatus> users2ndPage = users2ndPageWrapper.getPageContent();
     List<ApiUserWithStatus> users2ndList = users2ndPage.stream().toList();
     assertEquals(3, users2ndList.size());
 
-    checkApiUserWithStatus(users2ndList.get(0), "invalid@example.com", "Irwin", UserStatus.ACTIVE);
-    checkApiUserWithStatus(users2ndList.get(1), "nobody@example.com", "Nixon", UserStatus.ACTIVE);
+    checkApiUserWithStatus(users2ndList.get(0), "nobody@example.com", "Nixon", UserStatus.ACTIVE);
+    checkApiUserWithStatus(
+        users2ndList.get(1), "notruby@example.com", "Reynolds", UserStatus.ACTIVE);
     checkApiUserWithStatus(
-        users2ndList.get(2), "notruby@example.com", "Reynolds", UserStatus.ACTIVE);
+        users2ndList.get(2), "allfacilities@example.com", "Williams", UserStatus.ACTIVE);
   }
 
   @Test
   @WithSimpleReportOrgAdminUser
   void searchUsersAndStatusInCurrentOrgPaged_success() {
     initSampleData();
     ManageUsersPageWrapper usersPageWrapper =
-        _service.searchUsersAndStatusInCurrentOrgPaged(0, 10, "Bob");
+        _service.getPagedUsersAndStatusInCurrentOrg(0, 10, "Bob");
     Page<ApiUserWithStatus> usersPage = usersPageWrapper.getPageContent();
     List<ApiUserWithStatus> users = usersPage.stream().toList();
     assertEquals(1, users.size());

diff --git a/frontend/src/generated/graphql.tsx b/frontend/src/generated/graphql.tsx
@@ -93,6 +93,18 @@ export type ApiUserWithStatus = {
   suffix?: Maybe<Scalars["String"]["output"]>;
 };
 
+export type ApiUserWithStatusAndCountPage = {
+  __typename?: "ApiUserWithStatusAndCountPage";
+  pageContent: ApiUserWithStatusPage;
+  totalUsersInOrg: Scalars["Int"]["output"];
+};
+
+export type ApiUserWithStatusPage = {
+  __typename?: "ApiUserWithStatusPage";
+  content?: Maybe<Array<ApiUserWithStatus>>;
+  totalElements: Scalars["Int"]["output"];
+};
+
 export enum ArchivedStatus {
   All = "ALL",
   Archived = "ARCHIVED",
@@ -768,6 +780,7 @@ export type Query = {
   user?: Maybe<User>;
   users?: Maybe<Array<Maybe<ApiUser>>>;
   usersWithStatus?: Maybe<Array<ApiUserWithStatus>>;
+  usersWithStatusPage: ApiUserWithStatusAndCountPage;
   whoami: User;
 };
 
@@ -919,6 +932,11 @@ export type QueryUserArgs = {
   id?: InputMaybe<Scalars["ID"]["input"]>;
 };
 
+export type QueryUsersWithStatusPageArgs = {
+  pageNumber?: InputMaybe<Scalars["Int"]["input"]>;
+  searchQuery?: InputMaybe<Scalars["String"]["input"]>;
+};
+
 export type Result = {
   __typename?: "Result";
   correctionStatus?: Maybe<Scalars["String"]["output"]>;