LinuxForHealth / FHIR

The LinuxForHealth FHIR® Server and related projects
https://linuxforhealth.github.io/FHIR
Apache License 2.0
326 stars 157 forks source link

Implicit base not picked up for the patient search parameter #1674

Closed lmsurpre closed 3 years ago

lmsurpre commented 3 years ago

Describe the bug I found a case that we missed when we implemented #300 for adding the implicit resource type of a reference: chained parameters.

Specifically, note this line from https://build.fhir.org/search.html#chaining

Because the Diagnostic Report subject can be one of a set of different resources, it's necessary to limit the search to a particular type: GET [base]/DiagnosticReport?subject:Patient.name=peter

To me, that implies that if the type was NOT ambiguous, then the type modifier should be optional (just like in the normal case). However, my query for Observation?patient.gender=female returns the following issue:

'issue': [{'severity': 'fatal', 'code': 'invalid', 'details': {'text': "Search parameter: 'patient' must have resource type name modifier"}

Expected behavior The type modifier is not required if the reference search parameter only targets a single type.

Additional context We should open an HL7 JIRA ticket on this section of the spec as well, because it contradicts itself in a bad way:

For instance, given that the resource DiagnosticReport has a search parameter named subject, which is usually a reference to a Patient resource, and the Patient resource includes a parameter name which searches on patient name, then the search
 GET [base]/DiagnosticReport?subject.name=peter
is a request to return all the lab reports that have a subject whose name includes "peter".
Because the Diagnostic Report subject can be one of a set of different resources, it's necessary to limit the search to a particular type:
 GET [base]/DiagnosticReport?subject:Patient.name=peter

If it's "necessary to limit the search to a particular type" then the first query they give isn't actually valid, but they don't make this very clear.

michaelwschroeder commented 3 years ago

@lmsurpre I believe this is working as designed. When parsing a chained search parameter, we check the SearchParameter.target contents. If the target contains more than one resource type and a type modifier is not specified on the search parameter, then an error (as shown above) is returned.

In the example above, the Observation?patient.gender=female search correctly returns an error because the patient search parameter specifies both the Patient and Group resource types as targets. Here's the full search parameter definition:

   {
      "resourceType" : "SearchParameter",
      "id" : "clinical-patient",
      "extension" : [{
        "url" : "http://hl7.org/fhir/StructureDefinition/structuredefinition-standards-status",
        "valueCode" : "trial-use"
      }],
      "url" : "http://hl7.org/fhir/SearchParameter/clinical-patient",
      "version" : "4.0.1",
      "name" : "patient",
      "status" : "draft",
      "experimental" : false,
      "date" : "2019-11-01T09:29:23+11:00",
      "publisher" : "Health Level Seven International (Patient Care)",
      "contact" : [{
        "telecom" : [{
          "system" : "url",
          "value" : "http://hl7.org/fhir"
        }]
      },
      {
        "telecom" : [{
          "system" : "url",
          "value" : "http://www.hl7.org/Special/committees/patientcare/index.cfm"
        }]
      }],
      "description" : "Multiple Resources: \r\n\r\n* [AllergyIntolerance](allergyintolerance.html): Who the sensitivity is for\r\n* [CarePlan](careplan.html): Who the care plan is for\r\n* [CareTeam](careteam.html): Who care team is for\r\n* [ClinicalImpression](clinicalimpression.html): Patient or group assessed\r\n* [Composition](composition.html): Who and/or what the composition is about\r\n* [Condition](condition.html): Who has the condition?\r\n* [Consent](consent.html): Who the consent applies to\r\n* [DetectedIssue](detectedissue.html): Associated patient\r\n* [DeviceRequest](devicerequest.html): Individual the service is ordered for\r\n* [DeviceUseStatement](deviceusestatement.html): Search by subject - a patient\r\n* [DiagnosticReport](diagnosticreport.html): The subject of the report if a patient\r\n* [DocumentManifest](documentmanifest.html): The subject of the set of documents\r\n* [DocumentReference](documentreference.html): Who/what is the subject of the document\r\n* [Encounter](encounter.html): The patient or group present at the encounter\r\n* [EpisodeOfCare](episodeofcare.html): The patient who is the focus of this episode of care\r\n* [FamilyMemberHistory](familymemberhistory.html): The identity of a subject to list family member history items for\r\n* [Flag](flag.html): The identity of a subject to list flags for\r\n* [Goal](goal.html): Who this goal is intended for\r\n* [ImagingStudy](imagingstudy.html): Who the study is about\r\n* [Immunization](immunization.html): The patient for the vaccination record\r\n* [List](list.html): If all resources have the same subject\r\n* [MedicationAdministration](medicationadministration.html): The identity of a patient to list administrations  for\r\n* [MedicationDispense](medicationdispense.html): The identity of a patient to list dispenses  for\r\n* [MedicationRequest](medicationrequest.html): Returns prescriptions for a specific patient\r\n* [MedicationStatement](medicationstatement.html): Returns statements for a specific patient.\r\n* [NutritionOrder](nutritionorder.html): The identity of the person who requires the diet, formula or nutritional supplement\r\n* [Observation](observation.html): The subject that the observation is about (if patient)\r\n* [Procedure](procedure.html): Search by subject - a patient\r\n* [RiskAssessment](riskassessment.html): Who/what does assessment apply to?\r\n* [ServiceRequest](servicerequest.html): Search by subject - a patient\r\n* [SupplyDelivery](supplydelivery.html): Patient for whom the item is supplied\r\n* [VisionPrescription](visionprescription.html): The identity of a patient to list dispenses for\r\n",
      "code" : "patient",
      "base" : ["AllergyIntolerance",
      "CarePlan",
      "CareTeam",
      "ClinicalImpression",
      "Composition",
      "Condition",
      "Consent",
      "DetectedIssue",
      "DeviceRequest",
      "DeviceUseStatement",
      "DiagnosticReport",
      "DocumentManifest",
      "DocumentReference",
      "Encounter",
      "EpisodeOfCare",
      "FamilyMemberHistory",
      "Flag",
      "Goal",
      "ImagingStudy",
      "Immunization",
      "List",
      "MedicationAdministration",
      "MedicationDispense",
      "MedicationRequest",
      "MedicationStatement",
      "NutritionOrder",
      "Observation",
      "Procedure",
      "RiskAssessment",
      "ServiceRequest",
      "SupplyDelivery",
      "VisionPrescription"],
      "type" : "reference",
      "expression" : "AllergyIntolerance.patient | CarePlan.subject.where(resolve() is Patient) | CareTeam.subject.where(resolve() is Patient) | ClinicalImpression.subject.where(resolve() is Patient) | Composition.subject.where(resolve() is Patient) | Condition.subject.where(resolve() is Patient) | Consent.patient | DetectedIssue.patient | DeviceRequest.subject.where(resolve() is Patient) | DeviceUseStatement.subject | DiagnosticReport.subject.where(resolve() is Patient) | DocumentManifest.subject.where(resolve() is Patient) | DocumentReference.subject.where(resolve() is Patient) | Encounter.subject.where(resolve() is Patient) | EpisodeOfCare.patient | FamilyMemberHistory.patient | Flag.subject.where(resolve() is Patient) | Goal.subject.where(resolve() is Patient) | ImagingStudy.subject.where(resolve() is Patient) | Immunization.patient | List.subject.where(resolve() is Patient) | MedicationAdministration.subject.where(resolve() is Patient) | MedicationDispense.subject.where(resolve() is Patient) | MedicationRequest.subject.where(resolve() is Patient) | MedicationStatement.subject.where(resolve() is Patient) | NutritionOrder.patient | Observation.subject.where(resolve() is Patient) | Procedure.subject.where(resolve() is Patient) | RiskAssessment.subject.where(resolve() is Patient) | ServiceRequest.subject.where(resolve() is Patient) | SupplyDelivery.patient | VisionPrescription.patient",
      "xpath" : "f:AllergyIntolerance/f:patient | f:CarePlan/f:subject | f:CareTeam/f:subject | f:ClinicalImpression/f:subject | f:Composition/f:subject | f:Condition/f:subject | f:Consent/f:patient | f:DetectedIssue/f:patient | f:DeviceRequest/f:subject | f:DeviceUseStatement/f:subject | f:DiagnosticReport/f:subject | f:DocumentManifest/f:subject | f:DocumentReference/f:subject | f:Encounter/f:subject | f:EpisodeOfCare/f:patient | f:FamilyMemberHistory/f:patient | f:Flag/f:subject | f:Goal/f:subject | f:ImagingStudy/f:subject | f:Immunization/f:patient | f:List/f:subject | f:MedicationAdministration/f:subject | f:MedicationDispense/f:subject | f:MedicationRequest/f:subject | f:MedicationStatement/f:subject | f:NutritionOrder/f:patient | f:Observation/f:subject | f:Procedure/f:subject | f:RiskAssessment/f:subject | f:ServiceRequest/f:subject | f:SupplyDelivery/f:patient | f:VisionPrescription/f:patient",
      "xpathUsage" : "normal",
      "target" : ["Patient",
      "Group"]
    }

Looking at the expression field, the only expression which can return something other than the Patient resource type is the DeviceUseStatement.subject expression, which can return Patient or Group. I don't know if that was done by design, but seems odd to specify that rather than DeviceUseStatement.subect.where(resolve() is Patient) (like the expressions for the other resource types) in a patient search parameter.

tbieste commented 3 years ago

@lmsurpre, was looking through backlog issues, and after looking into this one, I came to the same conclusion that Mike did. Can this one be closed?

lmsurpre commented 3 years ago

Its definitely an interesting case.

As Mike mentions, Observation.subject.where(resolve() is Patient) can only ever be a Patient. But I see that is quite difficult to infer from the SearchParameter definition and I agree its probably better just to rely on the target field of the search parameter definition.

The problem with this is that https://hl7.org/fhir/search.html#reference explicitly mentions this search parameter as an example of a reference search parameter that shouldn't need the :[type] modifier:

In some cases, search parameters are defined with an implicitly limited scope. For example, Observation has an element subject, which is a reference to one of a number of types. This has a matching search parameter subject, which refers to any of the possible types. In addition to this, there is another search parameter patient, which also refers to Observation.subject, but is limited to only include references of type Patient. When using the patient search parameter, there is no need to specify ":Patient" as a modifier, or "Patient/" in the search value, as this must always be true.

lmsurpre commented 3 years ago

I had a peak at the search parameters from R5 and it looks like they have updated the target for this clinical-subject search parameter to include just about every kind of resource type as its "target":

"code" : "patient",
      "base" : ["AllergyIntolerance",
      "CarePlan",
      "CareTeam",
      "ClinicalImpression",
      "Composition",
      "Condition",
      "Consent",
      "DetectedIssue",
      "DeviceRequest",
      "DeviceUsage",
      "DiagnosticReport",
      "DocumentManifest",
      "DocumentReference",
      "Encounter",
      "EpisodeOfCare",
      "FamilyMemberHistory",
      "Flag",
      "Goal",
      "ImagingStudy",
      "Immunization",
      "List",
      "MedicationAdministration",
      "MedicationDispense",
      "MedicationRequest",
      "MedicationUsage",
      "NutritionOrder",
      "Observation",
      "Procedure",
      "RiskAssessment",
      "ServiceRequest",
      "SupplyDelivery",
      "VisionPrescription"],
      "type" : "reference",
      "expression" : "AllergyIntolerance.patient | CarePlan.subject.where(resolve() is Patient) | CareTeam.subject.where(resolve() is Patient) | ClinicalImpression.subject.where(resolve() is Patient) | Composition.subject.where(resolve() is Patient) | Condition.subject.where(resolve() is Patient) | Consent.subject.where(resolve() is Patient) | DetectedIssue.patient | DeviceRequest.subject.where(resolve() is Patient) | DeviceUsage.subject | DiagnosticReport.subject.where(resolve() is Patient) | DocumentManifest.subject.where(resolve() is Patient) | DocumentReference.subject.where(resolve() is Patient) | Encounter.subject.where(resolve() is Patient) | EpisodeOfCare.patient | FamilyMemberHistory.patient | Flag.subject.where(resolve() is Patient) | Goal.subject.where(resolve() is Patient) | ImagingStudy.subject.where(resolve() is Patient) | Immunization.patient | List.subject.where(resolve() is Patient) | MedicationAdministration.subject.where(resolve() is Patient) | MedicationDispense.subject.where(resolve() is Patient) | MedicationRequest.subject.where(resolve() is Patient) | MedicationUsage.subject.where(resolve() is Patient) | NutritionOrder.patient | Observation.subject.where(resolve() is Patient) | Procedure.subject.where(resolve() is Patient) | RiskAssessment.subject.where(resolve() is Patient) | ServiceRequest.subject.where(resolve() is Patient) | SupplyDelivery.patient | VisionPrescription.patient",
"target" : ["Patient",
      "Group",
      "Account",
      "ActivityDefinition",
      "AdministrableProductDefinition",
      "AdverseEvent",
      "AllergyIntolerance",
      "Appointment",
      "AppointmentResponse",
      "AuditEvent",
      "Basic",
      "Binary",
      "BiologicallyDerivedProduct",
      "BodyStructure",
      "Bundle",
      "CapabilityStatement",
      "CapabilityStatement2",
      "CarePlan",
      "CareTeam",
      "CatalogEntry",
      "ChargeItem",
      "ChargeItemDefinition",
      "Citation",
      "Claim",
      "ClaimResponse",
      "ClinicalImpression",
      "ClinicalUseIssue",
      "CodeSystem",
      "Communication",
      "CommunicationRequest",
      "CompartmentDefinition",
      "Composition",
      "ConceptMap",
      "Condition",
      "ConditionDefinition",
      "Consent",
      "Contract",
      "Coverage",
      "CoverageEligibilityRequest",
      "CoverageEligibilityResponse",
      "DetectedIssue",
      "Device",
      "DeviceDefinition",
      "DeviceMetric",
      "DeviceRequest",
      "DeviceUsage",
      "DiagnosticReport",
      "DocumentManifest",
      "DocumentReference",
      "Encounter",
      "Endpoint",
      "EnrollmentRequest",
      "EnrollmentResponse",
      "EpisodeOfCare",
      "EventDefinition",
      "Evidence",
      "EvidenceReport",
      "EvidenceVariable",
      "ExampleScenario",
      "ExplanationOfBenefit",
      "FamilyMemberHistory",
      "Flag",
      "Goal",
      "GraphDefinition",
      "GuidanceResponse",
      "HealthcareService",
      "ImagingStudy",
      "Immunization",
      "ImmunizationEvaluation",
      "ImmunizationRecommendation",
      "ImplementationGuide",
      "Ingredient",
      "InsurancePlan",
      "InventoryReport",
      "Invoice",
      "Library",
      "Linkage",
      "List",
      "Location",
      "ManufacturedItemDefinition",
      "Measure",
      "MeasureReport",
      "Medication",
      "MedicationAdministration",
      "MedicationDispense",
      "MedicationKnowledge",
      "MedicationRequest",
      "MedicationUsage",
      "MedicinalProductDefinition",
      "MessageDefinition",
      "MessageHeader",
      "MolecularSequence",
      "NamingSystem",
      "NutritionIntake",
      "NutritionOrder",
      "NutritionProduct",
      "Observation",
      "ObservationDefinition",
      "OperationDefinition",
      "OperationOutcome",
      "Organization",
      "OrganizationAffiliation",
      "PackagedProductDefinition",
      "PaymentNotice",
      "PaymentReconciliation",
      "Permission",
      "Person",
      "PlanDefinition",
      "Practitioner",
      "PractitionerRole",
      "Procedure",
      "Provenance",
      "Questionnaire",
      "QuestionnaireResponse",
      "RegulatedAuthorization",
      "RelatedPerson",
      "RequestGroup",
      "ResearchStudy",
      "ResearchSubject",
      "RiskAssessment",
      "Schedule",
      "SearchParameter",
      "ServiceRequest",
      "Slot",
      "Specimen",
      "SpecimenDefinition",
      "StructureDefinition",
      "StructureMap",
      "Subscription",
      "SubscriptionStatus",
      "SubscriptionTopic",
      "Substance",
      "SubstanceDefinition",
      "SubstanceNucleicAcid",
      "SubstancePolymer",
      "SubstanceProtein",
      "SubstanceReferenceInformation",
      "SubstanceSourceMaterial",
      "SupplyDelivery",
      "SupplyRequest",
      "Task",
      "TerminologyCapabilities",
      "TestReport",
      "TestScript",
      "ValueSet",
      "VerificationResult",
      "VisionPrescription"]

I'm not sure where those are all coming from but I'd argue that, given the section I just quoted above, it should have a single target of type Patient.

lmsurpre commented 3 years ago

It looks like there is an open issue for this that resolved to change it to just "Patient": https://jira.hl7.org/browse/FHIR-13601 Personally, I think we should list this as erratta under our Conformance.md documentation and change the published search parameter to have a single target.

tbieste commented 3 years ago

That seems reasonable to do.

lmsurpre commented 3 years ago

What is funny is that issue actually links to https://jira.hl7.org/browse/FHIR-15903 in which I raised this same issue. I wish I would have linked to that from here...

Anyway, I think it provides further evidence for modifying the search parameter that we ship in our registry. I'll leave my comments https://jira.hl7.org/browse/FHIR-13601 to hopefully get it addressed in FHIR R5 so that we don't need to patch the spec resource.

UPDATE: FHIR-13601 was closed without actually fixing the issue, so I've opened https://jira.hl7.org/browse/FHIR-32876

lmsurpre commented 3 years ago

I also opened https://jira.hl7.org/browse/FHIR-32305 for the same issue in the US Core search parameter definitions. Depending on the result of that discussion, we may want to patch those ones as well.

tbieste commented 3 years ago

Successfully verified with searches such as these: Observation?patient.gender=female Observation?patient.name=john

lmsurpre commented 3 years ago

I just found that http://hl7.org/fhir/SearchParameter/Provenance-patient has the same issue. I added this as a comment to https://jira.hl7.org/browse/FHIR-32876 and I will open a separate issue for it here as well so that we can patch it.