cht-core icon indicating copy to clipboard operation
cht-core copied to clipboard

JSON forms with nested fields errors in the UI

Open garethbowen opened this issue 4 years ago • 6 comments

Describe the bug We have an old feature to allow JSON forms to have nested fields which no longer works as expected. This has been broken for some time and nobody has reported it so I'm leaning towards removing it.

Determine if this feature is still needed and either fix it and document it. or remove it.

To Reproduce Steps to reproduce the behavior:

  1. Configure a JSON with a field key with dots to signify nested fields.
      "NESTED": {
        "meta": {
          "code": "NESTED",
          "icon": "",
          "translation_key": "nested"
        },
        "fields": {
          "some.nested.field": {
            "labels": {
              "short": {
                "translation_key": "nested field"
              },
              "tiny": {
                "en": "nested"
              }
            },
            "position": 1,
            "flags": {},
            "type": "string",
            "required": false
          }
        },
        "use_sentinel": true
      },
  1. Go to App Management > SMS > Test Message
  2. Send a message using the new form, eg: NESTED hi
  3. To to Application > Reports and select the new report
  4. The report doesn't get selected and there is an error in the console.

Expected behavior The report should show without errors. If the feature is to be kept then the fields should be indented just like we do for Enketo reports.

Logs

angular.js:15567 Error selecting report TypeError: Cannot read property 'fields' of undefined
    at fieldsToHtml (format-data-record.js:77)
    at format-data-record.js:83
    at core.js:1052
    at Function.forEach (core.js:2012)
    at fieldsToHtml (format-data-record.js:79)
    at formatJsonFields (format-data-record.js:491)
    at makeDataRecordReadable (format-data-record.js:639)
    at format-data-record.js:673
    at processQueue (angular.js:17945)
    at angular.js:17993

Environment

  • Instance: localhost
  • Browser: Firefox, Chrome
  • Client platform: Linux
  • App: webapp
  • Version: master (3.9.0)

Additional context This PR fixes it a little so instead of erroring it just doesn't show the field at all.

garethbowen avatar Apr 22 '20 22:04 garethbowen

@abbyad Do you know if this feature is still useful?

garethbowen avatar Apr 22 '20 22:04 garethbowen

I don't remember ever using this, and to be honest, I don't know if I ever knew this existed!

@derickl do you know of this being used anywhere? We could search all existing configs for dot notation in the form names to see if we are using it.

Even if it wasn't in use, this does seem like a useful feature since JSON forms are now being used for integrations. We could accept more structured data via the reports API, which would then display nested data indented nicely in the reports tab as opposed to flatly all on the same level. @michaelkohn aside from the UI side, does this have any benefit when it comes to interoperability with other tools or FHIR?

abbyad avatar Apr 22 '20 23:04 abbyad

For what it's worth, this is the way the fields property looks when you submit this message:

  "fields": {
    "some": {
      "nested": {
        "field": "hi"
      }
    }
  },

garethbowen avatar Apr 23 '20 00:04 garethbowen

When I did the SIH work this was I think the first time we'd really touched JSON forms in a long time, and it wasn't pretty. I feel like this is a pretty neglected part of our codebase with a bunch of wild ideas from a long time ago, and not a particularly clear vision.

It might be worth us revising this from a higher level, looking at what else is out there in our space and learning from that. As this is currently the primary way data comes into our system from external sources this will presumably become more and more important as our integrations become more numerous and complex.

SCdF avatar Apr 23 '20 08:04 SCdF

To clarify, with the fix we can use nested fields, but any such field won't be displayed in the UI. If that's correct, we have the following options:

  1. Leave it as is, and nested fields are automatically hidden
  2. Remove the option to nest fields for JSON reports
  3. In the report view show nested fields in JSON reports without indenting
  4. In the report view show nested fields in JSON reports with proper indenting

Would be good to see if this would be used to make a decision on the option, and the priority.

abbyad avatar Apr 24 '20 19:04 abbyad

Testing on latest version (3.15~ish), the frontend code doesn't throw an error anymore. However the field is still not displayed: image

dianabarsan avatar Apr 29 '22 13:04 dianabarsan