medic / cht-core

The CHT Core Framework makes it faster to build responsive, offline-first digital health apps that equip health workers to provide better care in their communities. It is a central resource of the Community Health Toolkit.
https://communityhealthtoolkit.org
GNU Affero General Public License v3.0
441 stars 211 forks source link

Enable registration of a person from a report/action form #2912

Closed sglangevin closed 7 years ago

sglangevin commented 7 years ago

Currently, people can only be registered using a contact form. There are some workflows where it would be helpful to be able to register a new person using a report/action form.

For example, when a CHW does a postnatal care visit for a mother, this form collects information about the baby. Ideally, the postnatal care visit form would, upon submission, create a new person in the family. This should be configurable so that any report/action form can potentially register a person.

garethbowen commented 7 years ago

Is it ok to create the person only when the data is synced to the server? ie: in sentinel not on the phone.

sglangevin commented 7 years ago

If it's done that way, could we still create a set of follow-up tasks for the CHW who submitted the report based on that person's birthdate?

garethbowen commented 7 years ago

Yes once the person is created on the server they would replicate down to the CHP and then tasks would be generated as per usual. The only problem is those tasks won't show up right away if the CHP is offline.

abbyad commented 7 years ago

I think we'd want this workflow to work while a person is offline, especially since postnatal workflows are very time sensitive. A followup may be needed day of or next day, and it is rather likely that they will be offline in the time period.

sglangevin commented 7 years ago

Yeah, I agree with Marc - offline would be preferred.

garethbowen commented 7 years ago

Can't blame a guy for trying...

ranjuts commented 7 years ago

Another user story for this feature -- Confirmation of a delivery/birth is usually followed by a PNC and/or immunization workflow which requires registering the child (children if multiple). Automatically creating the child reduces a task for the CHW, also ensures her likelihood to timely report on any events concerning the child that happens at or immediately after birth (eg. birth polio vaccination). ++1 for needing this to work offline.

abbyad commented 7 years ago

@sglangevin does the person need to be created directly by the action form, or is it sufficient to be able to create a person from a task? If we could open and submit a create person form from a task then we can send all the necessary info via the task to make the person creation very streamlined.

Doing it this way does require a separate task but this could be clearer to a user than creating people silently in the background. Also, via tasks would likely be quicker to implement on the dev side, as well as be more familiar on the configuration side since without the need to introduce a new notation to create people from action forms. If we do in fact want to create people from action forms we need to design that for this issue.

sglangevin commented 7 years ago

Ideally, the person is created directly from the action form as opposed to via a task. The task helps, but it still requires an extra step for the CHW. PNC and immunizations workflows begin immediately after birth, so creating the person at the time the delivery is reported is important.

We had discussed the option of having this as a task, but there was a strong preference for having it be automatic to make things a lot easier for the CHP.

abbyad commented 7 years ago

Ok, so to do that we'll need to figure out how to specify that a form should create a person.

An option would be to create a person with the info from any group named person, or for groups that have a specific appearance value, such as new_person. @sglangevin do you have any preferences or other suggestions?

sglangevin commented 7 years ago

I think either of those would work. I know that in family registration (clinic form) we are using a group named person, so it might make sense to have a similar format for creating a person from a report form. The appearance value could also work well, though it might be easiest to stay consistent with the family/household registration form.

SCdF commented 7 years ago

Currently with families we have a special element in the xform that we look for, and when we find it we take the elements out of it and save them as individual docs. We could just do that here. The downside of this approach though, is that the "inner" form must be copied into the form that contains it. This means if you embed the create person form inside the create family form, if you update the create person form you have to do it in two places.

Here is another idea for discussion.

Instead of storing entire documents in the xform to be pulled out and saved, we instead store the name of the real form (eg create person) as well as the initial input data (eg the parent). Then when the user saves the report those are pulled out, and the user is redirected to fill out those forms correctly using the standard outer form. You could even queue multiple up by converting more than one into one form whereby the inputs contained the later forms (and so when you hit save the second time you would again be redirected back into Enketo).

Here is an example to show how it could work in comparison to the current saving of families:

SCdF commented 7 years ago

This idea has some cons though, so here is another idea to think about.

The con here, is that if you have a family with a child in it, and then you save the family and then get distracted / the app crashes etc, you will not get prompted to create the child (as opposed to what happens now, where the save happens at the same time).

You could solve this by holding saves somewhere or something. Or, you could reflect the relationship between a parent and linked documents in rules, so that if the linked documents don't get filled out you get a task. For example, you could have some rule that if there is a new family and it's supposed to have N kids, create N tasks to have a kid in that family.

This is, ironically, duplicating logic again, just like what we're doing now with forms.

This makes me wonder if we could somehow represent linked documents in rules config. Something like:

