fix: cdr fix to correctly show actions that are due, due soon, past due in expanded listings

[bradymiller]

cdr fix to correctly show actions that are due, due soon, past due in expanded listings

Reported by @hanksterr7

Note that the code change here is very minimal (just ensuring $dateCounter == 1 before running the code that states it is Not Due). This link will show the code changes without whitespace to show this:
https://github.com/openemr/openemr/pull/7211/files?w=1

Testing well in both reminder and report mode.

[stephenwaite]

[hanksterr7]

@bradymiller , sorry but your change does not correctly solve the issue

Original block of code (with added indenting of the top half):
`

                if ($passTarget) {
                        // increment pass target counter
                        $pass_target++;
                        // If report itemization is turned on, then record the "passed" item and set the flag
                        if (!empty($GLOBALS['report_itemizing_temp_flag_and_id'])) {
                            insertItemReportTracker($GLOBALS['report_itemizing_temp_flag_and_id'], $GLOBALS['report_itemized_test_id_iterator'], 1, $rowPatient['pid']);
                            $temp_track_pass = 1;
                        }
                        // send to reminder results
                        if ($mode == "reminders-all") {
                            // place the completed actions into the reminder return array
                            $actionArray = resolve_action_sql($rowRule['id'], '1');
                            foreach ($actionArray as $action) {
                                $action_plus = $action;
                                $action_plus['due_status'] = "not_due";
                                $action_plus['pid'] = $rowPatient['pid'];
                                $action_plus['rule_id'] = $rowRule['id'];
                                $results = reminder_results_integrate($results, $action_plus);
                            }
                        }

                    break;
                } else {
                    // send to reminder results
                    if ($mode != "report") {
                        // place the uncompleted actions into the reminder return array
                        $actionArray = resolve_action_sql($rowRule['id'], '1');
                        foreach ($actionArray as $action) {
                            $action_plus = $action;
                            $action_plus['due_status'] = $reminder_due;
                            $action_plus['pid'] = $rowPatient['pid'];
                            $action_plus['rule_id'] = $rowRule['id'];
                            $results = reminder_results_integrate($results, $action_plus);
                        }
                    }
                }

`

You wrapped the code that executes if $passTarget is true (except for the final break;) with a check for $dateCounter == 1

