Fix notices on manage premiums page
authorEileen McNaughton <emcnaughton@wikimedia.org>
Tue, 21 Nov 2023 04:55:36 +0000 (17:55 +1300)
committerEileen McNaughton <emcnaughton@wikimedia.org>
Tue, 21 Nov 2023 05:01:39 +0000 (18:01 +1300)
This addresses notices at
https://dmaster.localhost:32353/civicrm/admin/contribute/managePremiums?reset=1

These are probably also addressed by the admin ui / sk conversion but note that this also
covers the tab on the contribution page

Note that combined with https://github.com/civicrm/civicrm-core/pull/28238
this fixes most of the issues I encountered trying to configure a premimum
but there are still a bunch of notices when I attempt to use one.

Also, having switched to using apiv4 I have added some tpl escaping

CRM/Contribute/Page/ManagePremiums.php
templates/CRM/Contribute/Form/Contribution/PremiumBlock.tpl
templates/CRM/Contribute/Page/ManagePremiums.tpl
templates/CRM/Contribute/Page/Premium.tpl

index 0c08a37687d28e8eea7a22ddb00217da081953f8..9e7a45334101297daa0d59bb5e88401665d80b9b 100644 (file)
@@ -15,6 +15,8 @@
  * @copyright CiviCRM LLC https://civicrm.org/licensing
  */
 
+use Civi\Api4\Product;
+
 /**
  * Page for displaying list of Premiums.
  */
