Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add safe call in validateQuestionnaireResponseItems to handle custom widget #936

Closed
FikriMilano opened this issue Nov 23, 2021 · 6 comments · Fixed by #937
Closed

Add safe call in validateQuestionnaireResponseItems to handle custom widget #936

FikriMilano opened this issue Nov 23, 2021 · 6 comments · Fixed by #937
Assignees
Labels
effort:xsmall Extra small effort - 1 day P3 Low priority issue type:enhancement New feature or request

Comments

@FikriMilano
Copy link
Collaborator

FikriMilano commented Nov 23, 2021

Is your feature request related to a problem? Please describe.
When validating a custom questionnaire widget which has null item type it will throw an NPE.
For example:

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

Related problem: opensrp/fhircore#761

Describe the solution you'd like
Add safe call on this part

if (questionnaireItem.type.equals(Questionnaire.QuestionnaireItemType.GROUP)) {

Describe alternatives you've considered
N/A

Additional context
N/A

Would you like to work on the issue?
Yes

@FikriMilano FikriMilano added the type:enhancement New feature or request label Nov 23, 2021
@FikriMilano FikriMilano changed the title Add null check in validateQuestionnaireResponse to handle custom widget Add null check in validateQuestionnaireResponseItems to handle custom widget Nov 23, 2021
@FikriMilano FikriMilano changed the title Add null check in validateQuestionnaireResponseItems to handle custom widget Add safe call in validateQuestionnaireResponseItems to handle custom widget Nov 23, 2021
@jingtang10
Copy link
Collaborator

this is not a valid questionnaire item. see https://www.hl7.org/fhir/questionnaire-definitions.html#Questionnaire.item.type. type is mandatory.

@ekigamba
Copy link
Contributor

  1. That's correct, no such question type exists on the spec. Could we swap the check to prevent NPEs?

i.e.

if (Questionnaire.QuestionnaireItemType.GROUP.equals(questionnaireItem.type)) { 
  1. Support for custom widget types was added and it seems that this is how such a question is authored. The link is for the example on the Android FHIR SDK datacapturegallery

https://github.com/google/android-fhir/blob/master/datacapturegallery/src/main/assets/hl7-questionnaire-example-f201-lifelines.json#L52..L60 . Based on the issue raised, the feature seems to have broken due to this check. I tested this on FHIR Core and it causes a crash. However, this doesn't crash on datacapturegallery because the validation check only happens when you provide a QuestionnaireResponse to the QuestionnaireFragment. I guess we should have this issue filed as a bug and see if we can get this solution or any other solution accepted? @jingtang10

@joiskash
Copy link
Collaborator

joiskash commented Nov 24, 2021

I think according to the spec we should be using the attachment type for storing images as answers.

https://www.hl7.org/fhir/codesystem-item-type.html#item-type-attachment

We need to fix that questionnaire in the repo and put the right type. Since it is a number picker, the right type is probably integer.

@FikriMilano
Copy link
Collaborator Author

For giving example of questionnaires that has custom widget in it.
Have you considered using both valid item type and extension so that it will still show the correct custom widget while also preventing this NPE error?
@jingtang10 @joiskash

@joiskash
Copy link
Collaborator

Well Im not sure if its the responsibility of the datacapture library to check if questionnaire item has a type or not for custom widgets. I am surprised that HAPI FHIR is not throwing an error due to missing type, since that is the library that we are using to parse the questionnaire JSON.

In the case of canonical widgets , if item.type is missing , the datacapture library throws a kotlin.NotImplementedError: Question type null not supported. . In the case of custom widget , we can only have documentation and sample code in datacapturegallery recommending developers to check the validity of the questionnaire item since the implementation of the custom widget factory is in the hands of the developer.

FYI I am working on improving the function validateQuestionnaireResponseItems in PR #926

Also using tools like https://lhcformbuilder.nlm.nih.gov/ helps author valid questionnaires.

@jingtang10
Copy link
Collaborator

To summarise our discussion today, every questionnaire item needs to have a type. Custom widget doesn't mean custom data type. So you could define a custom widget to collect data in a custom way (for example, you might tap the screen to increment an integer and collect that integer rather than typing in an integer) but the answer you collect still needs to be one of the defined fhir questionnaire item data types.

With this clarified, we still have the question of how we should deal with errors like this. We already have the validation API which @joiskash worked on. We probably should consider providing developers with a more consistent experience (for example, but not necessarily this, making a checked exception).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort:xsmall Extra small effort - 1 day P3 Low priority issue type:enhancement New feature or request
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants