dev/financial#84 Move sequential credit notes from 'deeply embedeed in BAO functions...
authoreileen <emcnaughton@wikimedia.org>
Wed, 12 Feb 2020 13:23:25 +0000 (02:23 +1300)
committereileen <emcnaughton@wikimedia.org>
Sun, 16 Feb 2020 03:37:08 +0000 (16:37 +1300)
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.

16 files changed:
CRM/Admin/Page/Extensions.php
CRM/Contribute/BAO/Contribution.php
CRM/Contribute/Form/Task/Invoice.php
CRM/Extension/Mapper.php
CRM/Upgrade/Incremental/php/FiveTwentyFour.php
CoreExtensions/Financial/sequentialcreditnotes/README.md [new file with mode: 0644]
CoreExtensions/Financial/sequentialcreditnotes/info.xml [new file with mode: 0644]
CoreExtensions/Financial/sequentialcreditnotes/phpunit.xml.dist [new file with mode: 0644]
CoreExtensions/Financial/sequentialcreditnotes/sequentialcreditnotes.civix.php [new file with mode: 0644]
CoreExtensions/Financial/sequentialcreditnotes/sequentialcreditnotes.php [new file with mode: 0644]
CoreExtensions/Financial/sequentialcreditnotes/settings/creditnote.setting.php [new file with mode: 0644]
CoreExtensions/Financial/sequentialcreditnotes/tests/phpunit/SequentialcreditnotesTest.php [new file with mode: 0644]
CoreExtensions/Financial/sequentialcreditnotes/tests/phpunit/bootstrap.php [new file with mode: 0644]
settings/Contribute.setting.php
tests/phpunit/CRM/Contribute/BAO/ContributionTest.php
tests/phpunit/api/v3/ContributionTest.php

index b7b26ad1d0fcd2edc4b8f16e20274f76b4eb4742..3226e0e6ea71272730f11191c44249e19924db3c 100644 (file)
@@ -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'] = '';
index edf648b09856038889835b423ac86dd5f4d97d98..359e3f9dd7d0f0a5db78dba288df91c592a3456c 100644 (file)
@@ -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.
    *
