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

Support XForms that "compile" into SMS messages #4812

Closed
SCdF opened this issue Sep 7, 2018 · 18 comments
Closed

Support XForms that "compile" into SMS messages #4812

SCdF opened this issue Sep 7, 2018 · 18 comments
Assignees
Labels
MUSO IN Related to the Muso partnership Priority: 2 - Medium Normal priority Type: Feature Add something new

Comments

@SCdF
Copy link
Contributor

SCdF commented Sep 7, 2018

See this section of this document for discussion specific pertaining to the project that requires this: https://docs.google.com/document/d/1Ygpi5JzVKYbfWF-uEIc1RCvAPSGq-ut0N8cHMSn5eXY/edit#heading=h.2vg9c5lgu2ld

The developer should also work with @maremegaye so they have the specific form that is needed to test with and make sure the implementation works for that form. This is important because:

  • We need to make all the information needed later fits inside one SMS (multi-sms is not globally supported by mobile networks and is generally dodgy AFAICT), and this form has the potential to be quite large
  • We need to either have this report also possibly contain patient demos if the patient was created offline (you can tell: if they have a patient shortcode they made it to the server) or also configure the new patient form to send SMS

We could also consider hooking into PouchDB and only sending SMS if the form can't be replicated upward immediately. That could be hard though, and for time constraint reasons it's fine to not do this

@SCdF
Copy link
Contributor Author

SCdF commented Sep 7, 2018

This should hook up to this: #4811

@SCdF
Copy link
Contributor Author

SCdF commented Sep 7, 2018

Additionally, we need to come up with a strategy for "consolidating" SMS messages back to their data counterparts. In other words:

  • XML form is created offline, written to PouchDB as JSON (report a)
  • SMS form is sent to server, where it is written to CouchDB as JSON (report b)
  • ... various things happen that might refer to report a and report b ...
  • Offline phone goes online, data sync happens and report a replicates up to CouchDB
  • … now report a and report b should be merged.

@SCdF SCdF mentioned this issue Sep 7, 2018
@garethbowen
Copy link
Member

Can we not generate a doc if we're sending an SMS? That would solve the report merging problem but would leave the user in an unknown state until they replicate the report from the server.

If the UI is too bad we could generate a different kind of doc which records that the SMS was sent but never gets replicated to the server and throw it away once the real report gets generated.

This might be sufficient for the first round with a better solution later if needed?

@garethbowen garethbowen added Status: 1 - Triaged Type: Feature Add something new Priority: 2 - Medium Normal priority MUSO IN Related to the Muso partnership labels Sep 17, 2018
@maremegaye
Copy link

maremegaye commented Sep 25, 2018

Please find below the XLS for patient assessment under 5 and over 5 forms.
CL: CHW - CSCOM: This tab describes the information that must be extracted from the entire form (tab 1). All this information must be shared with the CSCOM either received by the Cashier or the Doctor or both at once
CL: CSCOM - CHW: This tab describes the SMS that the CHW will receive from the CSCOM information
CL: Data CSCOM - AS: this last tab describes the list of medications prescribed by the Doctor and sent directly (via internet) in the CHW's App

https://docs.google.com/spreadsheets/d/1OtHfrXQw4sDc716RbIig8ZoNmmCzluXeRccMK81_43s/edit#gid=1095104700

https://docs.google.com/spreadsheets/d/1z8-5S0q4X2s-V8pi5QiM3ntehvhaWavhV5ESRs5sDBs/edit#gid=1967584422

Let me know if there is anything missing
cc @garethbowen @Stefan @Fatoufall

@alxndrsn
Copy link
Contributor

Further to comment above, form XML is available at patient_assessment_over_5.* and patient_assessment.* in https://github.com/medic/medic-projects/tree/master/muso_in/forms/app.

@SCdF
Copy link
Contributor Author

SCdF commented Nov 7, 2018

@alxndrsn reviewed and back to you. So sorry it took this long, that was very much unintentional!

@SCdF
Copy link
Contributor Author

SCdF commented Dec 21, 2018

For AT, you should read https://github.com/medic/medic-docs/blob/master/configuration/forms.md#sending-reports-as-sms and make sure that both the document makes sense and the code works as described.

@dianabarsan
Copy link
Member

I gave this a try.

I didn't test the integration with Android at all, I hacked webapp in a branch to not do the android wrapper checks, and instead of calling the wrapper sms_send function, to log the parameters that would have been passed to said function, namely the doc._id, the gateway phone number (serving as recipient afaict) and the contents of the sms to be sent. (The branch: https://github.com/medic/medic/tree/4812-simulation)

At first, I did not update any of my forms (forms from standard config).
It appears that the documentation says that the property supposed to define the custom SMS definition is called report2sms, but the app checks for xml2sms.

Also, by default, all forms send sms.

When a form doesn't have the xml2sms property or its value evaluates to false (fe empty string), upon being submitted, the SMS generation always falls back to ODK compact record representation.

So, in order to have a form not send an SMS, it requires you to either:

  • update its couchdb doc to have an xml2sms field which, when parsed, returns a falsy value. (fe: "false")
  • update the form xml so it doesn't have a prefix, so the ODK compact record representation returns nothing (all standard forms have the optional prefix property)

The custom SMS generation looks good: expressions evaluate as described. Invalid expressions throw an error which is logged in console and submitted as feedback.

@newtewt newtewt assigned newtewt and unassigned newtewt May 29, 2019
@ngaruko ngaruko self-assigned this May 29, 2019
@newtewt
Copy link
Contributor

newtewt commented May 29, 2019

