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 frontend support for Manage Users pagination #8371

Merged
merged 26 commits into from
Feb 13, 2025
Merged

Add frontend support for Manage Users pagination #8371

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
f7d6075
Add frontend pagination on manage users
mpbrown Dec 18, 2024
a792c4b
Add paginated search frontend
mpbrown Dec 24, 2024
7882003
Update snapshots
mpbrown Dec 26, 2024
3a698f0
Handle total users in org
mpbrown Jan 17, 2025
812ad30
Update styling for Manage Users
mpbrown Jan 27, 2025
e6a3da8
Run graphql codegen
mpbrown Jan 28, 2025
9cb7122
Update snapshot
mpbrown Jan 28, 2025
0c83837
Handle out of bounds requested page index
mpbrown Jan 29, 2025
625bd39
Update page size to 14
mpbrown Feb 3, 2025
1cb6838
Remove left padding for pagination on manage users
mpbrown Feb 3, 2025
485485f
Move search to query param for easier redirects
mpbrown Feb 3, 2025
e178422
Update filter effect
mpbrown Feb 3, 2025
ffe8864
Use debounced effect hook for query string
mpbrown Feb 4, 2025
1115978
Update test
mpbrown Feb 4, 2025
75f3ee5
Update page size to 12
mpbrown Feb 4, 2025
f95d192
Set page group size to 5
mpbrown Feb 4, 2025
a6ce8d3
Use constant page size to fix frontend pagination math bug
mpbrown Feb 5, 2025
9ad3181
Update mock name in test
mpbrown Feb 5, 2025
31e1517
Remove button id
mpbrown Feb 10, 2025
3c3ff4f
Add settings page index redirect
mpbrown Feb 10, 2025
e47766c
Add more flexible search filtering
mpbrown Feb 10, 2025
3452eca
Update test entries per page
mpbrown Feb 10, 2025
b5ee413
Update lighthouse url for settings page
mpbrown Feb 10, 2025
f23eb3c
Add active facility id for new redirect
mpbrown Feb 11, 2025
c818e21
Appease eslint import order
mpbrown Feb 11, 2025
e44c7c0
Fix facility query param redirect
mpbrown Feb 11, 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
19 changes: 13 additions & 6 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 @@ -35,6 +35,7 @@
import gov.cdc.usds.simplereport.service.model.IdentitySupplier;
import gov.cdc.usds.simplereport.service.model.OrganizationRoles;
import gov.cdc.usds.simplereport.service.model.UserInfo;
import java.util.Arrays;
import java.util.EnumSet;
import java.util.HashSet;
import java.util.List;
Expand Down Expand Up @@ -631,12 +632,18 @@ public ManageUsersPageWrapper getPagedUsersAndStatusInCurrentOrg(
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());
String cleanedSearchQuery = searchQuery.replace(",", " ");
List<String> querySubstringList =
Arrays.stream(cleanedSearchQuery.toLowerCase().split("\\s+")).toList();

String fullName =
u.getNameInfo().getFirstName()
+ " "
+ u.getNameInfo().getMiddleName()
+ " "
+ u.getNameInfo().getLastName();

return querySubstringList.stream().allMatch(fullName.toLowerCase()::contains);
})
.toList();
}
Expand Down
22 changes: 22 additions & 0 deletions backend/src/test/java/gov/cdc/usds/simplereport/service/ApiUserServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,28 @@ void searchUsersAndStatusInCurrentOrgPaged_success() {
checkApiUserWithStatus(users.get(0), "[email protected]", "Bobberoo", UserStatus.ACTIVE);
}

@Test
@WithSimpleReportOrgAdminUser
void searchUsersAndStatusInCurrentOrgPaged_lastFirstPartial_success() {
initSampleData();
ManageUsersPageWrapper usersPageWrapper =
_service.getPagedUsersAndStatusInCurrentOrg(0, 10, "be, bo");
List<ApiUserWithStatus> users = usersPageWrapper.getPageContent().stream().toList();
assertEquals(1, users.size());
checkApiUserWithStatus(users.get(0), "[email protected]", "Bobberoo", UserStatus.ACTIVE);
}

@Test
@WithSimpleReportOrgAdminUser
void searchUsersAndStatusInCurrentOrgPaged_firstLastPartial_success() {
initSampleData();
ManageUsersPageWrapper usersPageWrapper =
_service.getPagedUsersAndStatusInCurrentOrg(0, 10, "ru re");
List<ApiUserWithStatus> users = usersPageWrapper.getPageContent().stream().toList();
assertEquals(1, users.size());
checkApiUserWithStatus(users.get(0), "[email protected]", "Reynolds", UserStatus.ACTIVE);
}

