dev/core#1855 - Allow different output formats for CiviReport results and untangle code

[demeritcowboy]

Overview

https://lab.civicrm.org/dev/core/-/issues/1855

The code to output csv/pdf/etc is hardcoded and hard to follow - I've previously struggled looking through the code a few times - but there's also several uses for being able to allow extensions to provide their own output formats or alter the core ones. I've expanded on this a bit in the lab ticket.

Inspired by and borrows from #17145 and #17693.

There's no change to existing report functions proposed here. A stock install should function the same as before.

There's a docs PR coming. It adds two varTypes to hook_civicrm_alterReportVar.

Before

CiviReports

After

Still CiviReports, but you can add your own formats. They also automatically become available as formats for the mail_report scheduled job.

If you want to get fancy, it's flexible enough to let you provide different output handlers for different report instances, or different report classes.

Technical Details

Since it looks big, here's a walkthrough:

1. Three of the files are for tests. I also recently added several tests here, here, and here to cover existing functionality.

2. There's three files in the Civi/Report folder:

• One is an interface, so by definition is boilerplate.
• One is a base class, which is identical to the interface but implements some defaults.
• The other is the factory, which is mostly just a hook for you to advertise your extension's outputhandler(s), with some fluff around it. On its own without the hook it just helps replace the big "if/else/if/then" that was hardcoded in the form by instead registering builtin types for 'pdf', 'csv', 'print'.

3. There's three files in the CRM/Report/OutputHandler folder, which are all copy/paste from the base class except for the parts that are different for the specific type, which are copy/paste from the if blocks. A key simplification here was that emailing a report and downloading a report start with the same thing: getting the output string.

4. That just leaves CRM_Report_Form itself.

• There's some getters added.
• It's hard to see from the github UI but if you look at the function endPostProcess before, you'll see it's this big nested if statement. By using the simplification above and moving the code into the outputhandler files, the if reduces mostly to:

if ($this->_sendmail) {
  $this->sendEmail();
}
elseif (!empty($this->outputHandler)) {
  $this->outputHandler->download();
  CRM_Utils_System::civiExit();
}

• There's a hook for the dropdown actions on the civireport page. Technically, this could be done already with hook_buildForm, but when you do that you have to do something like this: https://lab.civicrm.org/extensions/civiexportexcel/-/blob/ad7e59a587b5e7d6fe334fe03773e8173e2b64f1/civiexportexcel.php#L87-118, and that's without the extra code or core patch to make it actually do something when you choose it. With the new addition to alterReportVar it looks like this, and will automatically call out to your outputhandler when chosen:

function examplereportoutputhandler_civicrm_alterReportVar($type, &$vars, $form) {
switch ($type) {
case 'actions':
  $vars['report_instance.excel2007'] = ['title' => ts('Export to Excel')];
  break;
}

Comments

There's an example outputhandler extension which implements an "emoji" format at https://lab.civicrm.org/DaveD/examplereportoutputhandler

On the civireports instance listing pages, there are some links on the righthand side. This PR doesn't automatically alter those links, but there is already a hook for those that you could implement in your extension (hook_links with $op = 'view.report.links').

This PR does not fully address the hardcoding of the email body text when using the mail_report job - it's the same as before - but this might make it easier to make that more configurable via UI later since it's now out of the big if block. Even without a UI, you can now override it by extending one of the core outputhandlers and changing just the related function(s). You could do it before with hook_alterMailParams, but it wasn't very robust since you had to guess if the email was a civireport.

[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).

[demeritcowboy]

The test fail is because that test uses a dataprovider that calculates time-sensitive things, which is a no-no because it does the calculation at a different unknown time from the run of the test. I'm just surprised it hasn't come up before. Could be done via callback instead, or just not a dataprovider. Will make a ticket.

[eileenmcnaughton]

OK - gonna kick off tests again - here is the old link https://test.civicrm.org/job/CiviCRM-Core-PR/35526/

[eileenmcnaughton]

test this please

[demeritcowboy]

MySQL server has gone away. Jenkins test this please.

[eileenmcnaughton]

Yay it's passing - @seamuslee001 wanna check this fully replaces your PR?

[homotechsual]

WIP docsPR @ civicrm/civicrm-dev-docs#834

[seamuslee001]

I tested this by testing the original actions and confirmed that they were all working with this change. I also tested sending a report via email and confirmed that the base64 encoded content was correct and the mail_report job works correctly

[demeritcowboy]

Wow less than 24 hours. Thanks for testing!

[eileenmcnaughton]

Thanks for the work @demeritcowboy !