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

Jump to bottom

Add syphilis history field to bulk uploader #7809

Merged
merged 3 commits into from
Jun 17, 2024
Merged

Add syphilis history field to bulk uploader #7809

merged 3 commits into from
Jun 17, 2024

Conversation

mpbrown
Copy link
Collaborator

@mpbrown mpbrown commented Jun 14, 2024 •
edited
Loading

BACKEND PULL REQUEST

Related Issue

Changes Proposed

  • Adds support for the syphilis_history column to the bulk results uploader

Additional Information

  • We decided to maintain the current design of only asking for whether the patient is symptomatic for the disease rather than asking for individual symptoms
  • A separate ticket exists to create the syphilis FHIR bundle for both single entry and bulk upload

Testing

  • Deployed to dev5
  • Attempt to submit a bulk results upload file that contains a positive syphilis result but blank values for genders of sexual partners, pregnant, previous syphilis diagnosis, and/or symptomatic for disease
  • You should get a missing data error message in the bulk uploader screen
  • A syphilis device is configured on dev5 with syphilis as a test performed code of 76272004 and a model name of syphilis device model

image

@mpbrown mpbrown marked this pull request as ready for review June 14, 2024 19:24
emyl3
emyl3 reviewed Jun 14, 2024
View reviewed changes
backend/src/main/java/gov/cdc/usds/simplereport/validators/CsvValidatorUtils.java
@@ -176,7 +181,7 @@ public class CsvValidatorUtils {
"n", "no",
"u", UNKNOWN_CODE);
private static final Set<String> TEST_RESULT_VALUES =
Set.of("positive", "negative", "not detected", "detected", "invalid result");
Set.of(POSITIVE_LITERAL, "negative", "not detected", DETECTED_LITERAL, "invalid result");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unrelated to this change but I realized we need to update the view for the device code lookup because we put "Undetermined" as the test result in the device code lookup tool but we don't support that in the bulk upload and accept "Not detected" instead.

See:

prime-simplereport/frontend/src/app/uploads/DeviceLookup/DeviceDetails.tsx

Line 18 in db562d2

[TEST_RESULTS.UNDETERMINED]: "419984006",

Device code lookup
Screenshot 2024-06-14 at 16 08 20

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nice find on this! Yeah I think this is definitely worth creating a new ticket to update the device code lookup, especially given the recent issues with unmapped SNOMED code descriptions in the bulk uploader. Plus noticing that what we have listed as "Positive" is actually the SNOMED code for "Detected" whereas "Positive" SNOMED is 10828004. Seems like we're moving in the direction where it's important that certain tests distinguish the particular test result codes they use, so might be something else that needs updating too

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok! Will make a ticket for this!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ticket created here: #7826 Feel free to add/edit anything as you see fit!

emyl3
emyl3 previously approved these changes Jun 14, 2024
View reviewed changes
Copy link
Collaborator

@emyl3 emyl3 left a comment

Choose a reason for hiding this comment

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

LGTM! Left one unrelated comment -- I can create a ticket for that if you think it's something to address! :) Nice work on this Mike!

fzhao99
fzhao99 reviewed Jun 15, 2024
View reviewed changes
Copy link
Contributor

@fzhao99 fzhao99 left a comment

Choose a reason for hiding this comment

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

lgtm on dev5! left some questions

@mpbrown
Copy link
Collaborator Author

mpbrown commented Jun 17, 2024

Refactored that validator so now there's a common method used for validating both HIV and Syphilis AOE fields when a test result is positive

@mpbrown mpbrown requested review from fzhao99 and emyl3 June 17, 2024 10:44
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
95.2% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

fzhao99
fzhao99 approved these changes Jun 17, 2024
View reviewed changes
Copy link
Contributor

@fzhao99 fzhao99 left a comment

Choose a reason for hiding this comment

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

lgtm! love the way you separated out the field specification from the validation logic: think this will make future extensions much easier.

@mpbrown mpbrown enabled auto-merge June 17, 2024 14:34
emyl3
emyl3 approved these changes Jun 17, 2024
View reviewed changes
Copy link
Collaborator

@emyl3 emyl3 left a comment

Choose a reason for hiding this comment

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

👍🏼

@mpbrown mpbrown added this pull request to the merge queue Jun 17, 2024
Merged via the queue into main with commit e3f33ff Jun 17, 2024
39 checks passed
@mpbrown mpbrown deleted the mike/7452-bulk-upload-syphilis-support branch June 17, 2024 17:15
shanice-skylight pushed a commit that referenced this pull request Jun 26, 2024
@mpbrown @shanice-skylight
Add syphilis history field to bulk uploader (#7809)
a98d307 
* Add syphilis history field to bulk uploader

* Use string literals

* Refactor using list of fields
@mpbrown mpbrown mentioned this pull request Sep 25, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emyl3 emyl3 emyl3 approved these changes

@fzhao99 fzhao99 fzhao99 approved these changes

@mehansen mehansen Awaiting requested review from mehansen

@DanielSass DanielSass Awaiting requested review from DanielSass

@bobbywells52 bobbywells52 Awaiting requested review from bobbywells52

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Syphilis] CSV Bulk Results Uploader supports Syphilis test results
3 participants
@mpbrown @emyl3 @fzhao99