The $passTarget test is called during each of the passes through the
foreach ($target_dates as $dateFocus) {
loop
If the target passes during a pass through the loop, the
if ($passTarget)
success code is executed, and the break; causes further passes through the loop to be skipped.
Otherwise, the
} else {
section is executed
I believe the code in the top part of the success section needs to run when the target passes, regardless of whether $dateCounter is 1, 2, or 3
Yes, if the target passes during the 2nd or 3rd passes, the "else" code will execute this line during the preceding failure pass:
$results = reminder_results_integrate($results, $action_plus);
and contain a $reminder_due value that is appropriate for the rule passing during the current pass through the loop.
But I believe you still need to increment $pass_target and also do the "If report itemization is turned on" lines when the target does pass. Your change causes those actions to get skipped if the target passes during the 2nd or 3rd passes through the loop

I haven't studied what reminder_results_integrate() does. I'm assuming that if it executes on a failure pass (one or more times, for $dateCounter == 1 and $dateCounter == 2), it does not need to execute if the target passes when $dateCounter == 3
As such, all you need to do is relocate your new test on
if ($dateCounter == 1) {
to wrap just the
if ($mode == "reminders-all") {
section and not the full block of code that follows
if ($passTarget) {

Hope this makes sense.
Thanks

[hanksterr7]

I posted this comment in #7160. Entering it here as well
What does "an official pass" mean? Is there a difference between "not due" and "due soon", for some reporting purpose, such that you want to increment $pass_target only if the status is "not due" and not if it is "due soon"? If you want to increment $pass_target only for "not due" and not for "due soon", "due", or "past due", then I understand and agree with where you placed the test for $dateCounter == 1

[bradymiller]

hi @hanksterr7 , sorry for the delay on replying to this. Planning to research this more over next couple days and will get back to you.

[bradymiller]

hi @hanksterr7 ,
Updated the fix to be more focal and clear as you suggested. Agree should not encompass that entire section especially for clarity sake (that other part is for reporting which only runs on the target date which is why it was testing ok). I still need to test this some more.

And plan to analyze the reminder_results_integrate() a bit since something seems to be off there (or i just don't remember what i was doing there). One of objectives of the CDR engine was to support the same action from different rules without duplicating them, but i don't see how that function will do this correctly (quite possible i was supporting this somewhere else but something doesn't seem right).

[bradymiller]

hi @hanksterr7 ,
Sorry for the delay on this. After I test out the fixes in this PR (plan to by end of this week), then can bring it over to your PR (and at that point should be ready to bring in your PR). I'll plan to analyze/test the reminder_results_integrate() code after that since it's not related to this PR or your PR.

[bradymiller]

hi @hanksterr7 , This is testing well in the patient summary screen and the reports. If you are ok with this fix, then let me know and I'll bring it into the codebase (and you can incorporate it into your PR).

[hanksterr7]

Hi @bradymiller
I like the updated version of your changes. I agree with committing it.
Please confirm, though, that you want to execute the reminder_results_integrate() stuff only if the target status is "not due". I would think you would want to do this for both "not due" and "due soon". If "due soon", the patient is still in compliance with the rule criteria, but will not be for much longer. Would seem to be appropriate to consider such cases as meeting criteria for reporting purposes (unless I'm not fully understanding the purposes of the reports). Thanks

[bradymiller]

hi @hanksterr7 ,
The report mode doesn't use the reminder_results_integrate() function. Also note that report mode only does one date (the target get), so it is either a hit or a miss (so the tracking of it is more straightforward). In the non-report modes, every non-pass iteration will run reminder_results_integrate(), which essentially just then keeps the most recent one that was set when it escapes after the pass (except in the special case when reminders-all mode is used and passes on the first iteration where it is set to not due). Note I need to adjust reminder_results_integrate() a little so it works when separate rules have same action.

[hanksterr7]

Hi @bradymiller
I'm still not following your explanation, and I'm not sure you are understanding the question I am asking

Here is the block of code in question

                    // Check if pass target
                    $passTarget = test_targets($rowPatient['pid'], $rowRule['id'], '', $dateFocus);
                    if ($passTarget) {

(old)                        // increment pass target counter
(new)                       // increment pass target counter (used for reporting)
                        $pass_target++;
                        // If report itemization is turned on, then record the "passed" item and set the flag
                        if (!empty($GLOBALS['report_itemizing_temp_flag_and_id'])) {
                            insertItemReportTracker($GLOBALS['report_itemizing_temp_flag_and_id'], $GLOBALS['report_itemized_test_id_iterator'], 1, $rowPatient['pid']);
                            $temp_track_pass = 1;
                        }
(old)                        // send to reminder results
(old)                        if ($mode == "reminders-all") {
(new)                        // send to reminder results for reminders-all when not_due (ie. $dateCounter == 1)
(new)                        if (($mode == "reminders-all") && ($dateCounter == 1)) {
                            // place the completed actions into the reminder return array
                            $actionArray = resolve_action_sql($rowRule['id'], '1');
                            foreach ($actionArray as $action) {
                                $action_plus = $action;
                                $action_plus['due_status'] = "not_due";
                                $action_plus['pid'] = $rowPatient['pid'];
                                $action_plus['rule_id'] = $rowRule['id'];
                                $results = reminder_results_integrate($results, $action_plus);
                            }
                        }
                        break;
                    } else {

You are replacing
if ($mode == "reminders-all") {
with
if (($mode == "reminders-all") && ($dateCounter == 1)) {

So, as the comment says, you will "send to reminder results for reminders-all when not_due"

My question is: does it also make sense if to send to reminder results if "due soon"?

If a target passes when $dateCounter == 2 (which means the target did not pass when $dateCounter == 1), then the target is "due soon".

If sending to reminder results if either "not due" or "due soon" is desired behavior, then you need to test for $dateCounter == 1 or $dateCounter == 2

I would think "due soon" means the rule criteria have been satisfied, but won't be for much longer. So it is still appropriate to include the rule in success reports.

If you really don't want to send to reminder results if "due soon", and instead only if "not due", then the current code is fine

Thanks for pondering
-- Hank

[bradymiller]

Hi @hanksterr7 ,

Note the due_soon gets set when $datecounter == 1 if do not pass via the reminder_results_integrate() call in the else block below your code highlighted above. Then when it passes during $datecounter == 2, nothing else happens and it keeps the due_soon status. I agree this is super confusing though and I don't know what I was thinking 10 years ago when did this. Also note the reports are not using this stuff (it only runs through the loop once with the target date and either passes or fails ie, not due or due).

I just made the code more clear in a PR I just posted:
#7268
(Had to do fixes in reminder_results_integrate() to support duplicate actions across rules, which made that function more resource intensive. So it then made a lot of sense to only run that function when needed rather than running it multiple times to capture the correct reminder status)