I have no idea how you'd a) watch the nools engine synchronously or b) deal with multiple tasks, though b is certainly easier to think about than a.

sglangevin commented 7 years ago

@SCdF I like the idea of linked forms. I'm wondering if we could do that in a similar way to how we are currently saving the add family form (which has children within it). What if instead of hitting "Save" at the end of the first form, you just hit "Next" and started the second form. Then you'd hit "Save" at the end of the second form and it would save everything at the same time.

The other thing to consider is that this issue is talking about linking an action/report form to a contact form and this could be configured within the action/report form so that we could potentially render both forms from the start as opposed to looking at the linked form when the first form is saved.

SCdF commented 7 years ago

Talked to @garethbowen . For now for simplicity we're going to go with duplicating how it works with families + children. We could also look into changing form uploads in the future to support "inlining" sub forms, i.e. you wouldn't have to duplicate the child form and have potential errors.

sglangevin commented 7 years ago

Seems like this one is pretty much ready to be implemented. It's something that LG has been waiting on for quite some time and is also needed for death reporting, which is being done in our community based disease surveillance project in Uganda. Sometimes we will have CHWs reporting deaths of people who were not previously registered, so ideally they could create the person and report the death from within the same form.

Any takers on this one? I'd like to get this into the next feature release.

alxndrsn commented 7 years ago

Proposal: process the model so that anything with the db-doc attribute set true will create a new doc in the db.

N.B. will need to check of attributes can be set conveniently from XLS.

TODO:

alxndrsn commented 7 years ago

@sglangevin this is implemented. I'd like to walk you through the feature and check that it fulfills your needs.

sglangevin commented 7 years ago

Sounds good - can we choose a time to meet? I'll ping you on slack.

alxndrsn commented 7 years ago

Extensions to the form Model:

Extra docs

Example Form Model

<data>
  <root_prop_1>val A</root_prop_1>
  <other_doc db-doc="true">
    <type>whatever</type>
    <other_prop>val B</other_prop>
  </other_doc>
</data>

Resulting docs

Report (as before):

{
  _id: '...',
  _rev: '...',
  type: 'report',
  _attachments: { xml: ... ],
  fields: {
    root_prop_1: 'val A',
  }
}

Other doc:

{
  _id: '...',
  _rev: '...',
  type: 'whatever',
  other_prop: 'val B',
}

Linked docs

Example Form Model

<sickness>
  <sufferer db-doc-ref="/sickness/new">
  <new db-doc="true">
    <type>person</type>
    <name>Gómez</name>
    <original_report db-doc-ref="/sickness"/>
  </new>
</sickness>

Resulting docs

Report:

{
  _id: 'abc-123',
  _rev: '...',
  type: 'report',
  _attachments: { xml: ... ],
  fields: {
    sufferer: 'def-456',
  }
}

Other doc:

{
  _id: 'def-456',
  _rev: '...',
  type: 'person',
  name: 'Gómez',
  original_report: 'abc-123',
}

Repeated docs

Example Form

<thing>
  <name>Ab</name>
  <related db-doc="true">
    <type>relative</type>
    <name>Bo</name>
    <parent db-doc-ref="/thing"/>
  </related>
  <related db-doc="true">
    <type>relative</type>
    <name>Ca</name>
    <parent db-doc-ref="/thing"/>
  </related>
</artist>

Resulting docs

Report:

{
  _id: 'abc-123',
  _rev: '...',
  type: 'report',
  _attachments: { xml: ... ],
  fields: {
    name: 'Ab',
  }
}

Other docs:

{
  _id: '...',
  _rev: '...',
  type: 'relative',
  name: 'Bo',
  parent: 'abc-123',
}
{
  _id: '...',
  _rev: '...',
  type: 'relative',
  name: 'Ch',
  parent: 'abc-123',
}

@sglangevin Does this fulfill your needs? Will it be possible to configure using these attributes via XLSForm?

alxndrsn commented 7 years ago

Things to note at code review

SCdF commented 7 years ago

all docs are now assigned IDs by the client, and saved using db.put() instead of db.post()

@alxndrsn I believe this will break logic in places, because we tend to use "doesn't have an _id" as shorthand for "it's a new report". For example, what used to be persist() and is now prepare() in contacts-edit.js:

    function prepare(doc) {
      if(!doc._id) {
        doc.reported_date = Date.now();
        doc._id = uuidV4();
      }

      return doc;
    }

There may be more, this is just the first one that comes to mind.

alxndrsn commented 7 years ago

TODO

alxndrsn commented 7 years ago

@SCdF

I believe this will break logic in places, because we tend to use "doesn't have an _id" as shorthand for "it's a new report"

This is a shame. "Doesn't have a _rev" is a better metric to determine that.

