New options for Patient masking

[julianxcarter]

Summary

Makes the following elements in the Patient resource maskable: telecom, multipleBirth (both multipleBirthBoolean and multipleBirthInteger), photo, contact, generalPractitioner, managingOrganization, and link.

New behavior

No changed behavior. The above elements are now supported as options within the mask array of the PatientExtractor config.

Code changes

1. Support for new fields added to the maskPatientData function.
2. A new text fixture (extended-patient-bundle.json) was added to test masking, given that the newly supported fields are not supported by the CSVExtractors and will only ever occur in resources obtained via a FHIR search.
3. Updated readme to include the new masking fields.

Testing guidance

1. Ensure that updated unit tests pass.
2. Ensure that the updated masking test fixture has elements consistent with what would be expected on the CancerPatient resource (namely that _s are placed before primitive elements, and that array elements have their dataAbsentReason extension contained within an array).
3. Ensure that the updated mask fixture is valid FHIR. This can be done using the Inferno FHIR Validator

[Dtphelan1]

Suggested change

	delete patient.phoo;
	delete patient.photo;

Superrrr small typo

[julianxcarter]

Good catch! Addressed in commit 713e4ce

[Dtphelan1]

Overall this looks great to me. The one other thing I'll remark on is the resulting bundle is failing validation according to Inferno's online validator. The errors I see are screncapped below. That said, I feel confident that our modelling of data-absent-reason for primitive values is correct based on FHIR's documentation. I raise the point in case I'm 1) getting different validator results than you, 2) missing another aspect of this problem that might be interesting.

tl;dr once my super small typo above is addressed, I'm good to approve; modelling data-absent extensions on primitives is hard; validating against them is harder.

[Dtphelan1]

Validation errors/warnings

[julianxcarter]

Overall this looks great to me. The one other thing I'll remark on is the resulting bundle is failing validation according to Inferno's online validator. The errors I see are screncapped below. That said, I feel confident that our modelling of data-absent-reason for primitive values is correct based on FHIR's documentation. I raise the point in case I'm 1) getting different validator results than you, 2) missing another aspect of this problem that might be interesting.

tl;dr once my super small typo above is addressed, I'm good to approve; modelling data-absent extensions on primitives is hard; validating against them is harder.

I got the same errors and warnings when I ran the validation myself. They seem to be unavoidable for us in the sense that the elements causing them do actually require the properties being detected by the validator, but that including those required bits does amount to PII that's being masked in the first place. Some of these errors could be avoided by including some nebulously appropriate values for some of these elements (perhaps included "Masked" as a value for name.given and name.family, "unknown" as a coded value for gender, or "UNK" as a coded value for the birthSex extension). This would inch us closer to valid FHIR in the technical sense, but it's not clear to me that these codes and values are meant to be used in that way. Unfortunately I think in regards to masking we're gonna have to make a choice between semantically correct FHIR, and FHIR that follows the specification to a T.

[Dtphelan1]

Overall this looks great to me. The one other thing I'll remark on is the resulting bundle is failing validation according to Inferno's online validator. The errors I see are screncapped below. That said, I feel confident that our modelling of data-absent-reason for primitive values is correct based on FHIR's documentation. I raise the point in case I'm 1) getting different validator results than you, 2) missing another aspect of this problem that might be interesting.
tl;dr once my super small typo above is addressed, I'm good to approve; modelling data-absent extensions on primitives is hard; validating against them is harder.

I got the same errors and warnings when I ran the validation myself. They seem to be unavoidable for us in the sense that the elements causing them do actually require the properties being detected by the validator, but that including those required bits does amount to PII that's being masked in the first place. Some of these errors could be avoided by including some nebulously appropriate values for some of these elements (perhaps included "Masked" as a value for name.given and name.family, "unknown" as a coded value for gender, or "UNK" as a coded value for the birthSex extension). This would inch us closer to valid FHIR in the technical sense, but it's not clear to me that these codes and values are meant to be used in that way. Unfortunately I think in regards to masking we're gonna have to make a choice between semantically correct FHIR, and FHIR that follows the specification to a T.

To be clear, I think what we have so far is perfect and the ideal. Just bizarre that validators don't seem keen on handling the else-if case where, if that property cannot be found, then check to see if there is an appropriate data-absent-reason. Might be something to discuss with the inferno team at some point.

As far as this goes, I'm good to approve