[REF] spare a query

getInvoiceNumber returns NULL if the 'invoicing' setting is disabled, by checking slightly earlier
we can skip a query if it is not

Upgrade fix: Copy code to upgrade script rather than call a function.

The function will change as we fix the setting but we want the upgrade function to work off the old setting

Test fix: Use hard coded invoicePrefix

We know what it is - if it were not set right then the test would not pick it up because it is comparing
based on the assumption it is - using the string is more reliable

diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php
index 437bbfc190422dbf2a660ba0f45830f1fb9ee02e..31f5418ff91747bfe8eb5e232951b45f7dc3830a 100644 (file)
--- a/CRM/Contribute/BAO/Contribution.php
+++ b/CRM/Contribute/BAO/Contribution.php
@@ -131,7 +131,7 @@ class CRM_Contribute_BAO_Contribution extends CRM_Contribute_DAO_Contribution {
     if (!$contributionID) {
       CRM_Core_DAO::setCreateDefaults($params, self::getDefaults());
 
-      if (empty($params['invoice_number'])) {
+      if (empty($params['invoice_number']) && CRM_Invoicing_Utils::isInvoicingEnabled()) {
         $nextContributionID = CRM_Core_DAO::singleValueQuery("SELECT COALESCE(MAX(id) + 1, 1) FROM civicrm_contribution");
         $params['invoice_number'] = self::getInvoiceNumber($nextContributionID);
       }
@@ -5059,21 +5059,13 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac
    *
    *
    * @param string $name
-   * @param bool $checkInvoicing
+   *
    * @return string
    *
    */
-  public static function checkContributeSettings($name = NULL, $checkInvoicing = FALSE) {
+  public static function checkContributeSettings($name) {
     $contributeSettings = Civi::settings()->get('contribution_invoice_settings');
-
-    if ($checkInvoicing && empty($contributeSettings['invoicing'])) {
-      return NULL;
-    }
-
-    if ($name) {
-      return CRM_Utils_Array::value($name, $contributeSettings);
-    }
-    return $contributeSettings;
+    return CRM_Utils_Array::value($name, $contributeSettings);
   }
 
   /**
@@ -5888,7 +5880,7 @@ LIMIT 1;";
    * @return string
    */
   public static function getInvoiceNumber($contributionID) {
-    if ($invoicePrefix = self::checkContributeSettings('invoice_prefix', TRUE)) {
+    if ($invoicePrefix = self::checkContributeSettings('invoice_prefix')) {
       return $invoicePrefix . $contributionID;
     }
 

diff --git a/CRM/Contribute/Form/Contribution.php b/CRM/Contribute/Form/Contribution.php
index 4bbaabac47d0c429b5317fe430da30a59863a570..a47c573dd15ea0b72cc073d22a316e301a14fbee 100644 (file)
--- a/CRM/Contribute/Form/Contribution.php
+++ b/CRM/Contribute/Form/Contribution.php
@@ -1302,7 +1302,7 @@ class CRM_Contribute_Form_Contribution extends CRM_Contribute_Form_AbstractEditP
       ]);
       $this->_id = $params['id'];
       $this->_values = $existingContribution;
-      if (CRM_Contribute_BAO_Contribution::checkContributeSettings('invoicing')) {
+      if (CRM_Invoicing_Utils::isInvoicingEnabled()) {
         $this->_values['tax_amount'] = civicrm_api3('contribution', 'getvalue', [
           'id' => $params['id'],
           'return' => 'tax_amount',

diff --git a/CRM/Upgrade/Incremental/php/FourSeven.php b/CRM/Upgrade/Incremental/php/FourSeven.php
index eee123e7ce7b8c2fb0ed5c683f0f300450c5f524..30724c0508a7b17bdc644f3426a9a5c65748e681 100644 (file)
--- a/CRM/Upgrade/Incremental/php/FourSeven.php
+++ b/CRM/Upgrade/Incremental/php/FourSeven.php
@@ -455,9 +455,10 @@ class CRM_Upgrade_Incremental_php_FourSeven extends CRM_Upgrade_Incremental_Base
     $this->addTask(ts('Upgrade DB to %1: SQL', [1 => $rev]), 'runSql', $rev);
     $this->addTask('CRM-20572: Fix date fields in save search criteria of Contrib Sybunt custom search ', 'fixDateFieldsInSmartGroups');
     // CRM-20868 : Update invoice_numbers (in batch) with value in [invoice prefix][contribution id] format
-    if ($invoicePrefix = CRM_Contribute_BAO_Contribution::checkContributeSettings('invoice_prefix', TRUE)) {
+    $contributionSettings = Civi::settings()->get('contribution_invoice_settings');
+    if (!empty($contributionSettings['invoicing']) && !empty($contributionSettings['invoice_prefix'])) {
       list($minId, $maxId) = CRM_Core_DAO::executeQuery("SELECT coalesce(min(id),0), coalesce(max(id),0)
-        FROM civicrm_contribution ")->getDatabaseResult()->fetchRow();
+      FROM civicrm_contribution ")->getDatabaseResult()->fetchRow();
       for ($startId = $minId; $startId <= $maxId; $startId += self::BATCH_SIZE) {
         $endId = $startId + self::BATCH_SIZE - 1;
         $title = ts("Upgrade DB to %1: Update Contribution Invoice number (%2 => %3)", [
@@ -465,7 +466,7 @@ class CRM_Upgrade_Incremental_php_FourSeven extends CRM_Upgrade_Incremental_Base
           2 => $startId,
           3 => $endId,
         ]);
-        $this->addTask($title, 'updateContributionInvoiceNumber', $startId, $endId, $invoicePrefix);
+        $this->addTask($title, 'updateContributionInvoiceNumber', $startId, $endId, $contributionSettings['invoice_prefix']);
       }
     }
 

diff --git a/tests/phpunit/api/v3/ContributionTest.php b/tests/phpunit/api/v3/ContributionTest.php
index 1e71806386870e158e6ee0b7c4aa0c82102d073c..1b8f437e887b7cbe4a229d5a446d1cec1e5a2f70 100644 (file)
--- a/tests/phpunit/api/v3/ContributionTest.php
+++ b/tests/phpunit/api/v3/ContributionTest.php
@@ -132,8 +132,7 @@ class api_v3_ContributionTest extends CiviUnitTestCase {
    * @throws \CRM_Core_Exception
    */
   public function testGetContribution() {
-    $contributionSettings = $this->enableTaxAndInvoicing();
-    $invoice_prefix = CRM_Contribute_BAO_Contribution::checkContributeSettings('invoice_prefix', TRUE);
+    $this->enableTaxAndInvoicing();
     $p = [
       'contact_id' => $this->_individualId,
       'receive_date' => '2010-01-20',
@@ -171,7 +170,7 @@ class api_v3_ContributionTest extends CiviUnitTestCase {
     $this->assertEquals($contribution['net_amount'], 95.00);
     $this->assertEquals($contribution['trxn_id'], 23456);
     $this->assertEquals($contribution['invoice_id'], 78910);
-    $this->assertEquals($contribution['invoice_number'], $invoice_prefix . $contributions['id']);
+    $this->assertEquals($contribution['invoice_number'], 'INV_' . $contributions['id']);
     $this->assertEquals($contribution['contribution_source'], 'SSF');
     $this->assertEquals($contribution['contribution_status'], 'Completed');
     // Create a second contribution - we are testing that 'id' gets the right contribution id (not the contact id).