dev/core#1409 Remove net_amount from Addtional Payment form

This is causing a validation problem. We used to have an issue on the contribution form which we eventually
resolved by removing net_amount as it's best calculated anyway

In order to make this changed I had to ensure Payment.create adds the net_amount & had
to do a couple of tweaks for the test to pass

diff --git a/CRM/Contribute/Form/AdditionalPayment.php b/CRM/Contribute/Form/AdditionalPayment.php
index dbb3136f3fff8ec07747a92730152ca4b25bcc3a..49f7d01db1d69bfcff9d85bab4add3592041ec25 100644 (file)
--- a/CRM/Contribute/Form/AdditionalPayment.php
+++ b/CRM/Contribute/Form/AdditionalPayment.php
@@ -242,7 +242,7 @@ class CRM_Contribute_Form_AdditionalPayment extends CRM_Contribute_Form_Abstract
     $js = NULL;
     // render backoffice payment fields only on offline mode
     if (!$this->_mode) {
-      $js = ['onclick' => "return verify( );"];
+      $js = ['onclick' => 'return verify( );'];
 
       $this->add('select', 'payment_instrument_id',
         ts('Payment Method'),
@@ -258,11 +258,6 @@ class CRM_Contribute_Form_AdditionalPayment extends CRM_Contribute_Form_Abstract
         $attributes['fee_amount']
       );
       $this->addRule('fee_amount', ts('Please enter a valid monetary value for Fee Amount.'), 'money');
-
-      $this->add('text', 'net_amount', ts('Net Amount'),
-        $attributes['net_amount']
-      );
-      $this->addRule('net_amount', ts('Please enter a valid monetary value for Net Amount.'), 'money');
     }
 
     $buttonName = $this->_refund ? ts('Record Refund') : ts('Record Payment');
@@ -299,10 +294,7 @@ class CRM_Contribute_Form_AdditionalPayment extends CRM_Contribute_Form_Abstract
     if ($self->_paymentType == 'refund' && $fields['total_amount'] != abs($self->_refund)) {
       $errors['total_amount'] = ts('Refund amount must equal refund due amount.');
     }
-    $netAmt = (float) $fields['total_amount'] - (float) CRM_Utils_Array::value('fee_amount', $fields, 0);
-    if (!empty($fields['net_amount']) && $netAmt != $fields['net_amount']) {
-      $errors['net_amount'] = ts('Net amount should be equal to the difference between payment amount and fee amount.');
-    }
+
     if ($self->_paymentProcessor['id'] === 0 && empty($fields['payment_instrument_id'])) {
       $errors['payment_instrument_id'] = ts('Payment method is a required field');
     }

