From: Eileen McNaughton <emcnaughton@wikimedia.org>
Date: Thu, 29 Jul 2021 00:45:18 +0000 (+1200)
Subject: dev/core#2719 [REF] Deprecate legacy references to contribution_invoice_settings
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=a65a00109cda16b8bbfaa8f2fff120a38cea6af8;p=civicrm-core.git

dev/core#2719 [REF] Deprecate legacy references to contribution_invoice_settings
---

diff --git a/Civi/Core/SettingsBag.php b/Civi/Core/SettingsBag.php
index 948ea2b963..2c03016024 100644
--- a/Civi/Core/SettingsBag.php
+++ b/Civi/Core/SettingsBag.php
@@ -270,6 +270,7 @@ class SettingsBag {
    */
   public function updateVirtual($key, $value) {
     if ($key === 'contribution_invoice_settings') {
+      \CRM_Core_Error::deprecatedWarning('Invoicing settings should be directly accessed - eg Civi::setting()->set("invoicing")');
       foreach (SettingsBag::getContributionInvoiceSettingKeys() as $possibleKeyName => $settingName) {
         $keyValue = $value[$possibleKeyName] ?? '';
         if ($possibleKeyName === 'invoicing' && is_array($keyValue)) {
diff --git a/api/v3/Setting.php b/api/v3/Setting.php
index a2c422bb71..605ae2734d 100644
--- a/api/v3/Setting.php
+++ b/api/v3/Setting.php
@@ -296,7 +296,11 @@ function _civicrm_api3_setting_create_spec(&$params) {
  */
 function civicrm_api3_setting_get($params) {
   $domains = _civicrm_api3_setting_getDomainArray($params);
-  $result = CRM_Core_BAO_Setting::getItems($params, $domains, $params['return'] ?? []);
+  $returnSettings = (array) ($params['return'] ?? []);
+  if (in_array('contribution_invoice_settings', $returnSettings)) {
+    CRM_Core_Error::deprecatedWarning('contribution_invoice_settings is not a valid setting. Request the actual setting');
+  }
+  $result = CRM_Core_BAO_Setting::getItems($params, $domains, $returnSettings);
   return civicrm_api3_create_success($result, $params, 'Setting', 'get');
 }
 
diff --git a/tests/phpunit/CRM/Core/BAO/SettingTest.php b/tests/phpunit/CRM/Core/BAO/SettingTest.php
index 79ed20f4b8..b1412983e9 100644
--- a/tests/phpunit/CRM/Core/BAO/SettingTest.php
+++ b/tests/phpunit/CRM/Core/BAO/SettingTest.php
@@ -69,50 +69,6 @@ class CRM_Core_BAO_SettingTest extends CiviUnitTestCase {
     $this->assertFalse($result);
   }
 
-  /**
-   * Test temporary retrieval & setting of converted settings.
-   *
-   * As a transitional measure we allow the settings that were munged into
-   * contribution_invoice_setting. This tests that the current method of getting via the 'old' key
-   * works. This will be deprecated & removed over the next few versions but
-   * 1) some extensions use these settings &
-   * 2) there is a lot of work to fix this mess in core so a transitional method makes sense.
-   *
-   * https://lab.civicrm.org/dev/core/issues/1558
-   *
-   * @throws \CRM_Core_Exception
-   */
-  public function testHandlingOfContributionInvoiceSetting(): void {
-    $contributionSettings = [
-      'invoice_prefix' => 'G_',
-      'credit_notes_prefix' => 'XX_',
-      'due_date' => '20',
-      'due_date_period' => 'weeks',
-      'notes' => '<p>Give me money</p>',
-      'tax_term' => 'Extortion',
-      'tax_display_settings' => 'Exclusive',
-      // NOTE: This form of `invoicing` is accepted, but it may be normalized to the idiomatic form with a nested array.
-      'invoicing' => 1,
-      'is_email_pdf' => '1',
-    ];
-    Civi::settings()->set('contribution_invoice_settings', $contributionSettings);
-    $settingsFromGet = Civi::settings()->get('contribution_invoice_settings');
-    $settingsFromAPI = $this->callAPISuccess('Setting', 'get', ['return' => 'contribution_invoice_settings'])['values'][CRM_Core_Config::domainID()]['contribution_invoice_settings'];
-    $getVersion = $this->callAPISuccessGetValue('Setting', ['name' => 'contribution_invoice_settings']);
-    $this->assertEquals($settingsFromAPI, $settingsFromGet);
-    $this->assertAPIArrayComparison($getVersion, $settingsFromGet);
-    $this->assertEquals(['invoicing' => ['invoicing' => 1]] + $contributionSettings, $settingsFromGet);
-
-    // These are the preferred retrieval methods.
-    $this->assertEquals('G_', Civi::settings()->get('invoice_prefix'));
-    $this->assertEquals('XX_', Civi::settings()->get('credit_notes_prefix'));
-    $this->assertEquals('20', Civi::settings()->get('invoice_due_date'));
-    $this->assertEquals('weeks', Civi::settings()->get('invoice_due_date_period'));
-    $this->assertEquals('<p>Give me money</p>', Civi::settings()->get('invoice_notes'));
-    $this->assertEquals('Extortion', Civi::settings()->get('tax_term'));
-    $this->assertEquals('Exclusive', Civi::settings()->get('tax_display_settings'));
-  }
-
   /**
    * Ensure that overrides in $civicrm_setting apply when
    * using getItem($group,$name).