Skip to content

Conversation

@jafeltra
Copy link
Contributor

Summary

This PR makes it so that familyName, givenName, and gender are not required fields in CSVs. Because they are still required in the mCODE Cancer Patient profile, they are marked with dataAbsentReason extensions with a code 'unknown'.

New behavior

If familyName, givenName, or gender is not provided in the patient information CSV, no errors/warnings are logged and no data extracted. Further, if they are missing, the template will add a dataAbsentReason for name and gender in the patient resource because they have a 1..* cardinality in mCODE. I thought about adding a warning or debug message to say those fields are recommended, but decided against it. If anyone likes the idea of that message, I can definitely be convinced to add it back.

If name or gender is not provided but configuration says to mask it, it will get a dataAbsentReason for 'masked' instead. I thought this made sense because we'd want to mask any data we had, including the extension saying that we had no data (when I type it out, it sounds more confusing than I thought).

None of the other fields in the patient CSV are affected by this PR because they are not required in the profiled Patient resource, and if values are not provided in the CSV, we already do not include them in the extracted resource. Any field that we say to mask but was never in the resource is not masked, which I believe is still correct.

Code changes

  • Updates the requirements in the CSV validation schemas
  • Updates when we throw an error from the CSVPatientExtractor to only throw if the mrn is missing
  • Adds the dataAbsentReason extensions if applicable to name and gender

Testing guidance

  • Ensure the tests test this code appropriately.
  • Try running the CSV Patient extractor with only mrn and with a combination of the non-required data and ensure both approaches extract a Patient resource properly. Also try masking properties, including gender, name, mrn, and any other non-required, 0..* property.
  • Ensure this PR covers both STEAM-507 and STEAM-508

Comment on lines 124 to 125
// fhirpath.evaluate will return [] if there is no extension with the given URL
// so checking if the result is [] checks if the field exists to be masked
Copy link
Contributor

Choose a reason for hiding this comment

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

Might want to update this to reflect the new logic you've introduced 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in 235d66b

@Dtphelan1 Dtphelan1 self-assigned this Apr 12, 2021
Copy link
Contributor

@Dtphelan1 Dtphelan1 left a comment

Choose a reason for hiding this comment

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

This looks great Jules. AFAIK meets all the criteria for both tasks, local experiments produce expected results. No comments from me 😄

@julianxcarter julianxcarter self-assigned this Apr 13, 2021
Copy link
Contributor

@julianxcarter julianxcarter left a comment

Choose a reason for hiding this comment

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

This looks great to me, and everything Is working perfectly! Nice work 👍🏽

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants