dev/core#1366 - Make case activity audit print report work again

[demeritcowboy]

Overview

There is a print report link on manage case and an activity audit report dropdown on manage case. Internally the print report is implemented via the activity audit report which predates the print report. When the print report button was implemented it got implemented in a way that broke the intent of the audit report, likely because there was no real documentation for the audit report other than some hidden wiki project planning notes (my fault), combined with the fact that originally in civicase you couldn't include non-case activity types like Meeting on a case, so when that became possible there was a "flaw" in the audit report output which likely caused further confusion about what it was supposed to do.

What it's supposed to do is present you with a choice of activity set (timeline) to run against, and then use that choice when generating the output. It's currently overriding the selected choice and including all the activities on the case, so is really no different at the moment from the print report (which itself is actually giving the correct output even though the implementation isn't quite right).

There's more but that's all this PR addresses. See comments below for proposed next steps.

Before

Audit report including activities it's not supposed to.

After

Audit report includes only activities in the chosen activity set.

Technical Details

The unit test is in PR #15877 and what it's currently failing on against the current master is that when you choose to run an audit report on standard_timeline it's incorrectly including a Meeting activity that isn't in the standard timeline.

Note that

CRM_Case_XMLProcessor::allActivityTypes($indexName = TRUE, $all = FALSE)

is just an alias for

CRM_Case_PseudoConstant::caseActivityType($indexName = TRUE, $all = FALSE)

and so are the same function, with the same parameters. So the deleted block in Report.php that calls the PseudoConstant does the exact same thing as the change in line 779 that calls the other function, but in a different place, i.e. it now does it only when there is no Activity Set chosen, instead of always doing it regardless of what you chose.

Then the change in the .tpl doesn't change the output of the print report from current but makes it do the proper thing by not specifying an activity set. The earlier introduction of specifying "standard_timeline" to mean "everything" I assume was caused by the confusion mentioned in Overview.

So to test this, you need a case type with at least one timeline, and the activity types defined in the case type need to include at least one type that isn't in the timeline. So for example take a stock case type and add "Meeting" as an available activity type on the activity types tab. Then create a case and add a Meeting activity to the case.

Then if you click Print Report, it will include all the activities on the case including Meeting. If you click on Activity Audit and choose standard timeline, then click the audit's own print report button on the resulting screen, it will only include the activities in the standard timeline, i.e. not Meeting.

Comments

For future steps: The on-screen interactive version of the audit report that you get before getting the print button for the audit report has been broken since 2015 when the supporting javascript file was deleted and so nothing on that screen works. Since then obviously nobody is using it, the next two proposed steps are to bypass that screen and go straight to the audit print report when you choose to do an activity audit, and then at some future time remove all the code related to the on-screen javascripty page.

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

@agh1 are you or @alifrumin able to review?

[demeritcowboy]

Perhaps @adam-devapp could help review, e.g. trying out the test site here or applying the patch to a local site? Just to spread the load around (although of course I welcome any input from @agh1 or @alifrumin).

Note also that I neglected to put it in the summary description here but as mentioned in the lab ticket the changed function is only ever used in the audit report and print case report, so this PR doesn't affect anything else.

And I just realized my git commit comment got left as "wip". Does anyone read those anyway?

[eileenmcnaughton]

Ohh another person to pick on for case stuff :-)

[alifrumin]

I tested this and it looks good to me.

[seamuslee001]

Merging based on @alifrumin review

[demeritcowboy]

Thanks!