dev/financial#84 Remove sequential credit notes from core to an exten…

…sion that ships with core.

https://lab.civicrm.org/dev/financial/issues/84

The code that adds  sequentialcredtit notes is a hangover from pre-Lexim days when people's various needs just got added to core.
It would not have been added to core post Lexim for the following reasons
1) architectural - it assumes a one to one relationship between contributions & credit notes which is not reflective of our data model
2) performance - the code had significant performance impact on large sites
3) specificity - the code was for a specific localised use case of needing guaranteed sequential credit-notes
4) maintainability - we've had to put in a lot of work to get the code down to the few lines being removed. The goal is not
to allow organisations to transfer maintenance burden onto the community unless there is a clear rationale.

Medium term the intent is that this extension will not ship with core & will simply be available from the extensions
directory as any other extension. In order to reach this point we need a transitional approach. In the first instance sites
should need to take no action at all in order to experience no change of behaviour. In order to achieve this
we continue to ship this functionality with core as an extension. We enable the extension on upgrade (but on new
installs it is opt-in).

At this point sites who do not want this functionality can disable the extension.

The next step will be to get people who want the extension to install it to their extensions directory
so we can remove this from core later. This will involve us fixing the extension load code to prefer the extension
directory over core if the extension is in both places & adding checks & messaging to encourage people to
install the extension if they continue to want it.

At some point we will remove it from the core tarball. Ongoing maintenance will be provided by whoever wants to maintain  it - if
someone  wants to be added as a maintainer of the extension repo the answer is yes.

CRM/Contribute/BAO/Contribution.php

@@ -130,21 +130,12 @@ public static function add(&$params, $ids = []) {
     //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');
@@ -4688,31 +4679,6 @@ public static function generateFromEmailAndName($input, $contribution) {
     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.
    *

CRM/Upgrade/Incremental/php/FiveTwentyThree.php

@@ -36,6 +36,8 @@ public function setPreUpgradeMessage(&$preUpgradeMessage, $rev, $currentVer = NU
           2 => 'https://lab.civicrm.org/dev/core/issues/1387',
         ]);
       }
+      $preUpgradeMessage .= '<br/>' . ts('The code to create sequential credit notes when contributions are refunded has been moved to an extension.') .
+        ts('This extension has been enabled. If this feature is not important to you your performance will be improved by disabling it on the extension screen');
     }
   }
 
