opensrp / fhircore

FHIR Core / OpenSRP 2 is a Kotlin application for delivering offline-capable, mobile-first healthcare project implementations from local community to national and international scale using FHIR and WHO Smart Guidelines on Android.
https://smartregister.org
Apache License 2.0
54 stars 41 forks source link

Quest crashes when it loads photo capture widget #761

Closed FikriMilano closed 2 years ago

FikriMilano commented 2 years ago

Describe the bug Quest app crashes when it tries to load a questionnaire from HAPI with photo capture widget in it.

To Reproduce Steps to reproduce the behavior:

  1. Go to Quest app
  2. Login
  3. Click on one of the clients
  4. Click on 'G6PD TEST PHOTO RESULT'
  5. Wait and then crash

Expected behavior Quest app should not crash and successfully loads the questionnaire

Screenshots

Smartphone (please complete the following information):

Additional context

2021-11-18 14:53:21.390 21398-21398/org.smartregister.fhircore.quest E/AndroidRuntime: FATAL EXCEPTION: main
    Process: org.smartregister.fhircore.quest, PID: 21398
    java.lang.RuntimeException: An exception happened in constructor of class com.google.android.fhir.datacapture.QuestionnaireViewModel
        at androidx.lifecycle.SavedStateViewModelFactory.create(SavedStateViewModelFactory.java:132)
        at androidx.lifecycle.ViewModelProvider.get(ViewModelProvider.java:185)
        at androidx.lifecycle.ViewModelProvider.get(ViewModelProvider.java:150)
        at androidx.lifecycle.ViewModelLazy.getValue(ViewModelProvider.kt:54)
        at androidx.lifecycle.ViewModelLazy.getValue(ViewModelProvider.kt:41)
        at com.google.android.fhir.datacapture.QuestionnaireFragment.getViewModel(QuestionnaireFragment.kt:35)
        at com.google.android.fhir.datacapture.QuestionnaireFragment.access$getViewModel(QuestionnaireFragment.kt:34)
        at com.google.android.fhir.datacapture.QuestionnaireFragment$onViewCreated$3.invokeSuspend(QuestionnaireFragment.kt:71)
        at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
        at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:106)
        at androidx.lifecycle.DispatchQueue.drainQueue(DispatchQueue.kt:75)
        at androidx.lifecycle.DispatchQueue.resume(DispatchQueue.kt:54)
        at androidx.lifecycle.LifecycleController$observer$1.onStateChanged(LifecycleController.kt:40)
        at androidx.lifecycle.LifecycleRegistry$ObserverWithState.dispatchEvent(LifecycleRegistry.java:354)
        at androidx.lifecycle.LifecycleRegistry.forwardPass(LifecycleRegistry.java:265)
        at androidx.lifecycle.LifecycleRegistry.sync(LifecycleRegistry.java:307)
        at androidx.lifecycle.LifecycleRegistry.moveToState(LifecycleRegistry.java:148)
        at androidx.lifecycle.LifecycleRegistry.handleLifecycleEvent(LifecycleRegistry.java:134)
        at androidx.fragment.app.FragmentViewLifecycleOwner.handleLifecycleEvent(FragmentViewLifecycleOwner.java:88)
        at androidx.fragment.app.Fragment.restoreViewState(Fragment.java:653)
        at androidx.fragment.app.Fragment.restoreViewState(Fragment.java:3010)
        at androidx.fragment.app.Fragment.performActivityCreated(Fragment.java:3001)
        at androidx.fragment.app.FragmentStateManager.activityCreated(FragmentStateManager.java:580)
        at androidx.fragment.app.FragmentStateManager.moveToExpectedState(FragmentStateManager.java:285)
        at androidx.fragment.app.FragmentManager.executeOpsTogether(FragmentManager.java:2189)
        at androidx.fragment.app.FragmentManager.removeRedundantOperationsAndExecute(FragmentManager.java:2100)
        at androidx.fragment.app.FragmentManager.execPendingActions(FragmentManager.java:2002)
        at androidx.fragment.app.FragmentManager$5.run(FragmentManager.java:524)
        at android.os.Handler.handleCallback(Handler.java:938)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loop(Looper.java:223)
        at android.app.ActivityThread.main(ActivityThread.java:7656)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:592)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:947)
     Caused by: java.lang.NullPointerException: Attempt to invoke virtual method 'boolean org.hl7.fhir.r4.model.Questionnaire$QuestionnaireItemType.equals(java.lang.Object)' on a null object reference
        at com.google.android.fhir.datacapture.QuestionnaireViewModel.validateQuestionnaireResponseItems(QuestionnaireViewModel.kt:320)
        at com.google.android.fhir.datacapture.QuestionnaireViewModel.<init>(QuestionnaireViewModel.kt:56)
        at java.lang.reflect.Constructor.newInstance0(Native Method)
        at java.lang.reflect.Constructor.newInstance(Constructor.java:343)
        at androidx.lifecycle.SavedStateViewModelFactory.create(SavedStateViewModelFactory.java:122)
        at androidx.lifecycle.ViewModelProvider.get(ViewModelProvider.java:185) 
        at androidx.lifecycle.ViewModelProvider.get(ViewModelProvider.java:150) 
        at androidx.lifecycle.ViewModelLazy.getValue(ViewModelProvider.kt:54) 
        at androidx.lifecycle.ViewModelLazy.getValue(ViewModelProvider.kt:41) 
        at com.google.android.fhir.datacapture.QuestionnaireFragment.getViewModel(QuestionnaireFragment.kt:35) 
        at com.google.android.fhir.datacapture.QuestionnaireFragment.access$getViewModel(QuestionnaireFragment.kt:34) 
        at com.google.android.fhir.datacapture.QuestionnaireFragment$onViewCreated$3.invokeSuspend(QuestionnaireFragment.kt:71) 
        at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33) 
        at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:106) 
        at androidx.lifecycle.DispatchQueue.drainQueue(DispatchQueue.kt:75) 
        at androidx.lifecycle.DispatchQueue.resume(DispatchQueue.kt:54) 
        at androidx.lifecycle.LifecycleController$observer$1.onStateChanged(LifecycleController.kt:40) 
        at androidx.lifecycle.LifecycleRegistry$ObserverWithState.dispatchEvent(LifecycleRegistry.java:354) 
        at androidx.lifecycle.LifecycleRegistry.forwardPass(LifecycleRegistry.java:265) 
        at androidx.lifecycle.LifecycleRegistry.sync(LifecycleRegistry.java:307) 
        at androidx.lifecycle.LifecycleRegistry.moveToState(LifecycleRegistry.java:148) 
        at androidx.lifecycle.LifecycleRegistry.handleLifecycleEvent(LifecycleRegistry.java:134) 
        at androidx.fragment.app.FragmentViewLifecycleOwner.handleLifecycleEvent(FragmentViewLifecycleOwner.java:88) 
        at androidx.fragment.app.Fragment.restoreViewState(Fragment.java:653) 
        at androidx.fragment.app.Fragment.restoreViewState(Fragment.java:3010) 
        at androidx.fragment.app.Fragment.performActivityCreated(Fragment.java:3001) 
        at androidx.fragment.app.FragmentStateManager.activityCreated(FragmentStateManager.java:580) 
        at androidx.fragment.app.FragmentStateManager.moveToExpectedState(FragmentStateManager.java:285) 
        at androidx.fragment.app.FragmentManager.executeOpsTogether(FragmentManager.java:2189) 
        at androidx.fragment.app.FragmentManager.removeRedundantOperationsAndExecute(FragmentManager.java:2100) 
        at androidx.fragment.app.FragmentManager.execPendingActions(FragmentManager.java:2002) 
        at androidx.fragment.app.FragmentManager$5.run(FragmentManager.java:524) 
        at android.os.Handler.handleCallback(Handler.java:938) 
        at android.os.Handler.dispatchMessage(Handler.java:99) 
        at android.os.Looper.loop(Looper.java:223) 
        at android.app.ActivityThread.main(ActivityThread.java:7656) 
        at java.lang.reflect.Method.invoke(Native Method) 
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:592) 
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:947) 
FikriMilano commented 2 years ago