@Test
@WithSimpleReportOrgAdminUser
void searchUsersAndStatusInCurrentOrgPaged_pageNumberOutOfBounds_success() {
Expand Down
9 changes: 8 additions & 1 deletion frontend/src/app/Settings/Settings.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { Route, Routes } from "react-router-dom";
import { Navigate, Route, Routes } from "react-router-dom";

import { useSelectedFacility } from "../facilitySelect/useSelectedFacility";

import ManageOrganizationContainer from "./ManageOrganizationContainer";
import ManageFacilitiesContainer from "./Facility/ManageFacilitiesContainer";
Expand All @@ -10,6 +12,10 @@ import { ManageSelfRegistrationLinksContainer } from "./ManageSelfRegistrationLi
import "./Settings.scss";

const Settings = () => {
const [facility] = useSelectedFacility();
const activeFacilityId = facility?.id || "";
const settingsIndexRedirect = "/settings/users/1?" + activeFacilityId;

Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't we need the "/settings/users/1?facility=" + activeFacilityId here? (specifically adding facility=?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

😬 face palm yes we definitely should need that here! Somehow the redirect still worked but that's probably why it's still hitting that other facility select screen. Thank you for catching this!

return (
<div className="prime-home flex-1">
<div className="grid-container">
Expand All @@ -30,6 +36,7 @@ const Settings = () => {
element={<ManageSelfRegistrationLinksContainer />}
/>
<Route path="users/:pageNumber" element={<ManageUsersContainer />} />
<Route index element={<Navigate to={settingsIndexRedirect} />} />
Copy link
Collaborator

Choose a reason for hiding this comment

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

I forgot we could also add something like the following:

<Route index element={<Navigate replace to="users/1"/>}/>

to default it to the users page like before if someone were to navigate to /settings

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good idea! I just added this redirect

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated the urls we have configured for the Lighthouse checks since adding this new redirect confused it

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ooo ok so I just tested this with an org with multiple facilities and it prompts the user to select the facility dropdown twice... 😓 I'll look in to seeing how we can get this to work for multiple facilities... Feel free to remove this redirect since everything else works as expected and I don't this was even required but a nice to have! My bad, Mike!

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 wonder if that's because we'd have to add the active facility id to the search params like we do in the ManageUsersContainer component. I can test that out and see if it works

let searchParams: Record<string, string> = {
      facility: activeFacilityId,
    };
...
navigate({
      pathname: "/settings/users/1",
      search: new URLSearchParams(searchParams).toString(),
    });

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@emyl3 I'm redeploying to dev5 with a fix for this, although it will still prompt the user to select which facility but only once

Copy link
Collaborator

Choose a reason for hiding this comment

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

ooo wow ok! Thank you for that quick fix. That prompt happening once totally makes sense!

</Routes>
</div>
</div>
Expand Down
12 changes: 6 additions & 6 deletions frontend/src/app/Settings/Users/ManageUsers.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ describe("ManageUsers", () => {
updateUserName={updateUserName}
updateUserEmail={updateUserEmail}
currentPage={1}
entriesPerPage={10}
entriesPerPage={12}
totalEntries={3}
queryString={""}
setQueryString={setQueryString}
Expand Down Expand Up @@ -724,7 +724,7 @@ describe("ManageUsers", () => {
updateUserName={() => Promise.resolve()}
updateUserEmail={() => Promise.resolve()}
totalEntries={0}
entriesPerPage={10}
entriesPerPage={12}
currentPage={1}
queryString={""}
setQueryString={setQueryString}
Expand Down Expand Up @@ -794,7 +794,7 @@ describe("ManageUsers", () => {
updateUserName={() => Promise.resolve()}
updateUserEmail={() => Promise.resolve()}
totalEntries={2}
entriesPerPage={10}
entriesPerPage={12}
currentPage={1}
queryString={""}
setQueryString={setQueryString}
Expand Down Expand Up @@ -841,7 +841,7 @@ describe("ManageUsers", () => {
updateUserName={() => Promise.resolve()}
updateUserEmail={() => Promise.resolve()}
totalEntries={2}
entriesPerPage={10}
entriesPerPage={12}
currentPage={1}
queryString={""}
setQueryString={setQueryString}
Expand Down Expand Up @@ -889,7 +889,7 @@ describe("ManageUsers", () => {
updateUserName={() => Promise.resolve()}
updateUserEmail={() => Promise.resolve()}
totalEntries={2}
entriesPerPage={10}
entriesPerPage={12}
currentPage={1}
queryString={""}
setQueryString={setQueryString}
Expand Down Expand Up @@ -993,7 +993,7 @@ describe("ManageUsers", () => {
updateUserName={() => Promise.resolve()}
updateUserEmail={() => Promise.resolve()}
totalEntries={3}
entriesPerPage={10}
entriesPerPage={12}
currentPage={1}
queryString={""}
setQueryString={setQueryString}
Expand Down
1 change: 0 additions & 1 deletion frontend/src/app/Settings/Users/UsersSideNav.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ const UsersSideNav: React.FC<Props> = ({
<div className={"padding-1 padding-left-3 padding-bottom-4"}>
<Button
className={"clear-filter-button"}
id={`clear-filter-button`}
onClick={() => setQueryString("")}
label={"Clear search filter"}
></Button>
Expand Down
2 changes: 1 addition & 1 deletion lighthouserc.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ ci:
- https://localhost.simplereport.gov/app/results/1?facility=$FACILITY_ID
- https://localhost.simplereport.gov/app/patients?facility=$FACILITY_ID
- https://localhost.simplereport.gov/app/add-patient?facility=$FACILITY_ID
- https://localhost.simplereport.gov/app/settings?facility=$FACILITY_ID
- https://localhost.simplereport.gov/app/settings/users/1?facility=$FACILITY_ID
- https://localhost.simplereport.gov/app/settings/facilities?facility=$FACILITY_ID
- https://localhost.simplereport.gov/app/settings/add-facility/?facility=$FACILITY_ID
- https://localhost.simplereport.gov/app/upload-patients?facility=$FACILITY_ID
Expand Down
Loading