From 6e1fbb97106366bd7a6dcd2ed4a7c81ae5363a09 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Mon, 15 Nov 2021 14:50:33 +1300 Subject: [PATCH] Enotice fix --- CRM/Report/Form.php | 40 +++++++++++++++++-- CRM/Report/Utils/Report.php | 2 +- templates/CRM/Report/Form.tpl | 8 ++-- templates/CRM/Report/Form/Contact/Detail.tpl | 4 +- .../Form/Contribute/DeferredRevenue.tpl | 8 ++-- templates/CRM/Report/Form/Event/Income.tpl | 4 +- templates/CRM/Report/Form/Layout/Graph.tpl | 2 +- templates/CRM/Report/Form/Layout/Table.tpl | 18 ++++----- templates/CRM/Report/Form/Statistics.tpl | 38 ++++++++---------- .../CRM/Report/Form/Contribute/DetailTest.php | 5 ++- tests/phpunit/CRM/Report/Utils/ReportTest.php | 13 +++--- tests/phpunit/api/v3/ReportTemplateTest.php | 1 + 12 files changed, 86 insertions(+), 57 deletions(-) diff --git a/CRM/Report/Form.php b/CRM/Report/Form.php index 2d015a178e..92425afbf0 100644 --- a/CRM/Report/Form.php +++ b/CRM/Report/Form.php @@ -13,6 +13,16 @@ * Class CRM_Report_Form */ class CRM_Report_Form extends CRM_Core_Form { + /** + * Variables smarty expects to have set. + * + * We ensure these are assigned (value = NULL) when Smarty is instantiated in + * order to avoid e-notices / having to use empty or isset in the template layer. + * + * @var string[] + */ + public $expectedSmartyVariables = ['pager', 'skip', 'sections', 'grandStat']; + /** * Deprecated constant, Reports should be updated to use the getRowCount function. */ @@ -2471,11 +2481,11 @@ WHERE cg.extends IN ('" . implode("','", $this->_customGroupExtends) . "') AND if ($pager) { $this->setPager(); } - + $chartEnabled = !empty($this->_params['charts']) && !empty($rows); + $this->assign('chartEnabled', $chartEnabled); // allow building charts if any - if (!empty($this->_params['charts']) && !empty($rows)) { + if ($chartEnabled) { $this->buildChart($rows); - $this->assign('chartEnabled', TRUE); $this->_chartId = "{$this->_params['charts']}_" . ($this->_id ? $this->_id : substr(get_class($this), 16)) . '_' . CRM_Core_Config::singleton()->userSystem->getSessionId(); @@ -2487,6 +2497,12 @@ WHERE cg.extends IN ('" . implode("','", $this->_customGroupExtends) . "') AND if (!empty($value['no_display'])) { unset($this->_columnHeaders[$key]); } + foreach (['colspan', 'type'] as $expectedKey) { + if (!isset($this->_columnHeaders[$key][$expectedKey])) { + // Ensure it is set to prevent smarty notices. + $this->_columnHeaders[$key][$expectedKey] = FALSE; + } + } } // unset columns not to be displayed. @@ -2494,6 +2510,12 @@ WHERE cg.extends IN ('" . implode("','", $this->_customGroupExtends) . "') AND foreach ($this->_noDisplay as $noDisplayField) { foreach ($rows as $rowNum => $row) { unset($this->_columnHeaders[$noDisplayField]); + $expectedKeys = ['class']; + foreach ($expectedKeys as $expectedKey) { + if (!array_key_exists($expectedKey, $row)) { + $rows[$rowNum][$expectedKey] = NULL; + } + } } } } @@ -3300,7 +3322,7 @@ WHERE cg.extends IN ('" . implode("','", $this->_customGroupExtends) . "') AND * @param array $rows */ public function doTemplateAssignment(&$rows) { - $this->assign_by_ref('columnHeaders', $this->_columnHeaders); + $this->assign('columnHeaders', $this->_columnHeaders); $this->assign_by_ref('rows', $rows); $this->assign('statistics', $this->statistics($rows)); } @@ -3344,12 +3366,14 @@ WHERE cg.extends IN ('" . implode("','", $this->_customGroupExtends) . "') AND $statistics['counts']['rowCount'] = [ 'title' => ts('Row(s) Listed'), 'value' => $count, + 'type' => CRM_Utils_Type::T_INT, ]; if ($this->_rowsFound && ($this->_rowsFound > $count)) { $statistics['counts']['rowsFound'] = [ 'title' => ts('Total Row(s)'), 'value' => $this->_rowsFound, + 'type' => CRM_Utils_Type::T_INT, ]; } } @@ -3378,6 +3402,10 @@ WHERE cg.extends IN ('" . implode("','", $this->_customGroupExtends) . "') AND 'value' => implode(' & ', $combinations), ]; } + else { + // prevents an e-notice in statistics.tpl. + $statistics['groups'] = []; + } } /** @@ -3485,6 +3513,10 @@ WHERE cg.extends IN ('" . implode("','", $this->_customGroupExtends) . "') AND } } } + else { + // Prevents an e-notice in statistics.tpl. + $statistics['filters'] = []; + } } } diff --git a/CRM/Report/Utils/Report.php b/CRM/Report/Utils/Report.php index b3fd9556fa..7e05025016 100644 --- a/CRM/Report/Utils/Report.php +++ b/CRM/Report/Utils/Report.php @@ -394,7 +394,7 @@ WHERE inst.report_id = %1"; $_REQUEST['reset'] = CRM_Utils_Array::value('reset', $params, 1); $optionVal = self::getValueFromUrl($instanceId); - $messages = ["Report Mail Triggered..."]; + $messages = ['Report Mail Triggered...']; $templateInfo = CRM_Core_OptionGroup::getRowValues('report_template', $optionVal, 'value'); $obj = new CRM_Report_Page_Instance(); diff --git a/templates/CRM/Report/Form.tpl b/templates/CRM/Report/Form.tpl index db27dbab83..5f34d26e1f 100644 --- a/templates/CRM/Report/Form.tpl +++ b/templates/CRM/Report/Form.tpl @@ -16,7 +16,7 @@ {elseif $section eq 2}
{*include the table layout*} - {if empty($chartEnabled) || empty($chartSupported)} + {if !$chartEnabled || empty($chartSupported)} {include file="CRM/Report/Form/Layout/Table.tpl"} {/if}
@@ -32,19 +32,19 @@ {include file="CRM/Report/Form/Actions.tpl"} {*Statistics at the Top of the page*} - {include file="CRM/Report/Form/Statistics.tpl" top=true} + {include file="CRM/Report/Form/Statistics.tpl" top=true bottom=false} {*include the graph*} {include file="CRM/Report/Form/Layout/Graph.tpl"} {*include the table layout*} - {if empty($chartEnabled) || empty($chartSupported)} + {if !$chartEnabled || empty($chartSupported)} {include file="CRM/Report/Form/Layout/Table.tpl"} {/if}
{*Statistics at the bottom of the page*} - {include file="CRM/Report/Form/Statistics.tpl" bottom=true} + {include file="CRM/Report/Form/Statistics.tpl" top="false" bottom=true} {include file="CRM/Report/Form/ErrorMessage.tpl"} diff --git a/templates/CRM/Report/Form/Contact/Detail.tpl b/templates/CRM/Report/Form/Contact/Detail.tpl index 7dfdeb2eb7..80ec862f57 100644 --- a/templates/CRM/Report/Form/Contact/Detail.tpl +++ b/templates/CRM/Report/Form/Contact/Detail.tpl @@ -17,7 +17,7 @@
{include file="CRM/Report/Form/Actions.tpl"} {if !$section } -{include file="CRM/Report/Form/Statistics.tpl" top=true} +{include file="CRM/Report/Form/Statistics.tpl" top=true bottom=false} {/if} {if $rows}
@@ -180,7 +180,7 @@ {if !$section } {*Statistics at the bottom of the page*} - {include file="CRM/Report/Form/Statistics.tpl" bottom=true} + {include file="CRM/Report/Form/Statistics.tpl" top="false" bottom=true} {/if} {/if} {include file="CRM/Report/Form/ErrorMessage.tpl"} diff --git a/templates/CRM/Report/Form/Contribute/DeferredRevenue.tpl b/templates/CRM/Report/Form/Contribute/DeferredRevenue.tpl index 6707b40200..e519abfd46 100644 --- a/templates/CRM/Report/Form/Contribute/DeferredRevenue.tpl +++ b/templates/CRM/Report/Form/Contribute/DeferredRevenue.tpl @@ -18,12 +18,12 @@ {include file="CRM/Report/Form/Actions.tpl"} {*Statistics at the Top of the page*} - {include file="CRM/Report/Form/Statistics.tpl" top=true} - + {include file="CRM/Report/Form/Statistics.tpl" top=true bottom=false} + {foreach from=$rows item=row} - + {foreach from=$columnHeaders item=label key=header} @@ -43,7 +43,7 @@
{*Statistics at the bottom of the page*} - {include file="CRM/Report/Form/Statistics.tpl" bottom=true} + {include file="CRM/Report/Form/Statistics.tpl" top="false" bottom=true} {include file="CRM/Report/Form/ErrorMessage.tpl"} diff --git a/templates/CRM/Report/Form/Event/Income.tpl b/templates/CRM/Report/Form/Event/Income.tpl index 0030909b90..2ce5a713c6 100644 --- a/templates/CRM/Report/Form/Event/Income.tpl +++ b/templates/CRM/Report/Form/Event/Income.tpl @@ -18,7 +18,7 @@ {include file="CRM/Report/Form/Actions.tpl"} {*Statistics at the Top of the page*} {if !$section } - {include file="CRM/Report/Form/Statistics.tpl" top=true} + {include file="CRM/Report/Form/Statistics.tpl" top=true bottom=false} {/if} {if $events} @@ -70,7 +70,7 @@ {if !$section } {*Statistics at the bottom of the page*} - {include file="CRM/Report/Form/Statistics.tpl" bottom=true} + {include file="CRM/Report/Form/Statistics.tpl" top="false" bottom=true} {/if} {/if} {include file="CRM/Report/Form/ErrorMessage.tpl"} diff --git a/templates/CRM/Report/Form/Layout/Graph.tpl b/templates/CRM/Report/Form/Layout/Graph.tpl index 137753d1bd..176c36ad7c 100644 --- a/templates/CRM/Report/Form/Layout/Graph.tpl +++ b/templates/CRM/Report/Form/Layout/Graph.tpl @@ -9,7 +9,7 @@ *} {assign var=uploadURL value=$config->imageUploadURL|replace:'/persist/contribute/':'/persist/'} {* Display weekly,Quarterly,monthly and yearly contributions using pChart (Bar and Pie) *} -{if !empty($chartEnabled) and !empty($chartSupported)} +{if $chartEnabled and !empty($chartSupported)}
{if $outputMode eq 'print' OR $outputMode eq 'pdf'} diff --git a/templates/CRM/Report/Form/Layout/Table.tpl b/templates/CRM/Report/Form/Layout/Table.tpl index 1d567a6fb6..0f3e6ef22a 100644 --- a/templates/CRM/Report/Form/Layout/Table.tpl +++ b/templates/CRM/Report/Form/Layout/Table.tpl @@ -7,10 +7,10 @@ | and copyright information, see https://civicrm.org/licensing | +--------------------------------------------------------------------+ *} -{if empty($rows)} +{if !$rows}

