From 5d288dc4835fa81560db1b4c5101ac8e3f4748d1 Mon Sep 17 00:00:00 2001 From: eileen Date: Wed, 29 Jan 2020 14:22:42 +1300 Subject: [PATCH] [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 | 18 +++++------------- CRM/Contribute/Form/Contribution.php | 2 +- CRM/Upgrade/Incremental/php/FourSeven.php | 7 ++++--- tests/phpunit/api/v3/ContributionTest.php | 5 ++--- 4 files changed, 12 insertions(+), 20 deletions(-) diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php index 437bbfc190..31f5418ff9 100644 --- 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 4bbaabac47..a47c573dd1 100644 --- 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 eee123e7ce..30724c0508 100644 --- 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 1e71806386..1b8f437e88 100644 --- 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). -- 2.25.1