Fix contribution tab to work with escape by default

This removes some isset that affect contact contribution tab if escape on output is enabled.

This might regress some smarty e-notices. I think if tests pass that is OK at this stage
as we never eliminted them & getting to security enablable seems like a higher priority

diff --git a/CRM/Contact/BAO/Query.php b/CRM/Contact/BAO/Query.php
index cbe7debf28a06ac4cb131785994ffe8c30ad6dfc..54e6c641e25bec41b1a45a88afb21276a2288f2c 100644 (file)
--- a/CRM/Contact/BAO/Query.php
+++ b/CRM/Contact/BAO/Query.php
@@ -5136,7 +5136,7 @@ civicrm_relationship.start_date > {$today}
 
     $this->appendFinancialTypeWhereAndFromToQueryStrings($where, $from);
 
-    $summary = ['total' => []];
+    $summary = ['total' => [], 'soft_credit' => ['count' => 0, 'avg' => 0, 'amount' => 0]];
     $this->addBasicStatsToSummary($summary, $where, $from);
 
     if (CRM_Contribute_BAO_Query::isSoftCreditOptionEnabled()) {
@@ -6701,7 +6701,7 @@ AND   displayRelType.is_active = 1
     GROUP BY currency";
 
     $dao = CRM_Core_DAO::executeQuery($query);
-
+    $summary['cancel'] = ['count' => 0, 'amount' => 0, 'avg' => 0];
     if ($dao->N <= 1) {
       if ($dao->fetch()) {
         $summary['cancel']['count'] = $dao->cancel_count;

diff --git a/templates/CRM/Contribute/Form/Selector.tpl b/templates/CRM/Contribute/Form/Selector.tpl
index 0ab49308ebeae8900cec9b218759c05db28d88db..d5d39808d54df31b5982e66462c89a727767189a 100644 (file)
--- a/templates/CRM/Contribute/Form/Selector.tpl
+++ b/templates/CRM/Contribute/Form/Selector.tpl
@@ -22,7 +22,7 @@
       {/if}
       {foreach from=$columnHeaders item=header}
         <th scope="col">
-          {if isset($header.sort)}
+          {if $header.sort}
             {assign var='key' value=$header.sort}
             {$sort->_response.$key.link}
           {elseif (!empty($header.name))}
@@ -57,12 +57,12 @@
         </td>
       {foreach from=$columnHeaders item=column}
           {assign var='columnName' value=''}
-          {if isset($column.field_name)}
+          {if $column.field_name}
             {assign var='columnName' value=$column.field_name}
           {/if}
         {if !$columnName}{* if field_name has not been set skip, this helps with not changing anything not specifically edited *}
         {elseif $columnName === 'total_amount'}{* rendered above as soft credit columns = confusing *}
-        {elseif isset($column.type) && $column.type === 'actions'}{* rendered below as soft credit column handling = not fixed *}
+        {elseif $column.type === 'actions'}{* rendered below as soft credit column handling = not fixed *}
         {elseif $columnName == 'contribution_status'}
           <td class="crm-contribution-status">
             {$row.contribution_status}<br/>
@@ -71,13 +71,13 @@
             {/if}
           </td>
         {else}
-          {if isset($column.type) && $column.type == 'date'}
+          {if $column.type == 'date'}
             <td class="crm-contribution-{$columnName}">
               {$row.$columnName|crmDate}
             </td>
           {else}
-          <td class="crm-{$columnName} crm-{$columnName}_{if isset($row.columnName)}{$row.columnName}{/if}">
-            {if isset($row.$columnName)}{$row.$columnName}{/if}
+          <td class="crm-{$columnName} crm-{$columnName}_{if $row.columnName}{$row.columnName}{/if}">
+            {$row.$columnName}
           </td>
           {/if}
         {/if}

diff --git a/templates/CRM/Contribute/Page/ContributionTotals.tpl b/templates/CRM/Contribute/Page/ContributionTotals.tpl
index 0a105df7774bfc20ad0aee34c2c11f0e1d275adc..f95aa06c10b38da7b8e2b705b22975f0a3b1eeaa 100644 (file)
--- a/templates/CRM/Contribute/Page/ContributionTotals.tpl
+++ b/templates/CRM/Contribute/Page/ContributionTotals.tpl
@@ -29,11 +29,11 @@
             <th class="right"> &nbsp; {ts}# Completed{/ts} &ndash; {$contributionSummary.total.count}</th>
             <th class="right contriTotalRight"> &nbsp; {ts}Avg{/ts} &ndash; {$contributionSummary.total.avg}</th>
           {/if}
-          {if isset($contributionSummary.cancel.amount)}
+          {if $contributionSummary.cancel.amount}
             <th class="disabled right contriTotalRight"> &nbsp; {ts}Cancelled/Refunded{/ts} &ndash; {$contributionSummary.cancel.amount}</th>
           {/if}
       </tr>
-      {if isset($contributionSummary.soft_credit.count)}
+      {if $contributionSummary.soft_credit.count}
         {include file="CRM/Contribute/Page/ContributionSoftTotals.tpl" softCreditTotals=$contributionSummary.soft_credit}
       {/if}
     {/if}

diff --git a/tests/phpunit/CRM/Contact/BAO/QueryTest.php b/tests/phpunit/CRM/Contact/BAO/QueryTest.php
index 2147bb609ad33fe3c4119b9088fa5d4aca4bedc8..82e1f779414afc6fd835e016a09d5cb67b0ce780 100644 (file)
--- a/tests/phpunit/CRM/Contact/BAO/QueryTest.php
+++ b/tests/phpunit/CRM/Contact/BAO/QueryTest.php
@@ -1103,7 +1103,7 @@ civicrm_relationship.is_active = 1 AND
    *
    * @throws \CRM_Core_Exception
    */
-  public function testGetSummaryQueryWithFinancialACLDisabled() {
+  public function testGetSummaryQueryWithFinancialACLDisabled(): void {
     $this->createContributionsForSummaryQueryTests();
 
     // Test the function directly
@@ -1128,6 +1128,11 @@ civicrm_relationship.is_active = 1 AND
         'amount' => '$ 100.00',
         'avg' => '$ 50.00',
       ],
+      'soft_credit' => [
+        'count' => 0,
+        'avg' => 0,
+        'amount' => 0,
+      ],
     ], $summary);
   }
 
@@ -1136,7 +1141,7 @@ civicrm_relationship.is_active = 1 AND
    *
    * @throws \CRM_Core_Exception
    */
-  public function testGetSummaryQueryWithFinancialACLEnabled() {
+  public function testGetSummaryQueryWithFinancialACLEnabled(): void {
     $where = $from = NULL;
     $this->createContributionsForSummaryQueryTests();
     $this->enableFinancialACLs();
@@ -1166,6 +1171,11 @@ civicrm_relationship.is_active = 1 AND
         'amount' => '$ 50.00',
         'avg' => '$ 50.00',
       ],
+      'soft_credit' => [
+        'count' => 0,
+        'avg' => 0,
+        'amount' => 0,
+      ],
     ], $summary);
     $this->disableFinancialACLs();
   }