{ts}None found.{/ts}

{else} - {if !empty($pager) and $pager->_response and $pager->_response.numPages > 1} + {if $pager and $pager->_response and $pager->_response.numPages > 1}
{include file="CRM/common/pager.tpl" location="top"}
@@ -24,8 +24,8 @@ {else} {assign var=class value="class='reports-header'"} {/if} - {if empty($skip)} - {if !empty($header.colspan)} + {if !$skip} + {if $header.colspan}
{assign var=skip value=true} {assign var=skipCount value=`$header.colspan`} @@ -41,7 +41,7 @@ {/foreach} {/capture} - {if empty($sections)} {* section headers and sticky headers aren't playing nice yet *} + {if !$sections} {* section headers and sticky headers aren't playing nice yet *} {$tableHeader} @@ -92,14 +92,14 @@ {foreach from=$rows item=row key=rowid} {eval var=$sectionHeaderTemplate} - + {foreach from=$columnHeaders item=header key=field} {assign var=fieldLink value=$field|cat:"_link"} {assign var=fieldHover value=$field|cat:"_hover"} {assign var=fieldClass value=$field|cat:"_class"} {/foreach} - {if !empty($grandStat)} + {if $grandStat} {* foreach from=$grandStat item=row*} {foreach from=$columnHeaders item=header key=field} @@ -156,7 +156,7 @@ {* /foreach*} {/if}
{$row.label}
{$header.title|escape}
{if !empty($row.$fieldLink)} - + {/if} {if is_array($row.$field)} @@ -136,7 +136,7 @@
- {if !empty($pager) and $pager->_response and $pager->_response.numPages > 1} + {if $pager and $pager->_response and $pager->_response.numPages > 1}
{include file="CRM/common/pager.tpl" location="bottom"}
diff --git a/templates/CRM/Report/Form/Statistics.tpl b/templates/CRM/Report/Form/Statistics.tpl index f94e32db41..104f95466a 100644 --- a/templates/CRM/Report/Form/Statistics.tpl +++ b/templates/CRM/Report/Form/Statistics.tpl @@ -7,43 +7,39 @@ | and copyright information, see https://civicrm.org/licensing | +--------------------------------------------------------------------+ *} -{if !empty($top)} +{if $top} {if !empty($printOnly)}

