don't allow multiple currencies in a batch

civicrm · Jul 20, 2021 · a7838a7 · a7838a7
1 parent 1f3f8af
commit a7838a7
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 30 deletions.
diff --git a/CRM/Batch/BAO/Batch.php b/CRM/Batch/BAO/Batch.php
@@ -329,16 +329,7 @@ public static function getBatchList(&$params) {
         $values['id']
       );
       // CRM-21205
-      $values['currency'] = CRM_Core_DAO::singleValueQuery("
-        SELECT GROUP_CONCAT(DISTINCT ft.currency)
-        FROM  civicrm_batch batch
-        JOIN civicrm_entity_batch eb
-          ON batch.id = eb.batch_id
-        JOIN civicrm_financial_trxn ft
-          ON eb.entity_id = ft.id
-        WHERE batch.id = %1
-        GROUP BY batch.id
-      ", [1 => [$values['id'], 'Positive']]);
+      $values['currency'] = CRM_Batch_BAO_EntityBatch::getBatchCurrency($values['id']);
       $results[$values['id']] = $values;
     }
 

diff --git a/CRM/Batch/BAO/EntityBatch.php b/CRM/Batch/BAO/EntityBatch.php
@@ -23,9 +23,47 @@ class CRM_Batch_BAO_EntityBatch extends CRM_Batch_DAO_EntityBatch {
    * @return CRM_Batch_DAO_EntityBatch
    */
   public static function create($params) {
+    // Only write the EntityBatch record if the financial trxn and batch match on currency and payment instrument.
+    $batchCurrency = self::getBatchCurrency($params['batch_id']);
+    $batchPID = (int) CRM_Core_DAO::getFieldValue('CRM_Batch_DAO_Batch', $params['batch_id'], 'payment_instrument_id');
+    $trxn = \Civi\Api4\FinancialTrxn::get(FALSE)
+      ->addSelect('currency', 'payment_instrument_id')
+      ->addWhere('id', '=', $params['entity_id'])
+      ->execute()[0];
+    if ($batchCurrency && $batchCurrency !== $trxn['currency']) {
+      throw new \CRM_Core_Exception(ts('You can not add items of two different currencies to a single contribution batch.'));
+    }
+    if ($batchPID && $batchPID !== $trxn['payment_instrument_id']) {
+      $paymentInstrument = CRM_Core_PseudoConstant::getLabel('CRM_Batch_BAO_Batch', 'payment_instrument_id', $batchPID);
+      throw new \CRM_Core_Exception(ts('This batch is configured to include only transactions using %1 payment method. If you want to include other transactions, please edit the batch first and modify the Payment Method.', [1 => $paymentInstrument]));
+    }
     return self::writeRecord($params);
   }
 
+  /**
+   * Get the currency associated with a batch (if any).
+   *
+   * @param int $batchId
+   *
+   */
+  public static function getBatchCurrency($batchId) : ?string {
+    $sql = "SELECT DISTINCT ft.currency
+      FROM  civicrm_batch batch
+      JOIN civicrm_entity_batch eb
+        ON batch.id = eb.batch_id
+      JOIN civicrm_financial_trxn ft
+        ON eb.entity_id = ft.id
+      WHERE batch.id = %1";
+    $dao = CRM_Core_DAO::executeQuery($sql, [1 => [$batchId, 'Positive']]);
+    if ($dao->N === 0) {
+      return NULL;
+    }
+    else {
+      $dao->fetch();
+      return $dao->currency;
+    }
+  }
+
   /**
    * Remove entries from entity batch.
    * @param array|int $params

diff --git a/CRM/Financial/Page/AJAX.php b/CRM/Financial/Page/AJAX.php
@@ -170,19 +170,13 @@ public static function assignRemove() {
         switch ($op) {
           case 'assign':
           case 'remove':
-            $recordPID = CRM_Core_DAO::getFieldValue('CRM_Financial_DAO_FinancialTrxn', $recordID, 'payment_instrument_id');
-            $batchPID = CRM_Core_DAO::getFieldValue('CRM_Batch_DAO_Batch', $entityID, 'payment_instrument_id');
-            $paymentInstrument = CRM_Core_PseudoConstant::getLabel('CRM_Batch_BAO_Batch', 'payment_instrument_id', $batchPID);
-            if ($op == 'remove' || ($recordPID == $batchPID && $op == 'assign') || !isset($batchPID)) {
+            if ($op == 'remove' || $op == 'assign') {
               $params = [
                 'entity_id' => $recordID,
                 'entity_table' => 'civicrm_financial_trxn',
                 'batch_id' => $entityID,
               ];
             }
-            else {
-              $response = ['status' => ts("This batch is configured to include only transactions using %1 payment method. If you want to include other transactions, please edit the batch first and modify the Payment Method.", [1 => $paymentInstrument])];
-            }
             break;
 
           case 'close':
@@ -204,7 +198,12 @@ public static function assignRemove() {
         }
 
         if (method_exists($recordBAO, $methods[$op]) & !empty($params)) {
-          $updated = call_user_func_array(array($recordBAO, $methods[$op]), array(&$params));
+          try {
+            $updated = call_user_func_array(array($recordBAO, $methods[$op]), array(&$params));
+          }
+          catch (\CRM_Core_Exception $e) {
+            $errorMessage = $e->getMessage();
+          }
           if ($updated) {
             $redirectStatus = $updated->status_id;
             if ($batchStatus[$updated->status_id] == "Reopened") {
@@ -215,6 +214,9 @@ public static function assignRemove() {
               'status_id' => $redirectStatus,
             ];
           }
+          if ($errorMessage ?? FALSE) {
+            $response = ['status' => $errorMessage];
+          }
         }
       }
     }
@@ -465,29 +467,31 @@ public static function bulkAssignRemove() {
       }
     }
 
-    $batchPID = CRM_Core_DAO::getFieldValue('CRM_Batch_DAO_Batch', $entityID, 'payment_instrument_id');
-    $paymentInstrument = CRM_Core_PseudoConstant::getLabel('CRM_Batch_BAO_Batch', 'payment_instrument_id', $batchPID);
-    foreach ($cIDs as $key => $value) {
-      $recordPID = CRM_Core_DAO::getFieldValue('CRM_Financial_DAO_FinancialTrxn', $value, 'payment_instrument_id');
-      if ($action == 'Remove' || ($recordPID == $batchPID && $action == 'Assign') || !isset($batchPID)) {
+    foreach ($cIDs as $value) {
+      if ($action == 'Remove' || $action == 'Assign') {
         $params = [
           'entity_id' => $value,
           'entity_table' => 'civicrm_financial_trxn',
           'batch_id' => $entityID,
         ];
-        if ($action == 'Assign') {
-          $updated = CRM_Batch_BAO_EntityBatch::create($params);
+        try {
+          if ($action == 'Assign') {
+            CRM_Batch_BAO_EntityBatch::create($params);
+          }
+          else {
+            CRM_Batch_BAO_EntityBatch::del($params);
+          }
         }
-        else {
-          $updated = CRM_Batch_BAO_EntityBatch::del($params);
+        catch (\CRM_Core_Exception $e) {
+          $errorMessage = $e->getMessage();
         }
       }
     }
-    if ($updated) {
-      $status = ['status' => 'record-updated-success'];
+    if ($errorMessage ?? FALSE) {
+      $status = ['status' => $errorMessage];
     }
     else {
-      $status = ['status' => ts("This batch is configured to include only transactions using %1 payment method. If you want to include other transactions, please edit the batch first and modify the Payment Method.", [1 => $paymentInstrument])];
+      $status = ['status' => 'record-updated-success'];
     }
     CRM_Utils_JSON::output($status);
   }