[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

CRM/Contribute/BAO/Contribution.php

@@ -131,7 +131,7 @@ public static function add(&$params, $ids = []) {
     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 @@ protected function addContributionPageValuesToValuesHeavyHandedly(&$values) {
    *
    *
    * @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 @@ public static function getContributionTokenValues($id, $messageToken) {
    * @return string
    */
   public static function getInvoiceNumber($contributionID) {
-    if ($invoicePrefix = self::checkContributeSettings('invoice_prefix', TRUE)) {
+    if ($invoicePrefix = self::checkContributeSettings('invoice_prefix')) {
       return $invoicePrefix . $contributionID;
     }
 

CRM/Contribute/Form/Contribution.php

@@ -1302,7 +1302,7 @@ public function testSubmit($params, $action, $creditCardMode = NULL) {
       ]);
       $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',

CRM/Upgrade/Incremental/php/FourSeven.php

@@ -455,17 +455,18 @@ public function upgrade_4_7_28($rev) {
     $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)", [
           1 => $rev,
           2 => $startId,
           3 => $endId,
         ]);
-        $this->addTask($title, 'updateContributionInvoiceNumber', $startId, $endId, $invoicePrefix);
+        $this->addTask($title, 'updateContributionInvoiceNumber', $startId, $endId, $contributionSettings['invoice_prefix']);
       }
     }
 

tests/phpunit/api/v3/ContributionTest.php

@@ -132,8 +132,7 @@ public function tearDown() {
    * @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 @@ public function testGetContribution() {
     $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).