{$reportTitle}

{if !empty($reportDate)}{$reportDate}{/if}
{/if} {if !empty($statistics)} - {if !empty($statistics.groups)} - {foreach from=$statistics.groups item=row} - - - - - {/foreach} - {/if} - {if !empty($statistics.filters)} - {foreach from=$statistics.filters item=row} - - - - - {/foreach} - {/if} + {foreach from=$statistics.groups item=row} + + + + + {/foreach} + {foreach from=$statistics.filters item=row} + + + + + {/foreach}
{$row.title}{$row.value|escape}
{$row.title}{$row.value|escape}
{$row.title}{$row.value|escape}
{$row.title}{$row.value|escape}
{/if} {/if} -{if !empty($bottom) and !empty($rows) and !empty($statistics)} +{if $bottom and !empty($rows) and !empty($statistics)} - {if !empty($statistics.counts)} + {if $statistics.counts} {foreach from=$statistics.counts item=row}
{$row.title} - {if !empty($row.type) and $row.type eq 1024} + {if $row.type eq 1024} {$row.value|crmMoney|escape} - {elseif !empty($row.type) and $row.type eq 2} + {elseif $row.type eq 2} {$row.value|escape} {else} {$row.value|crmNumberFormat|escape} diff --git a/tests/phpunit/CRM/Report/Form/Contribute/DetailTest.php b/tests/phpunit/CRM/Report/Form/Contribute/DetailTest.php index 4fe96f8920..fdb938fb14 100644 --- a/tests/phpunit/CRM/Report/Form/Contribute/DetailTest.php +++ b/tests/phpunit/CRM/Report/Form/Contribute/DetailTest.php @@ -147,12 +147,12 @@ class CRM_Report_Form_Contribute_DetailTest extends CiviReportTestCase { * Make sure the total amount of a contribution doesn't multiply by the number * of soft credits. */ - public function testMultipleSoftCredits() { + public function testMultipleSoftCredits(): void { $this->quickCleanup($this->_tablesToTruncate); $solParams = [ 'first_name' => 'Solicitor 1', - 'last_name' => 'User ' . rand(), + 'last_name' => 'User', 'contact_type' => 'Individual', ]; $solicitor1Id = $this->individualCreate($solParams); @@ -210,6 +210,7 @@ class CRM_Report_Form_Contribute_DetailTest extends CiviReportTestCase { 'rowCount' => [ 'title' => 'Row(s) Listed', 'value' => 1, + 'type' => CRM_Utils_Type::T_INT, ], 'amount' => [ 'title' => 'Total Amount (Contributions)', diff --git a/tests/phpunit/CRM/Report/Utils/ReportTest.php b/tests/phpunit/CRM/Report/Utils/ReportTest.php index a757322475..cb85435e19 100644 --- a/tests/phpunit/CRM/Report/Utils/ReportTest.php +++ b/tests/phpunit/CRM/Report/Utils/ReportTest.php @@ -76,7 +76,7 @@ ENDOUTPUT; * tests for that - we're more interested in does it echo it in print * format. */ - public function testOutputPrint() { + public function testOutputPrint(): void { // Create many contacts, in particular so that the report would be more // than a one-pager. for ($i = 0; $i < 110; $i++) { @@ -114,8 +114,7 @@ ENDOUTPUT; ]); } catch (CRM_Core_Exception_PrematureExitException $e) { - $contents = ob_get_contents(); - ob_end_clean(); + $contents = ob_get_clean(); } $this->assertStringContainsString('CiviCRM Report', $contents); $this->assertStringContainsString('test report', $contents); @@ -133,7 +132,8 @@ ENDOUTPUT; * @runInSeparateProcess * @preserveGlobalState disabled */ - public function testOutputPdf() { + public function testOutputPdf(): void { + $contents = ''; // Create many contacts, in particular so that the report would be more // than a one-pager. for ($i = 0; $i < 110; $i++) { @@ -171,8 +171,7 @@ ENDOUTPUT; ]); } catch (CRM_Core_Exception_PrematureExitException $e) { - $contents = ob_get_contents(); - ob_end_clean(); + $contents = ob_get_clean(); } $this->assertStringStartsWith('%PDF', $contents); $this->assertStringContainsString("id_value={$last_contact['id']}", $contents); @@ -181,7 +180,7 @@ ENDOUTPUT; /** * Test when you choose Csv from the actions dropdown. */ - public function testOutputCsv() { + public function testOutputCsv(): void { // Create many contacts, in particular so that the report would be more // than a one-pager. for ($i = 0; $i < 110; $i++) { diff --git a/tests/phpunit/api/v3/ReportTemplateTest.php b/tests/phpunit/api/v3/ReportTemplateTest.php index 0b6d23f056..e246c4e145 100644 --- a/tests/phpunit/api/v3/ReportTemplateTest.php +++ b/tests/phpunit/api/v3/ReportTemplateTest.php @@ -1282,6 +1282,7 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase { 'civicrm_contact_contact_source_link' => '/index.php?q=civicrm/contact/view&reset=1&cid=' . $this->contactIDs[2], 'civicrm_contact_contact_source_hover' => 'View Contact Summary for this Contact', 'civicrm_activity_activity_type_id_hover' => 'View Activity Record', + 'class' => NULL, ]; $row = $rows[0]; // This link is not relative - skip for now -- 2.25.1