From 38c87aa27801454de6ba9864a34d4068e1f2fe0d Mon Sep 17 00:00:00 2001 From: eileen Date: Thu, 13 Feb 2020 02:23:25 +1300 Subject: [PATCH] dev/financial#84 Move sequential credit notes from 'deeply embedeed in BAO functions' to usin a listener structure https://lab.civicrm.org/dev/financial/issues/84 We have a significant problem in our code base that it is hard to cleanup our key processes and make them usable and Form-builder ready because of all the features deeply interwoven into them. It will be important that our Order->add payment flow works and can be utilsed by the form builder. The expectation is that adding a payment will update all the appropriate entities. We've been working pretty hard on this for a couple of years now (I think the time taken to clean up code is sadly greater than the time it takes to add it :-() and the Payment.create part is getting pretty solid now. However, we are consistently hampered by code that has been 'enmeshed' into deep BAO code. In the case of the credit note creation something that can easily be implemented as a pre hook and kept entirely out of the Contribution.create function is not only embedded in there but also in updateFinancialAccountsOnContributionStatusChange - which is one of the functions we are some dozens of hours into adding tests & 'sorting out'. We need to find a way to 1) Declutter key functions where possible and 2) Model good ways of implementing things by hook (& work through limitations in this approach) as part of cleaning up core and pushing towards a LeXIM approach. I originally agreed with the CiviDiet approach that we should try to move the myriad of things that are 'add ons' that someone thought was necessary & useful to others (or less charitably they thought that if they got it into core someone else would have to maintain it for them) moved out of core into extensions and eventually the maintenance of them moved out of the core team too. However, it's clear from the discussion on moving credit notes out of core that this is too hard for the forseeable future. We can try to avoid more features being added to core but most features in core are used by at least one organization & the politics of moving things out are too hard. On the bright side we let a lot less new features into core these days - not none - the membership status override date springs to mind and the ongoing adding of fields & filters to reports - but less. This leaves us with the question of whether we can still achieve the goal of making add-on features less of a developer burden. In addition to ensuring code is tested we can do this by getting it out of the 'deep functions' into a listener structure. I was able to covert the 'spaghetti-like-tendrils' in the credit note code to a simple 'pre' hook. So it felt like we needed to create something new. Something like .. an extension .. but not visible on the extensions screen and in core. Something like ... an extension in a folder in the core codebase that is suppressed from the extensions UI.... So this adds it as an extension to core in a new location & ensures GUI users cannot see it. I don't know exactly where we draw the line around packaging things this way - Partial payments is clearly part of the core architecture, but there might be a world in which we move something like event cart to be a more distinct bundle. As we move to form builder forms we are going to want forms being replaced to be able to be 'quarantined' for future amputation.... I would be looking to move financial type acls and invoicing to this structure - mostly because they are the biggest 'offenders' in terms of having code all over the place causing problems with other code initiatives & we've struggled to come up with a way to deal with that other than putting touching that code in the 'too hard basket'. But even things like PCPs & Premiums would be more maintainable if they were discrete & overcoming the challenges involved in that would solve a lot of extensibility issues. Note this approach permits an incremental cleanup-up-to-separation process. For example we could create an extension for a feature and move some code to it and later move some other code. For example we could move the invoicingPostProcessHook to a shell invoicing extension and at some later point address other parts of the invoicing code (check LineItem::getLineItems for the chunk of code that has been blocking other cleanup - moving this to a listener would help a lot Note this PR does not address the significant performance issues associated with the creditnotes code on larger sites, only the impact it has on our code maintainability. If we can agree a way to start to address how to split out or code into listeners in an agreed structure then I can look at that as a follow up. --- CRM/Admin/Page/Extensions.php | 6 + CRM/Contribute/BAO/Contribution.php | 46 -- CRM/Contribute/Form/Task/Invoice.php | 9 +- CRM/Extension/Mapper.php | 2 +- .../Incremental/php/FiveTwentyFour.php | 37 +- .../Financial/sequentialcreditnotes/README.md | 3 + .../Financial/sequentialcreditnotes/info.xml | 29 ++ .../sequentialcreditnotes/phpunit.xml.dist | 18 + .../sequentialcreditnotes.civix.php | 476 ++++++++++++++++++ .../sequentialcreditnotes.php | 72 +++ .../settings/creditnote.setting.php | 19 + .../phpunit/SequentialcreditnotesTest.php | 82 +++ .../tests/phpunit/bootstrap.php | 63 +++ settings/Contribute.setting.php | 16 - .../CRM/Contribute/BAO/ContributionTest.php | 34 -- tests/phpunit/api/v3/ContributionTest.php | 2 - 16 files changed, 795 insertions(+), 119 deletions(-) create mode 100644 CoreExtensions/Financial/sequentialcreditnotes/README.md create mode 100644 CoreExtensions/Financial/sequentialcreditnotes/info.xml create mode 100644 CoreExtensions/Financial/sequentialcreditnotes/phpunit.xml.dist create mode 100644 CoreExtensions/Financial/sequentialcreditnotes/sequentialcreditnotes.civix.php create mode 100644 CoreExtensions/Financial/sequentialcreditnotes/sequentialcreditnotes.php create mode 100644 CoreExtensions/Financial/sequentialcreditnotes/settings/creditnote.setting.php create mode 100644 CoreExtensions/Financial/sequentialcreditnotes/tests/phpunit/SequentialcreditnotesTest.php create mode 100644 CoreExtensions/Financial/sequentialcreditnotes/tests/phpunit/bootstrap.php diff --git a/CRM/Admin/Page/Extensions.php b/CRM/Admin/Page/Extensions.php index b7b26ad1d0..3226e0e6ea 100644 --- a/CRM/Admin/Page/Extensions.php +++ b/CRM/Admin/Page/Extensions.php @@ -148,7 +148,11 @@ class CRM_Admin_Page_Extensions extends CRM_Core_Page_Basic { $localExtensionRows = []; $keys = array_keys($manager->getStatuses()); sort($keys); + $hiddenExtensions = $mapper->getKeysByTag('hidden'); foreach ($keys as $key) { + if (in_array($key, $hiddenExtensions)) { + continue; + } try { $obj = $mapper->keyToInfo($key); } @@ -157,6 +161,8 @@ class CRM_Admin_Page_Extensions extends CRM_Core_Page_Basic { continue; } + $mapper = CRM_Extension_System::singleton()->getMapper(); + $row = self::createExtendedInfo($obj); $row['id'] = $obj->key; $row['action'] = ''; diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php index edf648b098..359e3f9dd7 100644 --- a/CRM/Contribute/BAO/Contribution.php +++ b/CRM/Contribute/BAO/Contribution.php @@ -130,21 +130,12 @@ class CRM_Contribute_BAO_Contribution extends CRM_Contribute_DAO_Contribution { //set defaults in create mode if (!$contributionID) { CRM_Core_DAO::setCreateDefaults($params, self::getDefaults()); - 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); } } - //if contribution is created with cancelled or refunded status, add credit note id - // do the same for chargeback - this entered the code 'accidentally' but moving it to here - // as part of cleanup maintains consistency. - if (self::isContributionStatusNegative(CRM_Utils_Array::value('contribution_status_id', $params))) { - if (empty($params['creditnote_id'])) { - $params['creditnote_id'] = self::createCreditNoteId(); - } - } $contributionStatusID = $params['contribution_status_id'] ?? NULL; if (CRM_Core_PseudoConstant::getName('CRM_Contribute_BAO_Contribution', 'contribution_status_id', (int) $contributionStatusID) === 'Partially paid' && empty($params['is_post_payment_create'])) { CRM_Core_Error::deprecatedFunctionWarning('Setting status to partially paid other than by using Payment.create is deprecated and unreliable'); @@ -1108,12 +1099,6 @@ class CRM_Contribute_BAO_Contribution extends CRM_Contribute_DAO_Contribution { if (self::isContributionUpdateARefund($params['prevContribution']->contribution_status_id, $params['contribution']->contribution_status_id)) { // @todo we should stop passing $params by reference - splitting this out would be a step towards that. $params['trxnParams']['total_amount'] = -$params['total_amount']; - if (empty($params['contribution']->creditnote_id)) { - // This is always set in the Contribution::create function. - CRM_Core_Error::deprecatedFunctionWarning('Logic says this line is never reached & can be removed'); - $creditNoteId = self::createCreditNoteId(); - CRM_Core_DAO::setFieldValue('CRM_Contribute_DAO_Contribution', $params['contribution']->id, 'creditnote_id', $creditNoteId); - } } elseif (($previousContributionStatus == 'Pending' && $params['prevContribution']->is_pay_later) || $previousContributionStatus == 'In Progress' @@ -1125,12 +1110,6 @@ class CRM_Contribute_BAO_Contribution extends CRM_Contribute_DAO_Contribution { // @todo we should stop passing $params by reference - splitting this out would be a step towards that. $params['trxnParams']['to_financial_account_id'] = $arAccountId; $params['trxnParams']['total_amount'] = -$params['total_amount']; - if (empty($params['contribution']->creditnote_id)) { - // This is always set in the Contribution::create function. - CRM_Core_Error::deprecatedFunctionWarning('Logic says this line is never reached & can be removed'); - $creditNoteId = self::createCreditNoteId(); - CRM_Core_DAO::setFieldValue('CRM_Contribute_DAO_Contribution', $params['contribution']->id, 'creditnote_id', $creditNoteId); - } } else { // @todo we should stop passing $params by reference - splitting this out would be a step towards that. @@ -4707,31 +4686,6 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac return CRM_Core_BAO_Domain::getDefaultReceiptFrom(); } - /** - * Generate credit note id with next avaible number - * - * @return string - * Credit Note Id. - * - * @throws \CiviCRM_API3_Exception - */ - public static function createCreditNoteId() { - - $creditNoteNum = CRM_Core_DAO::singleValueQuery("SELECT count(creditnote_id) as creditnote_number FROM civicrm_contribution WHERE creditnote_id IS NOT NULL"); - $creditNoteId = NULL; - - do { - $creditNoteNum++; - $creditNoteId = Civi::settings()->get('credit_notes_prefix') . '' . $creditNoteNum; - $result = civicrm_api3('Contribution', 'getcount', [ - 'sequential' => 1, - 'creditnote_id' => $creditNoteId, - ]); - } while ($result > 0); - - return $creditNoteId; - } - /** * Load related memberships. * diff --git a/CRM/Contribute/Form/Task/Invoice.php b/CRM/Contribute/Form/Task/Invoice.php index 9ff683534c..d3a55dbb60 100644 --- a/CRM/Contribute/Form/Task/Invoice.php +++ b/CRM/Contribute/Form/Task/Invoice.php @@ -268,14 +268,7 @@ class CRM_Contribute_Form_Task_Invoice extends CRM_Contribute_Form_Task { } if ($contribution->contribution_status_id == $refundedStatusId || $contribution->contribution_status_id == $cancelledStatusId) { - if (is_null($contribution->creditnote_id)) { - CRM_Core_Error::deprecatedFunctionWarning('This it the wrong place to add a credit note id since the id is added when the status is changed in the Contribution::Create function- hopefully it is never hit'); - $creditNoteId = CRM_Contribute_BAO_Contribution::createCreditNoteId(); - CRM_Core_DAO::setFieldValue('CRM_Contribute_DAO_Contribution', $contribution->id, 'creditnote_id', $creditNoteId); - } - else { - $creditNoteId = $contribution->creditnote_id; - } + $creditNoteId = $contribution->creditnote_id; } if (!$contribution->invoice_number) { $contribution->invoice_number = CRM_Contribute_BAO_Contribution::getInvoiceNumber($contribution->id); diff --git a/CRM/Extension/Mapper.php b/CRM/Extension/Mapper.php index 4a34a79117..c86226cba1 100644 --- a/CRM/Extension/Mapper.php +++ b/CRM/Extension/Mapper.php @@ -161,7 +161,7 @@ class CRM_Extension_Mapper { * @param bool $fresh * * @throws CRM_Extension_Exception - * @throws Exception + * * @return CRM_Extension_Info */ public function keyToInfo($key, $fresh = FALSE) { diff --git a/CRM/Upgrade/Incremental/php/FiveTwentyFour.php b/CRM/Upgrade/Incremental/php/FiveTwentyFour.php index b215e3c290..dc96840a26 100644 --- a/CRM/Upgrade/Incremental/php/FiveTwentyFour.php +++ b/CRM/Upgrade/Incremental/php/FiveTwentyFour.php @@ -52,18 +52,31 @@ class CRM_Upgrade_Incremental_php_FiveTwentyFour extends CRM_Upgrade_Incremental * (change the x in the function name): */ - // /** - // * Upgrade function. - // * - // * @param string $rev - // */ - // public function upgrade_5_0_x($rev) { - // $this->addTask(ts('Upgrade DB to %1: SQL', [1 => $rev]), 'runSql', $rev); - // $this->addTask('Do the foo change', 'taskFoo', ...); - // // Additional tasks here... - // // Note: do not use ts() in the addTask description because it adds unnecessary strings to transifex. - // // The above is an exception because 'Upgrade DB to %1: SQL' is generic & reusable. - // } + /** + * Upgrade function. + * + * @param string $rev + */ + public function upgrade_5_24_alpha1($rev) { + $this->addTask(ts('Upgrade DB to %1: SQL', [1 => $rev]), 'runSql', $rev); + $this->addTask('Install sequential creditnote extension', 'installCreditNotes'); + } + + /** + * Install sequentialCreditNotes extension. + * + * This feature is restructured as a core extension - which is primarily a code cleanup step. + * + * @param \CRM_Queue_TaskContext $ctx + * + * @return bool + * + * @throws \CiviCRM_API3_Exception + */ + public static function installCreditNotes(CRM_Queue_TaskContext $ctx) { + civicrm_api3('Extension', 'install', ['sequentialcreditnotes']); + return TRUE; + } // public static function taskFoo(CRM_Queue_TaskContext $ctx, ...) { // return TRUE; diff --git a/CoreExtensions/Financial/sequentialcreditnotes/README.md b/CoreExtensions/Financial/sequentialcreditnotes/README.md new file mode 100644 index 0000000000..6250b97582 --- /dev/null +++ b/CoreExtensions/Financial/sequentialcreditnotes/README.md @@ -0,0 +1,3 @@ +# sequentialcreditnotes + +Ensures a sequential credit note number is created whenever a contribution status is updated to refunded. diff --git a/CoreExtensions/Financial/sequentialcreditnotes/info.xml b/CoreExtensions/Financial/sequentialcreditnotes/info.xml new file mode 100644 index 0000000000..f471757746 --- /dev/null +++ b/CoreExtensions/Financial/sequentialcreditnotes/info.xml @@ -0,0 +1,29 @@ + + + sequentialcreditnotes + Sequential credit notes + Calculates and sets a sequential credit note whenever a contribution is refunded. + AGPL-3.0 + + eileen + eileen@civicrm.org + + + https://lab.civicrm.org/extensions/sequentialcreditnotes + https://lab.civicrm.org/extensions/sequentialcreditnotes + https://lab.civicrm.org/extensions/sequentialcreditnotes + http://www.gnu.org/licenses/agpl-3.0.html + + 2020-01-28 + 1.0 + + hidden + + stable + + 5.24 + + + CRM/Sequentialcreditnotes + + diff --git a/CoreExtensions/Financial/sequentialcreditnotes/phpunit.xml.dist b/CoreExtensions/Financial/sequentialcreditnotes/phpunit.xml.dist new file mode 100644 index 0000000000..0f9f25d307 --- /dev/null +++ b/CoreExtensions/Financial/sequentialcreditnotes/phpunit.xml.dist @@ -0,0 +1,18 @@ + + + + + ./tests/phpunit + + + + + ./ + + + + + + + + diff --git a/CoreExtensions/Financial/sequentialcreditnotes/sequentialcreditnotes.civix.php b/CoreExtensions/Financial/sequentialcreditnotes/sequentialcreditnotes.civix.php new file mode 100644 index 0000000000..0b17d88b11 --- /dev/null +++ b/CoreExtensions/Financial/sequentialcreditnotes/sequentialcreditnotes.civix.php @@ -0,0 +1,476 @@ +getUrl(self::LONG_NAME), '/'); + } + return CRM_Core_Resources::singleton()->getUrl(self::LONG_NAME, $file); + } + + /** + * Get the path of a resource file (in this extension). + * + * @param string|NULL $file + * Ex: NULL. + * Ex: 'css/foo.css'. + * @return string + * Ex: '/var/www/example.org/sites/default/ext/org.example.foo'. + * Ex: '/var/www/example.org/sites/default/ext/org.example.foo/css/foo.css'. + */ + public static function path($file = NULL) { + // return CRM_Core_Resources::singleton()->getPath(self::LONG_NAME, $file); + return __DIR__ . ($file === NULL ? '' : (DIRECTORY_SEPARATOR . $file)); + } + + /** + * Get the name of a class within this extension. + * + * @param string $suffix + * Ex: 'Page_HelloWorld' or 'Page\\HelloWorld'. + * @return string + * Ex: 'CRM_Foo_Page_HelloWorld'. + */ + public static function findClass($suffix) { + return self::CLASS_PREFIX . '_' . str_replace('\\', '_', $suffix); + } + +} + +use CRM_Sequentialcreditnotes_ExtensionUtil as E; + +/** + * (Delegated) Implements hook_civicrm_config(). + * + * @link https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_config + */ +function _sequentialcreditnotes_civix_civicrm_config(&$config = NULL) { + static $configured = FALSE; + if ($configured) { + return; + } + $configured = TRUE; + + $template =& CRM_Core_Smarty::singleton(); + + $extRoot = dirname(__FILE__) . DIRECTORY_SEPARATOR; + $extDir = $extRoot . 'templates'; + + if (is_array($template->template_dir)) { + array_unshift($template->template_dir, $extDir); + } + else { + $template->template_dir = [$extDir, $template->template_dir]; + } + + $include_path = $extRoot . PATH_SEPARATOR . get_include_path(); + set_include_path($include_path); +} + +/** + * (Delegated) Implements hook_civicrm_xmlMenu(). + * + * @param $files array(string) + * + * @link https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_xmlMenu + */ +function _sequentialcreditnotes_civix_civicrm_xmlMenu(&$files) { + foreach (_sequentialcreditnotes_civix_glob(__DIR__ . '/xml/Menu/*.xml') as $file) { + $files[] = $file; + } +} + +/** + * Implements hook_civicrm_install(). + * + * @link https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_install + */ +function _sequentialcreditnotes_civix_civicrm_install() { + _sequentialcreditnotes_civix_civicrm_config(); + if ($upgrader = _sequentialcreditnotes_civix_upgrader()) { + $upgrader->onInstall(); + } +} + +/** + * Implements hook_civicrm_postInstall(). + * + * @link https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_postInstall + */ +function _sequentialcreditnotes_civix_civicrm_postInstall() { + _sequentialcreditnotes_civix_civicrm_config(); + if ($upgrader = _sequentialcreditnotes_civix_upgrader()) { + if (is_callable([$upgrader, 'onPostInstall'])) { + $upgrader->onPostInstall(); + } + } +} + +/** + * Implements hook_civicrm_uninstall(). + * + * @link https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_uninstall + */ +function _sequentialcreditnotes_civix_civicrm_uninstall() { + _sequentialcreditnotes_civix_civicrm_config(); + if ($upgrader = _sequentialcreditnotes_civix_upgrader()) { + $upgrader->onUninstall(); + } +} + +/** + * (Delegated) Implements hook_civicrm_enable(). + * + * @link https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_enable + */ +function _sequentialcreditnotes_civix_civicrm_enable() { + _sequentialcreditnotes_civix_civicrm_config(); + if ($upgrader = _sequentialcreditnotes_civix_upgrader()) { + if (is_callable([$upgrader, 'onEnable'])) { + $upgrader->onEnable(); + } + } +} + +/** + * (Delegated) Implements hook_civicrm_disable(). + * + * @link https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_disable + * @return mixed + */ +function _sequentialcreditnotes_civix_civicrm_disable() { + _sequentialcreditnotes_civix_civicrm_config(); + if ($upgrader = _sequentialcreditnotes_civix_upgrader()) { + if (is_callable([$upgrader, 'onDisable'])) { + $upgrader->onDisable(); + } + } +} + +/** + * (Delegated) Implements hook_civicrm_upgrade(). + * + * @param $op string, the type of operation being performed; 'check' or 'enqueue' + * @param $queue CRM_Queue_Queue, (for 'enqueue') the modifiable list of pending up upgrade tasks + * + * @return mixed + * based on op. for 'check', returns array(boolean) (TRUE if upgrades are pending) for 'enqueue', returns void + * + * @link https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_upgrade + */ +function _sequentialcreditnotes_civix_civicrm_upgrade($op, CRM_Queue_Queue $queue = NULL) { + if ($upgrader = _sequentialcreditnotes_civix_upgrader()) { + return $upgrader->onUpgrade($op, $queue); + } +} + +/** + * @return CRM_Sequentialcreditnotes_Upgrader + */ +function _sequentialcreditnotes_civix_upgrader() { + if (!file_exists(__DIR__ . '/CRM/Sequentialcreditnotes/Upgrader.php')) { + return NULL; + } + else { + return CRM_Sequentialcreditnotes_Upgrader_Base::instance(); + } +} + +/** + * Search directory tree for files which match a glob pattern. + * + * Note: Dot-directories (like "..", ".git", or ".svn") will be ignored. + * Note: In Civi 4.3+, delegate to CRM_Utils_File::findFiles() + * + * @param string $dir base dir + * @param string $pattern , glob pattern, eg "*.txt" + * + * @return array(string) + */ +function _sequentialcreditnotes_civix_find_files($dir, $pattern) { + if (is_callable(['CRM_Utils_File', 'findFiles'])) { + return CRM_Utils_File::findFiles($dir, $pattern); + } + + $todos = [$dir]; + $result = []; + while (!empty($todos)) { + $subdir = array_shift($todos); + foreach (_sequentialcreditnotes_civix_glob("$subdir/$pattern") as $match) { + if (!is_dir($match)) { + $result[] = $match; + } + } + if ($dh = opendir($subdir)) { + while (FALSE !== ($entry = readdir($dh))) { + $path = $subdir . DIRECTORY_SEPARATOR . $entry; + if ($entry{0} == '.') { + } + elseif (is_dir($path)) { + $todos[] = $path; + } + } + closedir($dh); + } + } + return $result; +} + +/** + * (Delegated) Implements hook_civicrm_managed(). + * + * Find any *.mgd.php files, merge their content, and return. + * + * @link https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_managed + */ +function _sequentialcreditnotes_civix_civicrm_managed(&$entities) { + $mgdFiles = _sequentialcreditnotes_civix_find_files(__DIR__, '*.mgd.php'); + sort($mgdFiles); + foreach ($mgdFiles as $file) { + $es = include $file; + foreach ($es as $e) { + if (empty($e['module'])) { + $e['module'] = E::LONG_NAME; + } + if (empty($e['params']['version'])) { + $e['params']['version'] = '3'; + } + $entities[] = $e; + } + } +} + +/** + * (Delegated) Implements hook_civicrm_caseTypes(). + * + * Find any and return any files matching "xml/case/*.xml" + * + * Note: This hook only runs in CiviCRM 4.4+. + * + * @link https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_caseTypes + */ +function _sequentialcreditnotes_civix_civicrm_caseTypes(&$caseTypes) { + if (!is_dir(__DIR__ . '/xml/case')) { + return; + } + + foreach (_sequentialcreditnotes_civix_glob(__DIR__ . '/xml/case/*.xml') as $file) { + $name = preg_replace('/\.xml$/', '', basename($file)); + if ($name != CRM_Case_XMLProcessor::mungeCaseType($name)) { + $errorMessage = sprintf("Case-type file name is malformed (%s vs %s)", $name, CRM_Case_XMLProcessor::mungeCaseType($name)); + throw new CRM_Core_Exception($errorMessage); + } + $caseTypes[$name] = [ + 'module' => E::LONG_NAME, + 'name' => $name, + 'file' => $file, + ]; + } +} + +/** + * (Delegated) Implements hook_civicrm_angularModules(). + * + * Find any and return any files matching "ang/*.ang.php" + * + * Note: This hook only runs in CiviCRM 4.5+. + * + * @link https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_angularModules + */ +function _sequentialcreditnotes_civix_civicrm_angularModules(&$angularModules) { + if (!is_dir(__DIR__ . '/ang')) { + return; + } + + $files = _sequentialcreditnotes_civix_glob(__DIR__ . '/ang/*.ang.php'); + foreach ($files as $file) { + $name = preg_replace(':\.ang\.php$:', '', basename($file)); + $module = include $file; + if (empty($module['ext'])) { + $module['ext'] = E::LONG_NAME; + } + $angularModules[$name] = $module; + } +} + +/** + * (Delegated) Implements hook_civicrm_themes(). + * + * Find any and return any files matching "*.theme.php" + */ +function _sequentialcreditnotes_civix_civicrm_themes(&$themes) { + $files = _sequentialcreditnotes_civix_glob(__DIR__ . '/*.theme.php'); + foreach ($files as $file) { + $themeMeta = include $file; + if (empty($themeMeta['name'])) { + $themeMeta['name'] = preg_replace(':\.theme\.php$:', '', basename($file)); + } + if (empty($themeMeta['ext'])) { + $themeMeta['ext'] = E::LONG_NAME; + } + $themes[$themeMeta['name']] = $themeMeta; + } +} + +/** + * Glob wrapper which is guaranteed to return an array. + * + * The documentation for glob() says, "On some systems it is impossible to + * distinguish between empty match and an error." Anecdotally, the return + * result for an empty match is sometimes array() and sometimes FALSE. + * This wrapper provides consistency. + * + * @link http://php.net/glob + * @param string $pattern + * + * @return array, possibly empty + */ +function _sequentialcreditnotes_civix_glob($pattern) { + $result = glob($pattern); + return is_array($result) ? $result : []; +} + +/** + * Inserts a navigation menu item at a given place in the hierarchy. + * + * @param array $menu - menu hierarchy + * @param string $path - path to parent of this item, e.g. 'my_extension/submenu' + * 'Mailing', or 'Administer/System Settings' + * @param array $item - the item to insert (parent/child attributes will be + * filled for you) + * + * @return bool + */ +function _sequentialcreditnotes_civix_insert_navigation_menu(&$menu, $path, $item) { + // If we are done going down the path, insert menu + if (empty($path)) { + $menu[] = [ + 'attributes' => array_merge([ + 'label' => CRM_Utils_Array::value('name', $item), + 'active' => 1, + ], $item), + ]; + return TRUE; + } + else { + // Find an recurse into the next level down + $found = FALSE; + $path = explode('/', $path); + $first = array_shift($path); + foreach ($menu as $key => &$entry) { + if ($entry['attributes']['name'] == $first) { + if (!isset($entry['child'])) { + $entry['child'] = []; + } + $found = _sequentialcreditnotes_civix_insert_navigation_menu($entry['child'], implode('/', $path), $item); + } + } + return $found; + } +} + +/** + * (Delegated) Implements hook_civicrm_navigationMenu(). + */ +function _sequentialcreditnotes_civix_navigationMenu(&$nodes) { + if (!is_callable(['CRM_Core_BAO_Navigation', 'fixNavigationMenu'])) { + _sequentialcreditnotes_civix_fixNavigationMenu($nodes); + } +} + +/** + * Given a navigation menu, generate navIDs for any items which are + * missing them. + */ +function _sequentialcreditnotes_civix_fixNavigationMenu(&$nodes) { + $maxNavID = 1; + array_walk_recursive($nodes, function($item, $key) use (&$maxNavID) { + if ($key === 'navID') { + $maxNavID = max($maxNavID, $item); + } + }); + _sequentialcreditnotes_civix_fixNavigationMenuItems($nodes, $maxNavID, NULL); +} + +function _sequentialcreditnotes_civix_fixNavigationMenuItems(&$nodes, &$maxNavID, $parentID) { + $origKeys = array_keys($nodes); + foreach ($origKeys as $origKey) { + if (!isset($nodes[$origKey]['attributes']['parentID']) && $parentID !== NULL) { + $nodes[$origKey]['attributes']['parentID'] = $parentID; + } + // If no navID, then assign navID and fix key. + if (!isset($nodes[$origKey]['attributes']['navID'])) { + $newKey = ++$maxNavID; + $nodes[$origKey]['attributes']['navID'] = $newKey; + $nodes[$newKey] = $nodes[$origKey]; + unset($nodes[$origKey]); + $origKey = $newKey; + } + if (isset($nodes[$origKey]['child']) && is_array($nodes[$origKey]['child'])) { + _sequentialcreditnotes_civix_fixNavigationMenuItems($nodes[$origKey]['child'], $maxNavID, $nodes[$origKey]['attributes']['navID']); + } + } +} + +/** + * (Delegated) Implements hook_civicrm_alterSettingsFolders(). + * + * @link https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_alterSettingsFolders + */ +function _sequentialcreditnotes_civix_civicrm_alterSettingsFolders(&$metaDataFolders = NULL) { + $settingsDir = __DIR__ . DIRECTORY_SEPARATOR; + if (!in_array($settingsDir, $metaDataFolders) && is_dir($settingsDir)) { + $metaDataFolders[] = $settingsDir; + } +} + +/** + * (Delegated) Implements hook_civicrm_entityTypes(). + * + * Find any *.entityType.php files, merge their content, and return. + * + * @link https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_entityTypes + */ +function _sequentialcreditnotes_civix_civicrm_entityTypes(&$entityTypes) { + $entityTypes = array_merge($entityTypes, []); +} diff --git a/CoreExtensions/Financial/sequentialcreditnotes/sequentialcreditnotes.php b/CoreExtensions/Financial/sequentialcreditnotes/sequentialcreditnotes.php new file mode 100644 index 0000000000..2f2b749968 --- /dev/null +++ b/CoreExtensions/Financial/sequentialcreditnotes/sequentialcreditnotes.php @@ -0,0 +1,72 @@ +addWhere('id', '=', (int) $id)->setSelect(['creditnote_id'])->execute()->first(); + if ($existing['creditnote_id']) { + // Since we have it adding it makes is clearer. + $params['creditnote_id'] = $existing['creditnote_id']; + return; + } + } + $params['creditnote_id'] = sequentialcreditnotes_create_credit_note_id(); + } + } +} + +/** + * Generate credit note id with next available number + * + * @return string + * Credit Note Id. + * + * @throws \CiviCRM_API3_Exception + */ +function sequentialcreditnotes_create_credit_note_id() { + + $creditNoteNum = CRM_Core_DAO::singleValueQuery("SELECT count(creditnote_id) as creditnote_number FROM civicrm_contribution WHERE creditnote_id IS NOT NULL"); + $creditNoteId = NULL; + + do { + $creditNoteNum++; + $creditNoteId = Civi::settings()->get('credit_notes_prefix') . '' . $creditNoteNum; + $result = civicrm_api3('Contribution', 'getcount', [ + 'sequential' => 1, + 'creditnote_id' => $creditNoteId, + ]); + } while ($result > 0); + + return $creditNoteId; +} diff --git a/CoreExtensions/Financial/sequentialcreditnotes/settings/creditnote.setting.php b/CoreExtensions/Financial/sequentialcreditnotes/settings/creditnote.setting.php new file mode 100644 index 0000000000..23d007c0e5 --- /dev/null +++ b/CoreExtensions/Financial/sequentialcreditnotes/settings/creditnote.setting.php @@ -0,0 +1,19 @@ + [ + 'group_name' => 'Contribute Preferences', + 'group' => 'contribute', + 'name' => 'credit_notes_prefix', + 'html_type' => 'text', + 'quick_form_type' => 'Element', + 'add' => '5.23', + 'type' => CRM_Utils_Type::T_STRING, + 'title' => ts('Credit Notes Prefix'), + 'is_domain' => 1, + 'is_contact' => 0, + 'description' => ts('Prefix to be prepended to credit note ids'), + 'default' => 'CN_', + 'help_text' => ts('The credit note ID is generated when a contribution is set to Refunded, Cancelled or Chargeback. It is visible on invoices, if invoices are enabled'), + 'settings_pages' => ['contribute' => ['weight' => 80]], + ], +]; diff --git a/CoreExtensions/Financial/sequentialcreditnotes/tests/phpunit/SequentialcreditnotesTest.php b/CoreExtensions/Financial/sequentialcreditnotes/tests/phpunit/SequentialcreditnotesTest.php new file mode 100644 index 0000000000..507b39a921 --- /dev/null +++ b/CoreExtensions/Financial/sequentialcreditnotes/tests/phpunit/SequentialcreditnotesTest.php @@ -0,0 +1,82 @@ +installMe(__DIR__) + ->apply(); + } + + /** + * Check credit note id creation + * when a contribution is cancelled or refunded + * createCreditNoteId(); + * + * @throws \CRM_Core_Exception + * @throws \CiviCRM_API3_Exception + */ + public function testCreateCreditNoteId() { + $this->_apiversion = 4; + $contactId = $this->callAPISuccess('Contact', 'create', ['contact_type' => 'Individual', 'email' => 'b@example.com'])['id']; + + $params = [ + 'contact_id' => $contactId, + 'currency' => 'USD', + 'financial_type_id' => 1, + 'contribution_status_id' => 3, + 'payment_instrument_id' => 1, + 'source' => 'STUDENT', + 'receive_date' => '20080522000000', + 'receipt_date' => '20080522000000', + 'non_deductible_amount' => 0.00, + 'total_amount' => 300.00, + 'fee_amount' => 5, + 'net_amount' => 295, + 'trxn_id' => '76ereeswww835', + 'invoice_id' => '93ed39a9e9hd621bs0eafe3da82', + 'thankyou_date' => '20080522', + 'sequential' => TRUE, + ]; + + $creditNoteId = sequentialcreditnotes_create_credit_note_id(); + $contribution = $this->callAPISuccess('Contribution', 'create', $params)['values'][0]; + $this->assertEquals($contactId, $contribution['contact_id'], 'Check for contact id creation.'); + $this->assertEquals($creditNoteId, $contribution['creditnote_id'], 'Check if credit note id is created correctly.'); + + $params['id'] = $contribution['id']; + $this->callAPISuccess('Contribution', 'create', $params); + $contribution = $this->callAPISuccessGetSingle('Contribution', ['id' => $params['id']]); + $this->assertEquals($creditNoteId, $contribution['creditnote_id'], 'Check if credit note id was not altered.'); + } + +} diff --git a/CoreExtensions/Financial/sequentialcreditnotes/tests/phpunit/bootstrap.php b/CoreExtensions/Financial/sequentialcreditnotes/tests/phpunit/bootstrap.php new file mode 100644 index 0000000000..352e007050 --- /dev/null +++ b/CoreExtensions/Financial/sequentialcreditnotes/tests/phpunit/bootstrap.php @@ -0,0 +1,63 @@ +add('CRM_', __DIR__); +$loader->add('Civi\\', __DIR__); +$loader->add('api_', __DIR__); +$loader->add('api\\', __DIR__); +$loader->register(); + +/** + * Call the "cv" command. + * + * @param string $cmd + * The rest of the command to send. + * @param string $decode + * Ex: 'json' or 'phpcode'. + * @return string + * Response output (if the command executed normally). + * @throws \RuntimeException + * If the command terminates abnormally. + */ +function cv($cmd, $decode = 'json') { + $cmd = 'cv ' . $cmd; + $descriptorSpec = array(0 => array("pipe", "r"), 1 => array("pipe", "w"), 2 => STDERR); + $oldOutput = getenv('CV_OUTPUT'); + putenv("CV_OUTPUT=json"); + + // Execute `cv` in the original folder. This is a work-around for + // phpunit/codeception, which seem to manipulate PWD. + $cmd = sprintf('cd %s; %s', escapeshellarg(getenv('PWD')), $cmd); + + $process = proc_open($cmd, $descriptorSpec, $pipes, __DIR__); + putenv("CV_OUTPUT=$oldOutput"); + fclose($pipes[0]); + $result = stream_get_contents($pipes[1]); + fclose($pipes[1]); + if (proc_close($process) !== 0) { + throw new RuntimeException("Command failed ($cmd):\n$result"); + } + switch ($decode) { + case 'raw': + return $result; + + case 'phpcode': + // If the last output is /*PHPCODE*/, then we managed to complete execution. + if (substr(trim($result), 0, 12) !== "/*BEGINPHP*/" || substr(trim($result), -10) !== "/*ENDPHP*/") { + throw new \RuntimeException("Command failed ($cmd):\n$result"); + } + return $result; + + case 'json': + return json_decode($result, 1); + + default: + throw new RuntimeException("Bad decoder format ($decode)"); + } +} diff --git a/settings/Contribute.setting.php b/settings/Contribute.setting.php index fe9175d8ab..b9bd3699f0 100644 --- a/settings/Contribute.setting.php +++ b/settings/Contribute.setting.php @@ -73,22 +73,6 @@ return [ ], 'settings_pages' => ['contribute' => ['weight' => 90]], ], - 'credit_notes_prefix' => [ - 'group_name' => 'Contribute Preferences', - 'group' => 'contribute', - 'name' => 'credit_notes_prefix', - 'html_type' => 'text', - 'quick_form_type' => 'Element', - 'add' => '5.23', - 'type' => CRM_Utils_Type::T_STRING, - 'title' => ts('Credit Notes Prefix'), - 'is_domain' => 1, - 'is_contact' => 0, - 'description' => ts('Prefix to be prepended to credit note ids'), - 'default' => 'CN_', - 'help_text' => ts('The credit note ID is generated when a contribution is set to Refunded, Cancelled or Chargeback. It is visible on invoices, if invoices are enabled'), - 'settings_pages' => ['contribute' => ['weight' => 80]], - ], 'invoice_prefix' => [ 'html_type' => 'text', 'name' => 'invoice_prefix', diff --git a/tests/phpunit/CRM/Contribute/BAO/ContributionTest.php b/tests/phpunit/CRM/Contribute/BAO/ContributionTest.php index 52be410429..66244fbf98 100644 --- a/tests/phpunit/CRM/Contribute/BAO/ContributionTest.php +++ b/tests/phpunit/CRM/Contribute/BAO/ContributionTest.php @@ -522,40 +522,6 @@ class CRM_Contribute_BAO_ContributionTest extends CiviUnitTestCase { $this->assertEquals($contributionID, $contribution['id'], 'Check for duplicate transcation id .'); } - /** - * Check credit note id creation - * when a contribution is cancelled or refunded - * createCreditNoteId(); - */ - public function testCreateCreditNoteId() { - $contactId = $this->individualCreate(); - - $param = [ - 'contact_id' => $contactId, - 'currency' => 'USD', - 'financial_type_id' => 1, - 'contribution_status_id' => 3, - 'payment_instrument_id' => 1, - 'source' => 'STUDENT', - 'receive_date' => '20080522000000', - 'receipt_date' => '20080522000000', - 'id' => NULL, - 'non_deductible_amount' => 0.00, - 'total_amount' => 300.00, - 'fee_amount' => 5, - 'net_amount' => 295, - 'trxn_id' => '76ereeswww835', - 'invoice_id' => '93ed39a9e9hd621bs0eafe3da82', - 'thankyou_date' => '20080522', - 'sequential' => TRUE, - ]; - - $creditNoteId = CRM_Contribute_BAO_Contribution::createCreditNoteId(); - $contribution = $this->callAPISuccess('Contribution', 'create', $param)['values'][0]; - $this->assertEquals($contactId, $contribution['contact_id'], 'Check for contact id creation.'); - $this->assertEquals($creditNoteId, $contribution['creditnote_id'], 'Check if credit note id is created correctly.'); - } - /** * Create() method (create and update modes). */ diff --git a/tests/phpunit/api/v3/ContributionTest.php b/tests/phpunit/api/v3/ContributionTest.php index 4083a906af..b727d7cf61 100644 --- a/tests/phpunit/api/v3/ContributionTest.php +++ b/tests/phpunit/api/v3/ContributionTest.php @@ -1649,7 +1649,6 @@ class api_v3_ContributionTest extends CiviUnitTestCase { * @throws \CRM_Core_Exception */ public function testCreateUpdateContributionCancelPending() { - Civi::settings()->set('credit_notes_prefix', 'CN_'); $contribParams = [ 'contact_id' => $this->_individualId, 'receive_date' => '2012-01-01', @@ -1673,7 +1672,6 @@ class api_v3_ContributionTest extends CiviUnitTestCase { $contribution = $this->callAPISuccess('contribution', 'create', $newParams); $this->_checkFinancialTrxn($contribution, 'cancelPending', NULL, $checkTrxnDate); $this->_checkFinancialItem($contribution['id'], 'cancelPending'); - $this->assertEquals('CN_1', $contribution['values'][$contribution['id']]['creditnote_id']); } /** -- 2.25.1