@@ -27,7 +29,7 @@ class CRM_Contribute_Page_ManagePremiums extends CRM_Core_Page_Basic {
    *
    * @var array
    */
-  public static $_links = NULL;
+  public static $_links;
 
   /**
    * Get BAO Name.
@@ -35,7 +37,7 @@ class CRM_Contribute_Page_ManagePremiums extends CRM_Core_Page_Basic {
    * @return string
    *   Classname of BAO.
    */
-  public function getBAOName() {
+  public function getBAOName(): string {
     return 'CRM_Contribute_BAO_Product';
   }
 
@@ -45,7 +47,7 @@ class CRM_Contribute_Page_ManagePremiums extends CRM_Core_Page_Basic {
    * @return array
    *   (reference) of action links
    */
-  public function &links() {
+  public function &links(): array {
     if (!(self::$_links)) {
       self::$_links = [
         CRM_Core_Action::UPDATE => [
@@ -92,8 +94,10 @@ class CRM_Contribute_Page_ManagePremiums extends CRM_Core_Page_Basic {
    * This method is called after the page is created. It checks for the
    * type of action and executes that action.
    * Finally it calls the parent's run method.
+   *
+   * @throws \CRM_Core_Exception
    */
-  public function run() {
+  public function run(): void {
     $id = $this->getIdAndAction();
 
     // what action to take ?
@@ -104,45 +108,42 @@ class CRM_Contribute_Page_ManagePremiums extends CRM_Core_Page_Basic {
     $this->browse();
 
     // parent run
-    return CRM_Core_Page::run();
+    CRM_Core_Page::run();
   }
 
   /**
    * Browse all custom data groups.
+   *
+   * @throws \CRM_Core_Exception
    */
-  public function browse() {
-    // get all custom groups sorted by weight
-    $premiums = [];
-    $dao = new CRM_Contribute_DAO_Product();
-    $dao->orderBy('name');
-    $dao->find();
-
-    while ($dao->fetch()) {
-      $premiums[$dao->id] = [];
-      CRM_Core_DAO::storeValues($dao, $premiums[$dao->id]);
-      // form all action links
+  public function browse(): void {
+    // We could probably use checkPermissions here but historically didn't
+    // so have set it to FALSE to be safe while converting to api use.
+    $premiums = Product::get(FALSE)->addOrderBy('name')
+      ->addSelect('*', 'financial_type_id:name')
+      ->execute();
+
+    foreach ($premiums as $index => $premium) {
       $action = array_sum(array_keys($this->links()));
 
-      if ($dao->is_active) {
+      if ($premium['is_active']) {
         $action -= CRM_Core_Action::ENABLE;
       }
       else {
         $action -= CRM_Core_Action::DISABLE;
       }
 
-      $premiums[$dao->id]['action'] = CRM_Core_Action::formLink($this->links(),
+      $premiums[$index]['action'] = CRM_Core_Action::formLink($this->links(),
         $action,
-        ['id' => $dao->id],
+        ['id' => $premium['id']],
         ts('more'),
         FALSE,
         'premium.manage.row',
         'Premium',
-        $dao->id
+        $premium['id']
       );
-      // Financial Type
-      if (!empty($dao->financial_type_id)) {
-        $premiums[$dao->id]['financial_type'] = CRM_Core_PseudoConstant::getLabel('CRM_Contribute_BAO_Product', 'financial_type_id', $dao->financial_type_id);
-      }
+      $premiums[$index]['financial_type'] = $premium['financial_type_id:name'];
+      $premiums[$index]['class'] = '';
     }
     $this->assign('rows', $premiums);
   }
@@ -153,7 +154,7 @@ class CRM_Contribute_Page_ManagePremiums extends CRM_Core_Page_Basic {
    * @return string
    *   Classname of edit form.
    */
-  public function editForm() {
+  public function editForm(): string {
     return 'CRM_Contribute_Form_ManagePremiums';
   }
 
@@ -163,7 +164,7 @@ class CRM_Contribute_Page_ManagePremiums extends CRM_Core_Page_Basic {
    * @return string
    *   name of this page.
    */
-  public function editName() {
+  public function editName(): string {
     return 'Manage Premiums';
   }
 
@@ -175,7 +176,7 @@ class CRM_Contribute_Page_ManagePremiums extends CRM_Core_Page_Basic {
    * @return string
    *   user context.
    */
-  public function userContext($mode = NULL) {
+  public function userContext($mode = NULL): string {
     return 'civicrm/admin/contribute/managePremiums';
   }
 
index 309ced9349f353f81c6340b1b818389cf557b73a..9ad512c6e2bbece72b798eadeea2cf996b7fbbca 100644 (file)
       {foreach from=$products item=row}
         <div class="premium {if $showPremium}premium-selectable{/if}" id="premium_id-{$row.id}" min_contribution="{$row.min_contribution}">
           <div class="premium-short">
-            {if $row.thumbnail}<div class="premium-short-thumbnail"><img src="{$row.thumbnail}" alt="{$row.name|escape}" /></div>{/if}
-            <div class="premium-short-content">{$row.name}</div>
+            {if $row.thumbnail}<div class="premium-short-thumbnail"><img src="{$row.thumbnail|purify}" alt="{$row.name|escape}" /></div>{/if}
+            <div class="premium-short-content">{$row.name|escape}</div>
             <div style="clear:both"></div>
           </div>
 
           <div class="premium-full">
-            <div class="premium-full-image">{if $row.image}<img src="{$row.image}" alt="{$row.name|escape}" />{/if}</div>
+            <div class="premium-full-image">{if $row.image}<img src="{$row.image|escape}" alt="{$row.name|escape}" />{/if}</div>
             <div class="premium-full-content">
-              <div class="premium-full-title">{$row.name}</div>
+              <div class="premium-full-title">{$row.name|escape}</div>
               <div class="premium-full-disabled">
                 {ts 1=$row.min_contribution|crmMoney}You must contribute at least %1 to get this item{/ts}<br/>
                 <button type="button" amount="{$row.min_contribution}">
@@ -67,7 +67,7 @@
                 </button>
               </div>
               <div class="premium-full-description">
-                {$row.description}
+                {$row.description|escape}
               </div>
               {if $showSelectOptions}
                 {assign var="pid" value="options_"|cat:$row.id}
@@ -78,7 +78,7 @@
                 {/if}
               {else}
                 <div class="premium-full-options">
-                  <p><strong>{$row.options}</strong></p>
+                  <p><strong>{$row.options|purify}</strong></p>
                 </div>
               {/if}
               {if (($premiumBlock.premiums_display_min_contribution AND $context EQ "makeContribution") OR $preview EQ 1) AND $row.min_contribution GT 0}
index 9760a62a99368fe0e7d28af2b3a59aa4130bff5e..e73c541c650e11010e8199bcff2846d4b8645da1 100644 (file)
            </tr>
           </thead>
         {foreach from=$rows item=row}
-        <tr id="product-{$row.id}" class="crm-entity {cycle values="odd-row,even-row"}{if !empty($row.class)} {$row.class}{/if}{if NOT $row.is_active} disabled{/if}">
-          <td class="crm-contribution-form-block-name crm-editable" data-field="name">{$row.name}</td>
-          <td class="crm-contribution-form-block-sku crm-editable" data-field="sku">{$row.sku}</td>
+        <tr id="product-{$row.id}" class="crm-entity {cycle values="odd-row,even-row"} {$row.class}{if NOT $row.is_active} disabled{/if}">
+          <td class="crm-contribution-form-block-name crm-editable" data-field="name">{$row.name|escape}</td>
+          <td class="crm-contribution-form-block-sku crm-editable" data-field="sku">{$row.sku|escape}</td>
           <td class="crm-contribution-form-block-price">{$row.price|crmMoney}</td>
           <td class="crm-contribution-form-block-min_contribution">{$row.min_contribution|crmMoney}</td>
           <td class="crm-contribution-form-block-cost">{$row.cost|crmMoney}</td>
-          <td class="crm-contribution-form-block-financial_type">{$row.financial_type}</td>
+          <td class="crm-contribution-form-block-financial_type">{$row.financial_type|escape}</td>
           <td id="row_{$row.id}_status" >{if $row.is_active eq 1} {ts}Yes{/ts} {else} {ts}No{/ts} {/if}</td>
           <td id={$row.id}>{$row.action|smarty:nodefaults|replace:'xx':$row.id}</td>
         </tr>
index 377ff12a2fd9edac563516c54a3be77e23fa9af8..2899564840217724603ddf62c6da02b2cbb5a5f6 100644 (file)
         </tr>
         {foreach from=$rows item=row}
         <tr class="{cycle values='odd-row,even-row'}{if !empty($row.class)} {$row.class}{/if}{if NOT $row.is_active} disabled{/if}">
-          <td class="crm-contribution-form-block-product_name">{$row.product_name}</td>
-          <td class="crm-contribution-form-block-sku">{$row.sku}</td>
+          <td class="crm-contribution-form-block-product_name">{$row.product_name|escape}</td>
+          <td class="crm-contribution-form-block-sku">{$row.sku|escape}</td>
           <td class="crm-contribution-form-block-price">{$row.price|crmMoney}</td>
           <td class="crm-contribution-form-block-min_contribution">{$row.min_contribution|crmMoney}</td>
           <td class="crm-contribution-form-block-cost">{$row.cost|crmMoney}</td>
-          <td class="crm-contribution-form-block-financial_type">{$row.financial_type}</td>
+          <td class="crm-contribution-form-block-financial_type">{$row.financial_type|escape}</td>
           <td class="nowrap crm-contribution-form-block-weight">{$row.weight|smarty:nodefaults}</td>
-          <td class="crm-contribution-form-block-action">{$row.action}</td>
+          <td class="crm-contribution-form-block-action">{$row.action|smarty:nodefaults}</td>
         </tr>
         {/foreach}
         </table>