Registrations that clear previous registrations also clear themselves

[abbyad]

SMS registration forms may need to clear previous scheduled messages, but currently setting them as such also clears their own schedule too.

An example of where this would be needed is when a pregnancy registration may be resent by a CHW to update the LMP. That form would be configured in the registrations, and also in the patient_reports to clear schedules for previous registrations. From testing, SMS forms clear their own schedules (which is bad), but app workflows that trigger SMS schedules do not clear their own schedules (which is good). We would both json and xforms to avoid clearing their own newly created schedules.

@mandric suggested one approach to exclude clearing schedules on the current report would be to have an "expression" field on Patient Reports to decide which schedules to mute. His other suggestion was to do a rewrite combining Registrations and Patient Reports.

[mandric]

I'm pretty sure the configuration for patient_reports, notifications and registrations should be merged into one "form events" configuration/system. This is a bit of technical debt I'd like to see fixed to standardize how we configure forms on the backend, but it probably needs to wait for 3.0.

[abbyad]

@mandric I think you are right that at least the patient_reports and registrations need to be combined because it is becoming difficult to work with them for person-centered workflows.

For example, in a person-centered SMS workflow we submit a pregnancy registration with the person's 5 digit ID. If that ID is valid everything works more or less as expected, although it is possible to get auto-replies coming from each section (as seen in #3075), but that can be avoided by omitting one in config. The bigger problem is when the ID is invalid. In that case the errors and scheduled messages are created. We can use exists validation for registration events so that the schedule is not created, but can't however do the same for the form validation. The result is that we can't actually know if a registration report is about a valid person.

[mandric]

So #3075 would fix your issue?

[abbyad]

A workaround for now is to only use the events section or registrations. All validation and other messages should be done from the patient_reports. That means that once #3075 is fixed this issue is back to clearing schedules for previous submissions of the same form - while may be dealt with the tech debt of combining the config sections.

[abbyad]

The comment above was on the assumption that the registration could verify that the incoming report was for a valid person, using something like exists('N','patient_id') in the validation. This is not yet working, so either it is a config issue that can be resolved, a bug in exists, or a fundamental problem with this approach... I'll update once there is more info.

[abbyad]

I think it is the later, because exists looks to compare properties in doc.fields, whereas the ID ends up being added as doc.patient_id. Will open a separate issue for this if it is confirmed that there is no workaround.

[abbyad]

Having tried a couple of different ways, I always end up needing parts of registrations and part of patient_reports. This is fine when a report is completely valid. When it is invalid it is problematic because the registration transition proceeds when patient ID was not found by the accept_patient_reports transition. This means that SMS schedules would be sent for non existing patient IDs, unless it is caught by the regex validation in registrations which only checks the ID format. Even in that scenario the result is still not ideal since two error messages being sent, one from each transition:

Combining the transitions could help, but may have severe consequences (and testing) for existing projects, so it may be worth exploring other possibilities (eg finding way to configure registrations to run only after successful accept_patient_reports). That would also solve the original issue in this post that reports are clearing their own schedules.

[garethbowen]

CR please @SCdF https://github.com/medic/medic-sentinel/pull/103

It's a bit of refactoring and one small change.

[SCdF]

Good stuff bro.

[abbyad]

Need to bump sentinel to do acceptance testing.

[abbyad]

Testing on 2.10.0-alpha.4850 and this still happens.

For testing, send P 66837 on standard.app and it should add scheduled messages, and leave them as scheduled. Currently it shows them as scheduled for about 2 seconds then cleared

[garethbowen]

Not thoroughly enough tested on my side, sorry. Try again now.

[abbyad]

Works well in 4859, moving to Ready.