Some updates on this.

When a given questionnaire item tries to load:

    {
      "extension": [
        {
          "url": "http://doc-of-photo-capture",
          "valueString": "photo-capture"
        }
      ],
      "linkId": "photo",
      "text": "Photo of device",
      "enableWhen": [
        {
          "question": "result_type",
          "operator": "=",
          "answerCoding": {
            "system": "https://www.snomed.org",
            "code": "410680006"
          }
        },
        {
          "question": "result_type",
          "operator": "=",
          "answerCoding": {
            "system": "https://www.snomed.org",
            "code": "385432009"
          }
        }
      ],
      "enableBehavior": "any"
    }

It will throw an NPE because the QuestionnaireViewModel on data capture library tries to validate it's type. Which for a custom widget indeed so supposed to be null. https://github.com/google/android-fhir/blob/5c879cc94f3fd64e36b3cad540552c2004a483a0/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt#L346

@f-odhiambo @ekigamba

FikriMilano commented 2 years ago

So, in order to fix this:

FikriMilano commented 2 years ago

@f-odhiambo For the null check which is on datacapture library, where should I make the changes? on Android FHIR SDK or the preview version that FHIRCore use?

pld commented 2 years ago

@f-odhiambo For the null check which is on datacapture library, where should I make the changes? on Android FHIR SDK or the preview version that FHIRCore use?

