Cht-core: Immunization Condition Card not getting updated as per Collect Immunization Form submission

[mukesh2006]

Description

The Immunization Condition Card is not getting updated as per the Immunization report submitted from the collect app.

As per the code in contact-summary-extras.js, the card gets updated if the value of corresponding immunization key is set to yes in the document. i.e. https://github.com/medic/medic/blob/be2fb83b4df072fcbeaa1b1688c307d8b9f31923/config/standard/contact-summary-extras.js#L249

But the document created in couchdb corresponding to an immunization form submission has values Yes/No, hence the check vaccines_received['received_' + dose[0]] === 'yes'; fails and the card is not updated.

To patch this issue, the code can be updated as vaccines_received['received_' + dose[0]].toLowerCase() === 'yes'; but it does not answer the question that why the immunization report created in couchdb have values Yes/No instead of yes/no as set in the Immunization Form.

Investigation

The value yes/no is getting translated according to the translation key in the translation file (message-en.properties, message-ne.properties etc.). If translation file has entry of yes=Yes and no=No, then the translationCache gets set correspondingly in config.js: https://github.com/medic/medic/blob/be2fb83b4df072fcbeaa1b1688c307d8b9f31923/api/src/config.js#L81-L92
And then, when the immunization collect form submission is parsed with smsparser it returns Yes and No as the translated value of yes and no, and the same is set in the couchdb document. https://github.com/medic/medic/blob/be2fb83b4df072fcbeaa1b1688c307d8b9f31923/api/src/services/report/smsparser.js#L159-L178

Resolution

There could be multiple ways to resolve this:

1. Change yes=Yes to yes=yes and no=No to no=no in all the translation files.
2. Do not translate the value of string while parsing i.e. change return config.translate(raw); to return raw;. https://github.com/medic/medic/blob/be2fb83b4df072fcbeaa1b1688c307d8b9f31923/api/src/services/report/smsparser.js#L178
3. Find out what can be changed in config.js https://github.com/medic/medic/blob/be2fb83b4df072fcbeaa1b1688c307d8b9f31923/api/src/config.js#L119-L122

Please let me know your views on resolution of this issue.

@abbyad @medic/development-team

[abbyad]

I can't think of a good situation where we would want to translate the incoming value. I could see why we'd do the opposite, and normalize to the saved value. I can also see it being useful to localize the value when displaying the report, but not when receiving the value. This could perhaps be a relic from the list option used mostly with SIM apps, where a numeric value was translated back to a text value.

If we find out that there are scenarios where we need to translate it then we could add a field to the JSON Form schema to specify whether a field should be localized. However, if we don't have a reason to translate then it seems like the simplest and cleanest option is to never translate it (option 2, as per your possible resolutions @mukesh2006).

[mukesh2006]

@abbyad The code return config.translate(value); is at two more different places:

1. https://github.com/medic/medic/blob/be2fb83b4df072fcbeaa1b1688c307d8b9f31923/api/src/services/report/smsparser.js#L148-L156
2. https://github.com/medic/medic/blob/be2fb83b4df072fcbeaa1b1688c307d8b9f31923/api/src/services/report/smsparser.js#L169-L175

While we're here, shall I also modify these codes such that it returns value without tranlating?

[abbyad]

That seems to make sense, but worth having @garethbowen weigh in here too.

[garethbowen]

Yes I think those changes make sense, and I agree that we should store the non-translated value and translate just before showing it on screen.

Note that this will break backwards compatibility with other configurations that rely on this behaviour. Can you think of any way we can avoid break existing configurations?

[abbyad]

We could have it as a flag in the config, but not sure that we need that complexity. In general this is only relevant for projects that have SMS/Collect mixed with App workflows, and right now that is only Standard config.

The other place that this could have negative consequences is analytics, since it could now be expecting the name instead of the value. @derickl and @mukesh2006, can you review current projects to see if this would be a problem?

[mukesh2006]

@abbyad I looked into the postgres code for analytics in MoH-Nepal and Standard projects, they're comparing yes/no. So if we don't make this change then the analytics might break.

[abbyad]

Ok, sounds like those would be OK with the proposed change of always using the value, not the translated label.

@derickl, let us know if you have you seen it differently, otherwise we'll move forward with this.

[derickl]

I haven't seen this used differently. @smbuthia @eljhkrr ?

[smbuthia]

Neither have I.

[garethbowen]

Ready for AT in 5762-dont-translate-values

[newtewt]

This is removing the translation as expected and the value as it is sent gets stored. The standard config is still comparing a value against a lowercase 'yes' in the snippet below. When the sms comes in yes could be a variation of YES, YeS, etc. which would still cause the issue outlined above. I'm not sure whether the config code should handle that or we should but wanted to point out that the reason this issue was reported would could still happen.

vaccines_received['received_' + dose[0]] === 'yes'; 

[abbyad]

That particular example is with a Collect form, so the yes will always be as such and we don't have to worry about alternate forms of YES.

[newtewt]

Ok sounds good the translation is being removed as expected.