diff --git a/CRM/Core/BAO/FinancialTrxn.php b/CRM/Core/BAO/FinancialTrxn.php
index 9fe5b61e46895779362b5e80c0cbf95991de1913..ac05ff750ac722da38a760bd243e1f9b88f417e2 100644 (file)
--- a/CRM/Core/BAO/FinancialTrxn.php
+++ b/CRM/Core/BAO/FinancialTrxn.php
@@ -55,6 +55,14 @@ class CRM_Core_BAO_FinancialTrxn extends CRM_Financial_DAO_FinancialTrxn {
     $trxn = new CRM_Financial_DAO_FinancialTrxn();
     $trxn->copyValues($params);
 
+    if (isset($params['fee_amount']) && is_numeric($params['fee_amount'])) {
+      if (!isset($params['total_amount'])) {
+        $trxn->fetch();
+        $params['total_amount'] = $trxn->total_amount;
+      }
+      $trxn->net_amount = $params['total_amount'] - $params['fee_amount'];
+    }
+
     if (empty($params['id']) && !CRM_Utils_Rule::currencyCode($trxn->currency)) {
       $trxn->currency = CRM_Core_Config::singleton()->defaultCurrency;
     }

diff --git a/api/v3/Payment.php b/api/v3/Payment.php
index 5c490bd8654190fd112b076177291ec6ed73e436..afe22067b6b2812169749ac3b680197fd8f53772 100644 (file)
--- a/api/v3/Payment.php
+++ b/api/v3/Payment.php
@@ -39,6 +39,8 @@
  *
  * @return array
  *   Array of financial transactions which are payments, if error an array with an error id and error message
+ *
+ * @throws \CiviCRM_API3_Exception
  */
 function civicrm_api3_payment_get($params) {
   $financialTrxn = [];
@@ -50,7 +52,10 @@ function civicrm_api3_payment_get($params) {
   if (isset($params['trxn_id'])) {
     $params['financial_trxn_id.trxn_id'] = $params['trxn_id'];
   }
-  $eft = civicrm_api3('EntityFinancialTrxn', 'get', $params);
+  $eftParams = $params;
+  unset($eftParams['return']);
+  // @todo - why do we fetch EFT params at all?
+  $eft = civicrm_api3('EntityFinancialTrxn', 'get', $eftParams);
   if (!empty($eft['values'])) {
     $eftIds = [];
     foreach ($eft['values'] as $efts) {
@@ -83,9 +88,10 @@ function civicrm_api3_payment_get($params) {
  * @param array $params
  *   Input parameters.
  *
- * @throws API_Exception
  * @return array
  *   Api result array
+ *
+ * @throws \CiviCRM_API3_Exception
  */
 function civicrm_api3_payment_delete($params) {
   return civicrm_api3('FinancialTrxn', 'delete', $params);
@@ -135,7 +141,6 @@ function civicrm_api3_payment_cancel($params) {
  * @return array
  *   Api result array
  *
- * @throws \API_Exception
  * @throws \CRM_Core_Exception
  * @throws \CiviCRM_API3_Exception
  */
@@ -189,6 +194,10 @@ function _civicrm_api3_payment_create_spec(&$params) {
       'title' => ts('Total Payment Amount'),
       'type' => CRM_Utils_Type::T_FLOAT,
     ],
+    'fee_amount' => [
+      'title' => ts('Fee Amount'),
+      'type' => CRM_Utils_Type::T_FLOAT,
+    ],
     'payment_processor_id' => [
       'name' => 'payment_processor_id',
       'type' => CRM_Utils_Type::T_INT,
@@ -349,6 +358,11 @@ function _civicrm_api3_payment_get_spec(&$params) {
       'title' => ts('Payment Date'),
       'type' => CRM_Utils_Type::T_TIMESTAMP,
     ],
+    'financial_trxn_id' => [
+      'title' => ts('Payment ID'),
+      'type' => CRM_Utils_Type::T_INT,
+      'api.aliases' => ['payment_id', 'id'],
+    ],
   ];
 }
 

diff --git a/templates/CRM/Contribute/Form/AdditionalPayment.tpl b/templates/CRM/Contribute/Form/AdditionalPayment.tpl
index 23443f10fd5f261c039193f2143266610b6e29c9..ac7feb83fe8f3e8b05683a2b098ce8dd7aadbcae 100644 (file)
--- a/templates/CRM/Contribute/Form/AdditionalPayment.tpl
+++ b/templates/CRM/Contribute/Form/AdditionalPayment.tpl
@@ -110,8 +110,6 @@
           </tr>
           <tr class="crm-payment-form-block-fee_amount"><td class="label">{$form.fee_amount.label}</td><td{$valueStyle}>{$form.fee_amount.html|crmMoney:$currency:'XXX':'YYY'}<br />
             <span class="description">{ts}Processing fee for this transaction (if applicable).{/ts}</span></td></tr>
-           <tr class="crm-payment-form-block-net_amount"><td class="label">{$form.net_amount.label}</td><td>{$form.net_amount.html|crmMoney:$currency:'':1}<br />
-            <span class="description">{ts}Net value of the payment (Total Amount minus Fee).{/ts}</span></td></tr>
         </table>
       </div>
       {/if}

diff --git a/tests/phpunit/api/v3/PaymentTest.php b/tests/phpunit/api/v3/PaymentTest.php
index 5bc8ba4537231d29c2751eb9961437a7615fda8b..eb619078676140cfcee0c0ac450f64b3f6f466c6 100644 (file)
--- a/tests/phpunit/api/v3/PaymentTest.php
+++ b/tests/phpunit/api/v3/PaymentTest.php
@@ -770,6 +770,17 @@ class api_v3_PaymentTest extends CiviUnitTestCase {
     $this->validateAllPayments();
   }
 
+  /**
+   * Test net amount is set when fee amount is passed in.
+   *
+   * @throws \CRM_Core_Exception
+   */
+  public function testNetAmount() {
+    $order = $this->createPendingParticipantOrder();
+    $payment = $this->callAPISuccess('Payment', 'create', ['order_id' => $order['id'], 'total_amount' => 10, 'fee_amount' => .25]);
+    $this->assertEquals('9.75', $this->callAPISuccessGetValue('Payment', ['id' => $payment['id'], 'return' => 'net_amount']));
+  }
+
   /**
    * Test create payment api for pay later contribution with partial payment.
    *

diff --git a/tests/phpunit/api/v3/SyntaxConformanceTest.php b/tests/phpunit/api/v3/SyntaxConformanceTest.php
index c074d1a31449efc82d3160847a1a32ba2cf961ea..74bb2956915edce6284c52ca686f13f34bbc27df 100644 (file)
--- a/tests/phpunit/api/v3/SyntaxConformanceTest.php
+++ b/tests/phpunit/api/v3/SyntaxConformanceTest.php
@@ -663,6 +663,12 @@ class api_v3_SyntaxConformanceTest extends CiviUnitTestCase {
           'is_primary',
         ],
       ],
+      'FinancialTrxn' => [
+        'cant_update' => [
+          // Altering fee amount will also cause net_amount to be recalculated.
+          'fee_amount',
+        ],
+      ],
       'Navigation' => [
         'cant_update' => [
           // Weight is deliberately altered when this is changed - skip.