Is this a general issue w/the datacapture library? If so, you can do so in the google/android-fhir repo, we can cut a release from the branch and use that if needed before they merge it to main

ekigamba commented 2 years ago

@FikriMilano What happens if we set a type to prevent the NPE?

FikriMilano commented 2 years ago

Is this a general issue w/the datacapture library? If so, you can do so in the google/android-fhir repo, we can cut a release from the branch and use that if needed before they merge it to main

Okay

FikriMilano commented 2 years ago

@FikriMilano What happens if we set a type to prevent the NPE?

It will throw this error Caused by: ca.uhn.fhir.parser.DataFormatException: [element="type"] Invalid attribute value "photo-capture": Unknown QuestionnaireItemType code 'photo-capture'

ekigamba commented 2 years ago

I meant setting a valid type eg. string. Does it show a text input or the photo capture widget?

FikriMilano commented 2 years ago

It works, it shows the photo capture widget :D But we still need this extension:

"extension": [
        {
          "url": "http://doc-of-photo-capture",
          "valueString": "photo-capture"
        }
]

I think this approach is a good choice, combining type and extension to get the result that we wanted.

Should the type be string or attachment? because we expect the response to be valueString according with the previous agreement with @pld

ekigamba commented 2 years ago

@FikriMilano It's a workaround while we try and get this fixed on the Android FHIR SDK. Ideally, this should have been attachment but I don't know the impact of having it as string or attachment while string defining a custom widget extension. Test it out and see :smile:

FikriMilano commented 2 years ago

Yeah I did test it, works fine either string or attachment.

We could use attachment then response answer with valueString. Since we want the simplicity over valueAttachment. But not sure if this is a good practice or will cause problems on HAPI or FHIR Web because it's not following the spec. What do you think? Should we go with valueAttachment instead?

FYI i think the SDK also plans to update their custom questionnaire sample. https://github.com/google/android-fhir/issues/936#issuecomment-977481518 https://github.com/google/android-fhir/pull/937#discussion_r755986493

ekigamba commented 2 years ago

Yes, I think valueAttachment follows the spec better. To test if this can be uploaded without an issue, you can create a sample QuestionnaieResponse from this Questionnaire and see if the server rejects it due to the valueString with base64.

On the FHIR SDK issue discussion, I think we might not be on the same page regarding how custom widget extensions should be authored. See if you can get clarity on this during the call/issue

FikriMilano commented 2 years ago

Alright, let's use valueAttachment for now since using valueString seems to not make sense if the type is attachment