Merge pull request #19756 from eileenmcnaughton/mem2

[REF] Cleanup code to determine financial_type_id
civicrm · Mar 11, 2021 · dfaeeac · dfaeeac
2 parents c5afdcc + be2e79c
commit dfaeeac
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 22 deletions.
diff --git a/CRM/Financial/BAO/Order.php b/CRM/Financial/BAO/Order.php
@@ -78,6 +78,13 @@ class CRM_Financial_BAO_Order {
    */
   protected $priceFieldMetadata = [];
 
+  /**
+   * Metadata for price sets.
+   *
+   * @var array
+   */
+  protected $priceSetMetadata = [];
+
   /**
    * Get form object.
    *
@@ -245,14 +252,40 @@ public function getPriceFieldSpec(int $id) :array {
    */
   public function getPriceFieldsMetadata(): array {
     if (empty($this->priceFieldMetadata)) {
-      $this->priceFieldMetadata = CRM_Price_BAO_PriceSet::getCachedPriceSetDetail($this->getPriceSetID())['fields'];
+      $this->getPriceSetMetadata();
       if ($this->getForm()) {
         CRM_Utils_Hook::buildAmount($this->form->getFormContext(), $this->form, $this->priceFieldMetadata);
       }
     }
     return $this->priceFieldMetadata;
   }
 
+  /**
+   * Get the metadata for the fields in the price set.
+   *
+   * @return array
+   */
+  public function getPriceSetMetadata(): array {
+    if (empty($this->priceSetMetadata)) {
+      $priceSetMetadata = CRM_Price_BAO_PriceSet::getCachedPriceSetDetail($this->getPriceSetID());
+      $this->priceFieldMetadata = $priceSetMetadata['fields'];
+      unset($priceSetMetadata['fields']);
+      $this->priceSetMetadata = $priceSetMetadata;
+    }
+    return $this->priceSetMetadata;
+  }
+
+  /**
+   * Get the financial type id for the order.
+   *
+   * This may differ to the line items....
+   *
+   * @return int
+   */
+  public function getFinancialTypeID(): int {
+    return (int) $this->getOverrideFinancialTypeID() ?: $this->getPriceSetMetadata()['financial_type_id'];
+  }
+
   /**
    * Set the price field selection from an array of params containing price fields.
    *

diff --git a/CRM/Member/Form/Membership.php b/CRM/Member/Form/Membership.php
@@ -1014,10 +1014,7 @@ public function submit(): void {
       $this->_priceSet,
       $formValues
     );
-    if (empty($formValues['financial_type_id'])) {
-      $formValues['financial_type_id'] = $this->_priceSet['financial_type_id'];
-    }
-
+    $formValues['financial_type_id'] = $this->getFinancialTypeID();
     $membershipTypeValues = [];
     foreach ($this->_memTypeSelected as $memType) {
       $membershipTypeValues[$memType]['membership_type_id'] = $memType;
@@ -1146,7 +1143,6 @@ public function submit(): void {
     if (!empty($formValues['record_contribution'])) {
       $recordContribution = [
         'total_amount',
-        'financial_type_id',
         'payment_instrument_id',
         'trxn_id',
         'contribution_status_id',
@@ -1160,6 +1156,7 @@ public function submit(): void {
       foreach ($recordContribution as $f) {
         $params[$f] = $formValues[$f] ?? NULL;
       }
+      $params['financial_type_id'] = $this->getFinancialTypeID();
 
       if (empty($formValues['source'])) {
         $params['contribution_source'] = ts('%1 Membership: Offline signup (by %2)', [
@@ -1187,7 +1184,7 @@ public function submit(): void {
 
       //insert financial type name in receipt.
       $formValues['contributionType_name'] = CRM_Core_DAO::getFieldValue('CRM_Financial_DAO_FinancialType',
-        $formValues['financial_type_id']
+        $this->getFinancialTypeID()
       );
     }
 
@@ -1203,16 +1200,7 @@ public function submit(): void {
       //CRM-20264 : Store CC type and number (last 4 digit) during backoffice or online payment
       $params['card_type_id'] = $this->_params['card_type_id'] ?? NULL;
       $params['pan_truncation'] = $this->_params['pan_truncation'] ?? NULL;
-
-      if (!$isQuickConfig) {
-        $params['financial_type_id'] = CRM_Core_DAO::getFieldValue('CRM_Price_DAO_PriceSet',
-          $this->_priceSetId,
-          'financial_type_id'
-        );
-      }
-      else {
-        $params['financial_type_id'] = $formValues['financial_type_id'] ?? NULL;
-      }
+      $params['financial_type_id'] = $this->getFinancialTypeID();
 
       //get the payment processor id as per mode. Try removing in favour of beginPostProcess.
       $params['payment_processor_id'] = $formValues['payment_processor_id'] = $this->_paymentProcessor['id'];
@@ -1223,7 +1211,7 @@ public function submit(): void {
       $formValues['currencyID'] = $this->getCurrency();
       $formValues['description'] = ts("Contribution submitted by a staff person using member's credit card for signup");
       $formValues['invoiceID'] = $this->getInvoiceID();
-      $formValues['financial_type_id'] = $params['financial_type_id'];
+      $formValues['financial_type_id'] = $this->getFinancialTypeID();
 
       // at this point we've created a contact and stored its address etc
       // all the payment processors expect the name and address to be in the
@@ -1260,7 +1248,7 @@ public function submit(): void {
             'campaign_id' => $paymentParams['campaign_id'] ?? NULL,
             'source' => CRM_Utils_Array::value('source', $paymentParams, CRM_Utils_Array::value('description', $paymentParams)),
             'payment_instrument_id' => $paymentInstrumentID,
-            'financial_type_id' => $params['financial_type_id'],
+            'financial_type_id' => $this->getFinancialTypeID(),
             'receive_date' => CRM_Utils_Time::date('YmdHis'),
             'tax_amount' => $params['tax_amount'] ?? NULL,
             'invoice_id' => $this->getInvoiceID(),
@@ -1833,7 +1821,7 @@ protected function processContribution(
     $contactID = $contributionParams['contact_id'];
 
     // add these values for the recurringContrib function ,CRM-10188
-    $params['financial_type_id'] = $contributionParams['financial_type_id'];
+    $params['financial_type_id'] = $this->getFinancialTypeID();
     $params['is_recur'] = TRUE;
     $params['payment_instrument_id'] = $contributionParams['payment_instrument_id'] ?? NULL;
     $recurringContributionID = $this->legacyProcessRecurringContribution($params, $contactID);
@@ -1880,7 +1868,7 @@ protected function legacyProcessRecurringContribution(array $params, $contactID)
     $recurParams['frequency_unit'] = $params['frequency_unit'] ?? NULL;
     $recurParams['frequency_interval'] = $params['frequency_interval'] ?? NULL;
     $recurParams['installments'] = $params['installments'] ?? NULL;
-    $recurParams['financial_type_id'] = $params['financial_type_id'];
+    $recurParams['financial_type_id'] = $this->getFinancialTypeID();
     $recurParams['currency'] = $params['currency'] ?? NULL;
     $recurParams['payment_instrument_id'] = $params['payment_instrument_id'];
 
@@ -1912,4 +1900,16 @@ protected function isTest(): int {
     return ($this->_mode === 'test') ? TRUE : FALSE;
   }
 
+  /**
+   * Get the financial type id relevant to the contribution.
+   *
+   * Financial type id is optional when price sets are in use.
+   * Otherwise they are required for the form to submit.
+   *
+   * @return int
+   */
+  protected function getFinancialTypeID(): int {
+    return (int) $this->getSubmittedValue('financial_type_id') ?: $this->order->getFinancialTypeID();
+  }
+
 }
diff --git a/tests/phpunit/CRM/Member/Form/MembershipTest.php b/tests/phpunit/CRM/Member/Form/MembershipTest.php
@@ -833,11 +833,13 @@ public function testSubmitRecurTwoRows(): void {
       'frequency_interval' => 1,
       'frequency_unit' => 'month',
       'membership_type_id' => NULL,
+      // Set financial type id to null to check it is retrieved from the price set.
+      'financial_type_id' => NULL,
     ];
     $form->testSubmit(array_merge($this->getBaseSubmitParams(), $priceParams));
     $memberships = $this->callAPISuccess('Membership', 'get')['values'];
     $this->assertCount(2, $memberships);
-    $this->callAPISuccessGetSingle('Contribution', []);
+    $this->callAPISuccessGetSingle('Contribution', ['financial_type_id' => 1]);
     $this->callAPISuccessGetCount('MembershipPayment', [], 2);
     $lines = $this->callAPISuccess('LineItem', 'get', ['sequential' => 1])['values'];
     $this->assertCount(2, $lines);