Merge pull request #22684 from eileenmcnaughton/contacl

Move acl check for contributionView to the extension
civicrm · Feb 10, 2022 · 0ee4cba · 0ee4cba
2 parents 3a7d033 + 56c911b
commit 0ee4cba
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 10 deletions.
diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php
@@ -316,7 +316,7 @@ public static function getValues($params, &$values = [], &$ids = []) {
   }
 
   /**
-   * Get the values and resolve the most common mappings.
+   * Deprecated contact.get call.
    *
    * Since contribution status is resolved in almost every function that calls getValues it makes
    * sense to have an extra function to resolve it rather than repeat the code.
@@ -330,6 +330,8 @@ public static function getValues($params, &$values = [], &$ids = []) {
    * @return array
    *   Array of the found contribution.
    * @throws CRM_Core_Exception
+   *
+   * @deprecated
    */
   public static function getValuesWithMappings($params) {
     $values = $ids = [];

diff --git a/CRM/Contribute/Form/ContributionView.php b/CRM/Contribute/Form/ContributionView.php
@@ -9,6 +9,8 @@
  +--------------------------------------------------------------------+
  */
 
+use Civi\Api4\Contribution;
+
 /**
  *
  * @package CRM
@@ -24,16 +26,23 @@ class CRM_Contribute_Form_ContributionView extends CRM_Core_Form {
    * Set variables up before form is built.
    */
   public function preProcess() {
-    $id = $this->get('id');
-    if (empty($id)) {
-      throw new CRM_Core_Exception('Contribution ID is required');
-    }
-    $params = ['id' => $id];
+    $id = $this->getID();
+
     $context = CRM_Utils_Request::retrieve('context', 'Alphanumeric', $this);
     $this->assign('context', $context);
 
-    $values = CRM_Contribute_BAO_Contribution::getValuesWithMappings($params);
+    // Note than this get could be restricted by ACLs in an extension
+    $contribution = Contribution::get(TRUE)->addWhere('id', '=', $id)->addSelect('*')->execute()->first();
+    if (empty($contribution)) {
+      CRM_Core_Error::statusBounce(ts('Access to contribution not permitted'));
+    }
+    // We just cast here because it was traditionally an array called values - would be better
+    // just to use 'contribution'.
+    $values = (array) $contribution;
+    $contributionStatus = CRM_Core_PseudoConstant::getName('CRM_Contribute_BAO_Contribution', 'contribution_status_id', $values['contribution_status_id']);
 
+    // @todo - it might have been better to create a new form that extends this
+    // for template contributions rather than overloading this form.
     $force_create_template = CRM_Utils_Request::retrieve('force_create_template', 'Boolean', $this, FALSE, FALSE);
     if ($force_create_template && !empty($values['contribution_recur_id']) && empty($values['is_template'])) {
       // Create a template contribution.
@@ -70,7 +79,7 @@ public function preProcess() {
     }
 
     // get received into i.e to_financial_account_id from last trxn
-    $financialTrxnId = CRM_Core_BAO_FinancialTrxn::getFinancialTrxnId($values['contribution_id'], 'DESC');
+    $financialTrxnId = CRM_Core_BAO_FinancialTrxn::getFinancialTrxnId($this->getID(), 'DESC');
     $values['to_financial_account'] = '';
     if (!empty($financialTrxnId['financialTrxnId'])) {
       $values['to_financial_account_id'] = CRM_Core_DAO::getFieldValue('CRM_Financial_DAO_FinancialTrxn', $financialTrxnId['financialTrxnId'], 'to_financial_account_id');
@@ -129,7 +138,7 @@ public function preProcess() {
     }
 
     //assign soft credit record if exists.
-    $SCRecords = CRM_Contribute_BAO_ContributionSoft::getSoftContribution($values['contribution_id'], TRUE);
+    $SCRecords = CRM_Contribute_BAO_ContributionSoft::getSoftContribution($this->getID(), TRUE);
     if (!empty($SCRecords['soft_credit'])) {
       $this->assign('softContributions', $SCRecords['soft_credit']);
       unset($SCRecords['soft_credit']);
@@ -150,7 +159,7 @@ public function preProcess() {
       $campaigns = CRM_Campaign_BAO_Campaign::getCampaigns($campaignId);
       $values['campaign'] = $campaigns[$campaignId];
     }
-    if ($values['contribution_status'] == 'Refunded') {
+    if ($contributionStatus === 'Refunded') {
       $this->assign('refund_trxn_id', CRM_Core_BAO_FinancialTrxn::getRefundTransactionTrxnID($id));
     }
 
@@ -260,4 +269,17 @@ protected function assignPaymentInfoBlock($id) {
     return $title;
   }
 
+  /**
+   * Get the contribution ID.
+   *
+   * @return int
+   */
+  private function getID(): int {
+    $id = $this->get('id');
+    if (empty($id)) {
+      CRM_Core_Error::statusBounce('Contribution ID is required');
+    }
+    return $id;
+  }
+
 }