Change includeArchived to archivedStatus param #6128

emyl3 · 2023-07-13T23:58:52Z

BACKEND & FRONTEND PULL REQUEST

Related Issue

Resolves #6062

Changes Proposed

Update getPatients and patientsCount to accept a parameter that can return deleted users, undeleted users, or all users.

Additional Information

Test the new archivedStatus parameter!
Update the values below as you see fit.

patients

patients(
      facilityId: $facilityId
      pageNumber: 0
      pageSize: 100
      archivedStatus: $archivedStatus
      namePrefixMatch: $namePrefixMatch
      includeArchivedFacilities: $includeArchivedFacilities
    ) {
      internalId
      firstName
      lastName
      middleName
      birthDate
      gender
      telephone
      email
      emails
      phoneNumbers {
        type
        number
      }
    }

patientsCount

    patientsCount(
      facilityId: $facilityId
      archivedStatus: $archivedStatus
      namePrefixMatch: $namePrefixMatch
    )

Testing

Deployed on dev4 for testing

emyl3 · 2023-07-14T19:03:04Z

frontend/src/app/testResults/TestResultsList.tsx

@@ -452,11 +466,10 @@ export const DetachedTestResultsList = ({
                  setStartDateError("");
                  setEndDateError("");
                }}
-                disabled={


⚠️ updated this to address a code smell related to code complexity being over 15

fzhao99 · 2023-07-17T15:47:23Z

NB for reviewers: "b2daabe8-11c8-4006-aa5f-0163869d369b" is a facility Id you can used with archived (5 total) / unarchived (4 total) patients

fzhao99 · 2023-07-17T16:00:03Z

backend/src/test/java/gov/cdc/usds/simplereport/service/PersonServiceTest.java

@@ -145,12 +146,18 @@ void roundTrip() {
        "French",
        null);
    List<Person> all =


Feel free to point me to it if it's existing already, but could we add a test specifically to verify that the ArchivedStatus.Archived case is covered (ie it doesn't also return the other states?). Think I see most of the existing tests just being convered over to the UNARCHIVED/ALL states but not a new one for the new state we're introducing.

Yes! Nice catch! I forgot to push up that change 😶‍🌫️ Will tag you in some comments on this PR 😅

fzhao99

lgtm overall! left one suggestion.

…ery-update

emyl3 · 2023-07-17T18:24:13Z

backend/src/test/java/gov/cdc/usds/simplereport/service/PersonServiceTest.java

    assertPatientList(patients, GALE, JANNELLE, KACEY, ELIZABETH, HEINRICK);
+    // all facilities, deleted
+    List<Person> deletedPatients =


@fzhao99 -- testing patients for "ARCHIVED" status here, all facilities

emyl3 · 2023-07-17T18:24:46Z

backend/src/test/java/gov/cdc/usds/simplereport/service/PersonServiceTest.java

    assertPatientList(patients, JANNELLE, KACEY);
+    // site2, deleted, "fran"
+    deletedPatients = _service.getPatients(site2Id, 0, 100, ArchivedStatus.ARCHIVED, "fran", false);


@fzhao99 -- testing patients for "ARCHIVED" status here from a specific facility with text "fran"

emyl3 · 2023-07-17T18:25:10Z

backend/src/test/java/gov/cdc/usds/simplereport/service/PersonServiceTest.java

-    assertEquals(7, _service.getPatientsCount(site2Id, true, null, false));
+    assertEquals(10, _service.getPatientsCount(null, ArchivedStatus.UNARCHIVED, null, false));
+    assertEquals(12, _service.getPatientsCount(null, ArchivedStatus.ALL, null, false));
+    assertEquals(2, _service.getPatientsCount(null, ArchivedStatus.ARCHIVED, null, false));


@fzhao99 -- testing patients count for "ARCHIVED" status here, all facilities

emyl3 · 2023-07-17T18:25:31Z

backend/src/test/java/gov/cdc/usds/simplereport/service/PersonServiceTest.java

+    assertEquals(2, _service.getPatientsCount(null, ArchivedStatus.ARCHIVED, null, false));
+    assertEquals(6, _service.getPatientsCount(site2Id, ArchivedStatus.UNARCHIVED, null, false));
+    assertEquals(7, _service.getPatientsCount(site2Id, ArchivedStatus.ALL, null, false));
+    assertEquals(1, _service.getPatientsCount(site2Id, ArchivedStatus.ARCHIVED, null, false));


@fzhao99 -- testing patients for "ARCHIVED" status here, at a particular facility

emyl3 · 2023-07-17T18:26:34Z

backend/src/test/java/gov/cdc/usds/simplereport/service/PersonServiceTest.java

+    assertEquals(0, _service.getPatientsCount(null, ArchivedStatus.UNARCHIVED, "M", false));
+    assertEquals(0, _service.getPatientsCount(null, ArchivedStatus.UNARCHIVED, "", false));
+
+    assertEquals(1, _service.getPatientsCount(null, ArchivedStatus.ARCHIVED, "fra", false));


@fzhao99 -- testing patient count for "ARCHIVED" status here, all facilities or particular facility with text "fra" or "charles"

emyl3 · 2023-07-17T18:27:47Z

