Enotice fix

civicrm · Nov 22, 2021 · 6e1fbb9 · 6e1fbb9
1 parent c8cf43a
commit 6e1fbb9
Show file tree

Hide file tree

Showing 12 changed files with 86 additions and 57 deletions.
diff --git 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 @@ public function formatDisplay(&$rows, $pager = TRUE) {
     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,13 +2497,25 @@ public function formatDisplay(&$rows, $pager = TRUE) {
       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.
     if (!empty($rows)) {
       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 @@ public function moveSummaryColumnsToTheRightHandSide() {
    * @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 @@ public function countStat(&$statistics, $count) {
     $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 @@ public function groupByStat(&$statistics) {
         'value' => implode(' & ', $combinations),
       ];
     }
+    else {
+      // prevents an e-notice in statistics.tpl.
+      $statistics['groups'] = [];
+    }
   }
 
   /**
@@ -3485,6 +3513,10 @@ public function filterStat(&$statistics) {
           }
         }
       }
+      else {
+        // Prevents an e-notice in statistics.tpl.
+        $statistics['filters'] = [];
+      }
     }
   }
 

diff --git a/CRM/Report/Utils/Report.php b/CRM/Report/Utils/Report.php
@@ -394,7 +394,7 @@ public static function processReport($params) {
     $_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
@@ -16,7 +16,7 @@
 {elseif $section eq 2}
   <div class="crm-block crm-content-block crm-report-layoutTable-form-block">
     {*include the table layout*}
-    {if empty($chartEnabled) || empty($chartSupported)}
+    {if !$chartEnabled || empty($chartSupported)}
       {include file="CRM/Report/Form/Layout/Table.tpl"}
     {/if}
   </div>
@@ -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}
     <br />
     {*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"}
   </div>

diff --git a/templates/CRM/Report/Form/Contact/Detail.tpl b/templates/CRM/Report/Form/Contact/Detail.tpl
@@ -17,7 +17,7 @@
 <div class="crm-block crm-content-block crm-report-form-block">
 {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}
         <div class="report-pager">
@@ -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
@@ -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}
+
 <table class="report-layout display">
    {foreach from=$rows item=row}
      <thead><th colspan=16><font color="black" size="3">{$row.label}</font></th></thead>
-   
+
      <thead class="sticky">
      <tr>
        {foreach from=$columnHeaders item=label key=header}
@@ -43,7 +43,7 @@
 
   <br />
   {*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"}
 </div>

diff --git 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 @@
         </div>
         {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
@@ -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)}
   <div class='crm-chart'>
     {if $outputMode eq 'print' OR $outputMode eq 'pdf'}
       <img src="{$uploadURL|cat:$chartId}.png" />

diff --git 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}
   <p>{ts}None found.{/ts}</p>
 {else}
-    {if !empty($pager) and $pager->_response and $pager->_response.numPages > 1}
+    {if $pager and $pager->_response and $pager->_response.numPages > 1}
         <div class="report-pager">
             {include file="CRM/common/pager.tpl" location="top"}
         </div>
@@ -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}
                        <th colspan={$header.colspan}>{$header.title|escape}</th>
                       {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 *}
             <thead class="sticky">
             <tr>
                 {$tableHeader}
@@ -92,14 +92,14 @@
 
         {foreach from=$rows item=row key=rowid}
            {eval var=$sectionHeaderTemplate}
-            <tr  class="{cycle values="odd-row,even-row"} {if !empty($row.class)}{$row.class}{/if} crm-report" id="crm-report_{$rowid}">
+            <tr  class="{cycle values="odd-row,even-row"} {if $row.class}{$row.class}{/if} crm-report" id="crm-report_{$rowid}">
                 {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"}
                     <td class="crm-report-{$field}{if $header.type eq 1024 OR $header.type eq 1 OR $header.type eq 512} report-contents-right{elseif $row.$field eq 'Subtotal'} report-label{/if}">
                         {if !empty($row.$fieldLink)}
-                            <a title="{$row.$fieldHover|escape}" href="{$row.$fieldLink}"  {if !empty($row.$fieldClass)} class="{$row.$fieldClass}"{/if}>
+                            <a title="{$row.$fieldHover|escape}" href="{$row.$fieldLink}"  {if array_key_exists($fieldClass, $row)} class="{$row.$fieldClass}"{/if}>
                         {/if}
 
                         {if is_array($row.$field)}
@@ -136,7 +136,7 @@
             </tr>
         {/foreach}
 
-        {if !empty($grandStat)}
+        {if $grandStat}
             {* foreach from=$grandStat item=row*}
             <tr class="total-row">
                 {foreach from=$columnHeaders item=header key=field}
@@ -156,7 +156,7 @@
             {* /foreach*}
         {/if}
     </table>
-    {if !empty($pager) and $pager->_response and $pager->_response.numPages > 1}
+    {if $pager and $pager->_response and $pager->_response.numPages > 1}
         <div class="report-pager">
             {include file="CRM/common/pager.tpl" location="bottom"}
         </div>

diff --git 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)}
     <h1>{$reportTitle}</h1>
     <div id="report-date">{if !empty($reportDate)}{$reportDate}{/if}</div>
   {/if}
   {if !empty($statistics)}
     <table class="report-layout statistics-table">
-      {if !empty($statistics.groups)}
-        {foreach from=$statistics.groups item=row}
-          <tr>
-            <th class="statistics" scope="row">{$row.title}</th>
-            <td>{$row.value|escape}</td>
-          </tr>
-        {/foreach}
-      {/if}
-      {if !empty($statistics.filters)}
-        {foreach from=$statistics.filters item=row}
-          <tr>
-            <th class="statistics" scope="row">{$row.title}</th>
-            <td>{$row.value|escape}</td>
-          </tr>
-        {/foreach}
-      {/if}
+      {foreach from=$statistics.groups item=row}
+        <tr>
+          <th class="statistics" scope="row">{$row.title}</th>
+          <td>{$row.value|escape}</td>
+        </tr>
+      {/foreach}
+      {foreach from=$statistics.filters item=row}
+        <tr>
+          <th class="statistics" scope="row">{$row.title}</th>
+          <td>{$row.value|escape}</td>
+        </tr>
+      {/foreach}
     </table>
   {/if}
 {/if}
 
-{if !empty($bottom) and !empty($rows) and !empty($statistics)}
+{if $bottom and !empty($rows) and !empty($statistics)}
   <table class="report-layout">
-    {if !empty($statistics.counts)}
+    {if $statistics.counts}
       {foreach from=$statistics.counts item=row}
         <tr>
           <th class="statistics" scope="row">{$row.title}</th>
           <td>
-            {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
@@ -147,12 +147,12 @@ public function testPostalCodeSearchReportOutput($reportClass, $inputParams, $da
    * 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 @@ public function testMultipleSoftCredits() {
         'rowCount' => [
           'title' => 'Row(s) Listed',
           'value' => 1,
+          'type' => CRM_Utils_Type::T_INT,
         ],
         'amount' => [
           'title' => 'Total Amount (Contributions)',