dev/financial#84 Remove sequential credit notes from core

CRM/Admin/Form/Generic.php

@@ -50,14 +50,6 @@ public function setDefaultValues() {
    * Build the form object.
    */
   public function buildQuickForm() {
-    $filter = $this->getSettingPageFilter();
-    $settings = civicrm_api3('Setting', 'getfields', [])['values'];
-    foreach ($settings as $key => $setting) {
-      if (isset($setting['settings_pages'][$filter])) {
-        $this->_settings[$key] = $setting;
-      }
-    }
-
     $this->addFieldsDefinedInSettingsMetadata();
 
     // @todo look at sharing the code below in the settings trait.

CRM/Admin/Form/Preferences/Contribute.php

@@ -19,47 +19,37 @@
  * This class generates form components for the display preferences.
  */
 class CRM_Admin_Form_Preferences_Contribute extends CRM_Admin_Form_Preferences {
-  protected $_settings = [
-    'cvv_backoffice_required' => CRM_Core_BAO_Setting::CONTRIBUTE_PREFERENCES_NAME,
-    'update_contribution_on_membership_type_change' => CRM_Core_BAO_Setting::CONTRIBUTE_PREFERENCES_NAME,
-    'acl_financial_type' => CRM_Core_BAO_Setting::CONTRIBUTE_PREFERENCES_NAME,
-    'always_post_to_accounts_receivable' => CRM_Core_BAO_Setting::CONTRIBUTE_PREFERENCES_NAME,
-    'deferred_revenue_enabled' => CRM_Core_BAO_Setting::CONTRIBUTE_PREFERENCES_NAME,
-    'default_invoice_page' => CRM_Core_BAO_Setting::CONTRIBUTE_PREFERENCES_NAME,
-    'invoicing' => CRM_Core_BAO_Setting::CONTRIBUTE_PREFERENCES_NAME,
-  ];
 
   /**
    * Our standards for settings are to have a setting per value with defined metadata.
    *
    * Unfortunately the 'contribution_invoice_settings' has been added in non-compliance.
    * We use this array to hack-handle.
    *
-   * I think the best way forwards would be to covert to multiple individual settings.
+   * These are now stored as individual settings but this form still does weird & wonderful things.
+   *
+   * Note the 'real' settings on this form are added via metadata definition - ie
+   * 'settings_pages' => ['contribute' => ['weight' => 1]], in their metadata.
    *
    * @var array
    */
   protected $invoiceSettings = [];
 
   /**
    * Build the form object.
+   *
+   * @throws \CiviCRM_API3_Exception
+   * @throws \CRM_Core_Exception
    */
   public function buildQuickForm() {
     parent::buildQuickForm();
-    $config = CRM_Core_Config::singleton();
     $this->invoiceSettings = [
       'invoice_prefix' => [
         'html_type' => 'text',
         'title' => ts('Invoice Prefix'),
         'weight' => 1,
         'description' => ts('Enter prefix to be display on PDF for invoice'),
       ],
-      'credit_notes_prefix' => [
-        'html_type' => 'text',
-        'title' => ts('Credit Notes Prefix'),
-        'weight' => 2,
-        'description' => ts('Enter prefix to be display on PDF for credit notes.'),
-      ],
       'due_date' => [
         'html_type' => 'text',
         'title' => ts('Due Date'),

CRM/Admin/Form/SettingTrait.php

@@ -153,8 +153,10 @@ protected function getSettingsOrderedByWeight() {
    * Add fields in the metadata to the template.
    *
    * @throws \CRM_Core_Exception
+   * @throws \CiviCRM_API3_Exception
    */
   protected function addFieldsDefinedInSettingsMetadata() {
+    $this->addSettingsToFormFromMetadata();
     $settingMetaData = $this->getSettingsMetaData();
     $descriptions = [];
     foreach ($settingMetaData as $setting => $props) {
@@ -372,4 +374,19 @@ private function getReorderedSettingData($setting, $settingValue) {
     return array_intersect($order, $settingValueKeys);
   }
 
+  /**
+   * Add settings to form if the metadata designates they should be on the page.
+   *
+   * @throws \CiviCRM_API3_Exception
+   */
+  protected function addSettingsToFormFromMetadata() {
+    $filter = $this->getSettingPageFilter();
+    $settings = civicrm_api3('Setting', 'getfields', [])['values'];
+    foreach ($settings as $key => $setting) {
+      if (isset($setting['settings_pages'][$filter])) {
+        $this->_settings[$key] = $setting;
+      }
+    }
+  }
+
 }

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

@@ -32,6 +32,7 @@
     'is_contact' => 0,
     'description' => ts('Is the CVV code required for back office credit card transactions'),
     'help_text' => 'If set it back-office credit card transactions will required a cvv code. Leave as required unless you have a very strong reason to change',
+    'settings_pages' => ['contribute' => ['weight' => 1]],
   ],
   'contribution_invoice_settings' => [
     // @todo our standard is to have a setting per item not to hide settings in an array with
@@ -70,21 +71,7 @@
     'on_change' => [
       'CRM_Invoicing_Utils::onToggle',
     ],
-  ],
-  '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' => 9]],
   ],
   'invoice_prefix' => [
     'html_type' => 'text',
@@ -176,6 +163,7 @@
     'is_contact' => 0,
     'help_text' => NULL,
     'help' => ['id' => 'acl_financial_type'],
+    'settings_pages' => ['contribute' => ['weight' => 3]],
   ],
   'deferred_revenue_enabled' => [
     'group_name' => 'Contribute Preferences',
@@ -190,6 +178,7 @@
     'is_domain' => 1,
     'is_contact' => 0,
     'help_text' => NULL,
+    'settings_pages' => ['contribute' => ['weight' => 5]],
   ],
   'default_invoice_page' => [
     'group_name' => 'Contribute Preferences',
@@ -208,6 +197,7 @@
     'is_domain' => 1,
     'is_contact' => 0,
     'help_text' => NULL,
+    'settings_pages' => ['contribute' => ['weight' => 7]],
   ],
   'always_post_to_accounts_receivable' => [
     'group_name' => 'Contribute Preferences',
@@ -222,6 +212,7 @@
     'is_domain' => 1,
     'is_contact' => 0,
     'help_text' => NULL,
+    'settings_pages' => ['contribute' => ['weight' => 4]],
   ],
   'update_contribution_on_membership_type_change' => [
     'group_name' => 'Contribute Preferences',
@@ -237,5 +228,6 @@
     'is_contact' => 0,
     'description' => ts('Enabling this setting will update related contribution of membership(s) except if the membership is paid for with a recurring contribution.'),
     'help_text' => NULL,
+    'settings_pages' => ['contribute' => ['weight' => 2]],
   ],
 ];

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']);
   }
 
   /**