@SCdF do we need to make changes based on @dianabarsan review? Who's responsible for this now?

Also, we may have added code that handles the sending but we aren't requesting permissions to do so. Not sure if that was intentional or not but Logcat spits out the error below so we'd never be able to send SMS as far as I can tell. This is in medic-android.

2019-05-29 16:58:57.363 9997-10163/? W/System.err: java.lang.SecurityException: Sending SMS message: uid 10161 does not have android.permission.SEND_SMS.

@garethbowen
Copy link
Member

I wonder if we should check for permission and show an error in the UI if we don't have it. Otherwise this will just fail silently :/

@ngaruko ngaruko assigned newtewt and unassigned ngaruko May 30, 2019
@newtewt
Copy link
Contributor

newtewt commented May 30, 2019

Reading the docs from google we should be prompting or we'll never be able to send SMS.

If the device is running Android 6.0 (API level 23) or higher, and the app's targetSdkVersion is 23 or higher, the user isn't notified of any app permissions at install time. Your app must ask the user to grant the dangerous permissions at runtime. When your app requests permission, the user sees a system dialog (as shown in figure 1, left) telling the user which permission group your app is trying to access. The dialog includes a Deny and Allow button.

https://developer.android.com/guide/topics/permissions/overview#dangerous-permission-prompt

and details on requesting the permission.

https://developer.android.com/training/permissions/requesting.html#perm-request

@newtewt
Copy link
Contributor

newtewt commented May 30, 2019

Ok, I was able to build a version of the android app locally leveraging the permission requesting we already have and was able to use the Medic version of sending sms. However, when I try to follow the ODK standards for compact submission I cannot upload the app through the admin console.

https://opendatakit.github.io/xforms-spec/#compact-record-representation-(for-sms)

<?xml version="1.0"?>
<h:html xmlns="http://www.w3.org/2002/xforms" xmlns:ev="http://www.w3.org/2001/xml-events" xmlns:h="http://www.w3.org/1999/xhtml" xmlns:jr="http://openrosa.org/javarosa" xmlns:orx="http://openrosa.org/xforms" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
  <h:head>
    <h:title>MRDT Demo</h:title>
    <model>
      <instance>
        <mrdt delimiter="#" id="mrdt_demo" odk:prefix="mrdt" odk:delimiter="+">
          <name/>
          <mrdt_image_data type="binary"/>
          <meta>
            <instanceID/>
          </meta>
        </mrdt>
      </instance>
      <bind nodeset="/mrdt/name" type="string"/>
      <bind nodeset="/mrdt/mrdt_image_data" type="string"/>
    </model>
  </h:head>
  <h:body class="pages">
    <group appearance="field-list" ref="/mrdt">
      <label>Submit a photo of an MRDT test result</label>
      <input ref="/mrdt/name">
        <label>Patient name</label>
      </input>
      <input ref="/mrdt/mrdt_image_data" appearance="mrdt-verify">
        <label>MRDT image</label>
      </input>
    </group>
  </h:body>
</h:html>

@newtewt newtewt removed the Status: Blocked Unable to progress this. label May 30, 2019
@dianabarsan
Copy link
Member

dianabarsan commented May 30, 2019

It looks that our parser fails when you add the odk attribute namespace (maybe we don't accept namespaced attributes?).
Removing the namespace will allow the upload and the compaction will still work. (Also remove the double resulting "delimiter" attribute).

@newtewt
Copy link
Contributor

newtewt commented May 30, 2019

I tried without prefix and only delimiter and still failing.

<?xml version="1.0"?>
<h:html xmlns="http://www.w3.org/2002/xforms" xmlns:ev="http://www.w3.org/2001/xml-events" xmlns:h="http://www.w3.org/1999/xhtml" xmlns:jr="http://openrosa.org/javarosa" xmlns:orx="http://openrosa.org/xforms" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
  <h:head>
    <h:title>MRDT Demo</h:title>
    <model>
      <instance>
        <mrdt delimiter="#" id="mrdt_demo" delimiter="+">
          <name/>
          <mrdt_image_data type="binary"/>
          <meta>
            <instanceID/>
          </meta>
        </mrdt>
      </instance>
      <bind nodeset="/mrdt/name" type="string"/>
      <bind nodeset="/mrdt/mrdt_image_data" type="string"/>
    </model>
  </h:head>
  <h:body class="pages">
    <group appearance="field-list" ref="/mrdt">
      <label>Submit a photo of an MRDT test result</label>
      <input ref="/mrdt/name">
        <label>Patient name</label>
      </input>
      <input ref="/mrdt/mrdt_image_data" appearance="mrdt-verify">
        <label>MRDT image</label>
      </input>
    </group>
  </h:body>
</h:html>

@dianabarsan
Copy link
Member

dianabarsan commented May 30, 2019

You have 2 delimiter attributes :)
<mrdt delimiter="#" id="mrdt_demo" delimiter="+">

@newtewt
Copy link
Contributor

newtewt commented May 30, 2019

Thanks to @dianabarsan I'm able to send SMS in our standard and the ODK standard.

@garethbowen
Copy link
Member

Assigned to @dianabarsan to update the documentation and raise a new issue for any improvements.

newtewt pushed a commit to medic/medic-docs that referenced this issue Jun 7, 2019
Updated documentation about ODK compaction and medics SMS sending. 


medic/cht-core#4812
@newtewt
Copy link
Contributor

newtewt commented Jun 7, 2019

Docs have been updated .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MUSO IN Related to the Muso partnership Priority: 2 - Medium Normal priority Type: Feature Add something new
Projects
None yet
Development

No branches or pull requests

7 participants