Remove always-true IF
authoreileen <emcnaughton@wikimedia.org>
Tue, 20 Oct 2020 00:47:18 +0000 (13:47 +1300)
committereileen <emcnaughton@wikimedia.org>
Tue, 20 Oct 2020 00:47:18 +0000 (13:47 +1300)
As can be seen by searching for calls to recurLinks recurID is not
an optional parameter for the call - so the IF is extraneous.

I've updated almost all the places that call it to
pass strict-type-casted int (or in one place ensured it already is)
as a bit of 'tightening'

CRM/Contribute/Page/Tab.php
CRM/Contribute/Page/UserDashboard.php
CRM/Member/Page/RecurringContributions.php

index 49b0ec7e7d522a935939bc24b4c9f6eee37c1462..a3fb460d00396dd2bcb2a70c31091fcd9d282035 100644 (file)
@@ -42,12 +42,12 @@ class CRM_Contribute_Page_Tab extends CRM_Core_Page {
    * - Edit
    * - Cancel
    *
-   * @param bool $recurID
+   * @param int $recurID
    * @param string $context
    *
    * @return array
    */
-  public static function recurLinks($recurID = FALSE, $context = 'contribution') {
+  public static function recurLinks(int $recurID, $context = 'contribution') {
     $links = [
       CRM_Core_Action::VIEW => [
         'name' => ts('View'),
@@ -68,30 +68,28 @@ class CRM_Contribute_Page_Tab extends CRM_Core_Page {
       ],
     ];
 
-    if ($recurID) {
-      $paymentProcessorObj = Civi\Payment\System::singleton()->getById(CRM_Contribute_BAO_ContributionRecur::getPaymentProcessorID($recurID));
-      if ($paymentProcessorObj->supports('cancelRecurring')) {
-        unset($links[CRM_Core_Action::DISABLE]['extra'], $links[CRM_Core_Action::DISABLE]['ref']);
-        $links[CRM_Core_Action::DISABLE]['url'] = "civicrm/contribute/unsubscribe";
-        $links[CRM_Core_Action::DISABLE]['qs'] = "reset=1&crid=%%crid%%&cid=%%cid%%&context={$context}";
-      }
+    $paymentProcessorObj = Civi\Payment\System::singleton()->getById(CRM_Contribute_BAO_ContributionRecur::getPaymentProcessorID($recurID));
+    if ($paymentProcessorObj->supports('cancelRecurring')) {
+      unset($links[CRM_Core_Action::DISABLE]['extra'], $links[CRM_Core_Action::DISABLE]['ref']);
+      $links[CRM_Core_Action::DISABLE]['url'] = "civicrm/contribute/unsubscribe";
+      $links[CRM_Core_Action::DISABLE]['qs'] = "reset=1&crid=%%crid%%&cid=%%cid%%&context={$context}";
+    }
 
-      if ($paymentProcessorObj->supports('UpdateSubscriptionBillingInfo')) {
-        $links[CRM_Core_Action::RENEW] = [
-          'name' => ts('Change Billing Details'),
-          'title' => ts('Change Billing Details'),
-          'url' => 'civicrm/contribute/updatebilling',
-          'qs' => "reset=1&crid=%%crid%%&cid=%%cid%%&context={$context}",
-        ];
-      }
+    if ($paymentProcessorObj->supports('UpdateSubscriptionBillingInfo')) {
+      $links[CRM_Core_Action::RENEW] = [
+        'name' => ts('Change Billing Details'),
+        'title' => ts('Change Billing Details'),
+        'url' => 'civicrm/contribute/updatebilling',
+        'qs' => "reset=1&crid=%%crid%%&cid=%%cid%%&context={$context}",
+      ];
+    }
 
-      if (
-      (!CRM_Core_Permission::check('edit contributions') && $context === 'contribution') ||
-      (!$paymentProcessorObj->supports('ChangeSubscriptionAmount')
-        && !$paymentProcessorObj->supports('EditRecurringContribution')
-      )) {
-        unset($links[CRM_Core_Action::UPDATE]);
-      }
+    if (
+    (!CRM_Core_Permission::check('edit contributions') && $context === 'contribution') ||
+    (!$paymentProcessorObj->supports('ChangeSubscriptionAmount')
+      && !$paymentProcessorObj->supports('EditRecurringContribution')
+    )) {
+      unset($links[CRM_Core_Action::UPDATE]);
     }
 
     return $links;
@@ -234,7 +232,7 @@ class CRM_Contribute_Page_Tab extends CRM_Core_Page {
       // Is recurring contribution active?
       $recurContributions[$recurId]['is_active'] = !in_array(CRM_Contribute_PseudoConstant::contributionStatus($recurDetail['contribution_status_id'], 'name'), CRM_Contribute_BAO_ContributionRecur::getInactiveStatuses());
       if ($recurContributions[$recurId]['is_active']) {
-        $actionMask = array_sum(array_keys(self::recurLinks($recurId)));
+        $actionMask = array_sum(array_keys(self::recurLinks((int) $recurId)));
       }
       else {
         $actionMask = CRM_Core_Action::mask([CRM_Core_Permission::VIEW]);
@@ -253,7 +251,7 @@ class CRM_Contribute_Page_Tab extends CRM_Core_Page {
         $recurContributions[$recurId]['contribution_status'] = CRM_Core_PseudoConstant::getLabel('CRM_Contribute_BAO_ContributionRecur', 'contribution_status_id', $recurDetail['contribution_status_id']);
       }
 
-      $recurContributions[$recurId]['action'] = CRM_Core_Action::formLink(self::recurLinks($recurId), $actionMask,
+      $recurContributions[$recurId]['action'] = CRM_Core_Action::formLink(self::recurLinks((int) $recurId), $actionMask,
         [
           'cid' => $this->_contactId,
           'crid' => $recurId,
index dc8707155ce578ede8fdead8121352b88d30f4ca..cad2bc4aa4680b27f30aaeb5eb21eb970a12ada9 100644 (file)
@@ -98,7 +98,7 @@ class CRM_Contribute_Page_UserDashboard extends CRM_Contact_Page_View_UserDashBo
       $values['recur_status'] = $recurStatus[$values['contribution_status_id']];
       $recurRow[$values['id']] = $values;
 
-      $action = array_sum(array_keys(CRM_Contribute_Page_Tab::recurLinks($recur->id, 'dashboard')));
+      $action = array_sum(array_keys(CRM_Contribute_Page_Tab::recurLinks((int) $recur->id, 'dashboard')));
 
       $details = CRM_Contribute_BAO_ContributionRecur::getSubscriptionDetails($recur->id, 'recur');
       $hideUpdate = $details->membership_id & $details->auto_renew;
@@ -107,7 +107,7 @@ class CRM_Contribute_Page_UserDashboard extends CRM_Contact_Page_View_UserDashBo
         $action -= CRM_Core_Action::UPDATE;
       }
 
-      $recurRow[$values['id']]['action'] = CRM_Core_Action::formLink(CRM_Contribute_Page_Tab::recurLinks($recur->id, 'dashboard'),
+      $recurRow[$values['id']]['action'] = CRM_Core_Action::formLink(CRM_Contribute_Page_Tab::recurLinks((int) $recur->id, 'dashboard'),
         $action, [
           'cid' => $this->_contactId,
           'crid' => $values['id'],
index aac582c3892ec1aadfe8868bcdfa4ba77c641c2b..3a497b748ab9d3505576a27c690de7c37125084c 100644 (file)
@@ -79,7 +79,7 @@ class CRM_Member_Page_RecurringContributions extends CRM_Core_Page {
     $contributionStatuses = CRM_Contribute_PseudoConstant::contributionStatus(NULL, 'label');
 
     foreach ($result['values'] as $payment) {
-      $recurringContributionID = $payment['contribution_id.contribution_recur_id.id'];
+      $recurringContributionID = (int) $payment['contribution_id.contribution_recur_id.id'];
       $alreadyProcessed = isset($recurringContributions[$recurringContributionID]);
 
       if ($alreadyProcessed) {
@@ -110,7 +110,7 @@ class CRM_Member_Page_RecurringContributions extends CRM_Core_Page {
    * @param int $recurID
    * @param array $recurringContribution
    */
-  private function setActionsForRecurringContribution($recurID, &$recurringContribution) {
+  private function setActionsForRecurringContribution(int $recurID, &$recurringContribution) {
     $action = array_sum(array_keys(CRM_Contribute_Page_Tab::recurLinks($recurID, 'contribution')));
 
     // no action allowed if it's not active