@@ -88,6 +90,7 @@ public function upgrade_5_23_alpha1($rev) {
     if (!$this->hasConfigBackendData()) {
       $this->addTask('Drop column "civicrm_domain.config_backend"', 'dropColumn', 'civicrm_domain', 'config_backend');
     }
+    $this->addTask('Install sequential creditnote extension', 'installCreditNotes');
   }
 
   /**
@@ -167,4 +170,20 @@ private function hasConfigBackendData() {
     && CRM_Core_DAO::singleValueQuery('SELECT count(*) c FROM `civicrm_domain` WHERE config_backend IS NOT NULL') > 0;
   }
 
+  /**
+   * Install sequentialCreditNotes extension.
+   *
+   * This extension is being moved from core functionality to an extension.
+   *
+   * @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;
+  }
+
 }

Civi/Core/SettingsBag.php

@@ -423,6 +423,8 @@ protected function setDb($name, $value) {
    */
   public static function getContributionInvoiceSettingKeys(): array {
     $convertedKeys = [
+      // credit_notes_prefix is no longer core but leave it here for a few releases.
+      // it can be removed when we deprecate some others.
       'credit_notes_prefix' => 'credit_notes_prefix',
       'invoice_prefix' => 'invoice_prefix',
       'due_date' => 'invoice_due_date',

distmaker/dists/backdrop_php5.sh

@@ -24,6 +24,7 @@ dm_install_vendor "$SRC/vendor" "$TRG/vendor"
 dm_install_bower "$SRC/bower_components" "$TRG/bower_components"
 dm_install_drupal "$SRC/backdrop" "$TRG/backdrop"
 dm_install_cvext com.iatspayments.civicrm "$TRG/ext/iatspayments"
+dm_install_cvext sequentialcreditnotes "$TRG/ext/sequentialcreditnotes"
 
 # gen tarball
 cd $TRG/..

distmaker/dists/drupal6_php5.sh

@@ -24,6 +24,7 @@ dm_install_vendor "$SRC/vendor" "$TRG/vendor"
 dm_install_bower "$SRC/bower_components" "$TRG/bower_components"
 dm_install_drupal "$SRC/drupal" "$TRG/drupal"
 dm_install_cvext com.iatspayments.civicrm "$TRG/ext/iatspayments"
+dm_install_cvext sequentialcreditnotes "$TRG/ext/sequentialcreditnotes"
 
 # gen tarball
 cd $TRG/..

distmaker/dists/drupal7_dir_php5.sh

@@ -31,6 +31,7 @@ dm_install_vendor "$SRC/vendor" "$TRG/vendor"
 dm_install_bower "$SRC/bower_components" "$TRG/bower_components"
 dm_install_drupal "$DM_DRUPALDIR" "$TRG/drupal"
 dm_install_cvext com.iatspayments.civicrm "$TRG/ext/iatspayments"
+dm_install_cvext sequentialcreditnotes "$TRG/ext/sequentialcreditnotes"
 
 # gen tarball
 cd $TRG

distmaker/dists/drupal_php5.sh

@@ -24,6 +24,7 @@ dm_install_vendor "$SRC/vendor" "$TRG/vendor"
 dm_install_bower "$SRC/bower_components" "$TRG/bower_components"
 dm_install_drupal "$SRC/drupal" "$TRG/drupal"
 dm_install_cvext com.iatspayments.civicrm "$TRG/ext/iatspayments"
+dm_install_cvext sequentialcreditnotes "$TRG/ext/sequentialcreditnotes"
 
 # gen tarball
 cd $TRG/..

distmaker/dists/drupal_sk_php5.sh

@@ -24,6 +24,7 @@ dm_install_vendor "$SRC/vendor" "$TRG/vendor"
 dm_install_bower "$SRC/bower_components" "$TRG/bower_components"
 dm_install_drupal "$SRC/drupal" "$TRG/drupal"
 dm_install_cvext com.iatspayments.civicrm "$TRG/ext/iatspayments"
+dm_install_cvext sequentialcreditnotes "$TRG/ext/sequentialcreditnotes"
 
 # delete packages that distributions on Drupal.org repalce if present
 # also delete stuff that we dont really use and should not be included

distmaker/dists/joomla_php5.sh

@@ -23,6 +23,7 @@ dm_install_packages "$SRC/packages" "$TRG/packages"
 dm_install_vendor "$SRC/vendor" "$TRG/vendor"
 dm_install_bower "$SRC/bower_components" "$TRG/bower_components"
 dm_install_cvext com.iatspayments.civicrm "$TRG/ext/iatspayments"
+dm_install_cvext sequentialcreditnotes "$TRG/ext/sequentialcreditnotes"
 
 ## WTF: It's so good we'll install it twice!
 ## (The first is probably extraneous, but there could be bugs dependent on it.)

distmaker/dists/wordpress_php5.sh

@@ -24,6 +24,7 @@ dm_install_vendor "$SRC/vendor" "$TRG/civicrm/civicrm/vendor"
 dm_install_bower "$SRC/bower_components" "$TRG/civicrm/civicrm/bower_components"
 dm_install_wordpress "$SRC/WordPress" "$TRG/civicrm"
 dm_install_cvext com.iatspayments.civicrm "$TRG/civicrm/civicrm/ext/iatspayments"
+dm_install_cvext sequentialcreditnotes "$TRG/ext/sequentialcreditnotes"
 
 # gen tarball
 cd $TRG

settings/Contribute.setting.php

@@ -73,22 +73,6 @@
     ],
     'settings_pages' => ['contribute' => ['weight' => 9]],
   ],
-  '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' => 8]],
-  ],
   'invoice_prefix' => [
     'html_type' => 'text',
     'name' => 'invoice_prefix',

tests/phpunit/CRM/Contribute/BAO/ContributionTest.php

@@ -522,40 +522,6 @@ public function testcheckDuplicateIds() {
     $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).
    */

tests/phpunit/api/v3/ContributionTest.php

@@ -1649,7 +1649,6 @@ public function testCreateUpdateContributionInValidStatusChange() {
    * @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 @@ public function testCreateUpdateContributionCancelPending() {
     $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']);
   }
 
   /**