Skip to content

Commit

Permalink
Elisa/6062 patient query param addition (#6156)
Browse files Browse the repository at this point in the history
* Change includeArchived to archivedStatus param

* remove deprecated fields

* address code smells
  • Loading branch information
emyl3 authored Jul 18, 2023
1 parent 30acaee commit db010d3
Show file tree
Hide file tree
Showing 22 changed files with 455 additions and 231 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import gov.cdc.usds.simplereport.db.model.Facility;
import gov.cdc.usds.simplereport.db.model.Organization;
import gov.cdc.usds.simplereport.db.model.Person;
import gov.cdc.usds.simplereport.db.model.auxiliary.ArchivedStatus;
import gov.cdc.usds.simplereport.service.OrganizationService;
import gov.cdc.usds.simplereport.service.PersonService;
import java.time.LocalDate;
Expand All @@ -25,31 +26,42 @@ public PatientResolver(PersonService ps, OrganizationService os) {
_os = os;
}

private ArchivedStatus getArchivedStatus(Boolean includeArchived, ArchivedStatus archivedStatus) {
ArchivedStatus status;
if (includeArchived != null) {
status =
Boolean.TRUE.equals(includeArchived) ? ArchivedStatus.ALL : ArchivedStatus.UNARCHIVED;
} else {
status = archivedStatus != null ? archivedStatus : ArchivedStatus.UNARCHIVED;
}
return status;
}

// authorization happens in calls to PersonService
// will update as part of #6062
@QueryMapping
public List<Person> patients(
@Argument UUID facilityId,
@Argument int pageNumber,
@Argument int pageSize,
@Argument boolean includeArchived,
@Argument Boolean includeArchived,
@Argument ArchivedStatus archivedStatus,
@Argument String namePrefixMatch,
@Argument boolean includeArchivedFacilities) {
ArchivedStatus status = getArchivedStatus(includeArchived, archivedStatus);
return _ps.getPatients(
facilityId,
pageNumber,
pageSize,
includeArchived,
namePrefixMatch,
includeArchivedFacilities);
facilityId, pageNumber, pageSize, status, namePrefixMatch, includeArchivedFacilities);
}

// authorization happens in calls to PersonService
@QueryMapping
public long patientsCount(
@Argument UUID facilityId,
@Argument boolean includeArchived,
@Argument Boolean includeArchived,
@Argument ArchivedStatus archivedStatus,
@Argument String namePrefixMatch) {
return _ps.getPatientsCount(facilityId, includeArchived, namePrefixMatch, false);
ArchivedStatus status = getArchivedStatus(includeArchived, archivedStatus);
return _ps.getPatientsCount(facilityId, status, namePrefixMatch, false);
}