Thank you @fzhao99 for flagging that. Let me know what you think! 😸

sonarcloud · 2023-07-17T18:39:19Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

96.9% Coverage
0.0% Duplication

fzhao99

lgtm!

johanna-skylight

Code looks awesome overall. The only question that I have is whether or not this will be backwards compatible with the UI (as the UI changes are going in the same PR).

How will the two graphql operations behave for requests that are still relying on the old parameter? (case when the UI is running old code)

emyl3 · 2023-07-17T21:02:42Z

Code looks awesome overall. The only question that I have is whether or not this will be backwards compatible with the UI (as the UI changes are going in the same PR).

How will the two graphql operations behave for requests that are still relying on the old parameter? (case when the UI is running old code)

OOO that is a very good point. Thank you for bringing this up. This change will definitely break some views for the X number of mins a user has the old frontend.

I will put up two separate PRs and probably close this one:

one PR to introduce and leverage this new parameter (archivedStatus)
another PR to deprecate and remove support for the old parameter (includeArchived)

Let me know if this sounds good to you, @johanna-skylight! Thank you! 😸

emyl3 · 2023-07-18T19:20:46Z

closed in favor of #6156

emyl3 force-pushed the elisa/6062-patient-query-update branch 2 times, most recently from 78b1f70 to 2dce9bd Compare July 14, 2023 16:36

Change includeArchived to archivedStatus param

d4eac15

emyl3 temporarily deployed to Dev4 July 14, 2023 18:59 — with GitHub Actions Inactive

emyl3 commented Jul 14, 2023

View reviewed changes

emyl3 temporarily deployed to Dev4 July 17, 2023 14:51 — with GitHub Actions Inactive

emyl3 marked this pull request as ready for review July 17, 2023 15:23

emyl3 requested review from mehansen, BobanL, fzhao99 and johanna-skylight July 17, 2023 15:23

fzhao99 reviewed Jul 17, 2023

View reviewed changes

Merge remote-tracking branch 'origin/main' into elisa/6062-patient-qu…

3c27b01

…ery-update

emyl3 force-pushed the elisa/6062-patient-query-update branch from ed8fabc to 3c27b01 Compare July 17, 2023 18:17

emyl3 commented Jul 17, 2023

View reviewed changes

emyl3 requested a review from fzhao99 July 17, 2023 18:27

emyl3 temporarily deployed to Dev4 July 17, 2023 18:31 — with GitHub Actions Inactive

fzhao99 approved these changes Jul 17, 2023

View reviewed changes

johanna-skylight reviewed Jul 17, 2023

View reviewed changes

emyl3 mentioned this pull request Jul 18, 2023

Elisa/6062 patient query param addition #6156

Merged

emyl3 closed this Jul 18, 2023

emyl3 mentioned this pull request Jul 18, 2023

Deprecate includeArchived field #6164

Merged

emyl3 deleted the elisa/6062-patient-query-update branch July 19, 2023 18:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change includeArchived to archivedStatus param #6128

Change includeArchived to archivedStatus param #6128

emyl3 commented Jul 13, 2023 •

edited

Loading

emyl3 Jul 14, 2023

fzhao99 commented Jul 17, 2023

fzhao99 Jul 17, 2023 •

edited

Loading

emyl3 Jul 17, 2023

fzhao99 left a comment

emyl3 Jul 17, 2023

emyl3 Jul 17, 2023

emyl3 Jul 17, 2023

emyl3 Jul 17, 2023

emyl3 Jul 17, 2023

emyl3 commented Jul 17, 2023

sonarcloud bot commented Jul 17, 2023

fzhao99 left a comment

johanna-skylight left a comment

emyl3 commented Jul 17, 2023

emyl3 commented Jul 18, 2023

Change includeArchived to archivedStatus param #6128

Change includeArchived to archivedStatus param #6128

Conversation

emyl3 commented Jul 13, 2023 • edited Loading

BACKEND & FRONTEND PULL REQUEST

Related Issue

Changes Proposed

Additional Information

Testing

emyl3 Jul 14, 2023

Choose a reason for hiding this comment

fzhao99 commented Jul 17, 2023

fzhao99 Jul 17, 2023 • edited Loading

Choose a reason for hiding this comment

emyl3 Jul 17, 2023

Choose a reason for hiding this comment

fzhao99 left a comment

Choose a reason for hiding this comment

emyl3 Jul 17, 2023

Choose a reason for hiding this comment

emyl3 Jul 17, 2023

Choose a reason for hiding this comment

emyl3 Jul 17, 2023

Choose a reason for hiding this comment

emyl3 Jul 17, 2023

Choose a reason for hiding this comment

emyl3 Jul 17, 2023

Choose a reason for hiding this comment

emyl3 commented Jul 17, 2023

sonarcloud bot commented Jul 17, 2023

fzhao99 left a comment

Choose a reason for hiding this comment

johanna-skylight left a comment

Choose a reason for hiding this comment

emyl3 commented Jul 17, 2023

emyl3 commented Jul 18, 2023

emyl3 commented Jul 13, 2023 •

edited

Loading

fzhao99 Jul 17, 2023 •

edited

Loading