google / android-fhir

The Android FHIR SDK is a set of Kotlin libraries for building offline-capable, mobile-first healthcare applications using the HL7® FHIR® standard on Android.
https://google.github.io/android-fhir/
Apache License 2.0
497 stars 296 forks source link

SDC library, it does not set the answer to the definition field if the field type is choice type. #674

Closed santosh-pingle closed 3 years ago

santosh-pingle commented 3 years ago

Describe the bug Definition url is "definition": "http://hl7.org/fhir/StructureDefinition/Observation#Observation.value.value", Observation.value is abstract Type, it is declared as value property in Observation resource class. To update the answer to the definition field(Observation.value.value), it is required to create instances of fields present in the definition url.

"item": [
    {
      "text": "Temperature",
      "type": "group",
      "linkId": "5.0.0",
      "code": [
        {
          "code": "8310-5",
          "display": "Temperature",
          "system": "http://loinc.org"
        }
      ],
      "definition": "http://hl7.org/fhir/StructureDefinition/Observation",
      "extension": [
        {
          "url": "http://hl7.org/fhir/uv/sdc/StructureDefinition/sdc-questionnaire-itemExtractionContext",
          "valueExpression": {
            "language": "application/x-fhir-query",
            "expression": "Observation",
            "name": "temperature"
          }
        },
        {
          "url": "http://hl7.org/fhir/StructureDefinition/questionnaire-itemControl",
          "valueCodeableConcept": {
            "coding": [
              {
                "system": "http://hl7.org/fhir/questionnaire-item-control",
                "code": "page",
                "display": "Page"
              }
            ],
            "text": "Page"
          }
        }
      ],
      "item": [
        {
          "text": "Add instructions for capturing temperature",
          "type": "display",
          "linkId": "5.0.1"
        },
        {
          "type": "group",
          "definition": "http://hl7.org/fhir/StructureDefinition/Observation#Observation.value",
          "extension": [
            {
              "url": "http://hl7.org/fhir/uv/sdc/StructureDefinition/sdc-questionnaire-itemExtractionContext",
              "valueExpression": {
                "language": "application/x-fhir-query",
                "expression": "Quantity",
                "name": "quantity"
              }
            }
          ],
          "item": [
            {
              "definition": "http://hl7.org/fhir/StructureDefinition/Observation#Observation.value.value",
              "text": "Record temperature",
              "type": "decimal",
              "linkId": "5.1.0",
              "extension": [
                {
                  "url": "http://hl7.org/fhir/StructureDefinition/minValue",
                  "valueDecimal": 35.0
                },
                {
                  "url": "http://hl7.org/fhir/StructureDefinition/maxValue",
                  "valueDecimal": 45.0
                }
              ]
            },
            {
              "text": "Unit",

Component SDC library

To Reproduce Steps to reproduce the behavior:

  1. Go to '...'
  2. Click on '....'
  3. Scroll down to '....'
  4. See error

Expected behavior A clear and concise description of what you expected to happen.

Screenshots If applicable, add screenshots to help explain your problem.

Smartphone (please complete the following information):

Additional context Add any other context about the problem here.

Would you like to work on the issue? Please state if this issue should be assigned to you or who you think could help to solve this issue.

ekigamba commented 3 years ago

@santosh-pingle Can you print the error you are getting here?

santosh-pingle commented 3 years ago

@ekigamba Issue 1 : It was throwing an Instantiation exception java.lang.InstantiationException: java.lang.Class cannot be instantiated at java.lang.Class.newInstance(Native Method) at com.google.android.fhir.datacapture.mapping.ResourceMapper.extractField(ResourceMapper.kt:253) at com.google.android.fhir.datacapture.mapping.ResourceMapper.extractFields(ResourceMapper.kt:207) at com.google.android.fhir.datacapture.mapping.ResourceMapper.extractField(ResourceMapper.kt:233) at com.google.android.fhir.datacapture.mapping.ResourceMapper.extractFields(ResourceMapper.kt:207) at com.google.android.fhir.datacapture.mapping.ResourceMapper.extractByDefinitions(ResourceMapper.kt:116) at com.google.android.fhir.datacapture.mapping.ResourceMapper.extract(ResourceMapper.kt:92) at com.google.android.fhir.datacapture.mapping.ResourceMapper.extract$default(ResourceMapper.kt:89)

Below fix is added by considering expression value of extension which is declared in questionnaire json. https://github.com/google/android-fhir/pull/675/files#diff-4d2985aba5e8203b97cc5dc2eac7dbbd0c7832be34a27036d1ecfb8e4a3a3e50R261

Issue 2 Now, definition field returns null for nested field on abstract type and does not update the answer.

https://github.com/google/android-fhir/pull/675/files#diff-4d2985aba5e8203b97cc5dc2eac7dbbd0c7832be34a27036d1ecfb8e4a3a3e50R451 https://github.com/google/android-fhir/pull/675/files#diff-4d2985aba5e8203b97cc5dc2eac7dbbd0c7832be34a27036d1ecfb8e4a3a3e50R451

santosh-pingle commented 3 years ago

@epicadk

Could you please give me more insights on " I think maybe you could update the title to .... is choice type"

ekigamba commented 3 years ago

@santosh-pingle This seems to be an edge-case that we were not aware of. Is this the standard for properties of value for all Resources? If so, you can create a generic solution that will fix this. The only issue is that the specification clearly states that you should have valueDecimal and not just value as the property. However, I can see the ~HL7~ HAPI models only have value and uses the abstract Type to hold all other types

santosh-pingle commented 3 years ago

@ekigamba I am not sure about the standard for properties of value for all Resources, but its declared as value field/property name for QuantityValue data type and many more (CodeableConcept, StringType ..) in Observation resource. Therefore, definition url is declared as Observation.value in questionnaire and not as Observation.quantityValue.

User input value for temperature is required to be updated as Quantity.value, which is Observation.quantityValue.value as per specification and Observation.value.value as per hl7.fhir.r4.model model https://hl7.org/fhir/R4/bodytemp.html

ekigamba commented 3 years ago

@santosh-pingle The above is mostly correct. However, as per the spec if the property is described as value[X], the actual property is either of the listed sub-items such as valueString, valueDateTime, valueQuantity as seen in this QuestionnaireResponse example https://www.hl7.org/fhir/questionnaireresponse-example-f201-lifelines.json.html. The HAPI models don't follow this. I guess what I mean is that Authoring a questionnaire for definition-based extraction requires following the FHIR models/data-types and not the HAPI models. The current implementation infers the Type from the HAPI Model assuming that the models strictly follow the FHIR specification, in cases whereby this isn't the case, we need to handle that separately. I think that this is such a case and you can come up with a generic fix in cases where the inferred data-type is Type and the field starts with value

For the other errors above, I see that this assumes multiple resource generation for definition-based extraction was implemented. I am not sure of the current state but we had an issue tracking this here https://github.com/google/android-fhir/issues/504

epicadk commented 3 years ago

@epicadk

Could you please give me more insights on " I think maybe you could update the title to .... is choice type"

Sorry, I wasn't really sure so I wanted to research a bit before commenting. Anyways what I wanted to say was that Quantity.value is a choice type so maybe an appropriate title for the issue would be

SDC library, it does not set the answer to the definition field if the field type is choice type

because you'll face the same issue for other choice type fields (like Extension.Value)

refer http://www.hl7.org/fhir/formats.html#choice list of all choice types http://www.hl7.org/fhir/choice-elements.json

In hapi the Type type is only used for choice types, as the org.hl7.fhir.r4.model.Type javdoc says

An element that is a type. Used when the element type in FHIR is a choice of more than one data type

refer https://github.com/hapifhir/org.hl7.fhir.core/blob/99fcac64ac35ca7dc3a4aa9922684ac7fdb0ee7b/org.hl7.fhir.r4/src/main/java/org/hl7/fhir/r4/model/Type.java#L39

santosh-pingle commented 3 years ago

@epicadk Could you please give me more insights on " I think maybe you could update the title to .... is choice type"

Sorry, I wasn't really sure so I wanted to research a bit before commenting. Anyways what I wanted to say was that Quantity.value is a choice type so maybe an appropriate title for the issue would be

SDC library, it does not set the answer to the definition field if the field type is choice type

because you'll face the same issue for other choice type fields (like Extension.Value)

refer http://www.hl7.org/fhir/formats.html#choice list of all choice types http://www.hl7.org/fhir/choice-elements.json

In hapi the Type type is only used for choice types, as the org.hl7.fhir.r4.model.Type javdoc says

An element that is a type. Used when the element type in FHIR is a choice of more than one data type

refer https://github.com/hapifhir/org.hl7.fhir.core/blob/99fcac64ac35ca7dc3a4aa9922684ac7fdb0ee7b/org.hl7.fhir.r4/src/main/java/org/hl7/fhir/r4/model/Type.java#L39

Sure, I have updated title with choice type.

santosh-pingle commented 3 years ago

@ekigamba

@santosh-pingle The above is mostly correct. However, as per the spec if the property is described as value[X], the actual property is either of the listed sub-items such as valueString, valueDateTime, valueQuantity as seen in this QuestionnaireResponse example https://www.hl7.org/fhir/questionnaireresponse-example-f201-lifelines.json.html. The HAPI models don't follow this. I guess what I mean is that Authoring a questionnaire for definition-based extraction requires following the FHIR models/data-types and not the HAPI models. The current implementation infers the Type from the HAPI Model assuming that the models strictly follow the FHIR specification, in cases whereby this isn't the case, we need to handle that separately. I think that this is such a case and you can come up with a generic fix in cases where the inferred data-type is Type and the field starts with value

For the other errors above, I see that this assumes multiple resource generation for definition-based extraction was implemented. I am not sure of the current state but we had an issue tracking this here #504

Yes, FHIR model data types need to be considered in definition based extraction while fixing this issue with generic approach.

santosh-pingle commented 3 years ago

Working on it.

santosh-pingle commented 3 years ago

I will try to create PR by eod today to unblock #625