From 3057ec13e9856c417ce570c7fe31b44bdf3b0edd Mon Sep 17 00:00:00 2001 From: demeritcowboy Date: Sun, 19 Jul 2020 21:09:55 -0400 Subject: [PATCH] Allow extensions to provide their own formats and clean up tangled code --- CRM/Report/Form.php | 228 ++++++++++-------- CRM/Report/OutputHandler/Csv.php | 105 ++++++++ CRM/Report/OutputHandler/Pdf.php | 107 ++++++++ CRM/Report/OutputHandler/Print.php | 81 +++++++ Civi/Report/OutputHandlerBase.php | 146 +++++++++++ Civi/Report/OutputHandlerFactory.php | 101 ++++++++ Civi/Report/OutputHandlerInterface.php | 121 ++++++++++ tests/phpunit/CRM/Report/Form/SampleForm.php | 30 ++- .../Civi/Report/OutputHandlerFactoryTest.php | 80 ++++++ .../Civi/Report/SampleOutputHandler.php | 20 ++ 10 files changed, 910 insertions(+), 109 deletions(-) create mode 100644 CRM/Report/OutputHandler/Csv.php create mode 100644 CRM/Report/OutputHandler/Pdf.php create mode 100644 CRM/Report/OutputHandler/Print.php create mode 100644 Civi/Report/OutputHandlerBase.php create mode 100644 Civi/Report/OutputHandlerFactory.php create mode 100644 Civi/Report/OutputHandlerInterface.php create mode 100644 tests/phpunit/Civi/Report/OutputHandlerFactoryTest.php create mode 100644 tests/phpunit/Civi/Report/SampleOutputHandler.php diff --git a/CRM/Report/Form.php b/CRM/Report/Form.php index 945e794f4e..80308090e1 100644 --- a/CRM/Report/Form.php +++ b/CRM/Report/Form.php @@ -1113,6 +1113,37 @@ class CRM_Report_Form extends CRM_Core_Form { return $this->_id; } + /** + * Getter for _outputMode + * + * Note you can implement hook_civicrm_alterReportVar('actions', ...) + * which indirectly allows setting _outputMode if the user chooses + * your action. + * + * @return string + */ + public function getOutputMode():string { + return $this->_outputMode; + } + + /** + * Getter for report header form field value + * + * @return string + */ + public function getReportHeader():string { + return $this->_formValues['report_header'] ?? ''; + } + + /** + * Getter for report footer form field value + * + * @return string + */ + public function getReportFooter():string { + return $this->_formValues['report_footer'] ?? ''; + } + /** * Setter for $_force. * @@ -1681,6 +1712,8 @@ class CRM_Report_Form extends CRM_Core_Form { unset($actions['report_instance.csv']); } + CRM_Utils_Hook::alterReportVar('actions', $actions, $this); + return $actions; } @@ -2839,25 +2872,35 @@ WHERE cg.extends IN ('" . implode("','", $this->_customGroupExtends) . "') AND CRM_Core_DAO::$_nullObject ); - if ($this->_outputMode == 'print' || - ($this->_sendmail && !$this->_outputMode) - ) { - $this->printOnly = TRUE; - $this->addPaging = FALSE; + if ($this->_sendmail && !$this->_outputMode) { + // If we're here from the mail_report job, then the default there gets + // set to pdf before we get here, but if we're somehow here and sending + // by email and don't have a format set, then use print. + // @todo Is this on purpose - why would they be different defaults? $this->_outputMode = 'print'; + } + + // _outputMode means multiple things and can cover export to file formats, + // like csv, or actions with no output, like save. So this will only set + // a handler if it's one of the former. But it's also possible we have a + // really interesting handler out there. But the point is we don't need to + // know, just to know that a handler doesn't always get set by this call. + $this->setOutputHandler(); + + if (!empty($this->outputHandler)) { if ($this->_sendmail) { + // If we're sending by email these are the only options that make + // sense. + $this->printOnly = TRUE; + $this->addPaging = FALSE; $this->_absoluteUrl = TRUE; } - } - elseif ($this->_outputMode == 'pdf') { - $this->printOnly = TRUE; - $this->addPaging = FALSE; - $this->_absoluteUrl = TRUE; - } - elseif ($this->_outputMode == 'csv') { - $this->printOnly = TRUE; - $this->_absoluteUrl = TRUE; - $this->addPaging = FALSE; + else { + // otherwise ask the handler + $this->printOnly = $this->outputHandler->isPrintOnly(); + $this->addPaging = $this->outputHandler->isAddPaging(); + $this->_absoluteUrl = $this->outputHandler->isAbsoluteUrl(); + } } elseif ($this->_outputMode == 'copy' && $this->_criteriaForm) { $this->_createNew = TRUE; @@ -3413,98 +3456,21 @@ WHERE cg.extends IN ('" . implode("','", $this->_customGroupExtends) . "') AND $this->_resultSet = $rows; } - if ($this->_outputMode == 'print' || - $this->_outputMode == 'pdf' || - $this->_sendmail - ) { - - $content = $this->compileContent(); - $url = CRM_Utils_System::url("civicrm/report/instance/{$this->_id}", - "reset=1", TRUE - ); - + // Add contacts to group + if ($this->_outputMode == 'group') { + $group = $this->_params['groups']; + $this->add2group($group); + } + else { if ($this->_sendmail) { - $config = CRM_Core_Config::singleton(); - $attachments = []; - - if ($this->_outputMode == 'csv') { - $content - = $this->_formValues['report_header'] . '

' . ts('Report URL') . - ": {$url}

" . '

' . - ts('The report is attached as a CSV file.') . '

' . - $this->_formValues['report_footer']; - - $csvFullFilename = $config->templateCompileDir . - CRM_Utils_File::makeFileName('CiviReport.csv'); - $csvContent = CRM_Report_Utils_Report::makeCsv($this, $rows); - file_put_contents($csvFullFilename, $csvContent); - $attachments[] = [ - 'fullPath' => $csvFullFilename, - 'mime_type' => 'text/csv', - 'cleanName' => 'CiviReport.csv', - 'charset' => 'utf-8', - ]; - } - if ($this->_outputMode == 'pdf') { - // generate PDF content - $pdfFullFilename = $config->templateCompileDir . - CRM_Utils_File::makeFileName('CiviReport.pdf'); - file_put_contents($pdfFullFilename, - CRM_Utils_PDF_Utils::html2pdf($content, "CiviReport.pdf", - TRUE, ['orientation' => 'landscape'] - ) - ); - // generate Email Content - $content - = $this->_formValues['report_header'] . '

' . ts('Report URL') . - ": {$url}

" . '

' . - ts('The report is attached as a PDF file.') . '

' . - $this->_formValues['report_footer']; - - $attachments[] = [ - 'fullPath' => $pdfFullFilename, - 'mime_type' => 'application/pdf', - 'cleanName' => 'CiviReport.pdf', - ]; - } - - if (CRM_Report_Utils_Report::mailReport($content, $this->_id, - $this->_outputMode, $attachments - ) - ) { - CRM_Core_Session::setStatus(ts("Report mail has been sent."), ts('Sent'), 'success'); - } - else { - CRM_Core_Session::setStatus(ts("Report mail could not be sent."), ts('Mail Error'), 'error'); - } - return; + $this->sendEmail(); } - elseif ($this->_outputMode == 'print') { - echo $content; - } - else { - // Nb. Once upon a time we used a package called Open Flash Charts to - // draw charts, and we had a feature whereby a browser could send the - // server a PNG version of the chart, which could then be included in a - // PDF by including tags in the HTML for the conversion below. - // - // This feature stopped working when browsers stopped supporting Flash, - // and although we have a different client-side charting library in - // place, we decided not to reimplement the (rather convoluted) - // browser-sending-rendered-chart-to-server process. - // - // If this feature is required in future we should find a better way to - // render charts on the server side, e.g. server-created SVG. - CRM_Utils_PDF_Utils::html2pdf($content, "CiviReport.pdf", FALSE, ['orientation' => 'landscape']); + elseif (!empty($this->outputHandler)) { + $this->outputHandler->download(); + CRM_Utils_System::civiExit(); } - CRM_Utils_System::civiExit(); - } - elseif ($this->_outputMode == 'csv') { - CRM_Report_Utils_Report::export2csv($this, $rows); - } - elseif ($this->_outputMode == 'group') { - $group = $this->_params['groups']; - $this->add2group($group); + // else we don't need to do anything here since it must have been + // outputMode=save or something like that } } @@ -5989,4 +5955,58 @@ LEFT JOIN civicrm_contact {$field['alias']} ON {$field['alias']}.id = {$this->_a return ''; } + /** + * Retrieve a suitable object from the factory depending on the report + * parameters, which typically might just be dependent on outputMode. + * + * If there is no suitable output handler, e.g. if outputMode is "copy", + * then this sets it to NULL. + */ + public function setOutputHandler() { + $this->outputHandler = \Civi\Report\OutputHandlerFactory::singleton()->create($this); + } + + /** + * Send report by email + */ + public function sendEmail() { + if (empty($this->outputHandler)) { + // It's possible to end up here with outputMode unset, so we use + // the "print" handler which was the default before, i.e. include + // it as html in the body. + $oldOutputMode = $this->_outputMode ?? NULL; + $this->_outputMode = 'print'; + $this->setOutputHandler(); + $this->_outputMode = $oldOutputMode; + } + + $mailBody = $this->outputHandler->getMailBody(); + + $attachments = []; + $attachmentFileName = $this->outputHandler->getFileName(); + // It's not always in the form of an attachment, e.g. for 'print' the + // output ends up in $mailBody above. + if ($attachmentFileName) { + $fullFilename = CRM_Core_Config::singleton()->templateCompileDir . CRM_Utils_File::makeFileName($attachmentFileName); + file_put_contents($fullFilename, $this->outputHandler->getOutputString()); + $attachments[] = [ + 'fullPath' => $fullFilename, + 'mime_type' => $this->outputHandler->getMimeType(), + 'cleanName' => $attachmentFileName, + 'charset' => $this->outputHandler->getCharset(), + ]; + } + + // Send the email + // @todo outputMode doesn't seem to get used by mailReport, which is good + // since it shouldn't have any outputMode-related `if` statements in it. + // Someday could remove the param from the function call. + if (CRM_Report_Utils_Report::mailReport($mailBody, $this->_id, $this->_outputMode, $attachments)) { + CRM_Core_Session::setStatus(ts("Report mail has been sent."), ts('Sent'), 'success'); + } + else { + CRM_Core_Session::setStatus(ts("Report mail could not be sent."), ts('Mail Error'), 'error'); + } + } + } diff --git a/CRM/Report/OutputHandler/Csv.php b/CRM/Report/OutputHandler/Csv.php new file mode 100644 index 0000000000..475e20aed3 --- /dev/null +++ b/CRM/Report/OutputHandler/Csv.php @@ -0,0 +1,105 @@ +getOutputMode() === 'csv'); + } + + /** + * Return the download filename. This should be the "clean" name, not + * a munged temporary filename. + * + * @return string + */ + public function getFileName():string { + return 'CiviReport.csv'; + } + + /** + * Return the html body of the email. + * + * @return string + */ + public function getMailBody():string { + // @todo It would be nice if this was more end-user configurable, but + // keeping it the same as it was before for now. + $url = CRM_Utils_System::url('civicrm/report/instance/' . $this->getForm()->getID(), 'reset=1', TRUE); + return $this->getForm()->getReportHeader() . '

' . ts('Report URL') . + ": {$url}

" . '

' . + ts('The report is attached as a CSV file.') . '

' . + $this->getForm()->getReportFooter(); + } + + /** + * Return the report contents as a string, in this case the csv output. + * + * @return string + */ + public function getOutputString():string { + //@todo Hmm. See note in CRM_Report_Form::endPostProcess about $rows. + $rows = $this->getForm()->getTemplate()->get_template_vars('rows'); + + // avoid pass-by-ref warning + $form = $this->getForm(); + + return CRM_Report_Utils_Report::makeCsv($form, $rows); + } + + /** + * Set headers as appropriate and send the output to the browser. + */ + public function download() { + //@todo Hmm. See note in CRM_Report_Form::endPostProcess about $rows. + $rows = $this->getForm()->getTemplate()->get_template_vars('rows'); + + // avoid pass-by-ref warning + $form = $this->getForm(); + + CRM_Report_Utils_Report::export2csv($form, $rows); + } + + /** + * Mime type of the attachment. + * + * @return string + */ + public function getMimeType():string { + return 'text/csv'; + } + + /** + * Charset of the attachment. + * + * @return string + */ + public function getCharset():string { + return 'utf-8'; + } + +} diff --git a/CRM/Report/OutputHandler/Pdf.php b/CRM/Report/OutputHandler/Pdf.php new file mode 100644 index 0000000000..420b1eee78 --- /dev/null +++ b/CRM/Report/OutputHandler/Pdf.php @@ -0,0 +1,107 @@ +getOutputMode() === 'pdf'); + } + + /** + * Return the download filename. This should be the "clean" name, not + * a munged temporary filename. + * + * @return string + */ + public function getFileName():string { + return 'CiviReport.pdf'; + } + + /** + * Return the html body of the email. + * + * @return string + */ + public function getMailBody():string { + // @todo It would be nice if this was more end-user configurable, but + // keeping it the same as it was before for now. + $url = CRM_Utils_System::url('civicrm/report/instance/' . $this->getForm()->getID(), 'reset=1', TRUE); + return $this->getForm()->getReportHeader() . '

' . ts('Report URL') . + ": {$url}

" . '

' . + ts('The report is attached as a PDF file.') . '

' . + $this->getForm()->getReportFooter(); + } + + /** + * Return the report contents as a string, in this case the pdf file. + * + * @return string + */ + public function getOutputString():string { + return CRM_Utils_PDF_Utils::html2pdf( + $this->getForm()->compileContent(), + $this->getFileName(), + TRUE, + ['orientation' => 'landscape'] + ); + } + + /** + * Set headers as appropriate and send the output to the browser. + */ + public function download() { + // Nb. Once upon a time we used a package called Open Flash Charts to + // draw charts, and we had a feature whereby a browser could send the + // server a PNG version of the chart, which could then be included in a + // PDF by including tags in the HTML for the conversion below. + // + // This feature stopped working when browsers stopped supporting Flash, + // and although we have a different client-side charting library in + // place, we decided not to reimplement the (rather convoluted) + // browser-sending-rendered-chart-to-server process. + // + // If this feature is required in future we should find a better way to + // render charts on the server side, e.g. server-created SVG. + + CRM_Utils_PDF_Utils::html2pdf( + $this->getForm()->compileContent(), + $this->getFileName(), + FALSE, + ['orientation' => 'landscape'] + ); + } + + /** + * Mime type of the attachment. + * + * @return string + */ + public function getMimeType():string { + return 'application/pdf'; + } + +} diff --git a/CRM/Report/OutputHandler/Print.php b/CRM/Report/OutputHandler/Print.php new file mode 100644 index 0000000000..02e04ea184 --- /dev/null +++ b/CRM/Report/OutputHandler/Print.php @@ -0,0 +1,81 @@ +getOutputMode() === 'print'); + } + + /** + * Return the download filename. This should be the "clean" name, not + * a munged temporary filename. + * + * For 'print' there is no attachment. + * + * @return string + */ + public function getFileName():string { + return ''; + } + + /** + * Return the html body of the email. + * + * @return string + */ + public function getMailBody():string { + return $this->getOutputString(); + } + + /** + * Return the report contents as a string. + * + * @return string + */ + public function getOutputString():string { + return $this->getForm()->compileContent(); + } + + /** + * Set headers as appropriate and send the output to the browser. + * Here the headers are already text/html. + */ + public function download() { + echo $this->getOutputString(); + } + + /** + * Override so links displayed in the browser are relative. + * + * @return bool + */ + public function isAbsoluteUrl():bool { + return FALSE; + } + +} diff --git a/Civi/Report/OutputHandlerBase.php b/Civi/Report/OutputHandlerBase.php new file mode 100644 index 0000000000..247bc45f6d --- /dev/null +++ b/Civi/Report/OutputHandlerBase.php @@ -0,0 +1,146 @@ +form; + } + + /** + * Setter for $form + * + * @param \CRM_Report_Form $form + */ + public function setForm(\CRM_Report_Form $form) { + $this->form = $form; + } + + /** + * Are we a suitable output handler based on the given form? + * + * The class member $form isn't set yet at this point since we don't + * even know if we're in play yet, so the form is a parameter. + * + * @param \CRM_Report_Form $form + * + * @return bool + */ + public function isOutputHandlerFor(\CRM_Report_Form $form):bool { + return FALSE; + } + + /** + * Return the download filename. This should be the "clean" name, not + * a munged temporary filename. + * + * @return string + */ + public function getFileName():string { + return ''; + } + + /** + * Return the html body of the email. + * + * @return string + */ + public function getMailBody():string { + return ''; + } + + /** + * Return the report contents as a string. + * + * @return string + */ + public function getOutputString():string { + return ''; + } + + /** + * Set headers as appropriate and send the output to the browser. + */ + public function download() { + } + + /** + * Mime type of the attachment. + * + * @return string + */ + public function getMimeType():string { + return 'text/html'; + } + + /** + * Charset of the attachment. + * + * The default of '' means charset is not specified in the mimepart, + * which is normal for binary attachments, but for text attachments you + * should specify something like 'utf-8'. + * + * @return string + */ + public function getCharset():string { + return ''; + } + + /** + * Hide/show various elements in the output, but generally for a handler + * this is always set to TRUE. + * + * @return bool + */ + public function isPrintOnly():bool { + return TRUE; + } + + /** + * Use a pager, but for a handler this would be FALSE since paging + * is a UI element. + * + * @return bool + */ + public function isAddPaging():bool { + return FALSE; + } + + /** + * Create absolute urls for links. Generally for a handler + * this is always set to TRUE, but for example for 'print' it's displayed + * on the site so it can be relative. + * @todo Couldn't it just always be absolute? + * + * @return bool + */ + public function isAbsoluteUrl():bool { + return TRUE; + } + +} diff --git a/Civi/Report/OutputHandlerFactory.php b/Civi/Report/OutputHandlerFactory.php new file mode 100644 index 0000000000..a2f975410f --- /dev/null +++ b/Civi/Report/OutputHandlerFactory.php @@ -0,0 +1,101 @@ +isOutputHandlerFor($form)) { + $outputHandler->setForm($form); + return $outputHandler; + } + } + catch (\Exception $e) { + // no ts() since this is a sysadmin-y message + \Civi::log()->warn("Unable to use $candidate as an output handler. " . $e->getMessage()); + } + } + return NULL; + } + + /** + * Register an outputHandler to handle an output format. + * + * @param string $outputHandler + * The classname of a class that implements OutputHandlerInterface. + */ + public function register(string $outputHandler) { + // Use classname as index to (a) avoid duplicates and (b) make it easier + // to unset/overwrite one via hook. + self::$registered[$outputHandler] = $outputHandler; + } + + /** + * There are some handlers that were hard-coded in to the form before which + * have now been moved to outputhandlers. + */ + private static function registerBuiltins() { + self::$singleton->register('\CRM_Report_OutputHandler_Print'); + self::$singleton->register('\CRM_Report_OutputHandler_Csv'); + self::$singleton->register('\CRM_Report_OutputHandler_Pdf'); + } + +} diff --git a/Civi/Report/OutputHandlerInterface.php b/Civi/Report/OutputHandlerInterface.php new file mode 100644 index 0000000000..bae1097782 --- /dev/null +++ b/Civi/Report/OutputHandlerInterface.php @@ -0,0 +1,121 @@ +_outputMode; - } - + /** + * Getter for addPaging. + * + * @return bool + */ public function getAddPaging():bool { return $this->addPaging; } + /** + * Thin wrapper around protected function for testing. + * Just calls getActions. + * + * @param int $instanceId + * @return array + */ + public function getActionsForTesting($instanceId) { + return $this->getActions($instanceId); + } + + /** + * This just allows setting outputMode directly for testing. + * @param string $outputMode + */ + public function setOutputModeForTesting(string $outputMode) { + $this->_outputMode = $outputMode; + } + } diff --git a/tests/phpunit/Civi/Report/OutputHandlerFactoryTest.php b/tests/phpunit/Civi/Report/OutputHandlerFactoryTest.php new file mode 100644 index 0000000000..78397cdd8f --- /dev/null +++ b/tests/phpunit/Civi/Report/OutputHandlerFactoryTest.php @@ -0,0 +1,80 @@ +setOutputModeForTesting('csv'); + $outputHandler = OutputHandlerFactory::singleton()->create($form); + $this->assertEquals('CRM_Report_OutputHandler_Csv', get_class($outputHandler)); + + $form->setOutputModeForTesting('pdf'); + $outputHandler = OutputHandlerFactory::singleton()->create($form); + $this->assertEquals('CRM_Report_OutputHandler_Pdf', get_class($outputHandler)); + + $form->setOutputModeForTesting('print'); + $outputHandler = OutputHandlerFactory::singleton()->create($form); + $this->assertEquals('CRM_Report_OutputHandler_Print', get_class($outputHandler)); + } + + /** + * Test when no suitable handler available for given report parameters. + */ + public function testCreateNoMatch() { + $form = new \CRM_Report_Form_SampleForm(); + $form->setOutputModeForTesting('something_nonexistent'); + $outputHandler = OutputHandlerFactory::singleton()->create($form); + $this->assertNull($outputHandler); + } + + /** + * Test handler made available via hook. + */ + public function testCreateWithHook() { + \Civi::dispatcher()->addListener('hook_civicrm_alterReportVar', [$this, 'hookForAlterReportVar']); + $form = new \CRM_Report_Form_SampleForm(); + $form->setOutputModeForTesting('sample'); + $outputHandler = OutputHandlerFactory::singleton()->create($form); + $this->assertEquals('Civi\Report\SampleOutputHandler', get_class($outputHandler)); + } + + /** + * Test actions modified by hook. + */ + public function testAlterReportVarHookWithActions() { + \Civi::dispatcher()->addListener('hook_civicrm_alterReportVar', [$this, 'hookForAlterReportVar']); + $form = new \CRM_Report_Form_SampleForm(); + // NULL means no particular instance - running new report from template + $actions = $form->getActionsForTesting(NULL); + $this->assertEquals(['title' => 'Export Sample'], $actions['report_instance.sample']); + } + + /** + * This is the listener for hook_civicrm_alterReportVar + * + * @param \Civi\Core\Event\GenericHookEvent $e + * Should contain 'varType', 'var', and 'object' members corresponding + * to the hook parameters. + */ + public function hookForAlterReportVar(\Civi\Core\Event\GenericHookEvent $e) { + switch ($e->varType) { + case 'outputhandlers': + $e->var['\Civi\Report\SampleOutputHandler'] = '\Civi\Report\SampleOutputHandler'; + break; + + case 'actions': + $e->var['report_instance.sample'] = ['title' => 'Export Sample']; + break; + } + } + +} diff --git a/tests/phpunit/Civi/Report/SampleOutputHandler.php b/tests/phpunit/Civi/Report/SampleOutputHandler.php new file mode 100644 index 0000000000..35a1673079 --- /dev/null +++ b/tests/phpunit/Civi/Report/SampleOutputHandler.php @@ -0,0 +1,20 @@ +getOutputMode() === 'sample'; + } + +} -- 2.25.1