@QueryMapping
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ public class AuthorizationConfiguration {
private static final String SPEL_CAN_EXECUTE_SPECIFIC_PATIENT_SEARCH =
"@"
+ AUTHORIZER_BEAN
+ ".userHasSpecificPatientSearchPermission(#facilityId, #includeArchived, #namePrefixMatch, #includeArchivedFacilities)";
+ ".userHasSpecificPatientSearchPermission(#facilityId, #archivedStatus, #namePrefixMatch, #includeArchivedFacilities)";

/**
* Apply this annotation if the method should only be called by site-wide administrative users
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import gov.cdc.usds.simplereport.db.model.Person;
import gov.cdc.usds.simplereport.db.model.TestEvent;
import gov.cdc.usds.simplereport.db.model.TestOrder;
import gov.cdc.usds.simplereport.db.model.auxiliary.ArchivedStatus;
import gov.cdc.usds.simplereport.db.repository.ApiUserRepository;
import gov.cdc.usds.simplereport.db.repository.FacilityRepository;
import gov.cdc.usds.simplereport.db.repository.PatientLinkRepository;
Expand Down Expand Up @@ -222,15 +223,15 @@ public boolean userCanAccessPatientLink(UUID patientLinkId) {

public boolean userHasSpecificPatientSearchPermission(
UUID facilityId,
boolean includeArchived,
ArchivedStatus archivedStatus,
String namePrefixMatch,
boolean includeArchivedFacilities) {
Set<UserPermission> perms = new HashSet<>();

if (facilityId != null && !userCanAccessFacility(facilityId)) {
return false;
}
if (includeArchived) {
if (archivedStatus != ArchivedStatus.UNARCHIVED) {
perms.add(UserPermission.READ_ARCHIVED_PATIENT_LIST);
}
if (includeArchivedFacilities) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package gov.cdc.usds.simplereport.db.model.auxiliary;

public enum ArchivedStatus {
ALL,
ARCHIVED,
UNARCHIVED,
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import gov.cdc.usds.simplereport.db.model.Person;
import gov.cdc.usds.simplereport.db.model.Person.SpecField;
import gov.cdc.usds.simplereport.db.model.PhoneNumber;
import gov.cdc.usds.simplereport.db.model.auxiliary.ArchivedStatus;
import gov.cdc.usds.simplereport.db.model.auxiliary.PersonRole;
import gov.cdc.usds.simplereport.db.model.auxiliary.StreetAddress;
import gov.cdc.usds.simplereport.db.model.auxiliary.TestResultDeliveryPreference;
Expand Down Expand Up @@ -135,7 +136,7 @@ private Specification<Person> isDeletedFilter(boolean isDeleted) {
// called by List function and Count function
protected Specification<Person> buildPersonSearchFilter(
UUID facilityId,
boolean includeArchived,
ArchivedStatus archivedStatus,
String namePrefixMatch,
boolean includeArchivedFacilities) {

Expand All @@ -146,9 +147,12 @@ protected Specification<Person> buildPersonSearchFilter(

// build up filter based on params
Specification<Person> filter = inCurrentOrganizationFilter();
if (!includeArchived) {
if (archivedStatus == ArchivedStatus.UNARCHIVED) {
filter = filter.and(isDeletedFilter(false));
} else if (archivedStatus == ArchivedStatus.ARCHIVED) {
filter = filter.and(isDeletedFilter(true));
}

if (facilityId == null) {
filter = filter.and(inAccessibleFacilitiesFilter(includeArchivedFacilities));
} else {
Expand Down Expand Up @@ -179,7 +183,8 @@ protected Specification<Person> buildPersonMatchFilter(
* @param facilityId If null, then it means across accessible facilities in the whole organization
* @param pageOffset Pagination offset is zero based
* @param pageSize How many results to return, zero will result in the default page size (large)
* @param includeArchived Default is false. true will return both archived _and_ active users
* @param archivedStatus Default is UNARCHIVED. ARCHIVED will return only archived users. ALL will
* return both archived _and_ active users.
* @param namePrefixMatch Null returns all users, any string will filter by first,middle,last
* names that start with these characters. Case-insensitive. If fewer than
* @param includeArchivedFacilities setting to true will include patients in archived facilities,
Expand All @@ -191,7 +196,7 @@ public List<Person> getPatients(
UUID facilityId,
int pageOffset,
int pageSize,
boolean includeArchived,
ArchivedStatus archivedStatus,
String namePrefixMatch,
Boolean includeArchivedFacilities) {
if (pageOffset < 0) {
Expand All @@ -207,7 +212,7 @@ public List<Person> getPatients(

return _repo.findAll(
buildPersonSearchFilter(
facilityId, includeArchived, namePrefixMatch, includeArchivedFacilities),
facilityId, archivedStatus, namePrefixMatch, includeArchivedFacilities),
PageRequest.of(pageOffset, pageSize, NAME_SORT));
}

Expand All @@ -228,15 +233,15 @@ public boolean isDuplicatePatient(
@AuthorizationConfiguration.RequireSpecificPatientSearchPermission
public long getPatientsCount(
UUID facilityId,
boolean includeArchived,
ArchivedStatus archivedStatus,
String namePrefixMatch,
boolean includeArchivedFacilities) {
if (namePrefixMatch != null && namePrefixMatch.trim().length() < MINIMUM_CHAR_FOR_SEARCH) {
return 0;
}
return _repo.count(
buildPersonSearchFilter(
facilityId, includeArchived, namePrefixMatch, includeArchivedFacilities));
facilityId, archivedStatus, namePrefixMatch, includeArchivedFacilities));
}

// NO PERMISSION CHECK (make sure the caller has one!) getPatient()
Expand Down
4 changes: 4 additions & 0 deletions backend/src/main/resources/graphql/main.graphqls
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,8 @@ type Query {
pageSize: Int = 5000
includeArchived: Boolean = false
@requiredPermissions(allOf: ["READ_ARCHIVED_PATIENT_LIST"])
archivedStatus: ArchivedStatus = UNARCHIVED
@requiredPermissions(allOf: ["READ_ARCHIVED_PATIENT_LIST"])
namePrefixMatch: String
includeArchivedFacilities: Boolean = false
@requiredPermissions(allOf: ["VIEW_ARCHIVED_FACILITIES"])
Expand All @@ -392,6 +394,8 @@ type Query {
facilityId: ID
includeArchived: Boolean = false
@requiredPermissions(allOf: ["READ_ARCHIVED_PATIENT_LIST"])
archivedStatus: ArchivedStatus = UNARCHIVED
@requiredPermissions(allOf: ["READ_ARCHIVED_PATIENT_LIST"])
namePrefixMatch: String
): Int @requiredPermissions(anyOf: ["SEARCH_PATIENTS", "READ_PATIENT_LIST"])
patient(id: ID!): Patient
Expand Down
6 changes: 6 additions & 0 deletions backend/src/main/resources/graphql/wiring.graphqls
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,12 @@ enum PhoneType {
LANDLINE
}

enum ArchivedStatus {
ALL
ARCHIVED
UNARCHIVED
}

type NameInfo {
firstName: String
middleName: String
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ void queryingDeletedPatients_standardUser_fail() {
"deleted-person-query",
"getDeletedPatients",
null,
"Current user does not have permission to supply a non-default value for [includeArchived]");
"Current user does not have permission to supply a non-default value for [archivedStatus]");
assertLastAuditEntry(
TestUserIdentities.STANDARD_USER,
"getDeletedPatients",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import gov.cdc.usds.simplereport.db.model.auxiliary.ArchivedStatus;
import gov.cdc.usds.simplereport.service.BaseServiceTest;
import gov.cdc.usds.simplereport.service.OrganizationService;
import gov.cdc.usds.simplereport.service.PersonService;
import gov.cdc.usds.simplereport.test_util.TestDataBuilder;
import gov.cdc.usds.simplereport.test_util.TestDataFactory;
import java.time.LocalDate;
import java.util.Optional;
import java.util.UUID;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;

Expand Down Expand Up @@ -52,4 +54,68 @@ void patientExists_hasNoFacilityId_callsServiceWithNoFacility() {
verify(personService)
.isDuplicatePatient("John", "Schmidt", LocalDate.of(1990, 01, 01), org, Optional.empty());
}

@Test
void patients_withIncludeArchived_callsServiceWithArchivedStatus() {
var personService = mock(PersonService.class);
var orgService = mock(OrganizationService.class);
var org = TestDataBuilder.createValidOrganization();

when(orgService.getCurrentOrganization()).thenReturn(org);

var sut = new PatientResolver(personService, orgService);
var facilityId = UUID.randomUUID();

sut.patients(facilityId, 0, 100, true, null, null, false);

verify(personService).getPatients(facilityId, 0, 100, ArchivedStatus.ALL, null, false);
}

@Test
void patients_withArchivedStatus_callsServiceWithArchivedStatus() {
var personService = mock(PersonService.class);
var orgService = mock(OrganizationService.class);
var org = TestDataBuilder.createValidOrganization();

when(orgService.getCurrentOrganization()).thenReturn(org);

var sut = new PatientResolver(personService, orgService);
var facilityId = UUID.randomUUID();

sut.patients(facilityId, 0, 100, null, ArchivedStatus.ARCHIVED, null, false);

verify(personService).getPatients(facilityId, 0, 100, ArchivedStatus.ARCHIVED, null, false);
}

@Test
void patientsCount_withIncludeArchived_callsServiceWithArchivedStatus() {
var personService = mock(PersonService.class);
var orgService = mock(OrganizationService.class);
var org = TestDataBuilder.createValidOrganization();

when(orgService.getCurrentOrganization()).thenReturn(org);

var sut = new PatientResolver(personService, orgService);
var facilityId = UUID.randomUUID();

sut.patientsCount(facilityId, false, null, null);

verify(personService).getPatientsCount(facilityId, ArchivedStatus.UNARCHIVED, null, false);
}

@Test
void patientsCount_withArchivedStatus_callsServiceWithArchivedStatus() {
var personService = mock(PersonService.class);
var orgService = mock(OrganizationService.class);
var org = TestDataBuilder.createValidOrganization();

when(orgService.getCurrentOrganization()).thenReturn(org);

var sut = new PatientResolver(personService, orgService);
var facilityId = UUID.randomUUID();

sut.patients(facilityId, 0, 100, null, ArchivedStatus.UNARCHIVED, null, false);

verify(personService).getPatients(facilityId, 0, 100, ArchivedStatus.UNARCHIVED, null, false);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import gov.cdc.usds.simplereport.db.model.Facility;
import gov.cdc.usds.simplereport.db.model.Person;
import gov.cdc.usds.simplereport.db.model.PhoneNumber;
import gov.cdc.usds.simplereport.db.model.auxiliary.ArchivedStatus;
import gov.cdc.usds.simplereport.db.model.auxiliary.PersonRole;
import gov.cdc.usds.simplereport.db.model.auxiliary.PhoneType;
import gov.cdc.usds.simplereport.db.repository.PersonRepository;
Expand Down Expand Up @@ -405,11 +406,11 @@ private InputStream loadCsv(String csvFile) {

private List<Person> fetchDatabasePatients() {
return this._personService.getPatients(
null, PATIENT_PAGE_OFFSET, PATIENT_PAGE_SIZE, false, null, false);
null, PATIENT_PAGE_OFFSET, PATIENT_PAGE_SIZE, ArchivedStatus.UNARCHIVED, null, false);
}

private List<Person> fetchDatabasePatientsForFacility(UUID facilityId) {
return this._personService.getPatients(
facilityId, PATIENT_PAGE_OFFSET, PATIENT_PAGE_SIZE, false, null, false);
facilityId, PATIENT_PAGE_OFFSET, PATIENT_PAGE_SIZE, ArchivedStatus.UNARCHIVED, null, false);
}
}
Loading

0 comments on commit db010d3

Please sign in to comment.