Cht-core: Targets with date: 'now' show stale counts for first days of a reporting window

[kennsippell]

Describe the bug
Targets which should be counted "always" (not part of the monthly interval reporting) emit with date: Date.now(). We cache that emission for 7 days. If that emission is cached near the end of a reporting window, and then a new reporting interval begins -- we fail to count it since the cached date is outside the current interval. The result is taht targets with date: 'now' can show incorrect counts for the first seven days of a reporting window.

To Reproduce
Steps to reproduce:

1. On default configuration, register a pregnancy
2. Change clock to first few days of reporting window (eg. Feb 5)
3. Navigate to the targets tab. View "active pregnancies = 1"
4. Change clock to last few days of reporting window (eg. Feb 28)
5. Navigate to the targets tab. View "active pregnancies = 1"
6. Change clock to few days of next reporting window (eg. Mar 1)
7. Navigate to the targets tab.

Expected: View "active pregnancies = 1"
Observed: View "active pregnancies = 0"

If you move your clock forward six more days (eg. Mar 7), and refresh the target tab -- the proper values are displayed again.

Environment

• Instance: dev machine
• App: webapp
• Version: 3.8.0 and master

Additional context
We recognized clearly that the use of Date.now() was going to be a problem with our approach. But we thought this was rare - only mattering for calculations like "age of contact", etc. We could have used date: undefined instead of date: now to communicate that the emission should always be counted, but that's very awkward to do now in 3.9.0.

[SCdF]

@kennsippell / @garethbowen backport to 3.8.1?

[garethbowen]

Scheduled for 3.8.1

[dianabarsan]

In my mind, I've coupled the fix with a fix for https://github.com/medic/cht-core/issues/6209 (where no emissions from last day of the reporting interval are counted towards that month's targets doc) . The solution we're choosing here could maybe pave the way we're going to fix this secondary issue.
I've gone over a bunch of solutions for this problem, all with pros and cons and no perfect one:

A. Change cht-core to expire all task/targets calculations at the end of the reporting interval

pros:

• this is a minor change that switches the way calculations expire, from a time-to-live to a best-before, where the expiration date is min(reporting interval turnover, a week from now).
• no configuration changes are needed

cons:

• once a month, all calculations will expire (akin to a settings update)
• when we will implement the fix for 6209, the calculation will be a "best-effort", where we calculate old targets based on last saved (stale) state, disregarding "external" changes, instead of the current state (if we recalculate, all targets with date: now won't be counted for last month's targets)

Already have a PR with this fix.

B. Change medic-conf so docs that pass for targets with date: now would actually emit multiple times/doc, with multiple dates - maybe covering today, today - 1 month and today + 1 month, so it'd hit all "adjacent" reporting intervals.

pros:

• targets with date: now are always calculated correctly, and we don't need to expire all calculations at the end of an interval
• when the interval rolls over and we want to update last month's targets, we can just refresh the state and take all changes into account (even external ones) and have a correct calculation.

cons:

• the change is in medic-conf and everyone needs to update, recompile and re-upload configurations
• legacy configs (nools.rules.js) need manual updates to emit correctly. if not updated, they'll still experience this bug.

Already have a branch with this fix.

[garethbowen]

I'm concerned with option B and think we should go with option A in 3.8.1 and come up with an option C to solve 6209 in 3.9.0. @dianabarsan I'll endeavour to catch you online soon to discuss what that might be...

[dianabarsan]

Ready for AT on 6267-unstale-targets.

[ngaruko]

There is an issue on New Pregnancies card. If during registration, No Information is selected on the Gestational Age page, that registration does not show on New Pregnancies card.

Did you want to have a look at this @dianabarsan ?

[dianabarsan]

@ngaruko this is highly dependent on configuration.
Can you please share your config?

[ngaruko]

@dianabarsan This was tested on a fresh install with default config

[dianabarsan]

Thanks @ngaruko .
Looking at how such a report looks like:

...
"fields": {
   "patient_age_in_years": "33",
    "patient_uuid": "3fb7172a-74d8-4b00-9e29-6b0d6e55a3ce",
    "patient_id": "",
    "patient_name": "person3",
    "patient_short_name": "the woman",
    "patient_short_name_start": "The woman",
    "lmp_date_8601": "",
    "edd_8601": "",
    "lmp_method_approx": "no",
}
....

Notice that lmp_date_8601 field is empty.
Now looking at the target configuration: https://github.com/medic/cht-core/blob/master/config/default/targets.js#L40
In order to qualify a report as new pregnancy, it must have a way to determine LMP, which such report does not.
So it's expected not to be counted.

Also, this target has set date: "reported", while this issue covers targets that have date: "now" like active-pregnancies.
Does your report count as a active pregnancy?

[ngaruko]

Thanks @dianabarsan .Let us close this issue and continue to look at the new pregnancy issue separately. Feel free to merge!

[dianabarsan]

Merged into master and backported to 3.8.x.