Allow different output formats for CiviReport results, like native excel format, and untangle code
This started as a question like "without having to patch core or adding another if outputformat == 'x'
into some already awkward code in core, how can an extension implement a new output format for civireport results?"
My own motivation for this is a couple things:
- I've had to look at the related code blocks more than once and it takes at least 15 minutes just to wade through and get to a point where you can start looking into what you were originally trying to look into. Then you get lost again while circling back.
- I like the idea of being able to prevent people sending me listings of contacts in pdf format. No I will not import or analyze this data you've sent me in pdf format. (You can prevent this currently, it would just be easier after.)
- The email body when sent by the mail_report job is hardcoded. There's been one or two requests to have it be more configurable. You can sort of do it with hook_alterMailParams now but it's not the most robust. I'm not fully addressing that here with any UI changes, but the proposed changes set the stage for it being easier to get there. At the very least you would now be able to write an extension that simply extends the existing handler class (e.g. csv) and overrides the small function that returns the text.
- I'm currently doing this a different way and don't know if I would switch, but it opens up the possibility for an output handler that provides templates into which the data is inserted which then automatically is available in the existing UI and mail_report job. For most people this would mean a pdf template, but for people like me this means something like an excel template with a prebuilt pivot table and the handler would update the source rows so the pivot would automatically update.
And then beyond my own motivation some other possibilities which are theoretical:
- A variation of the last point above would be an outputhandler where when run by mail_report it connects to another network service and sends the data there server-to-server. You could set this up via cron exactly the same way you normally use the mail_report job. The equivalent download would then still also be automatically available to users to do manually the same as any other civireport.
So...
After doing some investigating, can summarize how the current output code works as
- type
sendmail
, which gets the output as a string and sends an email, OR - type
not sendmail
, which starts a download. Note that "download" technically is also a string, but sent to the browser instead of used in an email.
Then further
- Some of the convoluted nature of the existing code is partly because how to get either of those is different depending on the output format (e.g. csv/pdf/etc).
- Also note that "download" for type 'print' is actually the same as other downloads it's just that you don't need to change http headers first. So that's also adding to the awkwardness a bit in the current code because the way it's written it almost seems like a different code path.
THEREFORE --->: My thinking is to start on this by having a common interface to get the string. I mean interface in the general sense of the word, not necessarily OO Interface but maybe. But then it reduces the number of if/else's to just one:
- If sendmail,
- Get string (delegated to the output handler).
- Send email (for now extract this to its own function just for readability).
- Else
- Delegate to output handler to set headers and send string to browser (aka download).
There's a little more to it, and it can borrow a bit from the existing PRs, but that's what I'm thinking in terms of first steps to untangling. I will work on a PR.
Then the next steps after that would be to allow for other output handlers, whether it be by a new hook or existing hook or other.
Misc notes
-
save/copy/delete are handled in beginPostProcess and do nothing in endPostProcess
-
there's a difference between compileContent() and the output content. compileContent() is not used for csv, but is used for pdf and 'print'.
-
endPostProcess has 4 different "categories":
-
"add contacts to group", which doesn't output anything
-
_sendmail, which uses the other output handlers but requests the results as a string.
- defaults to print, but also does csv or pdf and then exits
- There is also some hardcoding of the email body, partly because it includes a computed url, but it would be nice not to hardcode this.
- Note pdf is called with TRUE for 3rd param, so that it returns the pdf as a string.
-
Download, which uses the other output handlers but requests the results as a download.
- Note that print is a download technically, it just goes to the browser window. Csv or pdf starts an attachment download.
- Here pdf is the default, but at the point in the code where that happens it has to be either print or pdf and print would have been already handled.
- Note pdf is called with FALSE for 3rd param.
- csv is done by a wrapper around the same function that is used in _sendmail
-
Tests that extend CiviReportTestCase use _resultSet/getResultSet() and have _outputMode blank, so endPostProcess (which gets called from preProcess because force=1 here) does none of the above and just stores the results in an array.