SCdF commented 7 years ago

@alxndrsn yeah almost certainly. And it might just be that one place I noted. Certainly for the code you're changing. I just didn't look into it any further.

sglangevin commented 7 years ago

@alxndrsn from what I am understanding, this fulfills the need. And actually, the fact that we can reference the original doc in the resulting docs is super helpful so I'm glad you added that.

I'm not 100% sure that I understand the Repeated Docs section above:

these currently cannot be linked from other docs, as no provision is made for indexing these docs

^^What does that part mean? Also, what is the difference between linked docs and repeated docs?

Will it be possible to configure using these attributes via XLSForm?

Probably. But I haven't tried yet so I'm not sure if we will run into issues with our conversion script or not. We can figure that out once this is ready to be tested and then see if we need to make any modifications either here or to the script.

alxndrsn commented 7 years ago

what is the difference between linked docs and repeated docs?

Repeated docs can also be linked docs, but don't have to be. With this PR, you can create multiple docs from a single report which optionally are inter-related. However, it's currently not possible to relate to docs created in a repeat section, because the syntax will be more complicated, and I didn't see any point implementing it unless it's wanted.

This means that if e.g. you created multiple children in a "family assessment report", each child could have a reference to the original report, but the report could not have a reference to any of the children.

alxndrsn commented 7 years ago

I think once merged this PR will allow for deprecation of legacy approaches to creating docs from a report. Separate tickets should be created for deprecation. This should involve:

  1. warning at form upload that a deprecated configuration style is being used
  2. creating a new ticket to be triggered after a certain time, to remove the old approach
alxndrsn commented 7 years ago

@sglangevin ready for testing cc @estellecomment

alxndrsn commented 7 years ago

It might be better to make each extra doc as a separate <instance>...

alxndrsn commented 7 years ago

...and work out how to do this in an XLS form.

sglangevin commented 7 years ago

@alxndrsn have you had time to look at this? I will try to make some time to see which parts I'm able to do in xlsform, probably next week.

alxndrsn commented 7 years ago

I'll work on this with @abbyad next week.

abbyad commented 7 years ago

The following XLSForm generates the proper XForm as described above: image

Resulting XForm snippet:

            <child_doc db-doc-ref=" /postnatal_care/group_delivery_summary/child "/>
            <child db-doc="true">
              <created_by_doc db-doc-ref="/postnatal_care"/>
              <dob/>
              <parent_id/>
            </child>
sglangevin commented 7 years ago

@abbyad does that mean that this can be set back to AT?

garethbowen commented 7 years ago

@alxndrsn It looks like all code has been reviewed. Closing for AT.

ngaruko commented 7 years ago

A bit of a long convo here..Can you please synthesize how to 'reproduce' this @sglangevin ?

sglangevin commented 7 years ago

This is not a bug so there are no steps to reproduce. I believe @abbyad and @alxndrsn have already AT-ed this, but you can follow the instructions in Marc's most recent comment or I can just AT this one.

sglangevin commented 7 years ago

@ngaruko can you confirm whether or not you did AT? In the meantime, I'm going to set this back to to acceptance testing as I'd like to make sure it's working as intended before we release the feature.

sglangevin commented 7 years ago

I've tried testing this using identical XML and the field that should contain the id of the child doc (new doc being generated by the form submission that is not the data_record) is not being populated. Are there any other tricks I'm missing? Also, xlsforms can't be converted if the calculate column is left blank as it @abbyad's example above. I tried adding various things there as well as manually removing the calculation from the xml and no luck.

@alxndrsn am I missing something? Were you and @abbyad able to get that id to be populated? I'm also not sure how I would find the documents that I may or may not have created in the db.

alxndrsn commented 7 years ago

@sglangevin what version of pyxform are you using? Can you share your XLS file and we can try again from this end?

sglangevin commented 7 years ago

@alxndrsn I'm using whatever is the latest version from Medic's fork, I believe it's 0.9.22. I'll send you the XLS file on Slack.

sglangevin commented 7 years ago

@alxndrsn was able to help me get this up and running this morning (thanks!) and I'll work today to get things working a little better. The doc was created but I need to work out how to make sure I'm generating the right fields and getting the person into the right place in the hierarchy. I'll report back.

I'm also going to file a bug because the child doc should be getting a reported_date added to it when it is created. I can work around that for testing, but that should be done automatically by the app the way we are doing so for creation of other docs.

sglangevin commented 7 years ago

Got this working on v2.13.0-beta.2, at least for registering a newborn from a PNC form, which was the original use case for this issue. It's awesome that we're able to link to the report that created the person, too - @michaelkohn will be happy. Moving to ready!

sglangevin commented 7 years ago

I have now tested this for creating docs from a repeat group and that is also working well. This can be used to register multiple births.