index 9ff683534ce741ff4ede8d5ce4001dd347f63cd9..d3a55dbb6059e155b153ce4af6ce5e36647dc955 100644 (file)
@@ -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);
index 4a34a79117b6b49a4e2a83f633be035a40a917bf..c86226cba17bad4ef168ab083fa51bdb10c47a3d 100644 (file)
@@ -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) {
index b215e3c2900d03a655b14d7d51ba64c52110f077..dc96840a2674fe24f8ca687c75a30fa5c30213ce 100644 (file)
@@ -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 (file)
index 0000000..6250b97
--- /dev/null
@@ -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 (file)
index 0000000..f471757
--- /dev/null
@@ -0,0 +1,29 @@
+<?xml version="1.0"?>
+<extension key="sequentialcreditnotes" type="module">
+  <file>sequentialcreditnotes</file>
+  <name>Sequential credit notes</name>
+  <description>Calculates and sets a sequential credit note whenever a contribution is refunded.</description>
+  <license>AGPL-3.0</license>
+  <maintainer>
+    <author>eileen</author>
+    <email>eileen@civicrm.org</email>
+  </maintainer>
+  <urls>
+    <url desc="Main Extension Page">https://lab.civicrm.org/extensions/sequentialcreditnotes</url>
+    <url desc="Documentation">https://lab.civicrm.org/extensions/sequentialcreditnotes</url>
+    <url desc="Support">https://lab.civicrm.org/extensions/sequentialcreditnotes</url>
+    <url desc="Licensing">http://www.gnu.org/licenses/agpl-3.0.html</url>
+  </urls>
+  <releaseDate>2020-01-28</releaseDate>
+  <version>1.0</version>
+  <tags>
+    <tag>hidden</tag>
+  </tags>
+  <develStage>stable</develStage>
+  <compatibility>
+    <ver>5.24</ver>
+  </compatibility>
+  <civix>
+    <namespace>CRM/Sequentialcreditnotes</namespace>
+  </civix>
+</extension>
diff --git a/CoreExtensions/Financial/sequentialcreditnotes/phpunit.xml.dist b/CoreExtensions/Financial/sequentialcreditnotes/phpunit.xml.dist
new file mode 100644 (file)
index 0000000..0f9f25d
--- /dev/null
@@ -0,0 +1,18 @@
+<?xml version="1.0"?>
+<phpunit backupGlobals="false" backupStaticAttributes="false" colors="true" convertErrorsToExceptions="true" convertNoticesToExceptions="true" convertWarningsToExceptions="true" processIsolation="false" stopOnFailure="false" syntaxCheck="false" bootstrap="tests/phpunit/bootstrap.php">
+  <testsuites>
+    <testsuite name="My Test Suite">
+      <directory>./tests/phpunit</directory>
+    </testsuite>
+  </testsuites>
+  <filter>
+    <whitelist>
+      <directory suffix=".php">./</directory>
+    </whitelist>
+  </filter>
+  <listeners>
+    <listener class="Civi\Test\CiviTestListener">
+      <arguments/>
+    </listener>
+  </listeners>
+</phpunit>
diff --git a/CoreExtensions/Financial/sequentialcreditnotes/sequentialcreditnotes.civix.php b/CoreExtensions/Financial/sequentialcreditnotes/sequentialcreditnotes.civix.php
new file mode 100644 (file)
index 0000000..0b17d88
--- /dev/null
@@ -0,0 +1,476 @@
+<?php
+
+// AUTO-GENERATED FILE -- Civix may overwrite any changes made to this file
+
+/**
+ * The ExtensionUtil class provides small stubs for accessing resources of this
+ * extension.
+ */
+class CRM_Sequentialcreditnotes_ExtensionUtil {
+  const SHORT_NAME = "sequentialcreditnotes";
+  const LONG_NAME = "sequentialcreditnotes";
+  const CLASS_PREFIX = "CRM_Sequentialcreditnotes";
+
+  /**
+   * Translate a string using the extension's domain.
+   *
+   * If the extension doesn't have a specific translation
+   * for the string, fallback to the default translations.
+   *
+   * @param string $text
+   *   Canonical message text (generally en_US).
+   * @param array $params
+   * @return string
+   *   Translated text.
+   * @see ts
+   */
+  public static function ts($text, $params = []) {
+    if (!array_key_exists('domain', $params)) {
+      $params['domain'] = [self::LONG_NAME, NULL];
+    }
+    return ts($text, $params);
+  }
+
+  /**
+   * Get the URL of a resource file (in this extension).
+   *
+   * @param string|NULL $file
+   *   Ex: NULL.
+   *   Ex: 'css/foo.css'.
+   * @return string
+   *   Ex: 'http://example.org/sites/default/ext/org.example.foo'.
+   *   Ex: 'http://example.org/sites/default/ext/org.example.foo/css/foo.css'.
+   */
+  public static function url($file = NULL) {
+    if ($file === NULL) {
+      return rtrim(CRM_Core_Resources::singleton()->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 (file)
index 0000000..2f2b749
--- /dev/null
@@ -0,0 +1,72 @@
+<?php
+
+require_once 'sequentialcreditnotes.civix.php';
+use Civi\Api4\Contribution;
+
+/**
+ * Implements hook_civicrm_alterSettingsFolders().
+ *
+ * @link https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_alterSettingsFolders
+ */
+function sequentialcreditnotes_civicrm_alterSettingsFolders(&$metaDataFolders = NULL) {
+  _sequentialcreditnotes_civix_civicrm_alterSettingsFolders($metaDataFolders);
+}
+
+/**
+ * Add a creditnote_id if appropriate.
+ *
+ * If the 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.
+ *
+ * Note that the
+ *
+ * @param \CRM_Core_DAO $op
+ * @param string $objectName
+ * @param int|null $id
+ * @param array $params
+ *
+ * @throws \CiviCRM_API3_Exception
+ * @throws \API_Exception
+ */
+function sequentialcreditnotes_civicrm_pre($op, $objectName, $id, &$params) {
+  if ($objectName === 'Contribution' && !empty($params['contribution_status_id'])) {
+    $reversalStatuses = ['Cancelled', 'Chargeback', 'Refunded'];
+    if (empty($params['creditnote_id']) && in_array(CRM_Core_PseudoConstant::getName('CRM_Contribute_BAO_Contribution', 'contribution_status_id', $params['contribution_status_id']), $reversalStatuses, TRUE)) {
+      if ($id) {
+        $existing = Contribution::get()->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 (file)
index 0000000..23d007c
--- /dev/null
@@ -0,0 +1,19 @@
+<?php
+return [
+  '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]],
+  ],
+];
diff --git a/CoreExtensions/Financial/sequentialcreditnotes/tests/phpunit/SequentialcreditnotesTest.php b/CoreExtensions/Financial/sequentialcreditnotes/tests/phpunit/SequentialcreditnotesTest.php
new file mode 100644 (file)
index 0000000..507b39a
--- /dev/null
@@ -0,0 +1,82 @@
+<?php
+
+use Civi\Test\HeadlessInterface;
+use Civi\Test\HookInterface;
+use Civi\Test\TransactionalInterface;
+
+/**
+ * FIXME - Add test description.
+ *
+ * Tips:
+ *  - With HookInterface, you may implement CiviCRM hooks directly in the test class.
+ *    Simply create corresponding functions (e.g. "hook_civicrm_post(...)" or similar).
+ *  - With TransactionalInterface, any data changes made by setUp() or test****() functions will
+ *    rollback automatically -- as long as you don't manipulate schema or truncate tables.
+ *    If this test needs to manipulate schema or truncate tables, then either:
+ *       a. Do all that using setupHeadless() and Civi\Test.
+ *       b. Disable TransactionalInterface, and handle all setup/teardown yourself.
+ *
+ * @group headless
+ */
+class SequentialcreditnotesTest extends \PHPUnit\Framework\TestCase implements HeadlessInterface, HookInterface, TransactionalInterface {
+
+  use \Civi\Test\Api3TestTrait;
+
+  /**
+   * Setup for headless test.
+   *
+   * @return \Civi\Test\CiviEnvBuilder
+   *
+   * @throws \CRM_Extension_Exception_ParseException
+   */
+  public function setUpHeadless(): \Civi\Test\CiviEnvBuilder {
+    // Civi\Test has many helpers, like install(), uninstall(), sql(), and sqlFile().
+    // See: https://docs.civicrm.org/dev/en/latest/testing/phpunit/#civitest
+    return \Civi\Test::headless()
+      ->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 (file)
index 0000000..352e007
--- /dev/null
@@ -0,0 +1,63 @@
+<?php
+
+ini_set('memory_limit', '2G');
+ini_set('safe_mode', 0);
+// phpcs:ignore
+eval(cv('php:boot --level=classloader', 'phpcode'));
+
+// Allow autoloading of PHPUnit helper classes in this extension.
+$loader = new \Composer\Autoload\ClassLoader();
+$loader->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)");
+  }
+}
index fe9175d8abceaae22142fb22c21a4a5e1425bf63..b9bd3699f0a8d7efbeaeac58f6d36fbfb4aa465f 100644 (file)
@@ -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',
index 52be41042974b257ece1b7c5b06cfc4a7b8caf54..66244fbf982cb11159e6a4a8f0317234f9296a50 100644 (file)
@@ -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).
    */
index 4083a906afd220557db899d148f95a085008984c..b727d7cf6133b1e1632cfba1b1141bbc376b286e 100644 (file)
@@ -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']);
   }
 
   /**